From 6159066c261a286a2e7ac4bb3cff3800c0a6d2b3 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Thu, 2 Apr 2020 16:07:56 -0700 Subject: [PATCH] refactor: ginify Menu (#22916) --- lib/browser/api/menu.js | 70 +++++---- patches/chromium/.patches | 1 + .../gin_forward_args_when_dispatching.patch | 94 ++++++++++++ .../api/electron_api_desktop_capturer.cc | 9 +- shell/browser/api/electron_api_menu.cc | 134 ++++++++---------- shell/browser/api/electron_api_menu.h | 42 +++--- shell/browser/api/electron_api_menu_mac.mm | 7 +- shell/browser/api/electron_api_menu_views.cc | 7 +- .../api/electron_api_top_level_window.cc | 1 - .../common/gin_helper/event_emitter_caller.cc | 2 +- .../common/gin_helper/event_emitter_caller.h | 6 +- shell/common/gin_helper/wrappable.h | 2 +- spec-main/ambient.d.ts | 6 +- spec-main/api-menu-item-spec.ts | 10 +- 14 files changed, 241 insertions(+), 150 deletions(-) create mode 100644 patches/chromium/gin_forward_args_when_dispatching.patch diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 2a1102e9a8a1..71e58f116f45 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -10,43 +10,51 @@ const { Menu } = bindings; let applicationMenu = null; let groupIdIndex = 0; -Object.setPrototypeOf(Menu.prototype, EventEmitter.prototype); - -// Menu Delegate. -// This object should hold no reference to |Menu| to avoid cyclic reference. -const delegate = { - isCommandIdChecked: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].checked : undefined, - isCommandIdEnabled: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].enabled : undefined, - shouldCommandIdWorkWhenHidden: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].acceleratorWorksWhenHidden : undefined, - isCommandIdVisible: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].visible : undefined, - getAcceleratorForCommandId: (menu, id, useDefaultAccelerator) => { - const command = menu.commandsMap[id]; - if (!command) return; - if (command.accelerator != null) return command.accelerator; - if (useDefaultAccelerator) return command.getDefaultRoleAccelerator(); - }, - shouldRegisterAcceleratorForCommandId: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].registerAccelerator : undefined, - executeCommand: (menu, event, id) => { - const command = menu.commandsMap[id]; - if (!command) return; - command.click(event, TopLevelWindow.getFocusedWindow(), webContents.getFocusedWebContents()); - }, - menuWillShow: (menu) => { - // Ensure radio groups have at least one menu item selected - for (const id of Object.keys(menu.groupsMap)) { - const found = menu.groupsMap[id].find(item => item.checked) || null; - if (!found) v8Util.setHiddenValue(menu.groupsMap[id][0], 'checked', true); - } - } -}; - /* Instance Methods */ Menu.prototype._init = function () { this.commandsMap = {}; this.groupsMap = {}; this.items = []; - this.delegate = delegate; +}; + +Menu.prototype._isCommandIdChecked = function (id) { + return this.commandsMap[id] ? this.commandsMap[id].checked : undefined; +}; + +Menu.prototype._isCommandIdEnabled = function (id) { + return this.commandsMap[id] ? this.commandsMap[id].enabled : undefined; +}; +Menu.prototype._shouldCommandIdWorkWhenHidden = function (id) { + return this.commandsMap[id] ? this.commandsMap[id].acceleratorWorksWhenHidden : undefined; +}; +Menu.prototype._isCommandIdVisible = function (id) { + return this.commandsMap[id] ? this.commandsMap[id].visible : undefined; +}; + +Menu.prototype._getAcceleratorForCommandId = function (id, useDefaultAccelerator) { + const command = this.commandsMap[id]; + if (!command) return; + if (command.accelerator != null) return command.accelerator; + if (useDefaultAccelerator) return command.getDefaultRoleAccelerator(); +}; + +Menu.prototype._shouldRegisterAcceleratorForCommandId = function (id) { + return this.commandsMap[id] ? this.commandsMap[id].registerAccelerator : undefined; +}; + +Menu.prototype._executeCommand = function (event, id) { + const command = this.commandsMap[id]; + if (!command) return; + command.click(event, TopLevelWindow.getFocusedWindow(), webContents.getFocusedWebContents()); +}; + +Menu.prototype._menuWillShow = function () { + // Ensure radio groups have at least one menu item selected + for (const id of Object.keys(this.groupsMap)) { + const found = this.groupsMap[id].find(item => item.checked) || null; + if (!found) v8Util.setHiddenValue(this.groupsMap[id][0], 'checked', true); + } }; Menu.prototype.popup = function (options = {}) { diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 99ac2206dd9b..edfb9ea8ee53 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -93,4 +93,5 @@ 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 +gin_forward_args_when_dispatching.patch fix_undo_redo_broken_in_webviews.patch diff --git a/patches/chromium/gin_forward_args_when_dispatching.patch b/patches/chromium/gin_forward_args_when_dispatching.patch new file mode 100644 index 000000000000..7d8c2b6f2880 --- /dev/null +++ b/patches/chromium/gin_forward_args_when_dispatching.patch @@ -0,0 +1,94 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Jeremy Apthorp +Date: Wed, 1 Apr 2020 01:06:45 +0000 +Subject: gin: forward args when dispatching + +This allows passing arguments with move-only semantics. + +Change-Id: I852eb343398e6f763abfe2a44e623f28353d79a5 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129029 +Commit-Queue: Jeremy Apthorp +Reviewed-by: Jeremy Roman +Auto-Submit: Jeremy Apthorp +Cr-Commit-Position: refs/heads/master@{#755196} + +diff --git a/gin/converter_unittest.cc b/gin/converter_unittest.cc +index 6f9205f06a3e4747e70149c440c4548e54045001..162572385bdd8a54612eb64414fe80e0f9d619d1 100644 +--- a/gin/converter_unittest.cc ++++ b/gin/converter_unittest.cc +@@ -12,6 +12,7 @@ + #include "base/stl_util.h" + #include "base/strings/string16.h" + #include "base/strings/utf_string_conversions.h" ++#include "gin/function_template.h" + #include "gin/handle.h" + #include "gin/public/isolate_holder.h" + #include "gin/test/v8_test.h" +@@ -224,4 +225,45 @@ TEST_F(ConverterTest, VectorOfWrappables) { + EXPECT_THAT(out_value2, testing::ContainerEq(vector)); + } + ++namespace { ++ ++class MoveOnlyObject { ++ public: ++ MoveOnlyObject() = default; ++ MoveOnlyObject(const MoveOnlyObject&) = delete; ++ MoveOnlyObject& operator=(const MoveOnlyObject&) = delete; ++ ++ MoveOnlyObject(MoveOnlyObject&&) noexcept = default; ++ MoveOnlyObject& operator=(MoveOnlyObject&&) noexcept = default; ++}; ++ ++} // namespace ++ ++template <> ++struct Converter { ++ static v8::Local ToV8(v8::Isolate* isolate, MoveOnlyObject in) { ++ return v8::Undefined(isolate); ++ } ++ static bool FromV8(v8::Isolate* isolate, ++ v8::Local val, ++ MoveOnlyObject* out) { ++ *out = MoveOnlyObject(); ++ return true; ++ } ++}; ++ ++TEST_F(ConverterTest, MoveOnlyParameters) { ++ v8::Isolate* isolate = instance_->isolate(); ++ v8::HandleScope handle_scope(isolate); ++ ++ auto receives_move_only_obj = [](MoveOnlyObject obj) {}; ++ auto func_templ = gin::CreateFunctionTemplate( ++ isolate, base::BindRepeating(receives_move_only_obj)); ++ ++ v8::Local context = instance_->isolate()->GetCurrentContext(); ++ auto func = func_templ->GetFunction(context).ToLocalChecked(); ++ v8::Local argv[] = {v8::Undefined(isolate)}; ++ func->Call(context, v8::Undefined(isolate), 1, argv).ToLocalChecked(); ++} ++ + } // namespace gin +diff --git a/gin/function_template.h b/gin/function_template.h +index 7edcc9e20dfa6367dde5f58237cca5f4ca637f7a..8c641d934fdeebb9a90f6eb49960e4fe06217913 100644 +--- a/gin/function_template.h ++++ b/gin/function_template.h +@@ -166,14 +166,15 @@ class Invoker, ArgTypes...> + template + void DispatchToCallback( + base::RepeatingCallback callback) { +- args_->Return(callback.Run(ArgumentHolder::value...)); ++ args_->Return( ++ callback.Run(std::move(ArgumentHolder::value)...)); + } + + // In C++, you can declare the function foo(void), but you can't pass a void + // expression to foo. As a result, we must specialize the case of Callbacks + // that have the void return type. + void DispatchToCallback(base::RepeatingCallback callback) { +- callback.Run(ArgumentHolder::value...); ++ callback.Run(std::move(ArgumentHolder::value)...); + } + + private: diff --git a/shell/browser/api/electron_api_desktop_capturer.cc b/shell/browser/api/electron_api_desktop_capturer.cc index 0a0657b5e920..6731d75d039b 100644 --- a/shell/browser/api/electron_api_desktop_capturer.cc +++ b/shell/browser/api/electron_api_desktop_capturer.cc @@ -155,6 +155,9 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) { // |media_list_sources|. if (!webrtc::DxgiDuplicatorController::Instance()->GetDeviceNames( &device_names)) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Locker locker(isolate); + v8::HandleScope scope(isolate); gin_helper::CallMethod(this, "_onerror", "Failed to get sources."); return; } @@ -184,9 +187,13 @@ void DesktopCapturer::UpdateSourcesList(DesktopMediaList* list) { std::back_inserter(captured_sources_)); } - if (!capture_window_ && !capture_screen_) + if (!capture_window_ && !capture_screen_) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Locker locker(isolate); + v8::HandleScope scope(isolate); gin_helper::CallMethod(this, "_onfinished", captured_sources_, fetch_window_icons_); + } } // static diff --git a/shell/browser/api/electron_api_menu.cc b/shell/browser/api/electron_api_menu.cc index 5308a4c928bd..62091e921bdd 100644 --- a/shell/browser/api/electron_api_menu.cc +++ b/shell/browser/api/electron_api_menu.cc @@ -16,21 +16,13 @@ #include "shell/common/gin_helper/object_template_builder.h" #include "shell/common/node_includes.h" -namespace { - -// We need this map to keep references to currently opened menus. -// Without this menus would be destroyed by js garbage collector -// even when they are still displayed. -std::map> g_menus; - -} // unnamed namespace - namespace electron { namespace api { +gin::WrapperInfo Menu::kWrapperInfo = {gin::kEmbedderNativeGin}; + Menu::Menu(gin::Arguments* args) : model_(new ElectronMenuModel(this)) { - InitWithArgs(args); model_->AddObserver(this); } @@ -40,74 +32,82 @@ Menu::~Menu() { } } -void Menu::AfterInit(v8::Isolate* isolate) { - gin::Dictionary wrappable(isolate, GetWrapper()); - gin::Dictionary delegate(nullptr); - if (!wrappable.Get("delegate", &delegate)) - return; - - delegate.Get("isCommandIdChecked", &is_checked_); - delegate.Get("isCommandIdEnabled", &is_enabled_); - delegate.Get("isCommandIdVisible", &is_visible_); - delegate.Get("shouldCommandIdWorkWhenHidden", &works_when_hidden_); - delegate.Get("getAcceleratorForCommandId", &get_accelerator_); - delegate.Get("shouldRegisterAcceleratorForCommandId", - &should_register_accelerator_); - delegate.Get("executeCommand", &execute_command_); - delegate.Get("menuWillShow", &menu_will_show_); +bool InvokeBoolMethod(const Menu* menu, + const char* method, + int command_id, + bool default_value = false) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::HandleScope scope(isolate); + // We need to cast off const here because GetWrapper() is non-const, but + // ui::SimpleMenuModel::Delegate's methods are const. + v8::Local val = gin_helper::CallMethod( + isolate, const_cast(menu), method, command_id); + bool ret = false; + return gin::ConvertFromV8(isolate, val, &ret) ? ret : default_value; } bool Menu::IsCommandIdChecked(int command_id) const { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - return is_checked_.Run(GetWrapper(), command_id); + return InvokeBoolMethod(this, "_isCommandIdChecked", command_id); } bool Menu::IsCommandIdEnabled(int command_id) const { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - return is_enabled_.Run(GetWrapper(), command_id); + return InvokeBoolMethod(this, "_isCommandIdEnabled", command_id); } bool Menu::IsCommandIdVisible(int command_id) const { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - return is_visible_.Run(GetWrapper(), command_id); + return InvokeBoolMethod(this, "_isCommandIdVisible", command_id); } bool Menu::ShouldCommandIdWorkWhenHidden(int command_id) const { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - return works_when_hidden_.Run(GetWrapper(), command_id); + return InvokeBoolMethod(this, "_shouldCommandIdWorkWhenHidden", command_id); } bool Menu::GetAcceleratorForCommandIdWithParams( int command_id, bool use_default_accelerator, ui::Accelerator* accelerator) const { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - v8::Local val = - get_accelerator_.Run(GetWrapper(), command_id, use_default_accelerator); - return gin::ConvertFromV8(isolate(), val, accelerator); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::HandleScope scope(isolate); + v8::Local val = gin_helper::CallMethod( + isolate, const_cast(this), "_getAcceleratorForCommandId", + command_id, use_default_accelerator); + return gin::ConvertFromV8(isolate, val, accelerator); } bool Menu::ShouldRegisterAcceleratorForCommandId(int command_id) const { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - return should_register_accelerator_.Run(GetWrapper(), command_id); + return InvokeBoolMethod(this, "_shouldRegisterAcceleratorForCommandId", + command_id); } void Menu::ExecuteCommand(int command_id, int flags) { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - execute_command_.Run(GetWrapper(), CreateEventFromFlags(flags), command_id); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::HandleScope scope(isolate); + gin_helper::CallMethod(isolate, const_cast(this), "_executeCommand", + CreateEventFromFlags(flags), command_id); } void Menu::OnMenuWillShow(ui::SimpleMenuModel* source) { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - menu_will_show_.Run(GetWrapper()); + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::HandleScope scope(isolate); + gin_helper::CallMethod(isolate, const_cast(this), "_menuWillShow"); +} + +base::OnceClosure Menu::BindSelfToClosure(base::OnceClosure callback) { + // return ((callback, ref) => { callback() }).bind(null, callback, this) + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Locker locker(isolate); + v8::HandleScope scope(isolate); + v8::Local self; + if (GetWrapper(isolate).ToLocal(&self)) { + v8::Global ref(isolate, self); + return base::BindOnce( + [](base::OnceClosure callback, v8::Global ref) { + std::move(callback).Run(); + }, + std::move(callback), std::move(ref)); + } else { + return base::DoNothing(); + } } void Menu::InsertItemAt(int index, @@ -208,32 +208,20 @@ bool Menu::WorksWhenHiddenAt(int index) const { } void Menu::OnMenuWillClose() { - g_menus.erase(weak_map_id()); + Unpin(); Emit("menu-will-close"); } void Menu::OnMenuWillShow() { - v8::HandleScope scope(isolate()); - g_menus[weak_map_id()] = v8::Global(isolate(), GetWrapper()); + Pin(v8::Isolate::GetCurrent()); Emit("menu-will-show"); } -base::OnceClosure Menu::BindSelfToClosure(base::OnceClosure callback) { - // return ((callback, ref) => { callback() }).bind(null, callback, this) - v8::Global ref(isolate(), GetWrapper()); - return base::BindOnce( - [](base::OnceClosure callback, v8::Global ref) { - std::move(callback).Run(); - }, - std::move(callback), std::move(ref)); -} - // static -void Menu::BuildPrototype(v8::Isolate* isolate, - v8::Local prototype) { - prototype->SetClassName(gin::StringToV8(isolate, "Menu")); - gin_helper::Destroyable::MakeDestroyable(isolate, prototype); - gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) +v8::Local Menu::FillObjectTemplate( + v8::Isolate* isolate, + v8::Local templ) { + return gin::ObjectTemplateBuilder(isolate, "Menu", templ) .SetMethod("insertItem", &Menu::InsertItemAt) .SetMethod("insertCheckItem", &Menu::InsertCheckItemAt) .SetMethod("insertRadioItem", &Menu::InsertRadioItemAt) @@ -256,7 +244,8 @@ void Menu::BuildPrototype(v8::Isolate* isolate, .SetMethod("worksWhenHiddenAt", &Menu::WorksWhenHiddenAt) .SetMethod("isVisibleAt", &Menu::IsVisibleAt) .SetMethod("popupAt", &Menu::PopupAt) - .SetMethod("closePopupAt", &Menu::ClosePopupAt); + .SetMethod("closePopupAt", &Menu::ClosePopupAt) + .Build(); } } // namespace api @@ -272,12 +261,9 @@ void Initialize(v8::Local exports, v8::Local context, void* priv) { v8::Isolate* isolate = context->GetIsolate(); - Menu::SetConstructor(isolate, base::BindRepeating(&Menu::New)); gin_helper::Dictionary dict(isolate, exports); - dict.Set( - "Menu", - Menu::GetConstructor(isolate)->GetFunction(context).ToLocalChecked()); + dict.Set("Menu", Menu::GetConstructor(context)); #if defined(OS_MACOSX) dict.SetMethod("setApplicationMenu", &Menu::SetApplicationMenu); dict.SetMethod("sendActionToFirstResponder", diff --git a/shell/browser/api/electron_api_menu.h b/shell/browser/api/electron_api_menu.h index b617267a4c65..110d06dd965e 100644 --- a/shell/browser/api/electron_api_menu.h +++ b/shell/browser/api/electron_api_menu.h @@ -11,21 +11,30 @@ #include "base/callback.h" #include "gin/arguments.h" #include "shell/browser/api/electron_api_top_level_window.h" +#include "shell/browser/event_emitter_mixin.h" #include "shell/browser/ui/electron_menu_model.h" -#include "shell/common/gin_helper/trackable_object.h" +#include "shell/common/gin_helper/constructible.h" +#include "shell/common/gin_helper/pinnable.h" namespace electron { namespace api { -class Menu : public gin_helper::TrackableObject, +class Menu : public gin::Wrappable, + public gin_helper::EventEmitterMixin, + public gin_helper::Constructible, + public gin_helper::Pinnable, public ElectronMenuModel::Delegate, public ElectronMenuModel::Observer { public: - static gin_helper::WrappableBase* New(gin::Arguments* args); + // gin_helper::Constructible + static gin::Handle New(gin::Arguments* args); + static v8::Local FillObjectTemplate( + v8::Isolate*, + v8::Local); - static void BuildPrototype(v8::Isolate* isolate, - v8::Local prototype); + // gin::Wrappable + static gin::WrapperInfo kWrapperInfo; #if defined(OS_MACOSX) // Set the global menubar. @@ -41,8 +50,9 @@ class Menu : public gin_helper::TrackableObject, explicit Menu(gin::Arguments* args); ~Menu() override; - // gin_helper::Wrappable: - void AfterInit(v8::Isolate* isolate) override; + // Returns a new callback which keeps references of the JS wrapper until the + // passed |callback| is called. + base::OnceClosure BindSelfToClosure(base::OnceClosure callback); // ui::SimpleMenuModel::Delegate: bool IsCommandIdChecked(int command_id) const override; @@ -71,11 +81,6 @@ class Menu : public gin_helper::TrackableObject, void OnMenuWillClose() override; void OnMenuWillShow() override; - protected: - // Returns a new callback which keeps references of the JS wrapper until the - // passed |callback| is called. - base::OnceClosure BindSelfToClosure(base::OnceClosure callback); - private: void InsertItemAt(int index, int command_id, const base::string16& label); void InsertSeparatorAt(int index); @@ -107,19 +112,6 @@ class Menu : public gin_helper::TrackableObject, bool IsVisibleAt(int index) const; bool WorksWhenHiddenAt(int index) const; - // Stored delegate methods. - base::RepeatingCallback, int)> is_checked_; - base::RepeatingCallback, int)> is_enabled_; - base::RepeatingCallback, int)> is_visible_; - base::RepeatingCallback, int)> works_when_hidden_; - base::RepeatingCallback(v8::Local, int, bool)> - get_accelerator_; - base::RepeatingCallback, int)> - should_register_accelerator_; - base::RepeatingCallback, v8::Local, int)> - execute_command_; - base::RepeatingCallback)> menu_will_show_; - DISALLOW_COPY_AND_ASSIGN(Menu); }; diff --git a/shell/browser/api/electron_api_menu_mac.mm b/shell/browser/api/electron_api_menu_mac.mm index 580f60c4c6c2..ecb0694c6a79 100644 --- a/shell/browser/api/electron_api_menu_mac.mm +++ b/shell/browser/api/electron_api_menu_mac.mm @@ -177,8 +177,11 @@ void Menu::SendActionToFirstResponder(const std::string& action) { } // static -gin_helper::WrappableBase* Menu::New(gin::Arguments* args) { - return new MenuMac(args); +gin::Handle Menu::New(gin::Arguments* args) { + auto handle = + gin::CreateHandle(args->isolate(), static_cast(new MenuMac(args))); + gin_helper::CallMethod(args->isolate(), handle.get(), "_init"); + return handle; } } // namespace api diff --git a/shell/browser/api/electron_api_menu_views.cc b/shell/browser/api/electron_api_menu_views.cc index 4594382651a4..4ff65ce94778 100644 --- a/shell/browser/api/electron_api_menu_views.cc +++ b/shell/browser/api/electron_api_menu_views.cc @@ -84,8 +84,11 @@ void MenuViews::OnClosed(int32_t window_id, base::OnceClosure callback) { } // static -gin_helper::WrappableBase* Menu::New(gin::Arguments* args) { - return new MenuViews(args); +gin::Handle Menu::New(gin::Arguments* args) { + auto handle = gin::CreateHandle(args->isolate(), + static_cast(new MenuViews(args))); + gin_helper::CallMethod(args->isolate(), handle.get(), "_init"); + return handle; } } // namespace api diff --git a/shell/browser/api/electron_api_top_level_window.cc b/shell/browser/api/electron_api_top_level_window.cc index d92b558a80ef..42bd20991162 100644 --- a/shell/browser/api/electron_api_top_level_window.cc +++ b/shell/browser/api/electron_api_top_level_window.cc @@ -687,7 +687,6 @@ void TopLevelWindow::SetMenu(v8::Isolate* isolate, v8::Local value) { gin::Handle menu; v8::Local object; if (value->IsObject() && value->ToObject(context).ToLocal(&object) && - gin::V8ToString(isolate, object->GetConstructorName()) == "Menu" && gin::ConvertFromV8(isolate, value, &menu) && !menu.IsEmpty()) { menu_.Reset(isolate, menu.ToV8()); window_->SetMenu(menu->model()); diff --git a/shell/common/gin_helper/event_emitter_caller.cc b/shell/common/gin_helper/event_emitter_caller.cc index c19479c85fda..515f055b186e 100644 --- a/shell/common/gin_helper/event_emitter_caller.cc +++ b/shell/common/gin_helper/event_emitter_caller.cc @@ -21,7 +21,7 @@ v8::Local CallMethodWithArgs(v8::Isolate* isolate, // Use node::MakeCallback to call the callback, and it will also run pending // tasks in Node.js. v8::MaybeLocal ret = node::MakeCallback( - isolate, obj, method, args->size(), &args->front(), {0, 0}); + isolate, obj, method, args->size(), args->data(), {0, 0}); // If the JS function throws an exception (doesn't return a value) the result // of MakeCallback will be empty and therefore ToLocal will be false, in this // case we need to return "false" as that indicates that the event emitter did diff --git a/shell/common/gin_helper/event_emitter_caller.h b/shell/common/gin_helper/event_emitter_caller.h index 5a5bdf6ad7a3..f2da1fede381 100644 --- a/shell/common/gin_helper/event_emitter_caller.h +++ b/shell/common/gin_helper/event_emitter_caller.h @@ -69,11 +69,11 @@ v8::Local CallMethod(v8::Isolate* isolate, gin::Wrappable* object, const char* method_name, Args&&... args) { - v8::HandleScope scope(isolate); + v8::EscapableHandleScope scope(isolate); v8::Local v8_object; if (object->GetWrapper(isolate).ToLocal(&v8_object)) - return CustomEmit(isolate, v8_object, method_name, - std::forward(args)...); + return scope.Escape(CustomEmit(isolate, v8_object, method_name, + std::forward(args)...)); else return v8::Local(); } diff --git a/shell/common/gin_helper/wrappable.h b/shell/common/gin_helper/wrappable.h index 7b86411a5543..ff6f06c6c2a5 100644 --- a/shell/common/gin_helper/wrappable.h +++ b/shell/common/gin_helper/wrappable.h @@ -25,7 +25,7 @@ class Wrappable : public WrappableBase { template static void SetConstructor(v8::Isolate* isolate, const base::Callback& constructor) { - v8::Local templ = CreateFunctionTemplate( + v8::Local templ = gin_helper::CreateFunctionTemplate( isolate, base::Bind(&internal::InvokeNew, constructor)); templ->InstanceTemplate()->SetInternalFieldCount(1); T::BuildPrototype(isolate, templ); diff --git a/spec-main/ambient.d.ts b/spec-main/ambient.d.ts index a3eded2479cd..a7efd44d76de 100644 --- a/spec-main/ambient.d.ts +++ b/spec-main/ambient.d.ts @@ -2,10 +2,8 @@ declare let standardScheme: string; declare namespace Electron { interface Menu { - delegate: { - executeCommand(menu: Menu, event: any, id: number): void; - menuWillShow(menu: Menu): void; - }; + _executeCommand(event: any, id: number): void; + _menuWillShow(): void; getAcceleratorTextAt(index: number): string; } diff --git a/spec-main/api-menu-item-spec.ts b/spec-main/api-menu-item-spec.ts index ca4b325840c6..f74d2139b14a 100644 --- a/spec-main/api-menu-item-spec.ts +++ b/spec-main/api-menu-item-spec.ts @@ -48,7 +48,7 @@ describe('MenuItems', () => { done(); } }]); - menu.delegate.executeCommand(menu, {}, menu.items[0].commandId); + menu._executeCommand({}, menu.items[0].commandId); }); }); @@ -60,7 +60,7 @@ describe('MenuItems', () => { }]); expect(menu.items[0].checked).to.be.false('menu item checked'); - menu.delegate.executeCommand(menu, {}, menu.items[0].commandId); + menu._executeCommand({}, menu.items[0].commandId); expect(menu.items[0].checked).to.be.true('menu item checked'); }); @@ -70,9 +70,9 @@ describe('MenuItems', () => { type: 'radio' }]); - menu.delegate.executeCommand(menu, {}, menu.items[0].commandId); + menu._executeCommand({}, menu.items[0].commandId); expect(menu.items[0].checked).to.be.true('menu item checked'); - menu.delegate.executeCommand(menu, {}, menu.items[0].commandId); + menu._executeCommand({}, menu.items[0].commandId); expect(menu.items[0].checked).to.be.true('menu item checked'); }); @@ -123,7 +123,7 @@ describe('MenuItems', () => { it('at least have one item checked in each group', () => { const menu = Menu.buildFromTemplate(template); - menu.delegate.menuWillShow(menu); + menu._menuWillShow(); const groups = findRadioGroups(template);