fixup! refactor: use gin::WeakCellFactory in GlobalCallbacks

fix: must Trace() the weak cell factory

.github/workflows/apply-patches.yml

@@ -26,7 +26,7 @@ jobs:
     # Use dorny/paths-filter instead of the path filter under the on: pull_request: block
     # so that the output can be used to conditionally run the apply-patches job, which lets
     # the job be marked as a required status check (conditional skip counts as a success).
-    - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
+    - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1
       id: filter
       with:
         filters: |

.github/workflows/audit-branch-ci.yml

@@ -155,7 +155,7 @@ jobs:
             await core.summary.write();
       - name: Send Slack message if errors
         if: ${{ always() && steps.audit-errors.outputs.errorsFound && github.ref == 'refs/heads/main' }}
-        uses: slackapi/slack-github-action@91efab103c0de0a537f72a35f6b8cda0ee76bf0a # v2.1.1
+        uses: slackapi/slack-github-action@af78098f536edbc4de71162a307590698245be95 # v3.0.1
         with:
           payload: |
             link: "https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}"

.github/workflows/build.yml

@@ -61,7 +61,7 @@ jobs:
     - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
       with:
         ref: ${{ github.event.pull_request.head.sha }}
-    - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
+    - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1
       id: filter
       with:
         filters: |

.github/workflows/pipeline-segment-electron-clang-tidy.yml

@@ -126,7 +126,7 @@ jobs:
         cd src/electron
         git pack-refs
     - name: Download Out Gen Artifacts
-      uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3
+      uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
       with:
         name: out_gen_artifacts_${{ env.ARTIFACT_KEY }}
         path: ./src/out/${{ env.ELECTRON_OUT_DIR }}/gen

.github/workflows/pipeline-segment-electron-test.yml

@@ -173,12 +173,12 @@ jobs:
         echo "DISABLE_CRASH_REPORTER_TESTS=true" >> $GITHUB_ENV
         echo "IS_ASAN=true" >> $GITHUB_ENV
     - name: Download Generated Artifacts
-      uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3
+      uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
       with:
         name: generated_artifacts_${{ env.ARTIFACT_KEY }}
         path: ./generated_artifacts_${{ matrix.build-type }}_${{ inputs.target-arch }}
     - name: Download Src Artifacts
-      uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3
+      uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
       with:
         name: src_artifacts_${{ env.ARTIFACT_KEY }}
         path: ./src_artifacts_${{ matrix.build-type }}_${{ inputs.target-arch }}
@@ -214,7 +214,7 @@ jobs:
 
     - name: Run Electron Tests
       shell: bash
-      timeout-minutes: 40
+      timeout-minutes: 60
       env:
         MOCHA_REPORTER: mocha-multi-reporters
         MOCHA_MULTI_REPORTERS: mocha-junit-reporter, tap

.github/workflows/pipeline-segment-node-nan-test.yml

@@ -65,12 +65,12 @@ jobs:
     - name: Install Dependencies
       uses: ./src/electron/.github/actions/install-dependencies
     - name: Download Generated Artifacts
-      uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3
+      uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
       with:
         name: generated_artifacts_${{ env.BUILD_TYPE }}_${{ env.TARGET_ARCH }}
         path: ./generated_artifacts_${{ env.BUILD_TYPE }}_${{ env.TARGET_ARCH }}
     - name: Download Src Artifacts
-      uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3
+      uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
       with:
         name: src_artifacts_linux_${{ env.TARGET_ARCH }}
         path: ./src_artifacts_linux_${{ env.TARGET_ARCH }}
@@ -121,12 +121,12 @@ jobs:
     - name: Install Dependencies
       uses: ./src/electron/.github/actions/install-dependencies
     - name: Download Generated Artifacts
-      uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3
+      uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
       with:
         name: generated_artifacts_${{ env.BUILD_TYPE }}_${{ env.TARGET_ARCH }}
         path: ./generated_artifacts_${{ env.BUILD_TYPE }}_${{ env.TARGET_ARCH }}
     - name: Download Src Artifacts
-      uses: actions/download-artifact@70fc10c6e5e1ce46ad2ea6f2b72d43f7d47b13c3
+      uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c
       with:
         name: src_artifacts_linux_${{ env.TARGET_ARCH }}
         path: ./src_artifacts_linux_${{ env.TARGET_ARCH }}

.github/workflows/pull-request-labeled.yml

@@ -14,7 +14,7 @@ jobs:
     permissions: {}
     steps:
       - name: Trigger Slack workflow
-        uses: slackapi/slack-github-action@91efab103c0de0a537f72a35f6b8cda0ee76bf0a # v2.1.1
+        uses: slackapi/slack-github-action@af78098f536edbc4de71162a307590698245be95 # v3.0.1
         with:
           webhook: ${{ secrets.BACKPORT_REQUESTED_SLACK_WEBHOOK_URL }} 
           webhook-type: webhook-trigger

.github/workflows/scorecards.yml

@@ -51,6 +51,6 @@ jobs:
 
       # Upload the results to GitHub's code scanning dashboard.
       - name: "Upload to code-scanning"
-        uses: github/codeql-action/upload-sarif@0d579ffd059c29b07949a3cce3983f0780820c98 # v3.29.5
+        uses: github/codeql-action/upload-sarif@b1bff81932f5cdfc8695c7752dcee935dcd061c8 # v3.29.5
         with:
           sarif_file: results.sarif

filenames.auto.gni

@@ -179,6 +179,7 @@ auto_filenames = {
     "lib/common/define-properties.ts",
     "lib/common/deprecate.ts",
     "lib/common/ipc-messages.ts",
+    "lib/common/timers-shim.ts",
     "lib/common/web-view-methods.ts",
     "lib/common/webpack-globals-provider.ts",
     "lib/renderer/api/context-bridge.ts",

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 }; };