Bug 1950885 - Part 1: Make disable_open_click_delay pref static r=asu…

…th,geckoview-reviewers,m_kato Differential Revision: https://phabricator.services.mozilla.com/D239940
mozilla · Feb 28, 2025 · 2d956e6 · 2d956e6
1 parent 2b4b16f
commit 2d956e6
Show file tree

Hide file tree

Showing 10 changed files with 20 additions and 26 deletions.
diff --git a/dom/notification/test/browser/browser_userContextId_openWindow.js b/dom/notification/test/browser/browser_userContextId_openWindow.js
@@ -45,7 +45,7 @@ add_setup(async function () {
       ["dom.serviceWorkers.exemptFromPerDomainMax", true],
       ["dom.serviceWorkers.enabled", true],
       ["dom.serviceWorkers.testing.enabled", true],
-      ["dom.serviceWorkers.disable_open_click_delay", 1000],
+      ["dom.webnotifications.disable_open_click_delay", 1000],
       ["dom.serviceWorkers.idle_timeout", 299999],
       ["dom.serviceWorkers.idle_extended_timeout", 299999],
       ["browser.link.open_newwindow", 3],

diff --git a/dom/notification/test/mochitest/test_notification_serviceworker_openWindow.html b/dom/notification/test/mochitest/test_notification_serviceworker_openWindow.html
@@ -19,7 +19,7 @@
     ["dom.serviceWorkers.exemptFromPerDomainMax", true],
     ["dom.serviceWorkers.enabled", true],
     ["dom.serviceWorkers.testing.enabled", true],
-    ["dom.serviceWorkers.disable_open_click_delay", 1000],
+    ["dom.webnotifications.disable_open_click_delay", 1000],
     ["dom.serviceWorkers.idle_timeout", 299999],
     ["dom.serviceWorkers.idle_extended_timeout", 299999]
   ]});

diff --git a/dom/notification/test/mochitest/test_notification_serviceworker_openWindow_nested.html b/dom/notification/test/mochitest/test_notification_serviceworker_openWindow_nested.html
@@ -99,7 +99,7 @@
     ["dom.serviceWorkers.exemptFromPerDomainMax", true],
     ["dom.serviceWorkers.enabled", true],
     ["dom.serviceWorkers.testing.enabled", true],
-    ["dom.serviceWorkers.disable_open_click_delay", 1000],
+    ["dom.webnotifications.disable_open_click_delay", 1000],
     ["dom.serviceWorkers.idle_timeout", 299999],
     ["dom.serviceWorkers.idle_extended_timeout", 299999],
     ["dom.securecontext.allowlist", "mochi.test,example.com"],

diff --git a/dom/notification/test/mochitest/test_notificationclick_focus.html b/dom/notification/test/mochitest/test_notificationclick_focus.html
@@ -40,7 +40,7 @@
   add_task(async function runTest() {
     await SpecialPowers.pushPrefEnv({"set": [
       // notificationclick_focus depends on this value
-      ["dom.serviceWorkers.disable_open_click_delay", 1000],
+      ["dom.webnotifications.disable_open_click_delay", 1000],
     ]});
     await NotificationTest.allowNotifications();
     await MockAlertsService.register();

diff --git a/dom/serviceworkers/ServiceWorkerOp.cpp b/dom/serviceworkers/ServiceWorkerOp.cpp
@@ -935,8 +935,8 @@ class NotificationEventOp : public ExtendableEventOp,
 
     // We swap first and then initialize the timer so that even if initializing
     // fails, we still clean the interaction count correctly.
-    uint32_t delay = mArgs.get_ServiceWorkerNotificationEventOpArgs()
-                         .disableOpenClickDelay();
+    uint32_t delay =
+        StaticPrefs::dom_webnotifications_disable_open_click_delay();
     rv = mTimer->InitWithCallback(this, delay, nsITimer::TYPE_ONE_SHOT);
 
     if (NS_WARN_IF(NS_FAILED(rv))) {

diff --git a/dom/serviceworkers/ServiceWorkerOpArgs.ipdlh b/dom/serviceworkers/ServiceWorkerOpArgs.ipdlh
@@ -64,7 +64,6 @@ struct ServiceWorkerNotificationEventOpArgs {
   nsString icon;
   nsString data;
   nsString scope;
-  uint32_t disableOpenClickDelay;
 };
 
 struct ServiceWorkerExtensionAPIEventOpArgs {

diff --git a/dom/serviceworkers/ServiceWorkerPrivate.cpp b/dom/serviceworkers/ServiceWorkerPrivate.cpp
@@ -114,14 +114,6 @@ uint32_t ServiceWorkerPrivate::sRunningServiceWorkersFetch = 0;
 uint32_t ServiceWorkerPrivate::sRunningServiceWorkersMax = 0;
 uint32_t ServiceWorkerPrivate::sRunningServiceWorkersFetchMax = 0;
 
-// Tracks the "dom.serviceWorkers.disable_open_click_delay" preference. Modified
-// on main thread, read on worker threads.
-// It is updated every time a "notificationclick" event is dispatched. While
-// this is done without synchronization, at the worst, the thread will just get
-// an older value within which a popup is allowed to be displayed, which will
-// still be a valid value since it was set prior to dispatching the runnable.
-Atomic<uint32_t> gDOMDisableOpenClickDelay(0);
-
 /**
  * KeepAliveToken
  */
@@ -998,10 +990,8 @@ nsresult ServiceWorkerPrivate::SendNotificationEvent(
     const nsAString& aScope) {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (aEventName.EqualsLiteral(NOTIFICATION_CLICK_EVENT_NAME)) {
-    gDOMDisableOpenClickDelay =
-        Preferences::GetInt("dom.serviceWorkers.disable_open_click_delay");
-  } else if (!aEventName.EqualsLiteral(NOTIFICATION_CLOSE_EVENT_NAME)) {
+  if (!aEventName.EqualsLiteral(NOTIFICATION_CLICK_EVENT_NAME) &&
+      !aEventName.EqualsLiteral(NOTIFICATION_CLOSE_EVENT_NAME)) {
     MOZ_ASSERT_UNREACHABLE("Invalid notification event name");
     return NS_ERROR_FAILURE;
   }
@@ -1017,7 +1007,6 @@ nsresult ServiceWorkerPrivate::SendNotificationEvent(
   args.icon() = nsString(aIcon);
   args.data() = nsString(aData);
   args.scope() = nsString(aScope);
-  args.disableOpenClickDelay() = gDOMDisableOpenClickDelay;
 
   return ExecServiceWorkerOp(
       std::move(args), ServiceWorkerLifetimeExtension(FullLifetimeExtension{}),

diff --git a/mobile/android/app/geckoview-prefs.js b/mobile/android/app/geckoview-prefs.js
@@ -201,10 +201,6 @@ pref("dom.meta-viewport.enabled", true);
 // subscription, to avoid duplicates for unacknowledged messages (bug 1207743)
 pref("dom.push.maxRecentMessageIDsPerSubscription", 0);
 
-// Allow service workers to open windows for a longer period after a notification
-// click on mobile. This is to account for some devices being quite slow (bug 1409761)
-pref("dom.serviceWorkers.disable_open_click_delay", 5000);
-
 // Enable WebShare support (bug 1402369)
 pref("dom.webshare.enabled", true);
 

diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -5379,6 +5379,18 @@
   value: Error
   mirror: never
 
+# Allow notificationclick to open window only during the limited timeframe.
+- name: dom.webnotifications.disable_open_click_delay
+  type: RelaxedAtomicUint32
+#if defined(ANDROID)
+  # Allow service workers to open windows for a longer period after a notification
+  # click on mobile. This is to account for some devices being quite slow (bug 1409761)
+  value: 5000
+#else
+  value: 1000
+#endif
+  mirror: always
+
 - name: dom.worker.canceling.timeoutMilliseconds
   type: RelaxedAtomicUint32
   value: 30000    # 30 seconds

diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js
@@ -804,8 +804,6 @@ pref("dom.mutation_events.forceEnable", "");
 
 pref("dom.popup_allowed_events", "change click dblclick auxclick mousedown mouseup pointerdown pointerup notificationclick reset submit touchend contextmenu");
 
-pref("dom.serviceWorkers.disable_open_click_delay", 1000);
-
 pref("dom.storage.shadow_writes", false);
 pref("dom.storage.snapshot_prefill", 16384);
 pref("dom.storage.snapshot_gradual_prefill", 4096);