Browse Source

feat: support global shortcuts via GlobalShortcutsPortal feature with ozone/wayland (#45171)

* fix: backport patch to fix systemd unit activation in Chromium

This backports a patch from Chromium, which fixes systemd unit
activation. That is, a globalShortcuts feature that Chromium has
needs to create a systemd unit and rename it properly. Portal's
global shortcuts uses that name afterwards to map the app with
the shortcuts bound. However, there might be a race between
Chromium binding shortcuts and renaming the unit.

This is a first step to add Portal's globalShortcuts to
Electron.

* feat: Support global shortcuts via GlobalShortcutsPortal feature

Chromium has a new feature called GlobalShortcutsPortal. It
allows clients to use Portal's globalShortcuts to register and
listen to shortcuts.

This patches adds necessary bits, which allows Electron to
use that feature.

In order to make it work, one has to add
--enable-features=GlobalShortcutsPortal

Test: tested manually with a sample app.

* docs: add GlobalShortcutsPortal feature to globalShortcuts docs

Electron supports Portal's globalShortcuts API now via Chromium, and Electron
apps can use that in a Wayland session. Update the docs with the required
feature flag that must be passed to be able to use that implementation.
Maksim Sisov 2 months ago
parent
commit
3ea623364b

+ 8 - 0
docs/api/global-shortcut.md

@@ -12,9 +12,17 @@ shortcuts.
 not have the keyboard focus. This module cannot be used before the `ready`
 event of the app module is emitted.
 
+Please also note that it is also possible to use Chromium's
+`GlobalShortcutsPortal` implementation, which allows apps to bind global
+shortcuts when running within a Wayland session.
+
 ```js
 const { app, globalShortcut } = require('electron')
 
+// Enable usage of Portal's globalShortcuts. This is essential for cases when
+// the app runs in a Wayland session.
+app.commandLine.appendSwitch('enable-features', 'GlobalShortcutsPortal')
+
 app.whenReady().then(() => {
   // Register a 'CommandOrControl+X' shortcut listener.
   const ret = globalShortcut.register('CommandOrControl+X', () => {

+ 1 - 0
patches/chromium/.patches

@@ -139,3 +139,4 @@ refactor_unfilter_unresponsive_events.patch
 build_disable_thin_lto_mac.patch
 build_add_public_config_simdutf_config.patch
 revert_code_health_clean_up_stale_macwebcontentsocclusion.patch
+check_for_unit_to_activate_before_notifying_about_success.patch

+ 497 - 0
patches/chromium/check_for_unit_to_activate_before_notifying_about_success.patch

@@ -0,0 +1,497 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Maksim Sisov <[email protected]>
+Date: Tue, 7 Jan 2025 23:46:56 -0800
+Subject: Check for unit to activate before notifying about success
+
+Portal's globalShortcuts interface relies on the unit name to
+properly assign a client for the bound commands. However, in
+some scenarious, there is a race between the service to be
+created, changed its name and activated. As a result, shortcuts
+might be bound before the name is changed. As a result, shortcuts
+might not correctly work and the client will not receive any
+signals.
+
+This is mostly not an issue for Chromium as it creates the
+global shortcuts portal linux object way earlier than it gets
+commands from the command service. But downstream project, which
+try to bind shortcuts at the same time as that instance is created,
+may experience this issue. As a result, they might not have
+shortcuts working correctly after system reboot or app restart as
+there is a race between those operations.
+
+Bug: None
+Change-Id: I8346d65e051d9587850c76ca0b8807669c161667
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6110782
+Reviewed-by: Thomas Anderson <[email protected]>
+Commit-Queue: Maksim Sisov <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#1403434}
+
+diff --git a/components/dbus/xdg/systemd.cc b/components/dbus/xdg/systemd.cc
+index 362a16447bf578923cb8a84674c277ae6c98228f..3cd9a55d540c07a4c53f5a62bec5cbea37c11838 100644
+--- a/components/dbus/xdg/systemd.cc
++++ b/components/dbus/xdg/systemd.cc
+@@ -4,9 +4,12 @@
+ 
+ #include "components/dbus/xdg/systemd.h"
+ 
++#include <string>
+ #include <vector>
+ 
+ #include "base/environment.h"
++#include "base/functional/bind.h"
++#include "base/functional/callback_helpers.h"
+ #include "base/memory/scoped_refptr.h"
+ #include "base/no_destructor.h"
+ #include "base/sequence_checker.h"
+@@ -17,7 +20,9 @@
+ #include "components/dbus/utils/name_has_owner.h"
+ #include "dbus/bus.h"
+ #include "dbus/message.h"
++#include "dbus/object_path.h"
+ #include "dbus/object_proxy.h"
++#include "dbus/property.h"
+ #include "third_party/abseil-cpp/absl/types/variant.h"
+ 
+ namespace dbus_xdg {
+@@ -37,6 +42,10 @@ constexpr char kServiceNameSystemd[] = "org.freedesktop.systemd1";
+ constexpr char kObjectPathSystemd[] = "/org/freedesktop/systemd1";
+ constexpr char kInterfaceSystemdManager[] = "org.freedesktop.systemd1.Manager";
+ constexpr char kMethodStartTransientUnit[] = "StartTransientUnit";
++constexpr char kMethodGetUnit[] = "GetUnit";
++
++constexpr char kInterfaceSystemdUnit[] = "org.freedesktop.systemd1.Unit";
++constexpr char kActiveStateProp[] = "ActiveState";
+ 
+ constexpr char kUnitNameFormat[] = "app-$1$2-$3.scope";
+ 
+@@ -67,6 +76,81 @@ const char* GetAppNameSuffix(const std::string& channel) {
+   return "";
+ }
+ 
++// Declare this helper for SystemdUnitActiveStateWatcher to be used.
++void SetStateAndRunCallbacks(SystemdUnitStatus result);
++
++// Watches the object to become active and fires callbacks via
++// SetStateAndRunCallbacks. The callbacks are fired whenever a response with the
++// state being "active" or "failed" (or similar) comes.
++//
++// PS firing callbacks results in destroying this object. So any references
++// to this become invalid.
++class SystemdUnitActiveStateWatcher : public dbus::PropertySet {
++ public:
++  SystemdUnitActiveStateWatcher(scoped_refptr<dbus::Bus> bus,
++                                dbus::ObjectProxy* object_proxy)
++      : dbus::PropertySet(object_proxy,
++                          kInterfaceSystemdUnit,
++                          base::BindRepeating(
++                              &SystemdUnitActiveStateWatcher::OnPropertyChanged,
++                              base::Unretained(this))),
++        bus_(bus) {
++    RegisterProperty(kActiveStateProp, &active_state_);
++    ConnectSignals();
++    GetAll();
++  }
++
++  ~SystemdUnitActiveStateWatcher() override {
++    bus_->RemoveObjectProxy(kServiceNameSystemd, object_proxy()->object_path(),
++                            base::DoNothing());
++  }
++
++ private:
++  void OnPropertyChanged(const std::string& property_name) {
++    DCHECK(active_state_.is_valid());
++    const std::string state_value = active_state_.value();
++    if (callbacks_called_ || state_value == "activating" ||
++        state_value == "reloading") {
++      // Ignore if callbacks have already been fired or continue waiting until
++      // the state changes to something else.
++      return;
++    }
++
++    // There are other states as failed, inactive, and deactivating. Treat all
++    // of them as failed.
++    callbacks_called_ = true;
++    SetStateAndRunCallbacks(state_value == "active"
++                                ? SystemdUnitStatus::kUnitStarted
++                                : SystemdUnitStatus::kFailedToStart);
++    MaybeDeleteSelf();
++  }
++
++  void OnGetAll(dbus::Response* response) override {
++    dbus::PropertySet::OnGetAll(response);
++    keep_alive_ = false;
++    MaybeDeleteSelf();
++  }
++
++  void MaybeDeleteSelf() {
++    if (!keep_alive_ && callbacks_called_) {
++      delete this;
++    }
++  }
++
++  // Registered property that this listens updates to.
++  dbus::Property<std::string> active_state_;
++
++  // Indicates whether callbacks for the unit's state have been called.
++  bool callbacks_called_ = false;
++
++  // Control variable that helps to defer the destruction of |this| as deleting
++  // self when the state changes to active during |OnGetAll| will result in a
++  // segfault.
++  bool keep_alive_ = true;
++
++  scoped_refptr<dbus::Bus> bus_;
++};
++
+ // Global state for cached result or pending callbacks.
+ StatusOrCallbacks& GetUnitNameState() {
+   static base::NoDestructor<StatusOrCallbacks> state(
+@@ -83,10 +167,52 @@ void SetStateAndRunCallbacks(SystemdUnitStatus result) {
+   }
+ }
+ 
+-void OnStartTransientUnitResponse(dbus::Response* response) {
++void OnGetPathResponse(scoped_refptr<dbus::Bus> bus, dbus::Response* response) {
++  dbus::MessageReader reader(response);
++  dbus::ObjectPath obj_path;
++  if (!response || !reader.PopObjectPath(&obj_path) || !obj_path.IsValid()) {
++    // We didn't get a valid response. Treat this as failed service.
++    SetStateAndRunCallbacks(SystemdUnitStatus::kFailedToStart);
++    return;
++  }
++
++  dbus::ObjectProxy* unit_proxy =
++      bus->GetObjectProxy(kServiceNameSystemd, obj_path);
++  // Create the active state property watcher. It will destroy itself once
++  // it gets notified about the state change.
++  std::unique_ptr<SystemdUnitActiveStateWatcher> active_state_watcher =
++      std::make_unique<SystemdUnitActiveStateWatcher>(bus, unit_proxy);
++  active_state_watcher.release();
++}
++
++void WaitUnitActivateAndRunCallbacks(scoped_refptr<dbus::Bus> bus,
++                                     std::string unit_name) {
++  // Get the path of the unit, which looks similar to
++  // /org/freedesktop/systemd1/unit/app_2dorg_2echromium_2eChromium_2d3182191_2escope
++  // and then wait for it activation.
++  dbus::ObjectProxy* systemd = bus->GetObjectProxy(
++      kServiceNameSystemd, dbus::ObjectPath(kObjectPathSystemd));
++
++  dbus::MethodCall method_call(kInterfaceSystemdManager, kMethodGetUnit);
++  dbus::MessageWriter writer(&method_call);
++  writer.AppendString(unit_name);
++
++  systemd->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
++                      base::BindOnce(&OnGetPathResponse, std::move(bus)));
++}
++
++void OnStartTransientUnitResponse(scoped_refptr<dbus::Bus> bus,
++                                  std::string unit_name,
++                                  dbus::Response* response) {
+   SystemdUnitStatus result = response ? SystemdUnitStatus::kUnitStarted
+                                       : SystemdUnitStatus::kFailedToStart;
+-  SetStateAndRunCallbacks(result);
++  // If the start of the unit failed, immediately notify the client. Otherwise,
++  // wait for its activation.
++  if (result == SystemdUnitStatus::kFailedToStart) {
++    SetStateAndRunCallbacks(result);
++  } else {
++    WaitUnitActivateAndRunCallbacks(std::move(bus), unit_name);
++  }
+ }
+ 
+ void OnNameHasOwnerResponse(scoped_refptr<dbus::Bus> bus,
+@@ -128,8 +254,9 @@ void OnNameHasOwnerResponse(scoped_refptr<dbus::Bus> bus,
+   properties.Write(&writer);
+   // No auxiliary units.
+   Dict<VarDict>().Write(&writer);
+-  systemd->CallMethod(&method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+-                      base::BindOnce(&OnStartTransientUnitResponse));
++  systemd->CallMethod(
++      &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
++      base::BindOnce(&OnStartTransientUnitResponse, std::move(bus), unit_name));
+ }
+ 
+ }  // namespace
+diff --git a/components/dbus/xdg/systemd_unittest.cc b/components/dbus/xdg/systemd_unittest.cc
+index 2e3baecabc4b479000c78d4f6bd30cb1f6e61d2e..67278d7033664d52fbbda02749a2aaa43352f402 100644
+--- a/components/dbus/xdg/systemd_unittest.cc
++++ b/components/dbus/xdg/systemd_unittest.cc
+@@ -16,7 +16,9 @@
+ #include "dbus/message.h"
+ #include "dbus/mock_bus.h"
+ #include "dbus/mock_object_proxy.h"
++#include "dbus/object_path.h"
+ #include "dbus/object_proxy.h"
++#include "dbus/property.h"
+ #include "testing/gmock/include/gmock/gmock.h"
+ #include "testing/gtest/include/gtest/gtest.h"
+ 
+@@ -32,6 +34,27 @@ constexpr char kServiceNameSystemd[] = "org.freedesktop.systemd1";
+ constexpr char kObjectPathSystemd[] = "/org/freedesktop/systemd1";
+ constexpr char kInterfaceSystemdManager[] = "org.freedesktop.systemd1.Manager";
+ constexpr char kMethodStartTransientUnit[] = "StartTransientUnit";
++constexpr char kMethodGetUnit[] = "GetUnit";
++
++constexpr char kFakeUnitPath[] = "/fake/unit/path";
++constexpr char kActiveState[] = "ActiveState";
++constexpr char kStateActive[] = "active";
++constexpr char kStateInactive[] = "inactive";
++
++std::unique_ptr<dbus::Response> CreateActiveStateGetAllResponse(
++    const std::string& state) {
++  auto response = dbus::Response::CreateEmpty();
++  dbus::MessageWriter writer(response.get());
++  dbus::MessageWriter array_writer(nullptr);
++  dbus::MessageWriter dict_entry_writer(nullptr);
++  writer.OpenArray("{sv}", &array_writer);
++  array_writer.OpenDictEntry(&dict_entry_writer);
++  dict_entry_writer.AppendString(kActiveState);
++  dict_entry_writer.AppendVariantOfString(state);
++  array_writer.CloseContainer(&dict_entry_writer);
++  writer.CloseContainer(&array_writer);
++  return response;
++}
+ 
+ class SetSystemdScopeUnitNameForXdgPortalTest : public ::testing::Test {
+  public:
+@@ -124,17 +147,48 @@ TEST_F(SetSystemdScopeUnitNameForXdgPortalTest, StartTransientUnitSuccess) {
+ 
+   EXPECT_CALL(*bus, GetObjectProxy(kServiceNameSystemd,
+                                    dbus::ObjectPath(kObjectPathSystemd)))
+-      .WillOnce(Return(mock_systemd_proxy.get()));
++      .Times(2)
++      .WillRepeatedly(Return(mock_systemd_proxy.get()));
++
++  auto mock_dbus_unit_proxy = base::MakeRefCounted<dbus::MockObjectProxy>(
++      bus.get(), kServiceNameSystemd, dbus::ObjectPath(kFakeUnitPath));
++  EXPECT_CALL(*bus, GetObjectProxy(kServiceNameSystemd,
++                                   dbus::ObjectPath(kFakeUnitPath)))
++      .WillOnce(Return(mock_dbus_unit_proxy.get()));
+ 
+   EXPECT_CALL(*mock_systemd_proxy, DoCallMethod(_, _, _))
+       .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
+                           dbus::ObjectProxy::ResponseCallback* callback) {
++        // Expect kMethodStartTransientUnit first.
+         EXPECT_EQ(method_call->GetInterface(), kInterfaceSystemdManager);
+         EXPECT_EQ(method_call->GetMember(), kMethodStartTransientUnit);
+ 
+         // Simulate a successful response
+         auto response = dbus::Response::CreateEmpty();
+         std::move(*callback).Run(response.get());
++      }))
++      .WillOnce(Invoke([obj_path = kFakeUnitPath](
++                           dbus::MethodCall* method_call, int timeout_ms,
++                           dbus::ObjectProxy::ResponseCallback* callback) {
++        // Then expect kMethodGetUnit. A valid path must be provided.
++        EXPECT_EQ(method_call->GetInterface(), kInterfaceSystemdManager);
++        EXPECT_EQ(method_call->GetMember(), kMethodGetUnit);
++
++        // Simulate a successful response and provide a fake path to the object.
++        auto response = dbus::Response::CreateEmpty();
++        dbus::MessageWriter writer(response.get());
++        writer.AppendObjectPath(dbus::ObjectPath(obj_path));
++        std::move(*callback).Run(response.get());
++      }));
++
++  EXPECT_CALL(*mock_dbus_unit_proxy, DoCallMethod(_, _, _))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        EXPECT_EQ(method_call->GetInterface(), dbus::kPropertiesInterface);
++        EXPECT_EQ(method_call->GetMember(), dbus::kPropertiesGetAll);
++        // Simulate a successful response with "active" state.
++        auto response = CreateActiveStateGetAllResponse(kStateActive);
++        std::move(*callback).Run(response.get());
+       }));
+ 
+   std::optional<SystemdUnitStatus> status;
+@@ -189,6 +243,142 @@ TEST_F(SetSystemdScopeUnitNameForXdgPortalTest, StartTransientUnitFailure) {
+   EXPECT_EQ(status, SystemdUnitStatus::kFailedToStart);
+ }
+ 
++TEST_F(SetSystemdScopeUnitNameForXdgPortalTest,
++       StartTransientUnitInvalidUnitPath) {
++  scoped_refptr<dbus::MockBus> bus =
++      base::MakeRefCounted<dbus::MockBus>(dbus::Bus::Options());
++
++  auto mock_dbus_proxy = base::MakeRefCounted<dbus::MockObjectProxy>(
++      bus.get(), DBUS_SERVICE_DBUS, dbus::ObjectPath(DBUS_PATH_DBUS));
++
++  EXPECT_CALL(
++      *bus, GetObjectProxy(DBUS_SERVICE_DBUS, dbus::ObjectPath(DBUS_PATH_DBUS)))
++      .WillRepeatedly(Return(mock_dbus_proxy.get()));
++
++  EXPECT_CALL(*mock_dbus_proxy, DoCallMethod(_, _, _))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        auto response = dbus::Response::CreateEmpty();
++        dbus::MessageWriter writer(response.get());
++        writer.AppendBool(true);
++        std::move(*callback).Run(response.get());
++      }));
++
++  auto mock_systemd_proxy = base::MakeRefCounted<dbus::MockObjectProxy>(
++      bus.get(), kServiceNameSystemd, dbus::ObjectPath(kObjectPathSystemd));
++
++  EXPECT_CALL(*bus, GetObjectProxy(kServiceNameSystemd,
++                                   dbus::ObjectPath(kObjectPathSystemd)))
++      .Times(2)
++      .WillRepeatedly(Return(mock_systemd_proxy.get()));
++
++  EXPECT_CALL(*mock_systemd_proxy, DoCallMethod(_, _, _))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        EXPECT_EQ(method_call->GetInterface(), kInterfaceSystemdManager);
++        EXPECT_EQ(method_call->GetMember(), kMethodStartTransientUnit);
++
++        // Simulate a successful response
++        auto response = dbus::Response::CreateEmpty();
++        std::move(*callback).Run(response.get());
++      }))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        EXPECT_EQ(method_call->GetInterface(), kInterfaceSystemdManager);
++        EXPECT_EQ(method_call->GetMember(), kMethodGetUnit);
++
++        // Simulate a failure response.
++        std::move(*callback).Run(nullptr);
++      }));
++
++  std::optional<SystemdUnitStatus> status;
++
++  SetSystemdScopeUnitNameForXdgPortal(
++      bus.get(), base::BindLambdaForTesting(
++                     [&](SystemdUnitStatus result) { status = result; }));
++
++  EXPECT_EQ(status, SystemdUnitStatus::kFailedToStart);
++}
++
++TEST_F(SetSystemdScopeUnitNameForXdgPortalTest,
++       StartTransientUnitFailToActivate) {
++  scoped_refptr<dbus::MockBus> bus =
++      base::MakeRefCounted<dbus::MockBus>(dbus::Bus::Options());
++
++  auto mock_dbus_proxy = base::MakeRefCounted<dbus::MockObjectProxy>(
++      bus.get(), DBUS_SERVICE_DBUS, dbus::ObjectPath(DBUS_PATH_DBUS));
++
++  EXPECT_CALL(
++      *bus, GetObjectProxy(DBUS_SERVICE_DBUS, dbus::ObjectPath(DBUS_PATH_DBUS)))
++      .WillRepeatedly(Return(mock_dbus_proxy.get()));
++
++  EXPECT_CALL(*mock_dbus_proxy, DoCallMethod(_, _, _))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        auto response = dbus::Response::CreateEmpty();
++        dbus::MessageWriter writer(response.get());
++        writer.AppendBool(true);
++        std::move(*callback).Run(response.get());
++      }));
++
++  auto mock_systemd_proxy = base::MakeRefCounted<dbus::MockObjectProxy>(
++      bus.get(), kServiceNameSystemd, dbus::ObjectPath(kObjectPathSystemd));
++
++  EXPECT_CALL(*bus, GetObjectProxy(kServiceNameSystemd,
++                                   dbus::ObjectPath(kObjectPathSystemd)))
++      .Times(2)
++      .WillRepeatedly(Return(mock_systemd_proxy.get()));
++
++  auto mock_dbus_unit_proxy = base::MakeRefCounted<dbus::MockObjectProxy>(
++      bus.get(), kServiceNameSystemd, dbus::ObjectPath(kFakeUnitPath));
++  EXPECT_CALL(*bus, GetObjectProxy(kServiceNameSystemd,
++                                   dbus::ObjectPath(kFakeUnitPath)))
++      .WillOnce(Return(mock_dbus_unit_proxy.get()));
++
++  EXPECT_CALL(*mock_systemd_proxy, DoCallMethod(_, _, _))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        EXPECT_EQ(method_call->GetInterface(), kInterfaceSystemdManager);
++        EXPECT_EQ(method_call->GetMember(), kMethodStartTransientUnit);
++
++        // Simulate a successful response
++        auto response = dbus::Response::CreateEmpty();
++        std::move(*callback).Run(response.get());
++      }))
++      .WillOnce(Invoke([obj_path = kFakeUnitPath](
++                           dbus::MethodCall* method_call, int timeout_ms,
++                           dbus::ObjectProxy::ResponseCallback* callback) {
++        EXPECT_EQ(method_call->GetInterface(), kInterfaceSystemdManager);
++        EXPECT_EQ(method_call->GetMember(), kMethodGetUnit);
++
++        // Simulate a successful response
++        auto response = dbus::Response::CreateEmpty();
++        dbus::MessageWriter writer(response.get());
++        writer.AppendObjectPath(dbus::ObjectPath(obj_path));
++        std::move(*callback).Run(response.get());
++      }));
++
++  EXPECT_CALL(*mock_dbus_unit_proxy, DoCallMethod(_, _, _))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        // Then expect kMethodGetUnit. A valid path must be provided.
++        EXPECT_EQ(method_call->GetInterface(), dbus::kPropertiesInterface);
++        EXPECT_EQ(method_call->GetMember(), dbus::kPropertiesGetAll);
++
++        // Simulate a successful response, but with inactive state.
++        auto response = CreateActiveStateGetAllResponse(kStateInactive);
++        std::move(*callback).Run(response.get());
++      }));
++
++  std::optional<SystemdUnitStatus> status;
++
++  SetSystemdScopeUnitNameForXdgPortal(
++      bus.get(), base::BindLambdaForTesting(
++                     [&](SystemdUnitStatus result) { status = result; }));
++
++  EXPECT_EQ(status, SystemdUnitStatus::kFailedToStart);
++}
++
+ TEST_F(SetSystemdScopeUnitNameForXdgPortalTest, UnitNameConstruction) {
+   scoped_refptr<dbus::MockBus> bus =
+       base::MakeRefCounted<dbus::MockBus>(dbus::Bus::Options());
+@@ -220,7 +410,14 @@ TEST_F(SetSystemdScopeUnitNameForXdgPortalTest, UnitNameConstruction) {
+ 
+   EXPECT_CALL(*bus, GetObjectProxy(kServiceNameSystemd,
+                                    dbus::ObjectPath(kObjectPathSystemd)))
+-      .WillOnce(Return(mock_systemd_proxy.get()));
++      .Times(2)
++      .WillRepeatedly(Return(mock_systemd_proxy.get()));
++
++  auto mock_dbus_unit_proxy = base::MakeRefCounted<dbus::MockObjectProxy>(
++      bus.get(), kServiceNameSystemd, dbus::ObjectPath(kFakeUnitPath));
++  EXPECT_CALL(*bus, GetObjectProxy(kServiceNameSystemd,
++                                   dbus::ObjectPath(kFakeUnitPath)))
++      .WillOnce(Return(mock_dbus_unit_proxy.get()));
+ 
+   EXPECT_CALL(*mock_systemd_proxy, DoCallMethod(_, _, _))
+       .WillOnce(Invoke([&](dbus::MethodCall* method_call, int timeout_ms,
+@@ -256,6 +453,30 @@ TEST_F(SetSystemdScopeUnitNameForXdgPortalTest, UnitNameConstruction) {
+ 
+         auto response = dbus::Response::CreateEmpty();
+         std::move(*callback).Run(response.get());
++      }))
++      .WillOnce(Invoke([obj_path = kFakeUnitPath](
++                           dbus::MethodCall* method_call, int timeout_ms,
++                           dbus::ObjectProxy::ResponseCallback* callback) {
++        EXPECT_EQ(method_call->GetInterface(), kInterfaceSystemdManager);
++        EXPECT_EQ(method_call->GetMember(), kMethodGetUnit);
++
++        // Simulate a successful response
++        auto response = dbus::Response::CreateEmpty();
++        dbus::MessageWriter writer(response.get());
++        writer.AppendObjectPath(dbus::ObjectPath(obj_path));
++        std::move(*callback).Run(response.get());
++      }));
++
++  EXPECT_CALL(*mock_dbus_unit_proxy, DoCallMethod(_, _, _))
++      .WillOnce(Invoke([](dbus::MethodCall* method_call, int timeout_ms,
++                          dbus::ObjectProxy::ResponseCallback* callback) {
++        // Then expect kMethodGetUnit. A valid path must be provided.
++        EXPECT_EQ(method_call->GetInterface(), dbus::kPropertiesInterface);
++        EXPECT_EQ(method_call->GetMember(), dbus::kPropertiesGetAll);
++
++        // Simulate a successful response
++        auto response = CreateActiveStateGetAllResponse(kStateActive);
++        std::move(*callback).Run(response.get());
+       }));
+ 
+   std::optional<SystemdUnitStatus> status;

+ 72 - 9
shell/browser/api/electron_api_global_shortcut.cc

@@ -4,9 +4,15 @@
 
 #include "shell/browser/api/electron_api_global_shortcut.h"
 
+#include <string>
 #include <vector>
 
 #include "base/containers/contains.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/uuid.h"
+#include "components/prefs/pref_service.h"
+#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/handle.h"
@@ -62,7 +68,12 @@ void GlobalShortcut::OnKeyPressed(const ui::Accelerator& accelerator) {
 
 void GlobalShortcut::ExecuteCommand(const extensions::ExtensionId& extension_id,
                                     const std::string& command_id) {
-  // Ignore extension commands
+  if (!base::Contains(command_callback_map_, command_id)) {
+    // This should never occur, because if it does, GlobalShortcutListener
+    // notifies us with wrong command.
+    NOTREACHED();
+  }
+  command_callback_map_[command_id].Run();
 }
 
 bool GlobalShortcut::RegisterAll(
@@ -103,13 +114,56 @@ bool GlobalShortcut::Register(const ui::Accelerator& accelerator,
   }
 #endif
 
-  if (!GlobalShortcutListener::GetInstance()->RegisterAccelerator(accelerator,
-                                                                  this)) {
+  auto* instance = GlobalShortcutListener::GetInstance();
+  if (!instance) {
     return false;
   }
 
-  accelerator_callback_map_[accelerator] = callback;
-  return true;
+  if (instance->IsRegistrationHandledExternally()) {
+    auto* context = ElectronBrowserContext::From("", false);
+    PrefService* prefs = context->prefs();
+
+    // Need a unique profile id. Set one if not generated yet, otherwise re-use
+    // the same so that the session for the globalShortcuts is able to get
+    // already registered shortcuts from the previous session. This will be used
+    // by GlobalShortcutListenerLinux as a session key.
+    std::string profile_id = prefs->GetString(kElectronGlobalShortcutsUuid);
+    if (profile_id.empty()) {
+      profile_id = base::Uuid::GenerateRandomV4().AsLowercaseString();
+      prefs->SetString(kElectronGlobalShortcutsUuid, profile_id);
+    }
+
+    // There is no way to get command id for the accelerator as it's extensions'
+    // thing. Instead, we can convert it to string in a following example form
+    // - std::string("Alt+Shift+K"). That must be sufficient enough for us to
+    // map this accelerator with registered commands.
+    const std::string command_str =
+        extensions::Command::AcceleratorToString(accelerator);
+    ui::CommandMap commands;
+    extensions::Command command(
+        command_str, base::UTF8ToUTF16("Electron shortcut " + command_str),
+        /*accelerator=*/std::string(), /*global=*/true);
+    command.set_accelerator(accelerator);
+    commands[command_str] = command;
+
+    // In order to distinguish the shortcuts, we must register multiple commands
+    // as different extensions. Otherwise, each shortcut will be an alternative
+    // for the very first registered and we'll not be able to distinguish them.
+    // For example, if Alt+Shift+K is registered first, registering and pressing
+    // Alt+Shift+M will trigger global shortcuts, but the command id that is
+    // received by GlobalShortcut will correspond to Alt+Shift+K as our command
+    // id is basically a stringified accelerator.
+    const std::string fake_extension_id = command_str + "+" + profile_id;
+    instance->OnCommandsChanged(fake_extension_id, profile_id, commands, this);
+    command_callback_map_[command_str] = callback;
+    return true;
+  } else {
+    if (instance->RegisterAccelerator(accelerator, this)) {
+      accelerator_callback_map_[accelerator] = callback;
+      return true;
+    }
+  }
+  return false;
 }
 
 void GlobalShortcut::Unregister(const ui::Accelerator& accelerator) {
@@ -127,8 +181,10 @@ void GlobalShortcut::Unregister(const ui::Accelerator& accelerator) {
   }
 #endif
 
-  GlobalShortcutListener::GetInstance()->UnregisterAccelerator(accelerator,
-                                                               this);
+  if (GlobalShortcutListener::GetInstance()) {
+    GlobalShortcutListener::GetInstance()->UnregisterAccelerator(accelerator,
+                                                                 this);
+  }
 }
 
 void GlobalShortcut::UnregisterSome(
@@ -139,7 +195,12 @@ void GlobalShortcut::UnregisterSome(
 }
 
 bool GlobalShortcut::IsRegistered(const ui::Accelerator& accelerator) {
-  return base::Contains(accelerator_callback_map_, accelerator);
+  if (base::Contains(accelerator_callback_map_, accelerator)) {
+    return true;
+  }
+  const std::string command_str =
+      extensions::Command::AcceleratorToString(accelerator);
+  return base::Contains(command_callback_map_, command_str);
 }
 
 void GlobalShortcut::UnregisterAll() {
@@ -149,7 +210,9 @@ void GlobalShortcut::UnregisterAll() {
     return;
   }
   accelerator_callback_map_.clear();
-  GlobalShortcutListener::GetInstance()->UnregisterAccelerators(this);
+  if (GlobalShortcutListener::GetInstance()) {
+    GlobalShortcutListener::GetInstance()->UnregisterAccelerators(this);
+  }
 }
 
 // static

+ 2 - 0
shell/browser/api/electron_api_global_shortcut.h

@@ -44,6 +44,7 @@ class GlobalShortcut final
  private:
   typedef std::map<ui::Accelerator, base::RepeatingClosure>
       AcceleratorCallbackMap;
+  typedef std::map<std::string, base::RepeatingClosure> CommandCallbackMap;
 
   bool RegisterAll(const std::vector<ui::Accelerator>& accelerators,
                    const base::RepeatingClosure& callback);
@@ -60,6 +61,7 @@ class GlobalShortcut final
                       const std::string& command_id) override;
 
   AcceleratorCallbackMap accelerator_callback_map_;
+  CommandCallbackMap command_callback_map_;
 };
 
 }  // namespace electron::api

+ 4 - 0
shell/browser/electron_browser_context.cc

@@ -458,6 +458,10 @@ void ElectronBrowserContext::InitPrefs() {
     }
   }
 #endif
+
+  // Unique uuid for global shortcuts.
+  registry->RegisterStringPref(electron::kElectronGlobalShortcutsUuid,
+                               std::string());
 }
 
 void ElectronBrowserContext::SetUserAgent(const std::string& user_agent) {

+ 7 - 0
shell/common/electron_constants.h

@@ -23,6 +23,13 @@ inline constexpr std::string_view kDeviceSerialNumberKey = "serialNumber";
 
 inline constexpr base::cstring_view kRunAsNode = "ELECTRON_RUN_AS_NODE";
 
+// Per-profile UUID to distinguish global shortcut sessions for
+// org.freedesktop.portal.GlobalShortcuts. This is a counterpart to
+// extensions::pref_names::kGlobalShortcutsUuid, which may be not defined
+// if extensions are disabled.
+inline constexpr char kElectronGlobalShortcutsUuid[] =
+    "electron.global_shortcuts.uuid";
+
 #if BUILDFLAG(ENABLE_PDF_VIEWER)
 inline constexpr std::string_view kPDFExtensionPluginName =
     "Chromium PDF Viewer";