ui: Cleanup class/struct forward declarations

extensions/renderer/api_binding_unittest.cc

Index: extensions/renderer/api_binding_unittest.cc
diff --git a/extensions/renderer/api_binding_unittest.cc b/extensions/renderer/api_binding_unittest.cc
index e231d8864b9f5ccd5cbbc1b1c585e36266ac8305..ede48cb1bf051899fa80d5c54ffe90a0b1670fce 100644
--- a/extensions/renderer/api_binding_unittest.cc
+++ b/extensions/renderer/api_binding_unittest.cc
@@ -10,6 +10,7 @@
 #include "content/public/child/v8_value_converter.h"
 #include "extensions/renderer/api_binding.h"
 #include "extensions/renderer/api_binding_test_util.h"
+#include "extensions/renderer/api_event_handler.h"
 #include "extensions/renderer/api_request_handler.h"
 #include "gin/converter.h"
 #include "gin/public/context_holder.h"
@@ -131,8 +132,36 @@ class APIBindingTest : public gin::V8Test {
 
   void TearDown() override {
     request_handler_.reset();
+
+    v8::Global<v8::Context> weak_context(instance_->isolate(), context_);
+    weak_context.SetWeak();
+
     holder_.reset();
-    gin::V8Test::TearDown();
+
+    // NOTE: We explicitly do NOT call gin::V8Test::TearDown() here because we
+    // do intermittent validation by doing a garbage collection after context
+    // destruction and ensuring the context is fully released (which wouldn't
+    // happen in cycles).
+    // TODO(devlin): It might be time to move off V8Test if we're doing this.
+    {
+      v8::HandleScope handle_scope(instance_->isolate());
+      v8::Local<v8::Context>::New(instance_->isolate(), context_)->Exit();
+      context_.Reset();
+    }
+
+    // Garbage collect everything so that we find any issues where we might be
+    // double-freeing.
+    // '5' is a magic number stolen from Blink; arbitrarily large enough to
+    // hopefully clean up all the various paths.
+    for (int i = 0; i < 5; i++) {
+      instance_->isolate()->RequestGarbageCollectionForTesting(
+          v8::Isolate::kFullGarbageCollection);
+    }
+
+    ASSERT_TRUE(weak_context.IsEmpty());
+
+    instance_->isolate()->Exit();
+    instance_.reset();
   }
 
   void ExpectPass(v8::Local<v8::Object> object,
@@ -202,7 +231,7 @@ TEST_F(APIBindingTest, Test) {
   ASSERT_TRUE(functions);
   ArgumentSpec::RefMap refs;
   APIBinding binding(
-      "test", *functions, nullptr,
+      "test", *functions, nullptr, nullptr,
       base::Bind(&APIBindingTest::OnFunctionCall, base::Unretained(this)),
       &refs);
   EXPECT_TRUE(refs.empty());
@@ -213,8 +242,11 @@ TEST_F(APIBindingTest, Test) {
   v8::Local<v8::Context> context =
       v8::Local<v8::Context>::New(isolate, context_);
 
+  APIEventHandler event_handler(
+      base::Bind(&RunFunctionOnGlobalAndIgnoreResult));
   v8::Local<v8::Object> binding_object =
-      binding.CreateInstance(context, isolate, base::Bind(&AllowAllAPIs));
+      binding.CreateInstance(context, isolate, &event_handler,
+                             base::Bind(&AllowAllAPIs));
 
   ExpectPass(binding_object, "obj.oneString('foo');", "['foo']");
   ExpectPass(binding_object, "obj.oneString('');", "['']");
@@ -304,7 +336,7 @@ TEST_F(APIBindingTest, TypeRefsTest) {
   ASSERT_TRUE(types);
   ArgumentSpec::RefMap refs;
   APIBinding binding(
-      "test", *functions, types.get(),
+      "test", *functions, types.get(), nullptr,
       base::Bind(&APIBindingTest::OnFunctionCall, base::Unretained(this)),
       &refs);
   EXPECT_EQ(2u, refs.size());
@@ -317,8 +349,11 @@ TEST_F(APIBindingTest, TypeRefsTest) {
   v8::Local<v8::Context> context =
       v8::Local<v8::Context>::New(isolate, context_);
 
+  APIEventHandler event_handler(
+      base::Bind(&RunFunctionOnGlobalAndIgnoreResult));
   v8::Local<v8::Object> binding_object =
-      binding.CreateInstance(context, isolate, base::Bind(&AllowAllAPIs));
+      binding.CreateInstance(context, isolate, &event_handler,
+                             base::Bind(&AllowAllAPIs));
 
   ExpectPass(binding_object, "obj.takesRefObj({prop1: 'foo'})",
              "[{'prop1':'foo'}]");
@@ -351,7 +386,7 @@ TEST_F(APIBindingTest, RestrictedAPIs) {
   ASSERT_TRUE(functions);
   ArgumentSpec::RefMap refs;
   APIBinding binding(
-      "test", *functions, nullptr,
+      "test", *functions, nullptr, nullptr,
       base::Bind(&APIBindingTest::OnFunctionCall, base::Unretained(this)),
       &refs);
 
@@ -368,8 +403,11 @@ TEST_F(APIBindingTest, RestrictedAPIs) {
     return name == "test.allowedOne" || name == "test.allowedTwo";
   };
 
+  APIEventHandler event_handler(
+      base::Bind(&RunFunctionOnGlobalAndIgnoreResult));
   v8::Local<v8::Object> binding_object =
-      binding.CreateInstance(context, isolate, base::Bind(is_available));
+      binding.CreateInstance(context, isolate, &event_handler,
+                             base::Bind(is_available));
 
   auto is_defined = [&binding_object, context](const std::string& name) {
     v8::Local<v8::Value> val =
@@ -384,4 +422,48 @@ TEST_F(APIBindingTest, RestrictedAPIs) {
   EXPECT_FALSE(is_defined("restrictedTwo"));
 }
 
+// Tests that events specified in the API are created as properties of the API
+// object.
+TEST_F(APIBindingTest, TestEventCreation) {
+  const char kEvents[] = "[{'name': 'onFoo'}, {'name': 'onBar'}]";
+  std::unique_ptr<base::ListValue> events = ListValueFromString(kEvents);
+  ASSERT_TRUE(events);
+  std::unique_ptr<base::ListValue> functions = ListValueFromString(kFunctions);
+  ASSERT_TRUE(functions);
+  ArgumentSpec::RefMap refs;
+  APIBinding binding(
+      "test", *functions, nullptr, events.get(),
+      base::Bind(&APIBindingTest::OnFunctionCall, base::Unretained(this)),
+      &refs);
+
+  v8::Isolate* isolate = instance_->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Context> context =
+      v8::Local<v8::Context>::New(isolate, context_);
+
+  APIEventHandler event_handler(
+      base::Bind(&RunFunctionOnGlobalAndIgnoreResult));
+  v8::Local<v8::Object> binding_object =
+      binding.CreateInstance(context, isolate, &event_handler,
+                             base::Bind(&AllowAllAPIs));
+
+  // Event behavior is tested in the APIEventHandler unittests as well as the
+  // APIBindingsSystem tests, so we really only need to check that the events
+  // are being initialized on the object.
+  v8::Maybe<bool> has_on_foo =
+      binding_object->Has(context, gin::StringToV8(isolate, "onFoo"));
+  EXPECT_TRUE(has_on_foo.IsJust());
+  EXPECT_TRUE(has_on_foo.FromJust());
+
+  v8::Maybe<bool> has_on_bar =
+      binding_object->Has(context, gin::StringToV8(isolate, "onBar"));
+  EXPECT_TRUE(has_on_bar.IsJust());
+  EXPECT_TRUE(has_on_bar.FromJust());
+
+  v8::Maybe<bool> has_on_baz =
+      binding_object->Has(context, gin::StringToV8(isolate, "onBaz"));
+  EXPECT_TRUE(has_on_baz.IsJust());
+  EXPECT_FALSE(has_on_baz.FromJust());
+}
+
 }  // namespace extensions