From a3e28788ce55dab11fd8d4f2f66e6b8a968e0cf5 Mon Sep 17 00:00:00 2001 From: Jeremy Apthorp Date: Sun, 29 Mar 2020 18:32:02 -0700 Subject: [PATCH] refactor: ginify Tray (#22822) * refactor: ginify Tray * lint * improve argument parsing logic * remove redundant imports from tray.js * new Tray produces an instanceof Tray * make Constructible generic * lint * clean up on exit --- filenames.gni | 5 + lib/browser/api/tray.js | 4 - shell/browser/api/electron_api_menu.cc | 5 +- shell/browser/api/electron_api_tray.cc | 136 +++++++++++++----- shell/browser/api/electron_api_tray.h | 45 +++--- shell/browser/api/ui_event.cc | 32 +++++ shell/browser/api/ui_event.h | 22 +++ shell/browser/event_emitter_mixin.h | 15 ++ shell/browser/javascript_environment.cc | 3 + shell/common/gin_helper/cleaned_up_at_exit.cc | 33 +++++ shell/common/gin_helper/cleaned_up_at_exit.h | 27 ++++ shell/common/gin_helper/constructible.h | 67 +++++++++ shell/common/gin_helper/event_emitter.cc | 16 --- shell/common/gin_helper/event_emitter.h | 11 -- .../gin_helper/function_template_extensions.h | 18 +++ spec-main/api-tray-spec.ts | 29 +++- 16 files changed, 380 insertions(+), 88 deletions(-) create mode 100644 shell/browser/api/ui_event.cc create mode 100644 shell/browser/api/ui_event.h create mode 100644 shell/common/gin_helper/cleaned_up_at_exit.cc create mode 100644 shell/common/gin_helper/cleaned_up_at_exit.h create mode 100644 shell/common/gin_helper/constructible.h diff --git a/filenames.gni b/filenames.gni index 01ccb1a0ac8f..2d484d8f9f47 100644 --- a/filenames.gni +++ b/filenames.gni @@ -129,6 +129,8 @@ filenames = { "shell/browser/api/process_metric.h", "shell/browser/api/save_page_handler.cc", "shell/browser/api/save_page_handler.h", + "shell/browser/api/ui_event.cc", + "shell/browser/api/ui_event.h", "shell/browser/auto_updater.cc", "shell/browser/auto_updater.h", "shell/browser/auto_updater_mac.mm", @@ -508,6 +510,9 @@ filenames = { "shell/common/gin_helper/arguments.h", "shell/common/gin_helper/callback.cc", "shell/common/gin_helper/callback.h", + "shell/common/gin_helper/cleaned_up_at_exit.cc", + "shell/common/gin_helper/cleaned_up_at_exit.h", + "shell/common/gin_helper/constructible.h", "shell/common/gin_helper/constructor.h", "shell/common/gin_helper/destroyable.cc", "shell/common/gin_helper/destroyable.h", diff --git a/lib/browser/api/tray.js b/lib/browser/api/tray.js index ac8cc88724fc..4781e538f94f 100644 --- a/lib/browser/api/tray.js +++ b/lib/browser/api/tray.js @@ -1,9 +1,5 @@ 'use strict'; -const { EventEmitter } = require('events'); -const { deprecate } = require('electron'); const { Tray } = process.electronBinding('tray'); -Object.setPrototypeOf(Tray.prototype, EventEmitter.prototype); - module.exports = Tray; diff --git a/shell/browser/api/electron_api_menu.cc b/shell/browser/api/electron_api_menu.cc index b952496b2f08..5308a4c928bd 100644 --- a/shell/browser/api/electron_api_menu.cc +++ b/shell/browser/api/electron_api_menu.cc @@ -7,6 +7,7 @@ #include #include +#include "shell/browser/api/ui_event.h" #include "shell/browser/native_window.h" #include "shell/common/gin_converters/accelerator_converter.h" #include "shell/common/gin_converters/callback_converter.h" @@ -100,9 +101,7 @@ bool Menu::ShouldRegisterAcceleratorForCommandId(int command_id) const { void Menu::ExecuteCommand(int command_id, int flags) { v8::Locker locker(isolate()); v8::HandleScope handle_scope(isolate()); - execute_command_.Run( - GetWrapper(), - gin_helper::internal::CreateEventFromFlags(isolate(), flags), command_id); + execute_command_.Run(GetWrapper(), CreateEventFromFlags(flags), command_id); } void Menu::OnMenuWillShow(ui::SimpleMenuModel* source) { diff --git a/shell/browser/api/electron_api_tray.cc b/shell/browser/api/electron_api_tray.cc index c6131909a0d5..cac09693cc8d 100644 --- a/shell/browser/api/electron_api_tray.cc +++ b/shell/browser/api/electron_api_tray.cc @@ -7,14 +7,17 @@ #include #include "base/threading/thread_task_runner_handle.h" +#include "gin/dictionary.h" +#include "gin/object_template_builder.h" #include "shell/browser/api/electron_api_menu.h" +#include "shell/browser/api/ui_event.h" #include "shell/browser/browser.h" #include "shell/common/api/electron_api_native_image.h" #include "shell/common/gin_converters/gfx_converter.h" #include "shell/common/gin_converters/guid_converter.h" #include "shell/common/gin_converters/image_converter.h" #include "shell/common/gin_helper/dictionary.h" -#include "shell/common/gin_helper/object_template_builder.h" +#include "shell/common/gin_helper/function_template_extensions.h" #include "shell/common/node_includes.h" #include "ui/gfx/image/image.h" @@ -55,50 +58,50 @@ namespace electron { namespace api { +gin::WrapperInfo Tray::kWrapperInfo = {gin::kEmbedderNativeGin}; + Tray::Tray(gin::Handle image, base::Optional guid, - gin_helper::Arguments* args) + gin::Arguments* args) : tray_icon_(TrayIcon::Create(guid)) { - SetImage(args->isolate(), image); + SetImage(image); tray_icon_->AddObserver(this); - - InitWithArgs(args); } Tray::~Tray() = default; // static -gin_helper::WrappableBase* Tray::New(gin_helper::ErrorThrower thrower, - gin::Handle image, - base::Optional guid, - gin_helper::Arguments* args) { +gin::Handle Tray::New(gin_helper::ErrorThrower thrower, + gin::Handle image, + base::Optional guid, + gin::Arguments* args) { if (!Browser::Get()->is_ready()) { thrower.ThrowError("Cannot create Tray before app is ready"); - return nullptr; + return gin::Handle(); } #if defined(OS_WIN) if (!guid.has_value() && args->Length() > 1) { thrower.ThrowError("Invalid GUID format"); - return nullptr; + return gin::Handle(); } #endif - return new Tray(image, guid, args); + return gin::CreateHandle(thrower.isolate(), new Tray(image, guid, args)); } void Tray::OnClicked(const gfx::Rect& bounds, const gfx::Point& location, int modifiers) { - EmitWithFlags("click", modifiers, bounds, location); + EmitCustomEvent("click", CreateEventFromFlags(modifiers), bounds, location); } void Tray::OnDoubleClicked(const gfx::Rect& bounds, int modifiers) { - EmitWithFlags("double-click", modifiers, bounds); + EmitCustomEvent("double-click", CreateEventFromFlags(modifiers), bounds); } void Tray::OnRightClicked(const gfx::Rect& bounds, int modifiers) { - EmitWithFlags("right-click", modifiers, bounds); + EmitCustomEvent("right-click", CreateEventFromFlags(modifiers), bounds); } void Tray::OnBalloonShow() { @@ -126,23 +129,23 @@ void Tray::OnDropText(const std::string& text) { } void Tray::OnMouseEntered(const gfx::Point& location, int modifiers) { - EmitWithFlags("mouse-enter", modifiers, location); + EmitCustomEvent("mouse-enter", CreateEventFromFlags(modifiers), location); } void Tray::OnMouseExited(const gfx::Point& location, int modifiers) { - EmitWithFlags("mouse-leave", modifiers, location); + EmitCustomEvent("mouse-leave", CreateEventFromFlags(modifiers), location); } void Tray::OnMouseMoved(const gfx::Point& location, int modifiers) { - EmitWithFlags("mouse-move", modifiers, location); + EmitCustomEvent("mouse-move", CreateEventFromFlags(modifiers), location); } void Tray::OnMouseUp(const gfx::Point& location, int modifiers) { - EmitWithFlags("mouse-up", modifiers, location); + EmitCustomEvent("mouse-up", CreateEventFromFlags(modifiers), location); } void Tray::OnMouseDown(const gfx::Point& location, int modifiers) { - EmitWithFlags("mouse-down", modifiers, location); + EmitCustomEvent("mouse-down", CreateEventFromFlags(modifiers), location); } void Tray::OnDragEntered() { @@ -157,7 +160,18 @@ void Tray::OnDragEnded() { Emit("drag-end"); } -void Tray::SetImage(v8::Isolate* isolate, gin::Handle image) { +void Tray::Destroy() { + menu_.Reset(); + tray_icon_.reset(); +} + +bool Tray::IsDestroyed() { + return !tray_icon_; +} + +void Tray::SetImage(gin::Handle image) { + if (!CheckDestroyed()) + return; #if defined(OS_WIN) tray_icon_->SetImage(image->GetHICON(GetSystemMetrics(SM_CXSMICON))); #else @@ -165,8 +179,9 @@ void Tray::SetImage(v8::Isolate* isolate, gin::Handle image) { #endif } -void Tray::SetPressedImage(v8::Isolate* isolate, - gin::Handle image) { +void Tray::SetPressedImage(gin::Handle image) { + if (!CheckDestroyed()) + return; #if defined(OS_WIN) tray_icon_->SetPressedImage(image->GetHICON(GetSystemMetrics(SM_CXSMICON))); #else @@ -175,16 +190,22 @@ void Tray::SetPressedImage(v8::Isolate* isolate, } void Tray::SetToolTip(const std::string& tool_tip) { + if (!CheckDestroyed()) + return; tray_icon_->SetToolTip(tool_tip); } void Tray::SetTitle(const std::string& title) { + if (!CheckDestroyed()) + return; #if defined(OS_MACOSX) tray_icon_->SetTitle(title); #endif } std::string Tray::GetTitle() { + if (!CheckDestroyed()) + return std::string(); #if defined(OS_MACOSX) return tray_icon_->GetTitle(); #else @@ -193,12 +214,16 @@ std::string Tray::GetTitle() { } void Tray::SetIgnoreDoubleClickEvents(bool ignore) { + if (!CheckDestroyed()) + return; #if defined(OS_MACOSX) tray_icon_->SetIgnoreDoubleClickEvents(ignore); #endif } bool Tray::GetIgnoreDoubleClickEvents() { + if (!CheckDestroyed()) + return false; #if defined(OS_MACOSX) return tray_icon_->GetIgnoreDoubleClickEvents(); #else @@ -208,6 +233,8 @@ bool Tray::GetIgnoreDoubleClickEvents() { void Tray::DisplayBalloon(gin_helper::ErrorThrower thrower, const gin_helper::Dictionary& options) { + if (!CheckDestroyed()) + return; TrayIcon::BalloonOptions balloon_options; if (!options.Get("title", &balloon_options.title) || @@ -236,27 +263,50 @@ void Tray::DisplayBalloon(gin_helper::ErrorThrower thrower, } void Tray::RemoveBalloon() { + if (!CheckDestroyed()) + return; tray_icon_->RemoveBalloon(); } void Tray::Focus() { + if (!CheckDestroyed()) + return; tray_icon_->Focus(); } -void Tray::PopUpContextMenu(gin_helper::Arguments* args) { +void Tray::PopUpContextMenu(gin::Arguments* args) { + if (!CheckDestroyed()) + return; gin::Handle menu; - args->GetNext(&menu); gfx::Point pos; - args->GetNext(&pos); + + v8::Local first_arg; + if (args->GetNext(&first_arg)) { + if (!gin::ConvertFromV8(args->isolate(), first_arg, &menu)) { + if (!gin::ConvertFromV8(args->isolate(), first_arg, &pos)) { + args->ThrowError(); + return; + } + } else if (args->Length() >= 2) { + if (!args->GetNext(&pos)) { + args->ThrowError(); + return; + } + } + } tray_icon_->PopUpContextMenu(pos, menu.IsEmpty() ? nullptr : menu->model()); } void Tray::CloseContextMenu() { + if (!CheckDestroyed()) + return; tray_icon_->CloseContextMenu(); } void Tray::SetContextMenu(gin_helper::ErrorThrower thrower, v8::Local arg) { + if (!CheckDestroyed()) + return; gin::Handle menu; if (arg->IsNull()) { menu_.Reset(); @@ -270,15 +320,29 @@ void Tray::SetContextMenu(gin_helper::ErrorThrower thrower, } gfx::Rect Tray::GetBounds() { + if (!CheckDestroyed()) + return gfx::Rect(); return tray_icon_->GetBounds(); } +bool Tray::CheckDestroyed() { + if (!tray_icon_) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Locker locker(isolate); + v8::HandleScope scope(isolate); + gin_helper::ErrorThrower(isolate).ThrowError("Tray is destroyed"); + return false; + } + return true; +} + // static -void Tray::BuildPrototype(v8::Isolate* isolate, - v8::Local prototype) { - prototype->SetClassName(gin::StringToV8(isolate, "Tray")); - gin_helper::Destroyable::MakeDestroyable(isolate, prototype); - gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate()) +v8::Local Tray::FillObjectTemplate( + v8::Isolate* isolate, + v8::Local templ) { + return gin::ObjectTemplateBuilder(isolate, "Tray", templ) + .SetMethod("destroy", &Tray::Destroy) + .SetMethod("isDestroyed", &Tray::IsDestroyed) .SetMethod("setImage", &Tray::SetImage) .SetMethod("setPressedImage", &Tray::SetPressedImage) .SetMethod("setToolTip", &Tray::SetToolTip) @@ -294,7 +358,8 @@ void Tray::BuildPrototype(v8::Isolate* isolate, .SetMethod("popUpContextMenu", &Tray::PopUpContextMenu) .SetMethod("closeContextMenu", &Tray::CloseContextMenu) .SetMethod("setContextMenu", &Tray::SetContextMenu) - .SetMethod("getBounds", &Tray::GetBounds); + .SetMethod("getBounds", &Tray::GetBounds) + .Build(); } } // namespace api @@ -310,12 +375,9 @@ void Initialize(v8::Local exports, v8::Local context, void* priv) { v8::Isolate* isolate = context->GetIsolate(); - Tray::SetConstructor(isolate, base::BindRepeating(&Tray::New)); - gin_helper::Dictionary dict(isolate, exports); - dict.Set( - "Tray", - Tray::GetConstructor(isolate)->GetFunction(context).ToLocalChecked()); + gin::Dictionary dict(isolate, exports); + dict.Set("Tray", Tray::GetConstructor(context)); } } // namespace diff --git a/shell/browser/api/electron_api_tray.h b/shell/browser/api/electron_api_tray.h index 0f48301ee502..c795704b08fb 100644 --- a/shell/browser/api/electron_api_tray.h +++ b/shell/browser/api/electron_api_tray.h @@ -10,11 +10,14 @@ #include #include "gin/handle.h" +#include "gin/wrappable.h" +#include "shell/browser/event_emitter_mixin.h" #include "shell/browser/ui/tray_icon.h" #include "shell/browser/ui/tray_icon_observer.h" #include "shell/common/gin_converters/guid_converter.h" +#include "shell/common/gin_helper/cleaned_up_at_exit.h" +#include "shell/common/gin_helper/constructible.h" #include "shell/common/gin_helper/error_thrower.h" -#include "shell/common/gin_helper/trackable_object.h" namespace gfx { class Image; @@ -26,27 +29,33 @@ class Dictionary; namespace electron { -class TrayIcon; - namespace api { class Menu; class NativeImage; -class Tray : public gin_helper::TrackableObject, public TrayIconObserver { +class Tray : public gin::Wrappable, + public gin_helper::EventEmitterMixin, + public gin_helper::Constructible, + public gin_helper::CleanedUpAtExit, + public TrayIconObserver { public: - static gin_helper::WrappableBase* New(gin_helper::ErrorThrower thrower, - gin::Handle image, - base::Optional guid, - gin_helper::Arguments* args); + // gin_helper::Constructible + static gin::Handle New(gin_helper::ErrorThrower thrower, + gin::Handle image, + base::Optional guid, + 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; - protected: + private: Tray(gin::Handle image, base::Optional guid, - gin_helper::Arguments* args); + gin::Arguments* args); ~Tray() override; // TrayIconObserver: @@ -70,8 +79,11 @@ class Tray : public gin_helper::TrackableObject, public TrayIconObserver { void OnMouseExited(const gfx::Point& location, int modifiers) override; void OnMouseMoved(const gfx::Point& location, int modifiers) override; - void SetImage(v8::Isolate* isolate, gin::Handle image); - void SetPressedImage(v8::Isolate* isolate, gin::Handle image); + // JS API: + void Destroy(); + bool IsDestroyed(); + void SetImage(gin::Handle image); + void SetPressedImage(gin::Handle image); void SetToolTip(const std::string& tool_tip); void SetTitle(const std::string& title); std::string GetTitle(); @@ -81,13 +93,14 @@ class Tray : public gin_helper::TrackableObject, public TrayIconObserver { const gin_helper::Dictionary& options); void RemoveBalloon(); void Focus(); - void PopUpContextMenu(gin_helper::Arguments* args); + void PopUpContextMenu(gin::Arguments* args); void CloseContextMenu(); void SetContextMenu(gin_helper::ErrorThrower thrower, v8::Local arg); gfx::Rect GetBounds(); - private: + bool CheckDestroyed(); + v8::Global menu_; std::unique_ptr tray_icon_; diff --git a/shell/browser/api/ui_event.cc b/shell/browser/api/ui_event.cc new file mode 100644 index 000000000000..27657753e189 --- /dev/null +++ b/shell/browser/api/ui_event.cc @@ -0,0 +1,32 @@ +// Copyright (c) 2020 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/ui_event.h" + +#include "gin/data_object_builder.h" +#include "ui/events/event_constants.h" +#include "v8/include/v8.h" + +namespace electron { +namespace api { + +constexpr int mouse_button_flags = + (ui::EF_RIGHT_MOUSE_BUTTON | ui::EF_LEFT_MOUSE_BUTTON | + ui::EF_MIDDLE_MOUSE_BUTTON | ui::EF_BACK_MOUSE_BUTTON | + ui::EF_FORWARD_MOUSE_BUTTON); + +v8::Local CreateEventFromFlags(int flags) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + const int is_mouse_click = static_cast(flags & mouse_button_flags); + return gin::DataObjectBuilder(isolate) + .Set("shiftKey", static_cast(flags & ui::EF_SHIFT_DOWN)) + .Set("ctrlKey", static_cast(flags & ui::EF_CONTROL_DOWN)) + .Set("altKey", static_cast(flags & ui::EF_ALT_DOWN)) + .Set("metaKey", static_cast(flags & ui::EF_COMMAND_DOWN)) + .Set("triggeredByAccelerator", !is_mouse_click) + .Build(); +} + +} // namespace api +} // namespace electron diff --git a/shell/browser/api/ui_event.h b/shell/browser/api/ui_event.h new file mode 100644 index 000000000000..700864fa86b0 --- /dev/null +++ b/shell/browser/api/ui_event.h @@ -0,0 +1,22 @@ +// Copyright (c) 2020 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_UI_EVENT_H_ +#define SHELL_BROWSER_API_UI_EVENT_H_ + +namespace v8 { +class Object; +template +class Local; +} // namespace v8 + +namespace electron { +namespace api { + +v8::Local CreateEventFromFlags(int flags); + +} // namespace api +} // namespace electron + +#endif // SHELL_BROWSER_API_UI_EVENT_H_ diff --git a/shell/browser/event_emitter_mixin.h b/shell/browser/event_emitter_mixin.h index 8e4ac680c4d0..781df5754173 100644 --- a/shell/browser/event_emitter_mixin.h +++ b/shell/browser/event_emitter_mixin.h @@ -33,6 +33,21 @@ class EventEmitterMixin { std::forward(args)...); } + // this.emit(name, event, args...); + template + bool EmitCustomEvent(base::StringPiece name, + v8::Local custom_event, + Args&&... args) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Local wrapper; + if (!static_cast(this)->GetWrapper(isolate).ToLocal(&wrapper)) + return false; + v8::Local event = + internal::CreateEvent(isolate, wrapper, custom_event); + return EmitWithEvent(isolate, wrapper, name, event, + std::forward(args)...); + } + protected: EventEmitterMixin() = default; diff --git a/shell/browser/javascript_environment.cc b/shell/browser/javascript_environment.cc index 746c5a1eeea0..e304e57fe067 100644 --- a/shell/browser/javascript_environment.cc +++ b/shell/browser/javascript_environment.cc @@ -16,6 +16,7 @@ #include "gin/array_buffer.h" #include "gin/v8_initializer.h" #include "shell/browser/microtasks_runner.h" +#include "shell/common/gin_helper/cleaned_up_at_exit.h" #include "shell/common/node_includes.h" #include "tracing/trace_event.h" @@ -39,7 +40,9 @@ JavascriptEnvironment::JavascriptEnvironment(uv_loop_t* event_loop) JavascriptEnvironment::~JavascriptEnvironment() { { + v8::Locker locker(isolate_); v8::HandleScope scope(isolate_); + gin_helper::CleanedUpAtExit::DoCleanup(); context_.Get(isolate_)->Exit(); } isolate_->Exit(); diff --git a/shell/common/gin_helper/cleaned_up_at_exit.cc b/shell/common/gin_helper/cleaned_up_at_exit.cc new file mode 100644 index 000000000000..9255aa3fca9b --- /dev/null +++ b/shell/common/gin_helper/cleaned_up_at_exit.cc @@ -0,0 +1,33 @@ +// Copyright (c) 2020 Slack Technologies, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "shell/common/gin_helper/cleaned_up_at_exit.h" + +#include "base/containers/flat_set.h" +#include "base/no_destructor.h" + +namespace gin_helper { + +base::flat_set& GetDoomed() { + static base::NoDestructor> doomed; + return *doomed; +} +CleanedUpAtExit::CleanedUpAtExit() { + GetDoomed().insert(this); +} +CleanedUpAtExit::~CleanedUpAtExit() { + GetDoomed().erase(this); +} + +// static +void CleanedUpAtExit::DoCleanup() { + auto& doomed = GetDoomed(); + while (!doomed.empty()) { + auto iter = doomed.begin(); + delete *iter; + // It removed itself from the list. + } +} + +} // namespace gin_helper diff --git a/shell/common/gin_helper/cleaned_up_at_exit.h b/shell/common/gin_helper/cleaned_up_at_exit.h new file mode 100644 index 000000000000..f3376c2f450f --- /dev/null +++ b/shell/common/gin_helper/cleaned_up_at_exit.h @@ -0,0 +1,27 @@ +// Copyright (c) 2020 Slack Technologies, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_GIN_HELPER_CLEANED_UP_AT_EXIT_H_ +#define SHELL_COMMON_GIN_HELPER_CLEANED_UP_AT_EXIT_H_ + +namespace gin_helper { + +// Objects of this type will be destroyed immediately prior to disposing the V8 +// Isolate. This should only be used for gin::Wrappable objects, whose lifetime +// is otherwise managed by V8. +// +// NB. This is only needed because v8::Global objects that have SetWeak +// finalization callbacks do not have their finalization callbacks invoked at +// Isolate teardown. +class CleanedUpAtExit { + public: + CleanedUpAtExit(); + virtual ~CleanedUpAtExit(); + + static void DoCleanup(); +}; + +} // namespace gin_helper + +#endif // SHELL_COMMON_GIN_HELPER_CLEANED_UP_AT_EXIT_H_ diff --git a/shell/common/gin_helper/constructible.h b/shell/common/gin_helper/constructible.h new file mode 100644 index 000000000000..72222220194b --- /dev/null +++ b/shell/common/gin_helper/constructible.h @@ -0,0 +1,67 @@ +// Copyright (c) 2020 Slack Technologies, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef SHELL_COMMON_GIN_HELPER_CONSTRUCTIBLE_H_ +#define SHELL_COMMON_GIN_HELPER_CONSTRUCTIBLE_H_ + +#include "gin/wrappable.h" +#include "shell/common/gin_helper/function_template_extensions.h" + +namespace gin_helper { +template +class EventEmitterMixin; + +// Helper class for Wrappable objects which should be constructible with 'new' +// in JavaScript. +// +// To use, inherit from gin::Wrappable and gin_helper::Constructible, and +// define the static methods New and FillObjectTemplate: +// +// class Example : public gin::Wrappable, +// public gin_helper::Constructible { +// public: +// static gin::Handle New(...usual gin method arguments...); +// static v8::Local FillObjectTemplate( +// v8::Isolate*, +// v8::Local); +// } +// +// Do NOT define the usual gin::Wrappable::GetObjectTemplateBuilder. It will +// not be called for Constructible classes. +// +// To expose the constructor, call GetConstructor: +// +// gin::Dictionary dict(isolate, exports); +// dict.Set("Example", Example::GetConstructor(context)); +template +class Constructible { + public: + static v8::Local GetConstructor( + v8::Local context) { + v8::Isolate* isolate = context->GetIsolate(); + gin::PerIsolateData* data = gin::PerIsolateData::From(isolate); + auto* wrapper_info = &T::kWrapperInfo; + v8::Local constructor = + data->GetFunctionTemplate(wrapper_info); + if (constructor.IsEmpty()) { + constructor = gin::CreateConstructorFunctionTemplate( + isolate, base::BindRepeating(&T::New)); + if (std::is_base_of, T>::value) { + constructor->Inherit( + gin_helper::internal::GetEventEmitterTemplate(isolate)); + } + constructor->InstanceTemplate()->SetInternalFieldCount( + gin::kNumberOfInternalFields); + v8::Local obj_templ = + T::FillObjectTemplate(isolate, constructor->InstanceTemplate()); + data->SetObjectTemplate(wrapper_info, obj_templ); + data->SetFunctionTemplate(wrapper_info, constructor); + } + return constructor->GetFunction(context).ToLocalChecked(); + } +}; + +} // namespace gin_helper + +#endif // SHELL_COMMON_GIN_HELPER_CONSTRUCTIBLE_H_ diff --git a/shell/common/gin_helper/event_emitter.cc b/shell/common/gin_helper/event_emitter.cc index d4d5ef712601..ebb52d2ac90c 100644 --- a/shell/common/gin_helper/event_emitter.cc +++ b/shell/common/gin_helper/event_emitter.cc @@ -8,7 +8,6 @@ #include "shell/browser/api/event.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/gin_helper/object_template_builder.h" -#include "ui/events/event_constants.h" namespace gin_helper { @@ -50,21 +49,6 @@ v8::Local CreateEvent(v8::Isolate* isolate, return event; } -v8::Local CreateEventFromFlags(v8::Isolate* isolate, int flags) { - const int mouse_button_flags = - (ui::EF_RIGHT_MOUSE_BUTTON | ui::EF_LEFT_MOUSE_BUTTON | - ui::EF_MIDDLE_MOUSE_BUTTON | ui::EF_BACK_MOUSE_BUTTON | - ui::EF_FORWARD_MOUSE_BUTTON); - const int is_mouse_click = static_cast(flags & mouse_button_flags); - Dictionary obj = gin::Dictionary::CreateEmpty(isolate); - obj.Set("shiftKey", static_cast(flags & ui::EF_SHIFT_DOWN)); - obj.Set("ctrlKey", static_cast(flags & ui::EF_CONTROL_DOWN)); - obj.Set("altKey", static_cast(flags & ui::EF_ALT_DOWN)); - obj.Set("metaKey", static_cast(flags & ui::EF_COMMAND_DOWN)); - obj.Set("triggeredByAccelerator", !is_mouse_click); - return obj.GetHandle(); -} - v8::Local CreateNativeEvent( v8::Isolate* isolate, v8::Local sender, diff --git a/shell/common/gin_helper/event_emitter.h b/shell/common/gin_helper/event_emitter.h index a929d8e7071d..9b0d5aaf82d0 100644 --- a/shell/common/gin_helper/event_emitter.h +++ b/shell/common/gin_helper/event_emitter.h @@ -25,7 +25,6 @@ v8::Local CreateEvent( v8::Isolate* isolate, v8::Local sender = v8::Local(), v8::Local custom_event = v8::Local()); -v8::Local CreateEventFromFlags(v8::Isolate* isolate, int flags); v8::Local CreateNativeEvent( v8::Isolate* isolate, v8::Local sender, @@ -59,16 +58,6 @@ class EventEmitter : public gin_helper::Wrappable { std::forward(args)...); } - // this.emit(name, new Event(flags), args...); - template - bool EmitWithFlags(base::StringPiece name, int flags, Args&&... args) { - v8::Locker locker(isolate()); - v8::HandleScope handle_scope(isolate()); - return EmitCustomEvent(name, - internal::CreateEventFromFlags(isolate(), flags), - std::forward(args)...); - } - // this.emit(name, new Event(), args...); template bool Emit(base::StringPiece name, Args&&... args) { diff --git a/shell/common/gin_helper/function_template_extensions.h b/shell/common/gin_helper/function_template_extensions.h index 7b9588ab9fd1..e4b118e4fd4c 100644 --- a/shell/common/gin_helper/function_template_extensions.h +++ b/shell/common/gin_helper/function_template_extensions.h @@ -37,6 +37,24 @@ inline bool GetNextArgument(Arguments* args, return true; } +// Like gin::CreateFunctionTemplate, but doesn't remove the template's +// prototype. +template +v8::Local CreateConstructorFunctionTemplate( + v8::Isolate* isolate, + base::RepeatingCallback callback, + InvokerOptions invoker_options = {}) { + typedef internal::CallbackHolder HolderT; + HolderT* holder = + new HolderT(isolate, std::move(callback), std::move(invoker_options)); + + v8::Local tmpl = v8::FunctionTemplate::New( + isolate, &internal::Dispatcher::DispatchToCallback, + ConvertToV8>(isolate, + holder->GetHandle(isolate))); + return tmpl; +} + } // namespace gin #endif // SHELL_COMMON_GIN_HELPER_FUNCTION_TEMPLATE_EXTENSIONS_H_ diff --git a/spec-main/api-tray-spec.ts b/spec-main/api-tray-spec.ts index 35e73c7bc1ba..890a3fde2316 100644 --- a/spec-main/api-tray-spec.ts +++ b/spec-main/api-tray-spec.ts @@ -18,7 +18,7 @@ describe('tray module', () => { const badPath = path.resolve('I', 'Do', 'Not', 'Exist'); expect(() => { tray = new Tray(badPath); - }).to.throw(/Image could not be created from .*/); + }).to.throw(/Error processing argument at index 0/); }); ifit(process.platform === 'win32')('throws a descriptive error if an invlaid guid is given', () => { @@ -32,6 +32,10 @@ describe('tray module', () => { tray = new Tray(nativeImage.createEmpty(), '0019A433-3526-48BA-A66C-676742C0FEFB'); }).to.not.throw(); }); + + it('is an instance of Tray', () => { + expect(tray).to.be.an.instanceOf(Tray); + }); }); ifdescribe(process.platform === 'darwin')('tray get/set ignoreDoubleClickEvents', () => { @@ -80,6 +84,29 @@ describe('tray module', () => { tray.popUpContextMenu(menu); }).to.not.throw(); }); + + it('can be called with a position', () => { + expect(() => { + tray.popUpContextMenu({ x: 0, y: 0 } as any); + }).to.not.throw(); + }); + + it('can be called with a menu and a position', () => { + const menu = Menu.buildFromTemplate([{ label: 'Test' }]); + expect(() => { + tray.popUpContextMenu(menu, { x: 0, y: 0 }); + }).to.not.throw(); + }); + + it('throws an error on invalid arguments', () => { + expect(() => { + tray.popUpContextMenu({} as any); + }).to.throw(/index 0/); + const menu = Menu.buildFromTemplate([{ label: 'Test' }]); + expect(() => { + tray.popUpContextMenu(menu, {} as any); + }).to.throw(/index 1/); + }); }); describe('tray.closeContextMenu()', () => {