[Extensions Bindings] Add Events support

extensions/renderer/api_event_handler.cc

Index: extensions/renderer/api_event_handler.cc
diff --git a/extensions/renderer/api_event_handler.cc b/extensions/renderer/api_event_handler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..de1ba714d054e39ba897b42e8e264af793b08feb
--- /dev/null
+++ b/extensions/renderer/api_event_handler.cc
@@ -0,0 +1,99 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "extensions/renderer/api_event_handler.h"
+
+#include <algorithm>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/values.h"
+#include "content/public/child/v8_value_converter.h"
+#include "gin/arguments.h"
+#include "gin/per_context_data.h"
+
+namespace extensions {
+
+namespace {
+
+const char kExtensionAPIEventPerContextKey[] = "extension_api_events";
+
+}  // namespace
+
+APIEventHandler::APIEventPerContextData::APIEventPerContextData() = default;
+APIEventHandler::APIEventPerContextData::~APIEventPerContextData() = default;
+
+APIEventHandler::APIEventHandler(const binding::RunJSFunction& call_js)
+    : call_js_(call_js), weak_factory_(this) {}
+APIEventHandler::~APIEventHandler() {}
+
+v8::Local<v8::Object> APIEventHandler::CreateEventInstance(
+    const std::string& event_name,
+    v8::Local<v8::Context> context) {
+  gin::PerContextData* per_context_data = gin::PerContextData::From(context);
+  DCHECK(per_context_data);
+  APIEventPerContextData* data = static_cast<APIEventPerContextData*>(
+      per_context_data->GetUserData(kExtensionAPIEventPerContextKey));
+  if (!data) {
+    auto api_data = base::MakeUnique<APIEventPerContextData>();
+    data = api_data.get();
+    per_context_data->SetUserData(kExtensionAPIEventPerContextKey,
+                                  api_data.release());
+  }
+
+  DCHECK(data->event_data.find(event_name) == data->event_data.end());
+
+  auto emitter = base::MakeUnique<EventEmitter>();
+  v8::Local<v8::Object> event_object =
+      emitter->GetWrapper(context->GetIsolate());
+  data->event_data.insert(std::make_pair(event_name, std::move(emitter)));

[Devlin, 2016/11/02 00:48:29]

Most places instead seem to do:

gin::CreateHandle(isolate, new EventEmitter())

However, it's unclear to me how the EventEmitter would be deleted in that case
(if it would).  If it wouldn't, then this is fine (and in my mind preferable,
since it ensures we don't leak the listeners).  If it would, then we have a
double-free that's not detected from the tests, and I'm confused. :)

[jbroman, 2016/11/02 19:50:14]

It's perhaps not obvious from the comment in gin/wrappable.h, but gin::Wrappable
objects are owned by their V8 wrapper objects. In particular, they register
themselves so that when their V8 wrapper is collected, they delete themselves.
As a result, keeping a unique_ptr around is actually a double-free. I expect
your unit tests simply aren't running a GC.

[Devlin, 2016/11/02 22:02:21]

is there a way to run a gc in a unittest so we can add it?  (Also addressed
double-free; see other comment.)

[jbroman, 2016/11/03 19:28:04]

Yup,
isolate->RequestGarbageCollectionForTesting(v8::Isolate::kFullGarbageCollection).
Blink calls it 5 times (to make sure paths that require multiple GCs to collect
are dealt with).

see, e.g.,
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

[Devlin, 2016/11/04 17:59:29]

Nifty, thanks!

+
+  return event_object;
+}
+
+void APIEventHandler::FireEventInContext(const std::string& event_name,
+                                         v8::Local<v8::Context> context,
+                                         const base::ListValue& args) {
+  gin::PerContextData* per_context_data = gin::PerContextData::From(context);
+  DCHECK(per_context_data);
+  APIEventPerContextData* data = static_cast<APIEventPerContextData*>(
+      per_context_data->GetUserData(kExtensionAPIEventPerContextKey));
+  if (!data)
+    return;
+
+  auto iter = data->event_data.find(event_name);
+  if (iter == data->event_data.end())
+    return;
+
+  EventEmitter::Listeners* listeners = iter->second->listeners();
+
+  std::vector<v8::Local<v8::Value>> v8_args;

[jbroman, 2016/11/02 19:50:14]

Just confirming that this is the behaviour you want (seems fine to me): if any
of these is an object (including an array), one callback could modify it, and
those modifications would be visible to subsequent callbacks in the same
context.

[Devlin, 2016/11/02 22:02:21]

Good call!  I've checked and this is what we're currently doing, and given it's
within the same context (and thus the same extension), I think this is probably
fine.  But I've added a comment to make it clear.

[jbroman, 2016/11/03 19:28:04]

Thanks!

+  v8_args.reserve(args.GetSize());
+  std::unique_ptr<content::V8ValueConverter> converter(
+      content::V8ValueConverter::create());
+  for (const auto& arg : args)
+    v8_args.push_back(converter->ToV8Value(arg.get(), context));
+
+  for (auto& listener : *listeners) {

[jbroman, 2016/11/02 19:50:14]

nit: const auto&

[Devlin, 2016/11/02 22:02:21]

Done.

+    call_js_.Run(listener.Get(context->GetIsolate()), context, v8_args.size(),
+                 v8_args.data());
+  }
+}
+
+const EventEmitter::Listeners&
+APIEventHandler::GetEventListenersForTesting(const std::string& event_name,
+                                             v8::Local<v8::Context> context) {
+  gin::PerContextData* per_context_data = gin::PerContextData::From(context);
+  DCHECK(per_context_data);
+  APIEventPerContextData* data = static_cast<APIEventPerContextData*>(
+      per_context_data->GetUserData(kExtensionAPIEventPerContextKey));
+  DCHECK(data);
+
+  auto iter = data->event_data.find(event_name);
+  DCHECK(iter != data->event_data.end());
+  return *iter->second->listeners();
+}
+
+}  // namespace extensions