gin: Use V8 Maybe APIs

TEST=gin_unittests BUG=479439 Review URL: https://codereview.chromium.org/1106393002 Cr-Commit-Position: refs/heads/master@{#331923}
stjohnalex · May 29, 2015 · 7a6acf6 · 7a6acf6
1 parent a719b2b
commit 7a6acf6
Show file tree

Hide file tree

Showing 17 changed files with 204 additions and 79 deletions.
diff --git a/gin/arguments.h b/gin/arguments.h
@@ -69,7 +69,10 @@ class GIN_EXPORT Arguments {
 
   template<typename T>
   void Return(T val) {
-    info_->GetReturnValue().Set(ConvertToV8(isolate_, val));
+    v8::Local<v8::Value> v8_value;
+    if (!TryConvertToV8(isolate_, val, &v8_value))
+      return;
+    info_->GetReturnValue().Set(v8_value);
   }
 
   v8::Local<v8::Value> PeekNext() const;

diff --git a/gin/converter.cc b/gin/converter.cc
@@ -10,23 +10,38 @@ using v8::ArrayBuffer;
 using v8::Boolean;
 using v8::External;
 using v8::Function;
+using v8::Int32;
 using v8::Integer;
 using v8::Isolate;
 using v8::Local;
+using v8::Maybe;
+using v8::MaybeLocal;
 using v8::Number;
 using v8::Object;
 using v8::String;
+using v8::Uint32;
 using v8::Value;
 
+namespace {
+
+template <typename T, typename U>
+bool FromMaybe(Maybe<T> maybe, U* out) {
+  if (maybe.IsNothing())
+    return false;
+  *out = static_cast<U>(maybe.FromJust());
+  return true;
+}
+
+}  // namespace
+
 namespace gin {
 
 Local<Value> Converter<bool>::ToV8(Isolate* isolate, bool val) {
   return Boolean::New(isolate, val).As<Value>();
 }
 
 bool Converter<bool>::FromV8(Isolate* isolate, Local<Value> val, bool* out) {
-  *out = val->BooleanValue();
-  return true;
+  return FromMaybe(val->BooleanValue(isolate->GetCurrentContext()), out);
 }
 
 Local<Value> Converter<int32_t>::ToV8(Isolate* isolate, int32_t val) {
@@ -38,7 +53,7 @@ bool Converter<int32_t>::FromV8(Isolate* isolate,
                                 int32_t* out) {
   if (!val->IsInt32())
     return false;
-  *out = val->Int32Value();
+  *out = val.As<Int32>()->Value();
   return true;
 }
 
@@ -51,7 +66,7 @@ bool Converter<uint32_t>::FromV8(Isolate* isolate,
                                  uint32_t* out) {
   if (!val->IsUint32())
     return false;
-  *out = val->Uint32Value();
+  *out = val.As<Uint32>()->Value();
   return true;
 }
 
@@ -66,8 +81,7 @@ bool Converter<int64_t>::FromV8(Isolate* isolate,
     return false;
   // Even though IntegerValue returns int64_t, JavaScript cannot represent
   // the full precision of int64_t, which means some rounding might occur.
-  *out = val->IntegerValue();
-  return true;
+  return FromMaybe(val->IntegerValue(isolate->GetCurrentContext()), out);
 }
 
 Local<Value> Converter<uint64_t>::ToV8(Isolate* isolate, uint64_t val) {
@@ -79,8 +93,7 @@ bool Converter<uint64_t>::FromV8(Isolate* isolate,
                                  uint64_t* out) {
   if (!val->IsNumber())
     return false;
-  *out = static_cast<uint64_t>(val->IntegerValue());
-  return true;
+  return FromMaybe(val->IntegerValue(isolate->GetCurrentContext()), out);
 }
 
 Local<Value> Converter<float>::ToV8(Isolate* isolate, float val) {
@@ -90,7 +103,7 @@ Local<Value> Converter<float>::ToV8(Isolate* isolate, float val) {
 bool Converter<float>::FromV8(Isolate* isolate, Local<Value> val, float* out) {
   if (!val->IsNumber())
     return false;
-  *out = static_cast<float>(val->NumberValue());
+  *out = static_cast<float>(val.As<Number>()->Value());
   return true;
 }
 
@@ -103,14 +116,16 @@ bool Converter<double>::FromV8(Isolate* isolate,
                                double* out) {
   if (!val->IsNumber())
     return false;
-  *out = val->NumberValue();
+  *out = val.As<Number>()->Value();
   return true;
 }
 
 Local<Value> Converter<base::StringPiece>::ToV8(Isolate* isolate,
                                                 const base::StringPiece& val) {
-  return String::NewFromUtf8(isolate, val.data(), String::kNormalString,
-                             static_cast<uint32_t>(val.length()));
+  return String::NewFromUtf8(isolate, val.data(),
+                             v8::NewStringType::kNormal,
+                             static_cast<uint32_t>(val.length()))
+      .ToLocalChecked();
 }
 
 Local<Value> Converter<std::string>::ToV8(Isolate* isolate,
@@ -194,10 +209,10 @@ bool Converter<Local<Value>>::FromV8(Isolate* isolate,
 
 v8::Local<v8::String> StringToSymbol(v8::Isolate* isolate,
                                       const base::StringPiece& val) {
-  return String::NewFromUtf8(isolate,
-                             val.data(),
-                             String::kInternalizedString,
-                             static_cast<uint32_t>(val.length()));
+  return String::NewFromUtf8(isolate, val.data(),
+                             v8::NewStringType::kInternalized,
+                             static_cast<uint32_t>(val.length()))
+      .ToLocalChecked();
 }
 
 std::string V8ToString(v8::Local<v8::Value> value) {

diff --git a/gin/converter.h b/gin/converter.h
@@ -8,12 +8,27 @@
 #include <string>
 #include <vector>
 
+#include "base/logging.h"
 #include "base/strings/string_piece.h"
 #include "gin/gin_export.h"
 #include "v8/include/v8.h"
 
 namespace gin {
 
+template<typename KeyType>
+bool SetProperty(v8::Isolate* isolate,
+                 v8::Local<v8::Object> object,
+                 KeyType key,
+                 v8::Local<v8::Value> value) {
+  auto maybe = object->Set(isolate->GetCurrentContext(), key, value);
+  return !maybe.IsNothing() && maybe.FromJust();
+}
+
+template<typename T>
+struct ToV8ReturnsMaybe {
+  static const bool value = false;
+};
+
 template<typename T, typename Enable = void>
 struct Converter {};
 
@@ -84,13 +99,15 @@ struct GIN_EXPORT Converter<double> {
 
 template<>
 struct GIN_EXPORT Converter<base::StringPiece> {
+  // This crashes when val.size() > v8::String::kMaxLength.
   static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
                                     const base::StringPiece& val);
   // No conversion out is possible because StringPiece does not contain storage.
 };
 
 template<>
 struct GIN_EXPORT Converter<std::string> {
+  // This crashes when val.size() > v8::String::kMaxLength.
   static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
                                     const std::string& val);
   static bool FromV8(v8::Isolate* isolate,
@@ -143,12 +160,15 @@ struct GIN_EXPORT Converter<v8::Local<v8::Value> > {
 
 template<typename T>
 struct Converter<std::vector<T> > {
-  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
-                                    const std::vector<T>& val) {
+  static v8::MaybeLocal<v8::Value> ToV8(v8::Local<v8::Context> context,
+                                        const std::vector<T>& val) {
+    v8::Isolate* isolate = context->GetIsolate();
     v8::Local<v8::Array> result(
         v8::Array::New(isolate, static_cast<int>(val.size())));
-    for (size_t i = 0; i < val.size(); ++i) {
-      result->Set(static_cast<int>(i), Converter<T>::ToV8(isolate, val[i]));
+    for (uint32_t i = 0; i < val.size(); ++i) {
+      auto maybe = result->Set(context, i, Converter<T>::ToV8(isolate, val[i]));
+      if (maybe.IsNothing() || !maybe.FromJust())
+        return v8::MaybeLocal<v8::Value>();
     }
     return result;
   }
@@ -163,8 +183,11 @@ struct Converter<std::vector<T> > {
     v8::Local<v8::Array> array(v8::Local<v8::Array>::Cast(val));
     uint32_t length = array->Length();
     for (uint32_t i = 0; i < length; ++i) {
+      v8::Local<v8::Value> v8_item;
+      if (!array->Get(isolate->GetCurrentContext(), i).ToLocal(&v8_item))
+        return false;
       T item;
-      if (!Converter<T>::FromV8(isolate, array->Get(i), &item))
+      if (!Converter<T>::FromV8(isolate, v8_item, &item))
         return false;
       result.push_back(item);
     }
@@ -174,18 +197,62 @@ struct Converter<std::vector<T> > {
   }
 };
 
+template<typename T>
+struct ToV8ReturnsMaybe<std::vector<T>> {
+  static const bool value = true;
+};
+
 // Convenience functions that deduce T.
 template<typename T>
 v8::Local<v8::Value> ConvertToV8(v8::Isolate* isolate, T input) {
   return Converter<T>::ToV8(isolate, input);
 }
 
+template<typename T>
+v8::MaybeLocal<v8::Value> ConvertToV8(v8::Local<v8::Context> context, T input) {
+  return Converter<T>::ToV8(context, input);
+}
+
+template<typename T, bool = ToV8ReturnsMaybe<T>::value> struct ToV8Traits;
+
+template <typename T>
+struct ToV8Traits<T, true> {
+  static bool TryConvertToV8(v8::Isolate* isolate,
+                             T input,
+                             v8::Local<v8::Value>* output) {
+    auto maybe = ConvertToV8(isolate->GetCurrentContext(), input);
+    if (maybe.IsEmpty())
+      return false;
+    *output = maybe.ToLocalChecked();
+    return true;
+  }
+};
+
+template <typename T>
+struct ToV8Traits<T, false> {
+  static bool TryConvertToV8(v8::Isolate* isolate,
+                             T input,
+                             v8::Local<v8::Value>* output) {
+    *output = ConvertToV8(isolate, input);
+    return true;
+  }
+};
+
+template <typename T>
+bool TryConvertToV8(v8::Isolate* isolate,
+                    T input,
+                    v8::Local<v8::Value>* output) {
+  return ToV8Traits<T>::TryConvertToV8(isolate, input, output);
+}
+
+// This crashes when input.size() > v8::String::kMaxLength.
 GIN_EXPORT inline v8::Local<v8::String> StringToV8(
     v8::Isolate* isolate,
     const base::StringPiece& input) {
   return ConvertToV8(isolate, input).As<v8::String>();
 }
 
+// This crashes when input.size() > v8::String::kMaxLength.
 GIN_EXPORT v8::Local<v8::String> StringToSymbol(v8::Isolate* isolate,
                                                  const base::StringPiece& val);
 

diff --git a/gin/converter_unittest.cc b/gin/converter_unittest.cc
@@ -116,19 +116,16 @@ TEST_F(ConverterTest, Vector) {
   expected.push_back(0);
   expected.push_back(1);
 
-  Local<Array> js_array = Local<Array>::Cast(
-      Converter<std::vector<int>>::ToV8(instance_->isolate(), expected));
-  ASSERT_FALSE(js_array.IsEmpty());
-  EXPECT_EQ(3u, js_array->Length());
+  auto maybe = Converter<std::vector<int>>::ToV8(
+      instance_->isolate()->GetCurrentContext(), expected);
+  Local<Value> js_value;
+  EXPECT_TRUE(maybe.ToLocal(&js_value));
+  Local<Array> js_array2 = Local<Array>::Cast(js_value);
+  EXPECT_EQ(3u, js_array2->Length());
   for (size_t i = 0; i < expected.size(); ++i) {
     EXPECT_TRUE(Integer::New(instance_->isolate(), expected[i])
-                    ->StrictEquals(js_array->Get(static_cast<int>(i))));
+                    ->StrictEquals(js_array2->Get(static_cast<int>(i))));
   }
-
-  std::vector<int> actual;
-  EXPECT_TRUE(Converter<std::vector<int> >::FromV8(instance_->isolate(),
-                                                   js_array, &actual));
-  EXPECT_EQ(expected, actual);
 }
 
 }  // namespace gin
diff --git a/gin/dictionary.h b/gin/dictionary.h
@@ -32,13 +32,23 @@ class GIN_EXPORT Dictionary {
 
   template<typename T>
   bool Get(const std::string& key, T* out) {
-    v8::Local<v8::Value> val = object_->Get(StringToV8(isolate_, key));
+    v8::Local<v8::Value> val;
+    if (!object_->Get(isolate_->GetCurrentContext(), StringToV8(isolate_, key))
+             .ToLocal(&val)) {
+      return false;
+    }
     return ConvertFromV8(isolate_, val, out);
   }
 
   template<typename T>
   bool Set(const std::string& key, T val) {
-    return object_->Set(StringToV8(isolate_, key), ConvertToV8(isolate_, val));
+    v8::Local<v8::Value> v8_value;
+    if (!TryConvertToV8(isolate_, val, &v8_value))
+      return false;
+    v8::Maybe<bool> result =
+        object_->Set(isolate_->GetCurrentContext(), StringToV8(isolate_, key),
+                    v8_value);
+    return !result.IsNothing() && result.FromJust();
   }
 
   v8::Isolate* isolate() const { return isolate_; }

diff --git a/gin/interceptor_unittest.cc b/gin/interceptor_unittest.cc
@@ -139,7 +139,7 @@ class InterceptorTest : public V8Test {
     v8::Local<v8::String> source = StringToV8(isolate, script_source);
     EXPECT_FALSE(source.IsEmpty());
 
-    gin::TryCatch try_catch;
+    gin::TryCatch try_catch(isolate);
     v8::Local<v8::Script> script = v8::Script::Compile(source);
     EXPECT_FALSE(script.IsEmpty());
     v8::Local<v8::Value> val = script->Run();

diff --git a/gin/modules/console.cc b/gin/modules/console.cc
@@ -43,7 +43,7 @@ v8::Local<v8::Value> Console::GetModule(v8::Isolate* isolate) {
         .Build();
     data->SetObjectTemplate(&g_wrapper_info, templ);
   }
-  return templ->NewInstance();
+  return templ->NewInstance(isolate->GetCurrentContext()).ToLocalChecked();
 }
 
 }  // namespace gin