From 232ca8af398c08aae5337bdc06d83efb9ac30362 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Wed, 18 Mar 2020 12:57:08 -0700 Subject: [PATCH] refactor: EventEmitters without gin_helper (#22726) --- filenames.gni | 4 + lib/browser/api/web-contents.js | 4 - lib/browser/init.ts | 3 + patches/chromium/.patches | 1 + ...ecttemplate_to_objecttemplatebuilder.patch | 43 ++++++++++ shell/browser/api/electron_api_debugger.cc | 61 ++++++--------- shell/browser/api/electron_api_debugger.h | 19 +++-- .../browser/api/electron_api_event_emitter.cc | 45 +++++++++++ .../browser/api/electron_api_event_emitter.h | 21 +++++ shell/browser/event_emitter_mixin.cc | 45 +++++++++++ shell/browser/event_emitter_mixin.h | 78 +++++++++++++++++++ shell/common/node_bindings.cc | 2 +- 12 files changed, 277 insertions(+), 49 deletions(-) create mode 100644 patches/chromium/gin_allow_passing_an_objecttemplate_to_objecttemplatebuilder.patch create mode 100644 shell/browser/api/electron_api_event_emitter.cc create mode 100644 shell/browser/api/electron_api_event_emitter.h create mode 100644 shell/browser/event_emitter_mixin.cc create mode 100644 shell/browser/event_emitter_mixin.h diff --git a/filenames.gni b/filenames.gni index d011f5290756c..2941d16985d1b 100644 --- a/filenames.gni +++ b/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", diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index a77c23d858e68..5dc0f887218ae 100644 --- a/lib/browser/api/web-contents.js +++ b/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 = {}) { diff --git a/lib/browser/init.ts b/lib/browser/init.ts index 958ec3a16a88d..3ac4b5e1397b1 100644 --- a/lib/browser/init.ts +++ b/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. diff --git a/patches/chromium/.patches b/patches/chromium/.patches index a2a61548792a9..d0aada4bd8a98 100644 --- a/patches/chromium/.patches +++ b/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 diff --git a/patches/chromium/gin_allow_passing_an_objecttemplate_to_objecttemplatebuilder.patch b/patches/chromium/gin_allow_passing_an_objecttemplate_to_objecttemplatebuilder.patch new file mode 100644 index 0000000000000..f7fcf9a487c28 --- /dev/null +++ b/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 +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 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 tmpl); + ObjectTemplateBuilder(const ObjectTemplateBuilder& other); + ~ObjectTemplateBuilder(); + diff --git a/shell/browser/api/electron_api_debugger.cc b/shell/browser/api/electron_api_debugger.cc index 0f25f775763de..bbbe9452ca3ec 100644 --- a/shell/browser/api/electron_api_debugger.cc +++ b/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 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(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 Debugger::SendCommand(gin_helper::Arguments* args) { - gin_helper::Promise promise(isolate()); +v8::Local Debugger::SendCommand(gin::Arguments* args) { + v8::Isolate* isolate = args->isolate(); + gin_helper::Promise promise(isolate); v8::Local handle = promise.GetHandle(); if (!agent_host_) { @@ -177,36 +180,20 @@ gin::Handle Debugger::Create(v8::Isolate* isolate, return gin::CreateHandle(isolate, new Debugger(isolate, web_contents)); } -// static -void Debugger::BuildPrototype(v8::Isolate* isolate, - v8::Local prototype) { - prototype->SetClassName(gin::StringToV8(isolate, "Debugger")); - gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) +gin::ObjectTemplateBuilder Debugger::GetObjectTemplateBuilder( + v8::Isolate* isolate) { + return gin_helper::EventEmitterMixin::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 exports, - v8::Local unused, - v8::Local 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 diff --git a/shell/browser/api/electron_api_debugger.h b/shell/browser/api/electron_api_debugger.h index ca6550bd2441f..170df89b45066 100644 --- a/shell/browser/api/electron_api_debugger.h +++ b/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, +class Debugger : public gin::Wrappable, + public gin_helper::EventEmitterMixin, public content::DevToolsAgentHostClient, public content::WebContentsObserver { public: static gin::Handle Create(v8::Isolate* isolate, content::WebContents* web_contents); - // gin_helper::TrackableObject: - static void BuildPrototype(v8::Isolate* isolate, - v8::Local 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, using PendingRequestMap = std::map>; - void Attach(gin_helper::Arguments* args); + void Attach(gin::Arguments* args); bool IsAttached(); void Detach(); - v8::Local SendCommand(gin_helper::Arguments* args); + v8::Local SendCommand(gin::Arguments* args); void ClearPendingRequests(); content::WebContents* web_contents_; // Weak Reference. diff --git a/shell/browser/api/electron_api_event_emitter.cc b/shell/browser/api/electron_api_event_emitter.cc new file mode 100644 index 0000000000000..40dfbe9655920 --- /dev/null +++ b/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 event_emitter_prototype; + +void SetEventEmitterPrototype(v8::Isolate* isolate, + v8::Local proto) { + event_emitter_prototype.Reset(isolate, proto); +} + +void Initialize(v8::Local exports, + v8::Local unused, + v8::Local context, + void* priv) { + v8::Isolate* isolate = context->GetIsolate(); + + gin::Dictionary dict(isolate, exports); + dict.Set("setEventEmitterPrototype", + base::BindRepeating(&SetEventEmitterPrototype)); +} + +} // namespace + +namespace electron { + +v8::Local 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) diff --git a/shell/browser/api/electron_api_event_emitter.h b/shell/browser/api/electron_api_event_emitter.h new file mode 100644 index 0000000000000..ab20c7c5fb8fe --- /dev/null +++ b/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 +class Local; +class Object; +class Isolate; +} // namespace v8 + +namespace electron { + +v8::Local GetEventEmitterPrototype(v8::Isolate* isolate); + +} // namespace electron + +#endif // SHELL_BROWSER_API_ELECTRON_API_EVENT_EMITTER_H_ diff --git a/shell/browser/event_emitter_mixin.cc b/shell/browser/event_emitter_mixin.cc new file mode 100644 index 0000000000000..f13201c64a171 --- /dev/null +++ b/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 GetEventEmitterTemplate(v8::Isolate* isolate) { + gin::PerIsolateData* data = gin::PerIsolateData::From(isolate); + v8::Local tmpl = + data->GetFunctionTemplate(&kWrapperInfo); + + if (tmpl.IsEmpty()) { + tmpl = v8::FunctionTemplate::New(isolate); + v8::Local context = isolate->GetCurrentContext(); + v8::Local func = tmpl->GetFunction(context).ToLocalChecked(); + + v8::Local eventemitter_prototype = + electron::GetEventEmitterPrototype(isolate); + + v8::Local func_prototype; + CHECK(func->Get(context, gin::StringToSymbol(isolate, "prototype")) + .ToLocal(&func_prototype)); + + CHECK(func_prototype.As() + ->SetPrototype(context, eventemitter_prototype) + .ToChecked()); + + data->SetFunctionTemplate(&kWrapperInfo, tmpl); + } + + return tmpl; +} + +} // namespace internal + +} // namespace gin_helper diff --git a/shell/browser/event_emitter_mixin.h b/shell/browser/event_emitter_mixin.h new file mode 100644 index 0000000000000..8e4ac680c4d01 --- /dev/null +++ b/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 + +#include "gin/object_template_builder.h" +#include "shell/common/gin_helper/event_emitter.h" + +namespace gin_helper { + +namespace internal { +v8::Local GetEventEmitterTemplate(v8::Isolate* isolate); +} // namespace internal + +template +class EventEmitterMixin { + public: + // this.emit(name, new Event(), args...); + template + bool Emit(base::StringPiece name, Args&&... args) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Locker locker(isolate); + v8::HandleScope handle_scope(isolate); + v8::Local wrapper; + if (!static_cast(this)->GetWrapper(isolate).ToLocal(&wrapper)) + return false; + v8::Local event = internal::CreateEvent(isolate, wrapper); + return EmitWithEvent(isolate, wrapper, name, event, + std::forward(args)...); + } + + protected: + EventEmitterMixin() = default; + + gin::ObjectTemplateBuilder GetObjectTemplateBuilder(v8::Isolate* isolate) { + gin::PerIsolateData* data = gin::PerIsolateData::From(isolate); + auto* wrapper_info = &(static_cast(this)->kWrapperInfo); + v8::Local 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(this)->GetTypeName(), + constructor->InstanceTemplate()); + } + + private: + // this.emit(name, event, args...); + template + static bool EmitWithEvent(v8::Isolate* isolate, + v8::Local wrapper, + base::StringPiece name, + v8::Local event, + Args&&... args) { + auto context = isolate->GetCurrentContext(); + gin_helper::EmitEvent(isolate, wrapper, name, event, + std::forward(args)...); + v8::Local 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_ diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index 78ad21d18adb4..b18bca67c89ba 100644 --- a/shell/common/node_bindings.cc +++ b/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) \