Browse Source

refactor: EventEmitters without gin_helper (#22726)

Jeremy Apthorp 5 years ago
parent
commit
232ca8af39

+ 4 - 0
filenames.gni

@@ -59,6 +59,8 @@ filenames = {
     "shell/browser/api/electron_api_download_item.cc",
     "shell/browser/api/electron_api_download_item.h",
     "shell/browser/api/electron_api_event.cc",
+    "shell/browser/api/electron_api_event_emitter.cc",
+    "shell/browser/api/electron_api_event_emitter.h",
     "shell/browser/api/electron_api_global_shortcut.cc",
     "shell/browser/api/electron_api_global_shortcut.h",
     "shell/browser/api/electron_api_in_app_purchase.cc",
@@ -175,6 +177,8 @@ filenames = {
     "shell/browser/electron_speech_recognition_manager_delegate.h",
     "shell/browser/electron_web_ui_controller_factory.cc",
     "shell/browser/electron_web_ui_controller_factory.h",
+    "shell/browser/event_emitter_mixin.cc",
+    "shell/browser/event_emitter_mixin.h",
     "shell/browser/feature_list.cc",
     "shell/browser/feature_list.h",
     "shell/browser/font_defaults.cc",

+ 0 - 4
lib/browser/api/web-contents.js

@@ -578,10 +578,6 @@ WebContents.prototype._init = function () {
   })
 }
 
-// JavaScript wrapper of Debugger.
-const { Debugger } = process.electronBinding('debugger')
-Object.setPrototypeOf(Debugger.prototype, EventEmitter.prototype)
-
 // Public APIs.
 module.exports = {
   create (options = {}) {

+ 3 - 0
lib/browser/init.ts

@@ -1,4 +1,5 @@
 import { Buffer } from 'buffer'
+import { EventEmitter } from 'events'
 import * as fs from 'fs'
 import * as path from 'path'
 import * as util from 'util'
@@ -15,6 +16,8 @@ require('../common/reset-search-paths')
 // Import common settings.
 require('@electron/internal/common/init')
 
+process.electronBinding('event_emitter').setEventEmitterPrototype(EventEmitter.prototype)
+
 if (process.platform === 'win32') {
   // Redirect node's console to use our own implementations, since node can not
   // handle console output when running as GUI program.

+ 1 - 0
patches/chromium/.patches

@@ -92,3 +92,4 @@ delay_lock_the_protocol_scheme_registry.patch
 gpu_notify_when_dxdiag_request_fails.patch
 feat_allow_embedders_to_add_observers_on_created_hunspell.patch
 feat_add_onclose_to_messageport.patch
+gin_allow_passing_an_objecttemplate_to_objecttemplatebuilder.patch

+ 43 - 0
patches/chromium/gin_allow_passing_an_objecttemplate_to_objecttemplatebuilder.patch

@@ -0,0 +1,43 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Jeremy Apthorp <[email protected]>
+Date: Tue, 17 Mar 2020 12:51:02 -0700
+Subject: gin: allow passing an ObjectTemplate to ObjectTemplateBuilder
+
+This allows us to pass an InstanceTemplate from a FunctionTemplate we
+construct ourselves. In particular, this means we can customize the
+prototype chain of the object.
+
+diff --git a/gin/object_template_builder.cc b/gin/object_template_builder.cc
+index 543cd694e7becec72c2e48b9f235ad02da777df8..dbc753ce357b4c18a3180528dcb3aafb86041428 100644
+--- a/gin/object_template_builder.cc
++++ b/gin/object_template_builder.cc
+@@ -143,11 +143,15 @@ void IndexedPropertyEnumerator(
+ ObjectTemplateBuilder::ObjectTemplateBuilder(v8::Isolate* isolate)
+     : ObjectTemplateBuilder(isolate, nullptr) {}
+ 
++ObjectTemplateBuilder::ObjectTemplateBuilder(v8::Isolate* isolate, const char *type_name)
++    : ObjectTemplateBuilder(isolate, nullptr, v8::ObjectTemplate::New(isolate)) {}
++
+ ObjectTemplateBuilder::ObjectTemplateBuilder(v8::Isolate* isolate,
+-                                             const char* type_name)
++                                             const char* type_name,
++                                             v8::Local<v8::ObjectTemplate> tmpl)
+     : isolate_(isolate),
+       type_name_(type_name),
+-      template_(v8::ObjectTemplate::New(isolate)) {
++      template_(tmpl) {
+   template_->SetInternalFieldCount(kNumberOfInternalFields);
+ }
+ 
+diff --git a/gin/object_template_builder.h b/gin/object_template_builder.h
+index 6fb331cbbfe242cb871edc5eec8ab8e620e0a17d..5ac479026eb040414961fb45913c91c74f2488dc 100644
+--- a/gin/object_template_builder.h
++++ b/gin/object_template_builder.h
+@@ -46,6 +46,7 @@ class GIN_EXPORT ObjectTemplateBuilder {
+  public:
+   explicit ObjectTemplateBuilder(v8::Isolate* isolate);
+   ObjectTemplateBuilder(v8::Isolate* isolate, const char* type_name);
++  ObjectTemplateBuilder(v8::Isolate* isolate, const char* type_name, v8::Local<v8::ObjectTemplate> tmpl);
+   ObjectTemplateBuilder(const ObjectTemplateBuilder& other);
+   ~ObjectTemplateBuilder();
+ 

+ 24 - 37
shell/browser/api/electron_api_debugger.cc

@@ -12,9 +12,9 @@
 #include "base/json/json_writer.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/web_contents.h"
+#include "gin/object_template_builder.h"
+#include "gin/per_isolate_data.h"
 #include "shell/common/gin_converters/value_converter.h"
-#include "shell/common/gin_helper/dictionary.h"
-#include "shell/common/gin_helper/object_template_builder.h"
 #include "shell/common/node_includes.h"
 
 using content::DevToolsAgentHost;
@@ -23,10 +23,10 @@ namespace electron {
 
 namespace api {
 
+gin::WrapperInfo Debugger::kWrapperInfo = {gin::kEmbedderNativeGin};
+
 Debugger::Debugger(v8::Isolate* isolate, content::WebContents* web_contents)
-    : content::WebContentsObserver(web_contents), web_contents_(web_contents) {
-  Init(isolate);
-}
+    : content::WebContentsObserver(web_contents), web_contents_(web_contents) {}
 
 Debugger::~Debugger() = default;
 
@@ -41,8 +41,10 @@ void Debugger::DispatchProtocolMessage(DevToolsAgentHost* agent_host,
                                        base::span<const uint8_t> message) {
   DCHECK(agent_host == agent_host_);
 
-  v8::Locker locker(isolate());
-  v8::HandleScope handle_scope(isolate());
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+
+  v8::Locker locker(isolate);
+  v8::HandleScope handle_scope(isolate);
 
   base::StringPiece message_str(reinterpret_cast<const char*>(message.data()),
                                 message.size());
@@ -96,24 +98,24 @@ void Debugger::RenderFrameHostChanged(content::RenderFrameHost* old_rfh,
   }
 }
 
-void Debugger::Attach(gin_helper::Arguments* args) {
+void Debugger::Attach(gin::Arguments* args) {
   std::string protocol_version;
   args->GetNext(&protocol_version);
 
   if (agent_host_) {
-    args->ThrowError("Debugger is already attached to the target");
+    args->ThrowTypeError("Debugger is already attached to the target");
     return;
   }
 
   if (!protocol_version.empty() &&
       !DevToolsAgentHost::IsSupportedProtocolVersion(protocol_version)) {
-    args->ThrowError("Requested protocol version is not supported");
+    args->ThrowTypeError("Requested protocol version is not supported");
     return;
   }
 
   agent_host_ = DevToolsAgentHost::GetOrCreateFor(web_contents_);
   if (!agent_host_) {
-    args->ThrowError("No target available");
+    args->ThrowTypeError("No target available");
     return;
   }
 
@@ -131,8 +133,9 @@ void Debugger::Detach() {
   AgentHostClosed(agent_host_.get());
 }
 
-v8::Local<v8::Promise> Debugger::SendCommand(gin_helper::Arguments* args) {
-  gin_helper::Promise<base::DictionaryValue> promise(isolate());
+v8::Local<v8::Promise> Debugger::SendCommand(gin::Arguments* args) {
+  v8::Isolate* isolate = args->isolate();
+  gin_helper::Promise<base::DictionaryValue> promise(isolate);
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
   if (!agent_host_) {
@@ -177,36 +180,20 @@ gin::Handle<Debugger> Debugger::Create(v8::Isolate* isolate,
   return gin::CreateHandle(isolate, new Debugger(isolate, web_contents));
 }
 
-// static
-void Debugger::BuildPrototype(v8::Isolate* isolate,
-                              v8::Local<v8::FunctionTemplate> prototype) {
-  prototype->SetClassName(gin::StringToV8(isolate, "Debugger"));
-  gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
+gin::ObjectTemplateBuilder Debugger::GetObjectTemplateBuilder(
+    v8::Isolate* isolate) {
+  return gin_helper::EventEmitterMixin<Debugger>::GetObjectTemplateBuilder(
+             isolate)
       .SetMethod("attach", &Debugger::Attach)
       .SetMethod("isAttached", &Debugger::IsAttached)
       .SetMethod("detach", &Debugger::Detach)
       .SetMethod("sendCommand", &Debugger::SendCommand);
 }
 
-}  // namespace api
-
-}  // namespace electron
-
-namespace {
-
-using electron::api::Debugger;
-
-void Initialize(v8::Local<v8::Object> exports,
-                v8::Local<v8::Value> unused,
-                v8::Local<v8::Context> context,
-                void* priv) {
-  v8::Isolate* isolate = context->GetIsolate();
-  gin_helper::Dictionary(isolate, exports)
-      .Set("Debugger", Debugger::GetConstructor(isolate)
-                           ->GetFunction(context)
-                           .ToLocalChecked());
+const char* Debugger::GetTypeName() {
+  return "Debugger";
 }
 
-}  // namespace
+}  // namespace api
 
-NODE_LINKED_MODULE_CONTEXT_AWARE(electron_browser_debugger, Initialize)
+}  // namespace electron

+ 12 - 7
shell/browser/api/electron_api_debugger.h

@@ -12,9 +12,11 @@
 #include "base/values.h"
 #include "content/public/browser/devtools_agent_host_client.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "gin/arguments.h"
 #include "gin/handle.h"
+#include "gin/wrappable.h"
+#include "shell/browser/event_emitter_mixin.h"
 #include "shell/common/gin_helper/promise.h"
-#include "shell/common/gin_helper/trackable_object.h"
 
 namespace content {
 class DevToolsAgentHost;
@@ -25,16 +27,19 @@ namespace electron {
 
 namespace api {
 
-class Debugger : public gin_helper::TrackableObject<Debugger>,
+class Debugger : public gin::Wrappable<Debugger>,
+                 public gin_helper::EventEmitterMixin<Debugger>,
                  public content::DevToolsAgentHostClient,
                  public content::WebContentsObserver {
  public:
   static gin::Handle<Debugger> Create(v8::Isolate* isolate,
                                       content::WebContents* web_contents);
 
-  // gin_helper::TrackableObject:
-  static void BuildPrototype(v8::Isolate* isolate,
-                             v8::Local<v8::FunctionTemplate> prototype);
+  // gin::Wrappable
+  static gin::WrapperInfo kWrapperInfo;
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
+      v8::Isolate* isolate) override;
+  const char* GetTypeName() override;
 
  protected:
   Debugger(v8::Isolate* isolate, content::WebContents* web_contents);
@@ -53,10 +58,10 @@ class Debugger : public gin_helper::TrackableObject<Debugger>,
   using PendingRequestMap =
       std::map<int, gin_helper::Promise<base::DictionaryValue>>;
 
-  void Attach(gin_helper::Arguments* args);
+  void Attach(gin::Arguments* args);
   bool IsAttached();
   void Detach();
-  v8::Local<v8::Promise> SendCommand(gin_helper::Arguments* args);
+  v8::Local<v8::Promise> SendCommand(gin::Arguments* args);
   void ClearPendingRequests();
 
   content::WebContents* web_contents_;  // Weak Reference.

+ 45 - 0
shell/browser/api/electron_api_event_emitter.cc

@@ -0,0 +1,45 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/browser/api/electron_api_event_emitter.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "gin/dictionary.h"
+#include "shell/common/gin_converters/callback_converter.h"
+#include "shell/common/node_includes.h"
+#include "v8/include/v8.h"
+
+namespace {
+
+v8::Global<v8::Object> event_emitter_prototype;
+
+void SetEventEmitterPrototype(v8::Isolate* isolate,
+                              v8::Local<v8::Object> proto) {
+  event_emitter_prototype.Reset(isolate, proto);
+}
+
+void Initialize(v8::Local<v8::Object> exports,
+                v8::Local<v8::Value> unused,
+                v8::Local<v8::Context> context,
+                void* priv) {
+  v8::Isolate* isolate = context->GetIsolate();
+
+  gin::Dictionary dict(isolate, exports);
+  dict.Set("setEventEmitterPrototype",
+           base::BindRepeating(&SetEventEmitterPrototype));
+}
+
+}  // namespace
+
+namespace electron {
+
+v8::Local<v8::Object> GetEventEmitterPrototype(v8::Isolate* isolate) {
+  CHECK(!event_emitter_prototype.IsEmpty());
+  return event_emitter_prototype.Get(isolate);
+}
+
+}  // namespace electron
+
+NODE_LINKED_MODULE_CONTEXT_AWARE(electron_browser_event_emitter, Initialize)

+ 21 - 0
shell/browser/api/electron_api_event_emitter.h

@@ -0,0 +1,21 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_BROWSER_API_ELECTRON_API_EVENT_EMITTER_H_
+#define SHELL_BROWSER_API_ELECTRON_API_EVENT_EMITTER_H_
+
+namespace v8 {
+template <typename T>
+class Local;
+class Object;
+class Isolate;
+}  // namespace v8
+
+namespace electron {
+
+v8::Local<v8::Object> GetEventEmitterPrototype(v8::Isolate* isolate);
+
+}  // namespace electron
+
+#endif  // SHELL_BROWSER_API_ELECTRON_API_EVENT_EMITTER_H_

+ 45 - 0
shell/browser/event_emitter_mixin.cc

@@ -0,0 +1,45 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/browser/event_emitter_mixin.h"
+
+#include "gin/public/wrapper_info.h"
+#include "shell/browser/api/electron_api_event_emitter.h"
+
+namespace gin_helper {
+
+namespace internal {
+
+gin::WrapperInfo kWrapperInfo = {gin::kEmbedderNativeGin};
+
+v8::Local<v8::FunctionTemplate> GetEventEmitterTemplate(v8::Isolate* isolate) {
+  gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
+  v8::Local<v8::FunctionTemplate> tmpl =
+      data->GetFunctionTemplate(&kWrapperInfo);
+
+  if (tmpl.IsEmpty()) {
+    tmpl = v8::FunctionTemplate::New(isolate);
+    v8::Local<v8::Context> context = isolate->GetCurrentContext();
+    v8::Local<v8::Function> func = tmpl->GetFunction(context).ToLocalChecked();
+
+    v8::Local<v8::Object> eventemitter_prototype =
+        electron::GetEventEmitterPrototype(isolate);
+
+    v8::Local<v8::Value> func_prototype;
+    CHECK(func->Get(context, gin::StringToSymbol(isolate, "prototype"))
+              .ToLocal(&func_prototype));
+
+    CHECK(func_prototype.As<v8::Object>()
+              ->SetPrototype(context, eventemitter_prototype)
+              .ToChecked());
+
+    data->SetFunctionTemplate(&kWrapperInfo, tmpl);
+  }
+
+  return tmpl;
+}
+
+}  // namespace internal
+
+}  // namespace gin_helper

+ 78 - 0
shell/browser/event_emitter_mixin.h

@@ -0,0 +1,78 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_BROWSER_EVENT_EMITTER_MIXIN_H_
+#define SHELL_BROWSER_EVENT_EMITTER_MIXIN_H_
+
+#include <utility>
+
+#include "gin/object_template_builder.h"
+#include "shell/common/gin_helper/event_emitter.h"
+
+namespace gin_helper {
+
+namespace internal {
+v8::Local<v8::FunctionTemplate> GetEventEmitterTemplate(v8::Isolate* isolate);
+}  // namespace internal
+
+template <typename T>
+class EventEmitterMixin {
+ public:
+  // this.emit(name, new Event(), args...);
+  template <typename... Args>
+  bool Emit(base::StringPiece name, Args&&... args) {
+    v8::Isolate* isolate = v8::Isolate::GetCurrent();
+    v8::Locker locker(isolate);
+    v8::HandleScope handle_scope(isolate);
+    v8::Local<v8::Object> wrapper;
+    if (!static_cast<T*>(this)->GetWrapper(isolate).ToLocal(&wrapper))
+      return false;
+    v8::Local<v8::Object> event = internal::CreateEvent(isolate, wrapper);
+    return EmitWithEvent(isolate, wrapper, name, event,
+                         std::forward<Args>(args)...);
+  }
+
+ protected:
+  EventEmitterMixin() = default;
+
+  gin::ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate) {
+    gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
+    auto* wrapper_info = &(static_cast<T*>(this)->kWrapperInfo);
+    v8::Local<v8::FunctionTemplate> constructor =
+        data->GetFunctionTemplate(wrapper_info);
+    if (constructor.IsEmpty()) {
+      constructor = v8::FunctionTemplate::New(isolate);
+      constructor->Inherit(internal::GetEventEmitterTemplate(isolate));
+      data->SetFunctionTemplate(wrapper_info, constructor);
+    }
+    return gin::ObjectTemplateBuilder(isolate,
+                                      static_cast<T*>(this)->GetTypeName(),
+                                      constructor->InstanceTemplate());
+  }
+
+ private:
+  // this.emit(name, event, args...);
+  template <typename... Args>
+  static bool EmitWithEvent(v8::Isolate* isolate,
+                            v8::Local<v8::Object> wrapper,
+                            base::StringPiece name,
+                            v8::Local<v8::Object> event,
+                            Args&&... args) {
+    auto context = isolate->GetCurrentContext();
+    gin_helper::EmitEvent(isolate, wrapper, name, event,
+                          std::forward<Args>(args)...);
+    v8::Local<v8::Value> defaultPrevented;
+    if (event->Get(context, gin::StringToV8(isolate, "defaultPrevented"))
+            .ToLocal(&defaultPrevented)) {
+      return defaultPrevented->BooleanValue(isolate);
+    }
+    return false;
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(EventEmitterMixin);
+};
+
+}  // namespace gin_helper
+
+#endif  // SHELL_BROWSER_EVENT_EMITTER_MIXIN_H_

+ 1 - 1
shell/common/node_bindings.cc

@@ -37,10 +37,10 @@
   V(electron_browser_auto_updater)       \
   V(electron_browser_browser_view)       \
   V(electron_browser_content_tracing)    \
-  V(electron_browser_debugger)           \
   V(electron_browser_dialog)             \
   V(electron_browser_download_item)      \
   V(electron_browser_event)              \
+  V(electron_browser_event_emitter)      \
   V(electron_browser_global_shortcut)    \
   V(electron_browser_in_app_purchase)    \
   V(electron_browser_menu)               \