[Extensions Bindings] Add a registerEventArgumentMassager

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
index 2391607e67ab5955be1ae634c9df4379cd6c6252..bf08fc054d65b7d4957d79a6c60c4d86b946ca55 100644
--- a/extensions/renderer/api_event_handler.cc
+++ b/extensions/renderer/api_event_handler.cc
@@ -33,7 +33,7 @@ struct APIEventPerContextData : public base::SupportsUserData::Data {
     // v8 objects. This helps us avoid cycles in v8 where an event listener
     // could hold a reference to the event, which in turn holds the reference
     // to the listener.
-    for (const auto& pair : event_data) {
+    for (const auto& pair : emitters) {
       EventEmitter* emitter = nullptr;
       gin::Converter<EventEmitter*>::FromV8(
           isolate, pair.second.Get(isolate), &emitter);
@@ -47,9 +47,59 @@ struct APIEventPerContextData : public base::SupportsUserData::Data {
   v8::Isolate* isolate;
 
   // A map from event name -> event emitter.
-  std::map<std::string, v8::Global<v8::Object>> event_data;
+  std::map<std::string, v8::Global<v8::Object>> emitters;
+
+  // A map from event name -> argument massager.
+  std::map<std::string, v8::Global<v8::Function>> massagers;
 };
 
+APIEventPerContextData* GetContextData(v8::Local<v8::Context> context,
+                                       bool should_create) {
+  gin::PerContextData* per_context_data = gin::PerContextData::From(context);
+  if (!per_context_data)
+    return nullptr;
+  auto* data = static_cast<APIEventPerContextData*>(
+      per_context_data->GetUserData(kExtensionAPIEventPerContextKey));
+
+  if (!data && should_create) {
+    auto api_data =
+        base::MakeUnique<APIEventPerContextData>(context->GetIsolate());
+    data = api_data.get();
+    per_context_data->SetUserData(kExtensionAPIEventPerContextKey,
+                                  api_data.release());
+  }
+
+  return data;
+}
+
+void DispatchEvent(const v8::FunctionCallbackInfo<v8::Value>& info) {
+  v8::Isolate* isolate = info.GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+  if (info.Length() != 1 || !info[0]->IsArray()) {
+    NOTREACHED();
+    return;
+  }
+
+  v8::Local<v8::Context> context = isolate->GetCurrentContext();
+  APIEventPerContextData* data = GetContextData(context, false);
+  DCHECK(data);
+  std::string event_name = gin::V8ToString(info.Data());
+  auto iter = data->emitters.find(event_name);
+  if (iter == data->emitters.end())
+    return;
+  v8::Global<v8::Object>& v8_emitter = iter->second;
+
+  std::vector<v8::Local<v8::Value>> args;
+  CHECK(gin::Converter<std::vector<v8::Local<v8::Value>>>::FromV8(
+      isolate, info[0], &args));
+
+  EventEmitter* emitter = nullptr;
+  gin::Converter<EventEmitter*>::FromV8(isolate, v8_emitter.Get(isolate),
+                                        &emitter);
+  CHECK(emitter);
+  emitter->Fire(context, &args);
+}
+
 }  // namespace
 
 APIEventHandler::APIEventHandler(
@@ -67,19 +117,8 @@ v8::Local<v8::Object> APIEventHandler::CreateEventInstance(
   // context directly.
   v8::Context::Scope context_scope(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>(context->GetIsolate());
-    data = api_data.get();
-    per_context_data->SetUserData(kExtensionAPIEventPerContextKey,
-                                  api_data.release());
-  }
-
-  DCHECK(data->event_data.find(event_name) == data->event_data.end());
+  APIEventPerContextData* data = GetContextData(context, true);
+  DCHECK(data->emitters.find(event_name) == data->emitters.end());
 
   gin::Handle<EventEmitter> emitter_handle = gin::CreateHandle(
       context->GetIsolate(),
@@ -89,7 +128,7 @@ v8::Local<v8::Object> APIEventHandler::CreateEventInstance(
   CHECK(emitter_value->IsObject());
   v8::Local<v8::Object> emitter_object =
       v8::Local<v8::Object>::Cast(emitter_value);
-  data->event_data[event_name] =
+  data->emitters[event_name] =
       v8::Global<v8::Object>(context->GetIsolate(), emitter_object);
 
   return emitter_object;
@@ -98,46 +137,76 @@ v8::Local<v8::Object> APIEventHandler::CreateEventInstance(
 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));
+  APIEventPerContextData* data = GetContextData(context, false);
   if (!data)
     return;
 
-  auto iter = data->event_data.find(event_name);
-  if (iter == data->event_data.end())
+  auto emitter_iter = data->emitters.find(event_name);
+  if (emitter_iter == data->emitters.end())
+    return;
+
+  EventEmitter* emitter = nullptr;
+  gin::Converter<EventEmitter*>::FromV8(
+      context->GetIsolate(), emitter_iter->second.Get(context->GetIsolate()),
+      &emitter);
+  CHECK(emitter);
+
+  if (emitter->listeners()->empty())
     return;
 
   // Note: since we only convert the arguments once, if a listener modifies an
   // object (including an array), other listeners will see that modification.
   // TODO(devlin): This is how it's always been, but should it be?
-  std::vector<v8::Local<v8::Value>> v8_args;
-  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));
 
-  EventEmitter* emitter = nullptr;
-  gin::Converter<EventEmitter*>::FromV8(
-      context->GetIsolate(), iter->second.Get(context->GetIsolate()), &emitter);
-  CHECK(emitter);
+  auto massager_iter = data->massagers.find(event_name);
+  if (massager_iter == data->massagers.end()) {
+    std::vector<v8::Local<v8::Value>> v8_args;
+    v8_args.reserve(args.GetSize());
+    for (const auto& arg : args)
+      v8_args.push_back(converter->ToV8Value(arg.get(), context));
+    emitter->Fire(context, &v8_args);
+  } else {
+    v8::Isolate* isolate = context->GetIsolate();
+    v8::HandleScope handle_scope(isolate);
+    v8::Local<v8::Function> massager = massager_iter->second.Get(isolate);
+    v8::Local<v8::Value> v8_args = converter->ToV8Value(&args, context);
+    DCHECK(!v8_args.IsEmpty());
+    DCHECK(v8_args->IsArray());
+
+    // Curry in the native dispatch function. Some argument massagers take
+    // extra liberties and call this asynchronously, so we can't just have the
+    // massager return a modified array of arguments.
+    // We don't store this in a template because the Data (event name) is
+    // different for each instance. Luckily, this is called during dispatching
+    // an event, rather than e.g. at initialization time.
+    v8::Local<v8::Function> dispatch_event = v8::Function::New(
+        isolate, &DispatchEvent, gin::StringToSymbol(isolate, event_name));
+
+    v8::Local<v8::Value> massager_args[] = {v8_args, dispatch_event};
+    call_js_.Run(massager, context, arraysize(massager_args), massager_args);
+  }
+}
 
-  emitter->Fire(context, &v8_args);
+void APIEventHandler::RegisterArgumentMassager(
+    v8::Local<v8::Context> context,
+    const std::string& event_name,
+    v8::Local<v8::Function> massager) {
+  APIEventPerContextData* data = GetContextData(context, true);
+  DCHECK(data->massagers.find(event_name) == data->massagers.end());
+  data->massagers[event_name] =

[jbroman, 2017/03/01 22:33:35]

nit: could use this if you prefer (slightly shorter):

data->massagers[event_name].Reset(context->GetIsolate(), massager);

[Devlin, 2017/03/02 17:19:26]

Sure, done.

I sometimes wonder which is faster under the hood, though - the former could in
theory be optimized by the compiler to construct the new global in-place in the
map with the proper value, the shorter Reset() version means that we first have
to construct an empty global, then call reset on it to initialize it to the
proper state.  It seems, to me, like that would be slower.

Of course, all of this is basically setting a pointer, so... not that important.
 But interesting to think about. :)

[jbroman, 2017/03/02 21:12:40]

From a language perspective, the compiler is required to call the assignment
operator here (and the STL called the default constructor earlier). After
inlining, Reset() actually has slightly less code (e.g. the move approach
requires writing the destructor for Persistent Base, etc.). That said, I'd
expect a compiler to generate nearly identical code (which calls the function to
make a global handle and then sets a pointer) either way, so I think you should
feel free to choose whichever looks better. :)

+      v8::Global<v8::Function>(context->GetIsolate(), massager);
 }
 
 size_t APIEventHandler::GetNumEventListenersForTesting(
     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));
+  APIEventPerContextData* data = GetContextData(context, false);
   DCHECK(data);
 
-  auto iter = data->event_data.find(event_name);
-  DCHECK(iter != data->event_data.end());
+  auto iter = data->emitters.find(event_name);
+  DCHECK(iter != data->emitters.end());
   EventEmitter* emitter = nullptr;
   gin::Converter<EventEmitter*>::FromV8(
       context->GetIsolate(), iter->second.Get(context->GetIsolate()), &emitter);