[gin] Make Converter<std::vector<T>> return v8::Local if Converter<T> does.

For consistency between the methods, have all converters take v8::Isolate*,
as all of the ones that presently take v8::Local<v8::Context> implicitly
assume it to be the current context anyhow.

Bug: None
Change-Id: I565fb54bdc7a1d5ac1796ef5d7c1c69ca83290bb
Reviewed-on: https://chromium-review.googlesource.com/876563
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532473}

gin/BUILD.gn

@@ -158,6 +158,8 @@ test("gin_unittests") {
     ":gin_shell",
     ":gin_test",
     "//base/test:test_support",
+    "//testing/gmock",
+    "//testing/gtest",
     "//v8",
   ]
 

gin/arguments_unittest.cc

@@ -74,8 +74,7 @@ TEST_F(ArgumentsTest, TestGetAll) {
 
   V8List list1 = {
       gin::ConvertToV8(isolate, 1), gin::StringToV8(isolate, "some string"),
-      gin::ConvertToV8(context, std::vector<double>({2.0, 3.0}))
-          .ToLocalChecked(),
+      gin::ConvertToV8(isolate, std::vector<double>({2.0, 3.0})),
   };
   bool called1 = false;
 

gin/converter.h

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <string>
+#include <type_traits>
 #include <vector>
 
 #include "base/logging.h"
@@ -163,15 +164,24 @@ struct GIN_EXPORT Converter<v8::Local<v8::Value> > {
 
 template<typename T>
 struct Converter<std::vector<T> > {
-  static v8::MaybeLocal<v8::Value> ToV8(v8::Local<v8::Context> context,
-                                        const std::vector<T>& val) {
-    v8::Isolate* isolate = context->GetIsolate();
+  static std::conditional_t<ToV8ReturnsMaybe<T>::value,
+                            v8::MaybeLocal<v8::Value>,
+                            v8::Local<v8::Value>>
+  ToV8(v8::Isolate* isolate, const std::vector<T>& val) {
+    v8::Local<v8::Context> context = isolate->GetCurrentContext();
     v8::Local<v8::Array> result(
         v8::Array::New(isolate, static_cast<int>(val.size())));
     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>();
+      v8::MaybeLocal<v8::Value> maybe = Converter<T>::ToV8(isolate, val[i]);
+      v8::Local<v8::Value> element;
+      if (!maybe.ToLocal(&element))
+        return {};
+      bool property_created;
+      if (!result->CreateDataProperty(context, i, element)
+               .To(&property_created) ||
+          !property_created) {
+        NOTREACHED() << "CreateDataProperty should always succeed here.";
+      }
     }
     return result;
   }
@@ -202,20 +212,18 @@ struct Converter<std::vector<T> > {
 
 template<typename T>
 struct ToV8ReturnsMaybe<std::vector<T>> {
-  static const bool value = true;
+  static const bool value = ToV8ReturnsMaybe<T>::value;
 };
 
 // Convenience functions that deduce T.
-template<typename T>
-v8::Local<v8::Value> ConvertToV8(v8::Isolate* isolate, T input) {
+template <typename T>
+std::conditional_t<ToV8ReturnsMaybe<T>::value,
+                   v8::MaybeLocal<v8::Value>,
+                   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>

gin/converter_unittest.cc

@@ -9,8 +9,11 @@
 #include <stdint.h>
 
 #include "base/compiler_specific.h"
+#include "gin/handle.h"
 #include "gin/public/isolate_holder.h"
 #include "gin/test/v8_test.h"
+#include "gin/wrappable.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "v8/include/v8.h"
 
@@ -116,16 +119,58 @@ TEST_F(ConverterTest, Vector) {
   expected.push_back(0);
   expected.push_back(1);
 
-  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());
+  auto js_array =
+      Converter<std::vector<int>>::ToV8(instance_->isolate(), expected)
+          .As<Array>();
+  EXPECT_EQ(3u, js_array->Length());
   for (size_t i = 0; i < expected.size(); ++i) {
     EXPECT_TRUE(Integer::New(instance_->isolate(), expected[i])
-                    ->StrictEquals(js_array2->Get(static_cast<int>(i))));
+                    ->StrictEquals(js_array->Get(static_cast<int>(i))));
   }
 }
 
+TEST_F(ConverterTest, VectorOfVectors) {
+  HandleScope handle_scope(instance_->isolate());
+
+  std::vector<std::vector<int>> vector_of_vectors = {
+      {1, 2, 3}, {4, 5, 6},
+  };
+
+  v8::Local<v8::Value> v8_value =
+      ConvertToV8(instance_->isolate(), vector_of_vectors);
+  std::vector<std::vector<int>> out_value;
+  ASSERT_TRUE(ConvertFromV8(instance_->isolate(), v8_value, &out_value));
+  EXPECT_THAT(out_value, testing::ContainerEq(vector_of_vectors));
+}
+
+namespace {
+
+class MyObject : public Wrappable<MyObject> {
+ public:
+  static WrapperInfo kWrapperInfo;
+
+  static gin::Handle<MyObject> Create(v8::Isolate* isolate) {
+    return CreateHandle(isolate, new MyObject());
+  }
+};
+
+WrapperInfo MyObject::kWrapperInfo = {kEmbedderNativeGin};
+
+}  // namespace
+
+TEST_F(ConverterTest, VectorOfWrappables) {
+  v8::Isolate* isolate = instance_->isolate();
+  v8::HandleScope handle_scope(isolate);
+
+  Handle<MyObject> obj = MyObject::Create(isolate);
+  std::vector<MyObject*> vector = {obj.get()};
+  v8::MaybeLocal<v8::Value> maybe = ConvertToV8(isolate, vector);
+  v8::Local<v8::Value> array;
+  ASSERT_TRUE(maybe.ToLocal(&array));
+
+  std::vector<MyObject*> out_value;
+  ASSERT_TRUE(ConvertFromV8(isolate, array, &out_value));
+  EXPECT_THAT(out_value, testing::ContainerEq(vector));
+}
+
 }  // namespace gin

gin/interceptor_unittest.cc

@@ -151,7 +151,7 @@ class InterceptorTest : public V8Test {
     v8::Local<v8::Function> func;
     EXPECT_TRUE(ConvertFromV8(isolate, val, &func));
     v8::Local<v8::Value> argv[] = {
-        ConvertToV8(isolate->GetCurrentContext(), obj.get()).ToLocalChecked(),
+        ConvertToV8(isolate, obj.get()).ToLocalChecked(),
     };
     func->Call(v8::Undefined(isolate), 1, argv);
     EXPECT_FALSE(try_catch.HasCaught());

gin/wrappable.h

@@ -115,10 +115,9 @@ template <typename T>
 struct Converter<T*,
                  typename std::enable_if<
                      std::is_convertible<T*, WrappableBase*>::value>::type> {
-  static v8::MaybeLocal<v8::Value> ToV8(v8::Local<v8::Context> context,
-                                        T* val) {
+  static v8::MaybeLocal<v8::Value> ToV8(v8::Isolate* isolate, T* val) {
     v8::Local<v8::Object> wrapper;
-    if (!val->GetWrapper(context->GetIsolate()).ToLocal(&wrapper))
+    if (!val->GetWrapper(isolate).ToLocal(&wrapper))
       return v8::MaybeLocal<v8::Value>();
     return v8::MaybeLocal<v8::Value>(wrapper);
   }

gin/wrappable_unittest.cc

@@ -74,7 +74,7 @@ TEST_F(WrappableTest, WrapAndUnwrap) {
   Handle<MyObject> obj = MyObject::Create(isolate);
 
   v8::Local<v8::Value> wrapper =
-      ConvertToV8(isolate->GetCurrentContext(), obj.get()).ToLocalChecked();
+      ConvertToV8(isolate, obj.get()).ToLocalChecked();
   EXPECT_FALSE(wrapper.IsEmpty());
 
   MyObject* unwrapped = NULL;
@@ -99,8 +99,7 @@ TEST_F(WrappableTest, UnwrapFailures) {
 
   // An object that's wrapping a C++ object of the wrong type.
   thing.Clear();
-  thing = ConvertToV8(isolate->GetCurrentContext(), new MyObject2())
-              .ToLocalChecked();
+  thing = ConvertToV8(isolate, new MyObject2()).ToLocalChecked();
   EXPECT_FALSE(ConvertFromV8(isolate, thing, &unwrapped));
   EXPECT_FALSE(unwrapped);
 }
@@ -128,7 +127,7 @@ TEST_F(WrappableTest, GetAndSetProperty) {
   v8::Local<v8::Function> func;
   EXPECT_TRUE(ConvertFromV8(isolate, val, &func));
   v8::Local<v8::Value> argv[] = {
-      ConvertToV8(isolate->GetCurrentContext(), obj.get()).ToLocalChecked(),
+      ConvertToV8(isolate, obj.get()).ToLocalChecked(),
   };
   func->Call(v8::Undefined(isolate), 1, argv);
   EXPECT_FALSE(try_catch.HasCaught());