From 032b902a899de0346cd49f6dcdeab3d8d9391c82 Mon Sep 17 00:00:00 2001 From: Cristian Tuns Date: Wed, 29 May 2024 14:55:24 -0400 Subject: [PATCH] Backed out 2 changesets (bug 1872993) for causing bc failures in browser_popupNotification_swapBrowser.js CLOSED TREE Backed out changeset 8518f8499c32 (bug 1872993) Backed out changeset b4f7d53db928 (bug 1872993) --- .../test/popupNotifications/browser.toml | 2 - ...rowser_popupNotification_security_delay.js | 24 +++++ .../browser_popupNotification_swapBrowser.js | 89 ------------------- .../content/test/popupNotifications/head.js | 28 ------ .../tests/browser/browser_webauthn_prompts.js | 2 +- toolkit/modules/PopupNotifications.sys.mjs | 23 +++-- 6 files changed, 39 insertions(+), 129 deletions(-) delete mode 100644 browser/base/content/test/popupNotifications/browser_popupNotification_swapBrowser.js diff --git a/browser/base/content/test/popupNotifications/browser.toml b/browser/base/content/test/popupNotifications/browser.toml index 3c991da139c1..6a7ff4e14e8b 100644 --- a/browser/base/content/test/popupNotifications/browser.toml +++ b/browser/base/content/test/popupNotifications/browser.toml @@ -91,8 +91,6 @@ skip-if = [ "os == 'linux' && asan", ] -["browser_popupNotification_swapBrowser.js"] - ["browser_reshow_in_background.js"] skip-if = [ "os == 'linux' && debug", diff --git a/browser/base/content/test/popupNotifications/browser_popupNotification_security_delay.js b/browser/base/content/test/popupNotifications/browser_popupNotification_security_delay.js index 148fcf1c691d..3c652b26a2ce 100644 --- a/browser/base/content/test/popupNotifications/browser_popupNotification_security_delay.js +++ b/browser/base/content/test/popupNotifications/browser_popupNotification_security_delay.js @@ -44,6 +44,30 @@ add_setup(async function () { }); }); +/** + * The security delay calculation in PopupNotification.sys.mjs is dependent on + * the monotonically increasing value of performance.now. This timestamp is + * not relative to a fixed date, but to runtime. + * We need to wait for the value performance.now() to be larger than the + * security delay in order to observe the bug. Only then does the + * timeSinceShown check in PopupNotifications.sys.mjs lead to a timeSinceShown + * value that is unconditionally greater than lazy.buttonDelay for + * notification.timeShown = null = 0. + * See: https://searchfox.org/mozilla-central/rev/f32d5f3949a3f4f185122142b29f2e3ab776836e/toolkit/modules/PopupNotifications.sys.mjs#1870-1872 + * + * When running in automation as part of a larger test suite performance.now() + * should usually be already sufficiently high in which case this check should + * directly resolve. + */ +async function ensureSecurityDelayReady() { + await TestUtils.waitForCondition( + () => performance.now() > TEST_SECURITY_DELAY, + "Wait for performance.now() > SECURITY_DELAY", + 500, + 50 + ); +} + /** * Test helper for security delay tests which performs the following steps: * 1. Shows a test notification. diff --git a/browser/base/content/test/popupNotifications/browser_popupNotification_swapBrowser.js b/browser/base/content/test/popupNotifications/browser_popupNotification_swapBrowser.js deleted file mode 100644 index 2ea9bd5c9c57..000000000000 --- a/browser/base/content/test/popupNotifications/browser_popupNotification_swapBrowser.js +++ /dev/null @@ -1,89 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - -"use strict"; - -const TEST_SECURITY_DELAY = 1; -const TEST_WINDOW_TIME_OPEN = 100; - -add_setup(async function () { - // Set the lowest non-zero security delay for PopupNotification actions so we can test - // if the delay causes inability to press the buttons on PopupNotifications when windows - // are swapped - // Geo Timeout Pref is set to 0, to ensure that the test does not wait location provider - // to start - await SpecialPowers.pushPrefEnv({ - set: [ - ["security.notification_enable_delay", TEST_SECURITY_DELAY], - ["geo.timeout", 0], - ], - }); -}); - -// Testing that the Popup Notification security delay is compared to the -// global process timer instead of the window specific process timer -// as if it is based on the window specific process timer and the tab -// with the Popup Notification is moved to another window with a lower -// window specific process counter, it would block the user from interacting -// with the buttons on the panel because of the security delay -add_task(async function transferPopupNotificationToNewWindowAndResolve() { - await ensureSecurityDelayReady(); - let tab = await BrowserTestUtils.openNewForegroundTab( - gBrowser, - "https://test1.example.com/" - ); - - let browser = tab.linkedBrowser; - - // This timeout is used simulate the a window being open for an extended period of - // time before a Popup Notification is shown so that when the tab containing the - // Popup Notification is moved to a new window there is large enough difference - // between the initial windows interal timer and the new windows interal timer so - // that it would impact the security delay if it was based on the windows interal timer - // eslint-disable-next-line mozilla/no-arbitrary-setTimeout - await new Promise(resolve => setTimeout(resolve, TEST_WINDOW_TIME_OPEN)); - - // Open Geolocation popup - let popupShownPromise = waitForNotificationPanel(); - await SpecialPowers.spawn(browser, [], async function () { - content.navigator.geolocation.getCurrentPosition(() => {}); - }); - await popupShownPromise; - - let notification = PopupNotifications.getNotification("geolocation"); - ok( - PopupNotifications.isPanelOpen && notification, - "Geolocation notification is open" - ); - - // Move Tab with Popup Notification to a new window with its own - // performance.now() counter - let promiseWin = BrowserTestUtils.waitForNewWindow(); - gBrowser.replaceTabWithWindow(tab); - let win = await promiseWin; - await waitForWindowReadyForPopupNotifications(win); - let timeNow = Cu.now(); - - // Ensure security delay is completed - await ensureSecurityDelayReady(timeNow); - - // Ensure Popup is still open - ok(win.PopupNotifications.isPanelOpen, "Geolocation notification is open"); - - let popupHidden = BrowserTestUtils.waitForEvent( - win.PopupNotifications.panel, - "popuphidden" - ); - - // Attempt to resolve the Popup - let acceptBtn = win.PopupNotifications.panel.querySelector( - ".popup-notification-primary-button" - ); - acceptBtn.click(); - - await popupHidden; - // Esnure the Popup has been resolved - Assert.ok(!win.PopupNotifications.isPanelOpen, "Geolocation popup is hidden"); - - await BrowserTestUtils.closeWindow(win); -}); diff --git a/browser/base/content/test/popupNotifications/head.js b/browser/base/content/test/popupNotifications/head.js index 8d34ffa25718..57c393e2a253 100644 --- a/browser/base/content/test/popupNotifications/head.js +++ b/browser/base/content/test/popupNotifications/head.js @@ -365,31 +365,3 @@ function triggerSecondaryCommand(popup, index) { altKey: !navigator.platform.includes("Mac"), }); } - -/** - * The security delay calculation in PopupNotification.sys.mjs is dependent on - * the monotonically increasing value of Cu.now. This timestamp is - * not relative to a fixed date, but to runtime. - * We need to wait for the value Cu.now() to be larger than the - * security delay in order to observe the bug. Only then does the - * timeSinceShown check in PopupNotifications.sys.mjs lead to a timeSinceShown - * value that is unconditionally greater than lazy.buttonDelay for - * notification.timeShown = null = 0. - * See: https://searchfox.org/mozilla-central/rev/f32d5f3949a3f4f185122142b29f2e3ab776836e/toolkit/modules/PopupNotifications.sys.mjs#1870-1872 - * - * When running in automation as part of a larger test suite Cu.now() - * should usually be already sufficiently high in which case this check should - * directly resolve. - */ -async function ensureSecurityDelayReady(timeNewWindowOpened = 0) { - let secDelay = Services.prefs.getIntPref( - "security.notification_enable_delay" - ); - - await TestUtils.waitForCondition( - () => Cu.now() - timeNewWindowOpened > secDelay, - "Wait for performance.now() > SECURITY_DELAY", - 500, - 50 - ); -} diff --git a/dom/webauthn/tests/browser/browser_webauthn_prompts.js b/dom/webauthn/tests/browser/browser_webauthn_prompts.js index 63d818a31319..50a2f87d8011 100644 --- a/dom/webauthn/tests/browser/browser_webauthn_prompts.js +++ b/dom/webauthn/tests/browser/browser_webauthn_prompts.js @@ -24,7 +24,7 @@ async function waitForPopupNotificationSecurityDelay() { ); await TestUtils.waitForCondition( () => { - let timeSinceShown = Cu.now() - notification.timeShown; + let timeSinceShown = performance.now() - notification.timeShown; return timeSinceShown > notificationEnableDelayMS; }, "Wait for security delay to expire", diff --git a/toolkit/modules/PopupNotifications.sys.mjs b/toolkit/modules/PopupNotifications.sys.mjs index 1237683fd660..bb53e0a5f69a 100644 --- a/toolkit/modules/PopupNotifications.sys.mjs +++ b/toolkit/modules/PopupNotifications.sys.mjs @@ -93,7 +93,7 @@ function Notification( this.isPrivate = PrivateBrowsingUtils.isWindowPrivate( this.browser.ownerGlobal ); - this.timeCreated = Cu.now(); + this.timeCreated = this.owner.window.performance.now(); } Notification.prototype = { @@ -823,9 +823,8 @@ PopupNotifications.prototype = { case "popuppositioned": if (this.isPanelOpen) { for (let elt of this.panel.children) { - let now = Cu.now(); elt.notification.timeShown = Math.max( - now, + this.window.performance.now(), elt.notification.timeShown ?? 0 ); } @@ -1243,7 +1242,7 @@ PopupNotifications.prototype = { }, _extendSecurityDelay(notifications) { - let now = Cu.now(); + let now = this.window.performance.now(); notifications.forEach(n => { n.timeShown = now + FULLSCREEN_TRANSITION_TIME_SHOWN_OFFSET_MS; }); @@ -1295,7 +1294,11 @@ PopupNotifications.prototype = { // Remember the time the notification was shown for the security delay. notificationsToShow.forEach( - n => (n.timeShown = Math.max(Cu.now(), n.timeShown ?? 0)) + n => + (n.timeShown = Math.max( + this.window.performance.now(), + n.timeShown ?? 0 + )) ); if (this.isPanelOpen && this._currentAnchorElement == anchorElement) { @@ -1809,7 +1812,8 @@ PopupNotifications.prototype = { // Record the time of the first notification dismissal if the main action // was not triggered in the meantime. - let timeSinceShown = Cu.now() - notificationObj.timeShown; + let timeSinceShown = + this.window.performance.now() - notificationObj.timeShown; if ( !notificationObj.wasDismissed && !notificationObj.recordedTelemetryMainAction @@ -1907,7 +1911,7 @@ PopupNotifications.prototype = { "_onButtonEvent: notification.timeShown is unset. Setting to now.", notification ); - notification.timeShown = Cu.now(); + notification.timeShown = this.window.performance.now(); } if (type == "dropmarkerpopupshown") { @@ -1923,7 +1927,8 @@ PopupNotifications.prototype = { if (type == "buttoncommand") { // Record the total timing of the main action since the notification was // created, even if the notification was dismissed in the meantime. - let timeSinceCreated = Cu.now() - notification.timeCreated; + let timeSinceCreated = + this.window.performance.now() - notification.timeCreated; if (!notification.recordedTelemetryMainAction) { notification.recordedTelemetryMainAction = true; notification._recordTelemetry( @@ -1947,7 +1952,7 @@ PopupNotifications.prototype = { return; } - let now = Cu.now(); + let now = this.window.performance.now(); let timeSinceShown = now - notification.timeShown; if (timeSinceShown < lazy.buttonDelay) { Services.console.logStringMessage(