fixup! refactor: use gin::WeakCellFactory in GlobalCallbacks

fix: must Trace() the weak cell factory

lib/browser/api/global-shortcut.ts

@@ -1,2 +1,36 @@
-const { globalShortcut } = process._linkedBinding('electron_browser_global_shortcut');
-export default globalShortcut;
+const { createGlobalShortcut } = process._linkedBinding('electron_browser_global_shortcut');
+
+let globalShortcut: Electron.GlobalShortcut;
+
+const createGlobalShortcutIfNeeded = () => {
+  if (globalShortcut === undefined) {
+    globalShortcut = createGlobalShortcut();
+  }
+};
+
+export default new Proxy({}, {
+  get: (_target, property: keyof Electron.GlobalShortcut) => {
+    createGlobalShortcutIfNeeded();
+    const value = globalShortcut[property];
+    if (typeof value === 'function') {
+      return value.bind(globalShortcut);
+    }
+    return value;
+  },
+  set: (_target, property: string, value: unknown) => {
+    createGlobalShortcutIfNeeded();
+    return Reflect.set(globalShortcut, property, value);
+  },
+  ownKeys: () => {
+    createGlobalShortcutIfNeeded();
+    return Reflect.ownKeys(globalShortcut);
+  },
+  has: (_target, property: string) => {
+    createGlobalShortcutIfNeeded();
+    return property in globalShortcut;
+  },
+  getOwnPropertyDescriptor: (_target, property: string) => {
+    createGlobalShortcutIfNeeded();
+    return Reflect.getOwnPropertyDescriptor(globalShortcut, property);
+  }
+});

patches/chromium/chore_add_electron_objects_to_wrappablepointertag.patch

@@ -8,10 +8,10 @@ electron objects that extend gin::Wrappable and gets
 allocated on the cpp heap
 
 diff --git a/gin/public/wrappable_pointer_tags.h b/gin/public/wrappable_pointer_tags.h
-index fee622ebde42211de6f702b754cfa38595df5a1c..6b524632ebb405e473cf4fe8e253bd13bf7b67e5 100644
+index fee622ebde42211de6f702b754cfa38595df5a1c..3bdcd3f3f0a36314694495ca7361be14d95da911 100644
 --- a/gin/public/wrappable_pointer_tags.h
 +++ b/gin/public/wrappable_pointer_tags.h
-@@ -77,7 +77,20 @@ enum WrappablePointerTag : uint16_t {
+@@ -77,7 +77,21 @@ enum WrappablePointerTag : uint16_t {
    kWebAXObjectProxy,             // content::WebAXObjectProxy
    kWrappedExceptionHandler,      // extensions::WrappedExceptionHandler
    kIndigoContext,                // indigo::IndigoContext
@@ -20,6 +20,7 @@ index fee622ebde42211de6f702b754cfa38595df5a1c..6b524632ebb405e473cf4fe8e253bd13
 +  kElectronDataPipeHolder,              // electron::api::DataPipeHolder
 +  kElectronDebugger,                    // electron::api::Debugger
 +  kElectronEvent,                       // gin_helper::internal::Event
++  kElectronGlobalShortcut,              // electron::api::GlobalShortcut
 +  kElectronMenu,                        // electron::api::Menu
 +  kElectronNetLog,                      // electron::api::NetLog
 +  kElectronPowerMonitor,                // electron::api::PowerMonitor

shell/browser/api/electron_api_global_shortcut.cc

@@ -15,14 +15,16 @@
 #include "electron/shell/browser/electron_browser_context.h"
 #include "electron/shell/common/electron_constants.h"
 #include "extensions/common/command.h"
-#include "gin/dictionary.h"
 #include "gin/object_template_builder.h"
+#include "gin/persistent.h"
 #include "shell/browser/api/electron_api_system_preferences.h"
 #include "shell/browser/browser.h"
 #include "shell/common/gin_converters/accelerator_converter.h"
 #include "shell/common/gin_converters/callback_converter.h"
-#include "shell/common/gin_helper/handle.h"
+#include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/node_includes.h"
+#include "v8/include/cppgc/allocation.h"
+#include "v8/include/v8-cppgc.h"
 
 #if BUILDFLAG(IS_MAC)
 #include "base/mac/mac_util.h"
@@ -50,8 +52,9 @@ bool MapHasMediaKeys(
 
 namespace electron::api {
 
-gin::DeprecatedWrapperInfo GlobalShortcut::kWrapperInfo = {
-    gin::kEmbedderNativeGin};
+const gin::WrapperInfo GlobalShortcut::kWrapperInfo = {
+    {gin::kEmbedderNativeGin},
+    gin::kElectronGlobalShortcut};
 
 GlobalShortcut::GlobalShortcut() = default;
 
@@ -60,7 +63,7 @@ GlobalShortcut::~GlobalShortcut() {
   if (instance && instance->IsRegistrationHandledExternally()) {
     // Eagerly cancel callbacks so PruneStaleCommands() can clear them before
     // the WeakPtrFactory destructor runs.
-    weak_ptr_factory_.InvalidateWeakPtrs();
+    weak_factory_.Invalidate();
     instance->PruneStaleCommands();
   }
 
@@ -69,7 +72,8 @@ GlobalShortcut::~GlobalShortcut() {
 
 void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
   if (auto* cb = base::FindOrNull(accelerator_callback_map_, accelerator)) {
-    cb->Run();
+    auto callback = *cb;
+    callback.Run();
   } else {
     // This should never occur, because if it does,
     // ui::GlobalAcceleratorListener notifies us with wrong accelerator.
@@ -80,7 +84,8 @@ void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
 void GlobalShortcut::ExecuteCommand(const extensions::ExtensionId& extension_id,
                                     const std::string& command_id) {
   if (auto* cb = base::FindOrNull(command_callback_map_, command_id)) {
-    cb->Run();
+    auto callback = *cb;
+    callback.Run();
   } else {
     // This should never occur, because if it does, GlobalAcceleratorListener
     // notifies us with wrong command.
@@ -112,11 +117,14 @@ bool GlobalShortcut::RegisterAll(
 
 bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
                               const base::RepeatingClosure& callback) {
+  v8::Isolate* const isolate = JavascriptEnvironment::GetIsolate();
+
   if (!electron::Browser::Get()->is_ready()) {
-    gin_helper::ErrorThrower(JavascriptEnvironment::GetIsolate())
-        .ThrowError("globalShortcut cannot be used before the app is ready");
+    gin_helper::ErrorThrower(isolate).ThrowError(
+        "globalShortcut cannot be used before the app is ready");
     return false;
   }
+
 #if BUILDFLAG(IS_MAC)
   if (accelerator.IsMediaKey()) {
     if (RegisteringMediaKeyForUntrustedClient(accelerator))
@@ -170,8 +178,10 @@ bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
     const std::string fake_extension_id = command_str + "+" + profile_id;
     instance->OnCommandsChanged(
         fake_extension_id, profile_id, commands, gfx::kNullAcceleratedWidget,
-        base::BindRepeating(&GlobalShortcut::ExecuteCommand,
-                            weak_ptr_factory_.GetWeakPtr()));
+        base::BindRepeating(
+            &GlobalShortcut::ExecuteCommand,
+            gin::WrapPersistent(weak_factory_.GetWeakCell(
+                isolate->GetCppHeap()->GetAllocationHandle()))));
     command_callback_map_[command_str] = callback;
     return true;
   } else {
@@ -233,16 +243,15 @@ void GlobalShortcut::UnregisterAll() {
 }
 
 // static
-gin_helper::Handle<GlobalShortcut> GlobalShortcut::Create(
-    v8::Isolate* isolate) {
-  return gin_helper::CreateHandle(isolate, new GlobalShortcut());
+GlobalShortcut* GlobalShortcut::Create(v8::Isolate* isolate) {
+  return cppgc::MakeGarbageCollected<GlobalShortcut>(
+      isolate->GetCppHeap()->GetAllocationHandle());
 }
 
 // static
 gin::ObjectTemplateBuilder GlobalShortcut::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
-  return gin_helper::DeprecatedWrappable<
-             GlobalShortcut>::GetObjectTemplateBuilder(isolate)
+  return gin::Wrappable<GlobalShortcut>::GetObjectTemplateBuilder(isolate)
       .SetMethod("registerAll", &GlobalShortcut::RegisterAll)
       .SetMethod("register", &GlobalShortcut::Register)
       .SetMethod("isRegistered", &GlobalShortcut::IsRegistered)
@@ -250,8 +259,17 @@ gin::ObjectTemplateBuilder GlobalShortcut::GetObjectTemplateBuilder(
       .SetMethod("unregisterAll", &GlobalShortcut::UnregisterAll);
 }
 
-const char* GlobalShortcut::GetTypeName() {
-  return "GlobalShortcut";
+void GlobalShortcut::Trace(cppgc::Visitor* visitor) const {
+  gin::Wrappable<GlobalShortcut>::Trace(visitor);
+  visitor->Trace(weak_factory_);
+}
+
+const gin::WrapperInfo* GlobalShortcut::wrapper_info() const {
+  return &kWrapperInfo;
+}
+
+const char* GlobalShortcut::GetHumanReadableName() const {
+  return "Electron / GlobalShortcut";
 }
 
 }  // namespace electron::api
@@ -263,8 +281,9 @@ void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Context> context,
                 void* priv) {
   v8::Isolate* const isolate = electron::JavascriptEnvironment::GetIsolate();
-  gin::Dictionary dict{isolate, exports};
-  dict.Set("globalShortcut", electron::api::GlobalShortcut::Create(isolate));
+  gin_helper::Dictionary dict{isolate, exports};
+  dict.SetMethod("createGlobalShortcut",
+                 base::BindRepeating(&electron::api::GlobalShortcut::Create));
 }
 
 }  // namespace

shell/browser/api/electron_api_global_shortcut.h

@@ -11,37 +11,35 @@
 #include "base/functional/callback_forward.h"
 #include "base/memory/weak_ptr.h"
 #include "extensions/common/extension_id.h"
-#include "shell/common/gin_helper/wrappable.h"
+#include "gin/weak_cell.h"
+#include "gin/wrappable.h"
+#include "shell/common/gin_helper/self_keep_alive.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/base/accelerators/global_accelerator_listener/global_accelerator_listener.h"
 
-namespace gin_helper {
-template <typename T>
-class Handle;
-}  // namespace gin_helper
-
 namespace electron::api {
 
-class GlobalShortcut final
-    : private ui::GlobalAcceleratorListener::Observer,
-      public gin_helper::DeprecatedWrappable<GlobalShortcut> {
+class GlobalShortcut final : private ui::GlobalAcceleratorListener::Observer,
+                             public gin::Wrappable<GlobalShortcut> {
  public:
-  static gin_helper::Handle<GlobalShortcut> Create(v8::Isolate* isolate);
+  static GlobalShortcut* Create(v8::Isolate* isolate);
 
-  // gin_helper::Wrappable
-  static gin::DeprecatedWrapperInfo kWrapperInfo;
+  // gin::Wrappable
+  static const gin::WrapperInfo kWrapperInfo;
+  void Trace(cppgc::Visitor* visitor) const override;
+  const gin::WrapperInfo* wrapper_info() const override;
+  const char* GetHumanReadableName() const override;
   gin::ObjectTemplateBuilder GetObjectTemplateBuilder(
       v8::Isolate* isolate) override;
-  const char* GetTypeName() override;
+
+  // Make public for cppgc::MakeGarbageCollected.
+  GlobalShortcut();
+  ~GlobalShortcut() override;
 
   // disable copy
   GlobalShortcut(const GlobalShortcut&) = delete;
   GlobalShortcut& operator=(const GlobalShortcut&) = delete;
 
- protected:
-  GlobalShortcut();
-  ~GlobalShortcut() override;
-
  private:
   typedef std::map<ui::Accelerator, base::RepeatingClosure>
       AcceleratorCallbackMap;
@@ -64,7 +62,9 @@ class GlobalShortcut final
   AcceleratorCallbackMap accelerator_callback_map_;
   CommandCallbackMap command_callback_map_;
 
-  base::WeakPtrFactory<GlobalShortcut> weak_ptr_factory_{this};
+  gin::WeakCellFactory<GlobalShortcut> weak_factory_{this};
+
+  gin_helper::SelfKeepAlive<GlobalShortcut> keep_alive_{this};
 };
 
 }  // namespace electron::api

typings/internal-ambient.d.ts

@@ -242,7 +242,7 @@ declare namespace NodeJS {
     _linkedBinding(name: 'electron_browser_crash_reporter'): CrashReporterBinding;
     _linkedBinding(name: 'electron_browser_desktop_capturer'): { createDesktopCapturer(): ElectronInternal.DesktopCapturer; isDisplayMediaSystemPickerAvailable(): boolean; };
     _linkedBinding(name: 'electron_browser_event_emitter'): { setEventEmitterPrototype(prototype: Object): void; };
-    _linkedBinding(name: 'electron_browser_global_shortcut'): { globalShortcut: Electron.GlobalShortcut };
+    _linkedBinding(name: 'electron_browser_global_shortcut'): { createGlobalShortcut(): Electron.GlobalShortcut };
     _linkedBinding(name: 'electron_browser_image_view'): { ImageView: any };
     _linkedBinding(name: 'electron_browser_in_app_purchase'): { inAppPurchase: Electron.InAppPurchase };
     _linkedBinding(name: 'electron_browser_message_port'): { createPair(): { port1: Electron.MessagePortMain, port2: Electron.MessagePortMain }; };