Bug 1679456 - Consume user activation on popup opening and remove popup opening tokens. r=smaug,edgar
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 30 Nov 2020 17:35:59 +0000
changeset 558819 68af110cd33e39111ef50541073b3c681a59bc2b
parent 558818 416d073941f5f9ca6b7b5262b98eb87100f0b8d0
child 558820 b12f5962b9d256d2018c03a9bb8dab09bd25bc16
push id37991
push usercsabou@mozilla.com
push dateTue, 01 Dec 2020 04:19:24 +0000
treeherdermozilla-central@99aa3dc8e185 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, edgar
bugs1679456
milestone85.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1679456 - Consume user activation on popup opening and remove popup opening tokens. r=smaug,edgar The test tweak is needed because user interaction isn't registered by a single mouseup, but the test is still valid with that tweak IMO. Depends on D98021 Differential Revision: https://phabricator.services.mozilla.com/D98022
dom/base/PopupBlocker.cpp
dom/base/PopupBlocker.h
dom/base/nsGlobalWindowOuter.cpp
dom/base/test/browser_multiple_popups.js
dom/locales/en-US/chrome/dom/dom.properties
--- a/dom/base/PopupBlocker.cpp
+++ b/dom/base/PopupBlocker.cpp
@@ -22,20 +22,16 @@ namespace {
 static char* sPopupAllowedEvents;
 
 static PopupBlocker::PopupControlState sPopupControlState =
     PopupBlocker::openAbused;
 static uint32_t sPopupStatePusherCount = 0;
 
 static TimeStamp sLastAllowedExternalProtocolIFrameTimeStamp;
 
-// This token is by default set to false. When a popup/filePicker is shown, it
-// is set to true.
-static bool sUnusedPopupToken = false;
-
 static uint32_t sOpenPopupSpamCount = 0;
 
 void PopupAllowedEventsChanged() {
   if (sPopupAllowedEvents) {
     free(sPopupAllowedEvents);
   }
 
   nsAutoCString str;
@@ -127,41 +123,22 @@ uint32_t PopupBlocker::GetPopupPermissio
     permissionManager->TestPermissionFromPrincipal(aPrincipal, "popup"_ns,
                                                    &permit);
   }
 
   return permit;
 }
 
 /* static */
-bool PopupBlocker::TryUsePopupOpeningToken(nsIPrincipal* aPrincipal) {
-  MOZ_ASSERT(sPopupStatePusherCount);
-
-  if (!sUnusedPopupToken) {
-    sUnusedPopupToken = true;
-    return true;
-  }
-
-  if (aPrincipal && aPrincipal->IsSystemPrincipal()) {
-    return true;
-  }
-
-  return false;
-}
-
-/* static */
 void PopupBlocker::PopupStatePusherCreated() { ++sPopupStatePusherCount; }
 
 /* static */
 void PopupBlocker::PopupStatePusherDestroyed() {
   MOZ_ASSERT(sPopupStatePusherCount);
-
-  if (!--sPopupStatePusherCount) {
-    sUnusedPopupToken = false;
-  }
+  --sPopupStatePusherCount;
 }
 
 // static
 PopupBlocker::PopupControlState PopupBlocker::GetEventPopupControlState(
     WidgetEvent* aEvent, Event* aDOMEvent) {
   // generally if an event handler is running, new windows are disallowed.
   // check for exceptions:
   PopupBlocker::PopupControlState abuse = PopupBlocker::openAbused;
--- a/dom/base/PopupBlocker.h
+++ b/dom/base/PopupBlocker.h
@@ -41,23 +41,16 @@ class PopupBlocker final {
 
   static PopupControlState GetPopupControlState();
 
   static void PopupStatePusherCreated();
   static void PopupStatePusherDestroyed();
 
   static uint32_t GetPopupPermission(nsIPrincipal* aPrincipal);
 
-  // This method returns true if the caller is allowed to show a popup, and it
-  // consumes the popup token for the current event. There is just 1 popup
-  // allowed per event.
-  // This method returns true if the token has been already consumed but
-  // aPrincipal is the system principal.
-  static bool TryUsePopupOpeningToken(nsIPrincipal* aPrincipal);
-
   static PopupBlocker::PopupControlState GetEventPopupControlState(
       WidgetEvent* aEvent, Event* aDOMEvent = nullptr);
 
   // Returns if a external protocol iframe is allowed.
   static bool ConsumeTimerTokenForExternalProtocolIframe();
 
   // Returns when the last external protocol iframe has been allowed.
   static TimeStamp WhenLastExternalProtocolIframeAllowed();
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -5761,17 +5761,20 @@ PopupBlocker::PopupControlState nsGlobal
     }
   }
 
   // If this popup is allowed, let's block any other for this event, forcing
   // PopupBlocker::openBlocked state.
   if ((abuse == PopupBlocker::openAllowed ||
        abuse == PopupBlocker::openControlled) &&
       StaticPrefs::dom_block_multiple_popups() && !IsPopupAllowed() &&
-      !PopupBlocker::TryUsePopupOpeningToken(mDoc->NodePrincipal())) {
+      !mDoc->ConsumeTransientUserGestureActivation()) {
+    nsContentUtils::ReportToConsole(nsIScriptError::warningFlag, "DOM"_ns, mDoc,
+                                    nsContentUtils::eDOM_PROPERTIES,
+                                    "MultiplePopupsBlockedNoUserActivation");
     abuse = PopupBlocker::openBlocked;
   }
 
   return abuse;
 }
 
 /* If a window open is blocked, fire the appropriate DOM events. */
 void nsGlobalWindowOuter::FireAbuseEvents(
--- a/dom/base/test/browser_multiple_popups.js
+++ b/dom/base/test/browser_multiple_popups.js
@@ -157,17 +157,17 @@ add_task(async _ => {
 
   let browser = gBrowser.getBrowserForTab(tab);
   await BrowserTestUtils.browserLoaded(browser);
 
   let obs = new WindowObserver(2);
 
   await BrowserTestUtils.synthesizeMouseAtCenter(
     "#input",
-    { type: "mouseup" },
+    {},
     tab.linkedBrowser
   );
 
   await obs;
   ok(true, "We had 2 windows.");
 
   await BrowserTestUtils.synthesizeMouseAtCenter(
     "#closeAllWindows",
@@ -276,17 +276,17 @@ add_task(async _ => {
       );
     });
   });
 
   let obs = new WindowObserver(1);
 
   await BrowserTestUtils.synthesizeMouseAtCenter(
     "#input",
-    { type: "mouseup" },
+    {},
     tab.linkedBrowser
   );
 
   await p;
   await obs;
   ok(true, "We had only 1 window.");
 
   await BrowserTestUtils.synthesizeMouseAtCenter(
--- a/dom/locales/en-US/chrome/dom/dom.properties
+++ b/dom/locales/en-US/chrome/dom/dom.properties
@@ -402,8 +402,9 @@ RequestStorageAccessUserGesture=document
 # LOCALIZATION NOTE: Do not translate "Location" and "History".
 LocChangeFloodingPrevented=Too many calls to Location or History APIs within a short timeframe.
 FolderUploadPrompt.title = Confirm Upload
 # LOCALIZATION NOTE: %S is the name of the folder the user selected in the file picker.
 FolderUploadPrompt.message = Are you sure you want to upload all files from ā€œ%Sā€? Only do this if you trust the site.
 FolderUploadPrompt.acceptButtonLabel = Upload
 InputPickerBlockedNoUserActivation=<input> picker was blocked due to lack of user activation.
 ExternalProtocolFrameBlockedNoUserActivation=Iframe with external protocol was blocked due to lack of user activation, or because not enough time has passed since the last such iframe was loaded.
+MultiplePopupsBlockedNoUserActivation=Opening multiple popups was blocked due to lack of user activation.