fix: Requested landing changes for Windows platform

YuriL180821 · Apr 8, 2023 · 1359c9d · 1359c9d
1 parent 993160f
commit 1359c9d
Show file tree

Hide file tree

Showing 10 changed files with 116 additions and 154 deletions.
diff --git a/docs/api/app.md b/docs/api/app.md
@@ -512,24 +512,20 @@ such as `--original-process-start-time`.
 Returns:
 
 * `event` Event
-* `appUserModelId` string - contains App module ID
-* `invokedArgs` string - contains arguments(in case of any) for toast button or data property in case of non-persistent notification
-* `dataCount` integer - contains amount of token pairs present inputData string (not null, default value 0 - which means abcense of inputData)
-* `inputData` string - valid JSON-string which contains result of User input into toast Reply fields. Empty string in case of absence of User input (i.e for case dataCount == 0)
+* `activationArguments` string - contains arguments(in case of any) for toast button or data property in case of non-persistent notification
+* `replyData` string - valid JSON-string which contains result of User input into toast Reply fields. Empty string in case of absence of User input
 
 Emitted after each completed user choise on notification.
 
 ```javascript
 const { app } = require('electron')
 
-app.on('notification-activation', (event, appUserModelId, invokedArgs, dataCount, inputData) => {
+app.on('notification-activation', (event, activationArguments, replyData) => {
   console.log('app.on : notification-activation')
-  console.log('appUserModelId : ' + appUserModelId)
-  console.log('invokedArgs : ' + invokedArgs)
-  console.log('dataCount : ' + dataCount)
-  console.log('inputData : ' + inputData)
-  if (dataCount) {
-    const jsonObj = JSON.parse(inputData)
+  console.log('activationArguments : ' + activationArguments)
+  console.log('replyData : ' + replyData)
+  if (replyData.length) {
+    const jsonObj = JSON.parse(replyData)
     console.log('jsonObj : ', jsonObj)
     Object.values(jsonObj).forEach((item) => {
       console.log('item : ', item)
@@ -625,7 +621,7 @@ Hides all application windows without minimizing them.
 
 ### `app.isHidden()` _macOS_
 
-Returns `boolean` - `true` if the application???including all of its windows???is hidden (e.g. with `Command-H`), `false` otherwise.
+Returns `boolean` - `true` if the application-including all of its windows???is hidden (e.g. with `Command-H`), `false` otherwise.
 
 ### `app.show()` _macOS_
 
@@ -1609,8 +1605,16 @@ your application when they are mistakenly running the x64 version under Rosetta
 
 ### `app.notificationsComServerCLSID` _Windows_
 
-A `string` which contains fully formed CLSID for notifications COM server.
-This is necessary only for apps with persistent notifications support.
-The current default value for this property is `empty` (not `nil`). It means that by the default no persistent notifications support is exist.
+A `string` which contains a fully formed CLSID used for the notifications COM server.
+This is only necessary for apps that need persistent notifications support.
+The current default value for this property is an empty string `''`. This means that by default persistent notifications are not supported on Windows.
 
-Changing for property value should be provided earlier than `ready` event triggered. Preferably changing for `app.notificationsComServerCLSID` is need to be provided into `will-finish-launching` event handler.
+This value should be set before the `ready` event.
+
+### `app.notificationsComDisplayName` _Windows_
+
+A `string` which contains a desired display name in System/Notifications & Actions Windows settings.
+Same name will be displayed as title for each toast created by Electron application.
+The current default value for this property is an empty string `''`. This means that by default toast titles will be displayed with application user model id.
+
+This value should be set before the `ready` event.
diff --git a/shell/browser/api/electron_api_app.cc b/shell/browser/api/electron_api_app.cc
@@ -714,7 +714,7 @@ void App::OnFinishLaunching(base::Value::Dict launch_info) {
   // applications. Only affects pulseaudio currently.
   media::AudioManager::SetGlobalAppName(Browser::Get()->GetName());
 #endif
-  // SAP-14667 handling notification from previous session requires
+  // Handling notification from previous session requires
   // ASAP COM server registration instead deffered call as exisit now
   // GetNotificationPresenter will trigger WindowsToastNotification::Initialize
   // and as result will register COM server too

diff --git a/shell/browser/electron_browser_client.cc b/shell/browser/electron_browser_client.cc
@@ -348,47 +348,25 @@ ElectronBrowserClient::~ElectronBrowserClient() {
   g_browser_client = nullptr;
 }
 
-absl::optional<int> ElectronBrowserClient::GetRenderFrameProcessID() {
-  absl::optional<int> proc_id;
-  // SAP-15679: Notification doesn't work when using Web URL in a restricted
-  // container
-  // search for pending renderer process which BrowserContext is same as
-  // current notifications BrowserContext
-  const auto last = std::end(pending_processes_);
-  const auto find = std::find_if(
-      pending_processes_.begin(), pending_processes_.end(),
-      [&](const std::pair<int, content::WebContents*>& proc_item) {
-        const auto& proc_id{proc_item.first};
-        auto* web_contents{proc_item.second};
-        if (!web_contents || web_contents->GetBrowserContext() !=
-                                 notifications_browser_context_) {
-          return false;
-        }
-
-        bool audio_muted{false};
-        bool notifications_allowed{false};
-        auto process_flags = [&](bool audio_muted_,
-                                 bool notifications_allowed_) {
-          audio_muted = audio_muted_;
-          notifications_allowed = notifications_allowed_;
-        };
-        WebNotificationAllowed(
-            GetWebContentsFromProcessID(proc_id)->GetPrimaryMainFrame(),
-            base::BindRepeating(
-                [](std::function<void(bool, bool)> process_flags_,
-                   // flags below will come from WebNotificationAllowed
-                   bool audio_muted_, bool notifications_allowed_) {
-                  process_flags_(audio_muted_, notifications_allowed_);
-                },
-                process_flags));
-
-        return notifications_allowed;
-      });
-
-  if (find != last)
-    proc_id = find->first;
-
-  return proc_id;
+absl::optional<int> ElectronBrowserClient::GetRenderFrameProcessID(
+    const GURL& service_worker_scope) {
+    absl::optional<int> proc_id;
+
+    const auto last = std::end(pending_processes_);
+    const auto find = std::find_if(
+        std::begin(pending_processes_), last,
+        [service_worker_scope](const std::pair<int, content::WebContents*>& proc_item) {
+          auto* web_contents{proc_item.second};
+          if (!web_contents || web_contents->GetURL().is_empty()) {
+            return false;
+          }
+          return web_contents->GetURL().spec().find(
+                 service_worker_scope.spec()) != std::string::npos;
+        });
+    if (find != last)
+      proc_id = find->first;
+
+    return proc_id;
 }
 
 content::WebContents* ElectronBrowserClient::GetWebContentsFromProcessID(

diff --git a/shell/browser/electron_browser_client.h b/shell/browser/electron_browser_client.h
@@ -56,12 +56,8 @@ class ElectronBrowserClient : public content::ContentBrowserClient,
 
   using Delegate = content::ContentBrowserClient;
   void set_delegate(Delegate* delegate) { delegate_ = delegate; }
-  // SAP-20060 - Port notifications to electron 16
-  void set_notify_context(content::BrowserContext* browser_context) {
-    notifications_browser_context_ = browser_context;
-  }
   // Returns regestered frame host process id
-  absl::optional<int> GetRenderFrameProcessID();
+  absl::optional<int> GetRenderFrameProcessID(const GURL& service_worker_scope);
 
   // Returns the WebContents for pending render processes.
   content::WebContents* GetWebContentsFromProcessID(int process_id);
@@ -109,10 +105,10 @@ class ElectronBrowserClient : public content::ContentBrowserClient,
 
   content::BluetoothDelegate* GetBluetoothDelegate() override;
 #if BUILDFLAG(IS_WIN)
-  // SAP-15762: Support COM activation registration at runtime
+  // feat: Support COM activation registration at runtime
   void SetNotificationsComServerCLSID(const std::string& com_server_clsid);
   std::string GetNotificationsComServerCLSID();
-  // SAP-21094: Application name displays in incorrect format on notification
+  // feat: Application name displays in incorrect format on notification
   void SetNotificationsComDisplayName(const std::string& com_display_name);
   std::string GetNotificationsComDisplayName();
 #endif
@@ -334,10 +330,10 @@ class ElectronBrowserClient : public content::ContentBrowserClient,
   std::string user_agent_override_ = "";
 
 #if defined(OS_WIN)
-  // SAP-15762: To support COM activation registration at runtime
+  // feat: To support COM activation registration at runtime
   std::string notifications_com_server_clsid_ =
       "";  // default empty i.e absence support for persistent notifications
-  // SAP-21094: Application name displays in incorrect format on notification
+  // feat: Application name displays in incorrect format on notification
   std::string notifications_com_display_name_ = "";
 #endif
 
@@ -355,9 +351,6 @@ class ElectronBrowserClient : public content::ContentBrowserClient,
 #if BUILDFLAG(IS_MAC)
   ElectronBrowserMainParts* browser_main_parts_ = nullptr;
 #endif
-  // SAP-15679: Notification doesn't work when using Web URL in a restricted
-  // container
-  content::BrowserContext* notifications_browser_context_;
 };
 
 }  // namespace electron

diff --git a/shell/browser/electron_browser_context.cc b/shell/browser/electron_browser_context.cc
@@ -16,6 +16,8 @@
 #include "base/path_service.h"
 #include "base/strings/escape.h"
 #include "base/strings/string_util.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "base/threading/thread_restrictions.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/pref_names.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
@@ -50,12 +52,10 @@
 #include "shell/browser/web_view_manager.h"
 #include "shell/browser/zoom_level_delegate.h"
 #include "shell/common/application_info.h"
-#include "shell/common/electron_constants.h"
 #include "shell/common/electron_paths.h"
 #include "shell/common/gin_converters/frame_converter.h"
 #include "shell/common/gin_helper/error_thrower.h"
 #include "shell/common/options_switches.h"
-#include "shell/common/thread_restrictions.h"
 #include "third_party/blink/public/mojom/mediastream/media_stream.mojom.h"
 
 #if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
@@ -175,7 +175,7 @@ ElectronBrowserContext::~ElectronBrowserContext() {
 
 void ElectronBrowserContext::InitPrefs() {
   auto prefs_path = GetPath().Append(FILE_PATH_LITERAL("Preferences"));
-  ScopedAllowBlockingForElectron allow_blocking;
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
   PrefServiceFactory prefs_factory;
   scoped_refptr<JsonPrefStore> pref_store =
       base::MakeRefCounted<JsonPrefStore>(prefs_path);
@@ -304,8 +304,6 @@ content::BrowserPluginGuestManager* ElectronBrowserContext::GetGuestManager() {
 
 content::PlatformNotificationService*
 ElectronBrowserContext::GetPlatformNotificationService() {
-  // SAP-20060 - Port notifications to electron 16
-  ElectronBrowserClient::Get()->set_notify_context(this);
   return ElectronBrowserClient::Get()->GetPlatformNotificationService();
 }
 
@@ -599,22 +597,19 @@ bool ElectronBrowserContext::DoesDeviceMatch(
     const base::Value* device_to_compare,
     blink::PermissionType permission_type) {
   if (permission_type ==
-          static_cast<blink::PermissionType>(
-              WebContentsPermissionHelper::PermissionType::HID) ||
-      permission_type ==
-          static_cast<blink::PermissionType>(
-              WebContentsPermissionHelper::PermissionType::USB)) {
-    if (device.GetDict().FindInt(kDeviceVendorIdKey) !=
-            device_to_compare->GetDict().FindInt(kDeviceVendorIdKey) ||
-        device.GetDict().FindInt(kDeviceProductIdKey) !=
-            device_to_compare->GetDict().FindInt(kDeviceProductIdKey)) {
+      static_cast<blink::PermissionType>(
+          WebContentsPermissionHelper::PermissionType::HID)) {
+    if (device.GetDict().FindInt(kHidVendorIdKey) !=
+            device_to_compare->GetDict().FindInt(kHidVendorIdKey) ||
+        device.GetDict().FindInt(kHidProductIdKey) !=
+            device_to_compare->GetDict().FindInt(kHidProductIdKey)) {
       return false;
     }
 
     const auto* serial_number =
-        device_to_compare->GetDict().FindString(kDeviceSerialNumberKey);
+        device_to_compare->GetDict().FindString(kHidSerialNumberKey);
     const auto* device_serial_number =
-        device.GetDict().FindString(kDeviceSerialNumberKey);
+        device.GetDict().FindString(kHidSerialNumberKey);
 
     if (serial_number && device_serial_number &&
         *device_serial_number == *serial_number)

diff --git a/shell/browser/notifications/notification.h b/shell/browser/notifications/notification.h
@@ -18,7 +18,7 @@ class NotificationDelegate;
 class NotificationPresenter;
 
 struct NotificationAction {
-  // SAP-15259 : Add the reply field on a notification
+  // feat: Add the reply field on a notification
   std::u16string type;  // type of the action (button or text input).
   std::u16string text;  // title of the button.
   std::u16string
@@ -52,17 +52,13 @@ struct NotificationOptions {
   std::u16string reply_placeholder;
   std::u16string sound;
   std::u16string urgency;  // Linux
-  std::u16string data;     // SAP-14775: support Notification.data property
+  std::u16string data;     // feat: Support Notification.data property
   std::vector<NotificationAction> actions;
   std::u16string close_button_text;
   std::u16string toast_xml;
-  bool is_persistent;  // 12-SAP-15399: correct processing for the persistent
-                       // notification without actions
-  bool require_interaction;  // SAP-17772: configuration toast show time
-                             // according to
-                             // Notification.requireInteraction property
-  bool should_be_presented;  // 16-SAP-18595: display toast according to
-                             // Notification.renotify
+  bool is_persistent;
+  bool require_interaction;
+  bool should_be_presented;
 
   NotificationOptions();
   ~NotificationOptions();
@@ -82,7 +78,7 @@ class Notification {
   void NotificationClicked();
   void NotificationDismissed(bool is_persistent = false);
   void NotificationFailed(const std::string& error = "");
-  // SAP-14036: Notification was replied to
+  // feat: Notification was replied to
   void NotificationReplied(const std::string& reply);
   void NotificationAction(int index);
 

diff --git a/shell/browser/notifications/platform_notification_service.cc b/shell/browser/notifications/platform_notification_service.cc
@@ -50,20 +50,21 @@ void OnWebNotificationAllowed(base::WeakPtr<Notification> notification,
     options.image = image;
     options.silent = audio_muted ? true : data.silent;
     options.has_reply = false;
+    // feat: Correct processing for the persistent notification without
+    // actions
     options.is_persistent = is_persistent;
-    // feat: configuration toast show time according to
+    // feat: Configuration toast show time according to
     // Notification.requireInteraction property
     options.require_interaction = data.require_interaction;
-    // feat: display toast according to Notification.renotify
+    // feat: Display toast according to Notification.renotify
     options.should_be_presented = data.renotify ? true : !is_replacing;
-    // feat: support Notification.data property
+    // feat: Support Notification.data property
     // header calculation bellow is caused by presence some not described hader
     // into data received see ReadData in
     // ./electron/src/third_party/blink/common/notifications/notification_mojom_traits.cc
     std::size_t data_start_pos(0);
-    if (data.data.size() > blink_data_header.size() &&  // blink::PlatformNotificationData::data header
-        std::equal(data.data.begin(), data.data.begin() + blink_data_header.size(),
-                   blink_data_header.data())) {
+    if (data.data.size() > 4 &&  // blink::PlatformNotificationData::data header
+        std::equal(data.data.begin(), data.data.begin() + 3, "\xFF\x14\xFF")) {
       // simple algorithm to find data start position after variable length
       // header
       if (data.data.size() > SHRT_MAX)
@@ -78,7 +79,7 @@ void OnWebNotificationAllowed(base::WeakPtr<Notification> notification,
     std::transform(data.data.begin() + data_start_pos, data.data.end(),
                    std::back_inserter(options.data),
                    [](const char ch) { return ch; });
-    // feat: support for actions(buttons) in toast
+    // feat: Support for actions(buttons) in toast
     std::transform(
         data.actions.begin(), data.actions.end(),
         std::back_inserter(options.actions),
@@ -164,7 +165,7 @@ class NotificationDelegateImpl final : public electron::NotificationDelegate {
  private:
   std::string notification_id_;
   content::BrowserContext*
-      context_;  // feat: context is necessary for Event dispatching
+      context_;  // context is necessary for Event dispatching
   GURL origin_;
 };
 
@@ -224,7 +225,7 @@ void PlatformNotificationService::DisplayPersistentNotification(
     const GURL& origin,
     const blink::PlatformNotificationData& notification_data,
     const blink::NotificationResources& notification_resources) {
-  absl::optional<int> proc_id = browser_client_->GetRenderFrameProcessID();
+  absl::optional<int> proc_id = browser_client_->GetRenderFrameProcessID(service_worker_scope);
   if (!proc_id.has_value())
     return;
 
@@ -246,7 +247,7 @@ void PlatformNotificationService::DisplayPersistentNotification(
         browser_client_->GetWebContentsFromProcessID(proc_id.value());
     if (web_ctx)
       delegate->SetBrowserContext(web_ctx->GetBrowserContext());
-    // feat: correct processing for the persistent notification without
+    // feat: Correct processing for the persistent notification without
     // actions
     const bool is_persistent(true);
     const bool is_replacing(prev_notif_count > curr_notif_count);

diff --git a/shell/browser/notifications/platform_notification_service.h b/shell/browser/notifications/platform_notification_service.h
@@ -51,7 +51,7 @@ class PlatformNotificationService
 
  private:
   ElectronBrowserClient* browser_client_;
-  int64_t next_persistent_id_;  // SAP-14036: counter for persitent
+  int64_t next_persistent_id_;  // counter for persitent
                                 // notifications
 };