Browse Source

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
Jeremy Apthorp 5 years ago
parent
commit
a3e28788ce

+ 5 - 0
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",

+ 0 - 4
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;

+ 2 - 3
shell/browser/api/electron_api_menu.cc

@@ -7,6 +7,7 @@
 #include <map>
 #include <utility>
 
+#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) {

+ 99 - 37
shell/browser/api/electron_api_tray.cc

@@ -7,14 +7,17 @@
 #include <string>
 
 #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<NativeImage> image,
            base::Optional<UUID> 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<NativeImage> image,
-                                     base::Optional<UUID> guid,
-                                     gin_helper::Arguments* args) {
+gin::Handle<Tray> Tray::New(gin_helper::ErrorThrower thrower,
+                            gin::Handle<NativeImage> image,
+                            base::Optional<UUID> guid,
+                            gin::Arguments* args) {
   if (!Browser::Get()->is_ready()) {
     thrower.ThrowError("Cannot create Tray before app is ready");
-    return nullptr;
+    return gin::Handle<Tray>();
   }
 
 #if defined(OS_WIN)
   if (!guid.has_value() && args->Length() > 1) {
     thrower.ThrowError("Invalid GUID format");
-    return nullptr;
+    return gin::Handle<Tray>();
   }
 #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<NativeImage> image) {
+void Tray::Destroy() {
+  menu_.Reset();
+  tray_icon_.reset();
+}
+
+bool Tray::IsDestroyed() {
+  return !tray_icon_;
+}
+
+void Tray::SetImage(gin::Handle<NativeImage> 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<NativeImage> image) {
 #endif
 }
 
-void Tray::SetPressedImage(v8::Isolate* isolate,
-                           gin::Handle<NativeImage> image) {
+void Tray::SetPressedImage(gin::Handle<NativeImage> 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> menu;
-  args->GetNext(&menu);
   gfx::Point pos;
-  args->GetNext(&pos);
+
+  v8::Local<v8::Value> 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<v8::Value> arg) {
+  if (!CheckDestroyed())
+    return;
   gin::Handle<Menu> 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<v8::FunctionTemplate> prototype) {
-  prototype->SetClassName(gin::StringToV8(isolate, "Tray"));
-  gin_helper::Destroyable::MakeDestroyable(isolate, prototype);
-  gin_helper::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
+v8::Local<v8::ObjectTemplate> Tray::FillObjectTemplate(
+    v8::Isolate* isolate,
+    v8::Local<v8::ObjectTemplate> 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<v8::Object> exports,
                 v8::Local<v8::Context> 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

+ 30 - 17
shell/browser/api/electron_api_tray.h

@@ -10,11 +10,14 @@
 #include <vector>
 
 #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<Tray>, public TrayIconObserver {
+class Tray : public gin::Wrappable<Tray>,
+             public gin_helper::EventEmitterMixin<Tray>,
+             public gin_helper::Constructible<Tray>,
+             public gin_helper::CleanedUpAtExit,
+             public TrayIconObserver {
  public:
-  static gin_helper::WrappableBase* New(gin_helper::ErrorThrower thrower,
-                                        gin::Handle<NativeImage> image,
-                                        base::Optional<UUID> guid,
-                                        gin_helper::Arguments* args);
-
-  static void BuildPrototype(v8::Isolate* isolate,
-                             v8::Local<v8::FunctionTemplate> prototype);
+  // gin_helper::Constructible
+  static gin::Handle<Tray> New(gin_helper::ErrorThrower thrower,
+                               gin::Handle<NativeImage> image,
+                               base::Optional<UUID> guid,
+                               gin::Arguments* args);
+  static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
+      v8::Isolate*,
+      v8::Local<v8::ObjectTemplate>);
+
+  // gin::Wrappable
+  static gin::WrapperInfo kWrapperInfo;
 
- protected:
+ private:
   Tray(gin::Handle<NativeImage> image,
        base::Optional<UUID> guid,
-       gin_helper::Arguments* args);
+       gin::Arguments* args);
   ~Tray() override;
 
   // TrayIconObserver:
@@ -70,8 +79,11 @@ class Tray : public gin_helper::TrackableObject<Tray>, 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<NativeImage> image);
-  void SetPressedImage(v8::Isolate* isolate, gin::Handle<NativeImage> image);
+  // JS API:
+  void Destroy();
+  bool IsDestroyed();
+  void SetImage(gin::Handle<NativeImage> image);
+  void SetPressedImage(gin::Handle<NativeImage> 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<Tray>, 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<v8::Value> arg);
   gfx::Rect GetBounds();
 
- private:
+  bool CheckDestroyed();
+
   v8::Global<v8::Value> menu_;
   std::unique_ptr<TrayIcon> tray_icon_;
 

+ 32 - 0
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<v8::Object> CreateEventFromFlags(int flags) {
+  v8::Isolate* isolate = v8::Isolate::GetCurrent();
+  const int is_mouse_click = static_cast<bool>(flags & mouse_button_flags);
+  return gin::DataObjectBuilder(isolate)
+      .Set("shiftKey", static_cast<bool>(flags & ui::EF_SHIFT_DOWN))
+      .Set("ctrlKey", static_cast<bool>(flags & ui::EF_CONTROL_DOWN))
+      .Set("altKey", static_cast<bool>(flags & ui::EF_ALT_DOWN))
+      .Set("metaKey", static_cast<bool>(flags & ui::EF_COMMAND_DOWN))
+      .Set("triggeredByAccelerator", !is_mouse_click)
+      .Build();
+}
+
+}  // namespace api
+}  // namespace electron

+ 22 - 0
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 <typename T>
+class Local;
+}  // namespace v8
+
+namespace electron {
+namespace api {
+
+v8::Local<v8::Object> CreateEventFromFlags(int flags);
+
+}  // namespace api
+}  // namespace electron
+
+#endif  // SHELL_BROWSER_API_UI_EVENT_H_

+ 15 - 0
shell/browser/event_emitter_mixin.h

@@ -33,6 +33,21 @@ class EventEmitterMixin {
                          std::forward<Args>(args)...);
   }
 
+  // this.emit(name, event, args...);
+  template <typename... Args>
+  bool EmitCustomEvent(base::StringPiece name,
+                       v8::Local<v8::Object> custom_event,
+                       Args&&... args) {
+    v8::Isolate* isolate = v8::Isolate::GetCurrent();
+    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, custom_event);
+    return EmitWithEvent(isolate, wrapper, name, event,
+                         std::forward<Args>(args)...);
+  }
+
  protected:
   EventEmitterMixin() = default;
 

+ 3 - 0
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();

+ 33 - 0
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<CleanedUpAtExit*>& GetDoomed() {
+  static base::NoDestructor<base::flat_set<CleanedUpAtExit*>> 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

+ 27 - 0
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_

+ 67 - 0
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 <typename T>
+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<Example>,
+//                   public gin_helper::Constructible<Example> {
+//    public:
+//     static gin::Handle<Tray> New(...usual gin method arguments...);
+//     static v8::Local<v8::ObjectTemplate> FillObjectTemplate(
+//         v8::Isolate*,
+//         v8::Local<v8::ObjectTemplate>);
+//   }
+//
+// 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 <typename T>
+class Constructible {
+ public:
+  static v8::Local<v8::Function> GetConstructor(
+      v8::Local<v8::Context> context) {
+    v8::Isolate* isolate = context->GetIsolate();
+    gin::PerIsolateData* data = gin::PerIsolateData::From(isolate);
+    auto* wrapper_info = &T::kWrapperInfo;
+    v8::Local<v8::FunctionTemplate> constructor =
+        data->GetFunctionTemplate(wrapper_info);
+    if (constructor.IsEmpty()) {
+      constructor = gin::CreateConstructorFunctionTemplate(
+          isolate, base::BindRepeating(&T::New));
+      if (std::is_base_of<EventEmitterMixin<T>, T>::value) {
+        constructor->Inherit(
+            gin_helper::internal::GetEventEmitterTemplate(isolate));
+      }
+      constructor->InstanceTemplate()->SetInternalFieldCount(
+          gin::kNumberOfInternalFields);
+      v8::Local<v8::ObjectTemplate> 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_

+ 0 - 16
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<v8::Object> CreateEvent(v8::Isolate* isolate,
   return event;
 }
 
-v8::Local<v8::Object> 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<bool>(flags & mouse_button_flags);
-  Dictionary obj = gin::Dictionary::CreateEmpty(isolate);
-  obj.Set("shiftKey", static_cast<bool>(flags & ui::EF_SHIFT_DOWN));
-  obj.Set("ctrlKey", static_cast<bool>(flags & ui::EF_CONTROL_DOWN));
-  obj.Set("altKey", static_cast<bool>(flags & ui::EF_ALT_DOWN));
-  obj.Set("metaKey", static_cast<bool>(flags & ui::EF_COMMAND_DOWN));
-  obj.Set("triggeredByAccelerator", !is_mouse_click);
-  return obj.GetHandle();
-}
-
 v8::Local<v8::Object> CreateNativeEvent(
     v8::Isolate* isolate,
     v8::Local<v8::Object> sender,

+ 0 - 11
shell/common/gin_helper/event_emitter.h

@@ -25,7 +25,6 @@ v8::Local<v8::Object> CreateEvent(
     v8::Isolate* isolate,
     v8::Local<v8::Object> sender = v8::Local<v8::Object>(),
     v8::Local<v8::Object> custom_event = v8::Local<v8::Object>());
-v8::Local<v8::Object> CreateEventFromFlags(v8::Isolate* isolate, int flags);
 v8::Local<v8::Object> CreateNativeEvent(
     v8::Isolate* isolate,
     v8::Local<v8::Object> sender,
@@ -59,16 +58,6 @@ class EventEmitter : public gin_helper::Wrappable<T> {
                          std::forward<Args>(args)...);
   }
 
-  // this.emit(name, new Event(flags), args...);
-  template <typename... Args>
-  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>(args)...);
-  }
-
   // this.emit(name, new Event(), args...);
   template <typename... Args>
   bool Emit(base::StringPiece name, Args&&... args) {

+ 18 - 0
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 <typename Sig>
+v8::Local<v8::FunctionTemplate> CreateConstructorFunctionTemplate(
+    v8::Isolate* isolate,
+    base::RepeatingCallback<Sig> callback,
+    InvokerOptions invoker_options = {}) {
+  typedef internal::CallbackHolder<Sig> HolderT;
+  HolderT* holder =
+      new HolderT(isolate, std::move(callback), std::move(invoker_options));
+
+  v8::Local<v8::FunctionTemplate> tmpl = v8::FunctionTemplate::New(
+      isolate, &internal::Dispatcher<Sig>::DispatchToCallback,
+      ConvertToV8<v8::Local<v8::External>>(isolate,
+                                           holder->GetHandle(isolate)));
+  return tmpl;
+}
+
 }  // namespace gin
 
 #endif  // SHELL_COMMON_GIN_HELPER_FUNCTION_TEMPLATE_EXTENSIONS_H_

+ 28 - 1
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()', () => {