Backed out changeset 0b1c2e4169ab (bug 1739145) for causing failures at browser_download_spam_protection.js. CLOSED TREE
authorButkovits Atila <abutkovits@mozilla.com>
Fri, 26 Nov 2021 19:10:38 +0200
changeset 600255 c0bd08bc5b382cd006a39fb3723025bb3f29cf11
parent 600254 aa66642c9d65e8f1ea15da90e6f87a3c6ec2d254
child 600256 9d69de646094ddde5f4f167c41a9eddfefd0f36d
push id39014
push usernfay@mozilla.com
push dateFri, 26 Nov 2021 21:20:28 +0000
treeherdermozilla-central@c0bd08bc5b38 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1739145
milestone96.0a1
backs out0b1c2e4169ab51ab857d46285302a3118273731a
first release with
nightly linux32
c0bd08bc5b38 / 96.0a1 / 20211126212028 / files
nightly linux64
c0bd08bc5b38 / 96.0a1 / 20211126212028 / files
nightly mac
c0bd08bc5b38 / 96.0a1 / 20211126212028 / files
nightly win32
c0bd08bc5b38 / 96.0a1 / 20211126212028 / files
nightly win64
c0bd08bc5b38 / 96.0a1 / 20211126212028 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset 0b1c2e4169ab (bug 1739145) for causing failures at browser_download_spam_protection.js. CLOSED TREE
browser/components/downloads/DownloadSpamProtection.jsm
browser/components/downloads/content/downloads.js
browser/components/downloads/test/browser/browser_download_spam_protection.js
toolkit/components/downloads/DownloadIntegration.jsm
uriloader/exthandler/tests/mochitest/browser_download_spam_permissions.js
--- a/browser/components/downloads/DownloadSpamProtection.jsm
+++ b/browser/components/downloads/DownloadSpamProtection.jsm
@@ -25,44 +25,50 @@ XPCOMUtils.defineLazyModuleGetters(this,
   DownloadList: "resource://gre/modules/DownloadList.jsm",
 });
 
 /**
  * Responsible for detecting events related to downloads spam and updating the
  * downloads UI with this information.
  */
 class DownloadSpamProtection {
+  static TOPIC = "blocked-automatic-download";
+
   constructor() {
     /**
      * Tracks URLs we have detected download spam for.
      * @type {Map<string, DownloadSpam>}
      */
     this._blockedURLToDownloadSpam = new Map();
     this._browserWin = BrowserWindowTracker.getTopWindow();
     this._indicator = DownloadsCommon.getIndicatorData(this._browserWin);
     this.list = new DownloadList();
   }
 
   get spamList() {
     return this.list;
   }
 
-  update(url) {
-    if (this._blockedURLToDownloadSpam.has(url)) {
-      let downloadSpam = this._blockedURLToDownloadSpam.get(url);
+  async observe(aSubject, aTopic, URL) {
+    if (aTopic != DownloadSpamProtection.TOPIC) {
+      return;
+    }
+
+    if (this._blockedURLToDownloadSpam.has(URL)) {
+      let downloadSpam = this._blockedURLToDownloadSpam.get(URL);
       this.spamList.remove(downloadSpam);
       downloadSpam.blockedDownloadsCount += 1;
       this.spamList.add(downloadSpam);
       this._indicator.onDownloadStateChanged(downloadSpam);
       return;
     }
 
-    let downloadSpam = new DownloadSpam(url);
+    let downloadSpam = new DownloadSpam(URL);
     this.spamList.add(downloadSpam);
-    this._blockedURLToDownloadSpam.set(url, downloadSpam);
+    this._blockedURLToDownloadSpam.set(URL, downloadSpam);
     let hasActiveDownloads = DownloadsCommon.summarizeDownloads(
       this._indicator._activeDownloads()
     ).numDownloading;
     if (!hasActiveDownloads) {
       this._browserWin.DownloadsPanel.showPanel();
     }
     this._indicator.onDownloadAdded(downloadSpam);
   }
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -157,21 +157,19 @@ var DownloadsPanel = {
     this.panel.hidden = false;
     DownloadsViewController.initialize();
     DownloadsCommon.log("Attaching DownloadsView...");
     DownloadsCommon.getData(window).addView(DownloadsView);
     DownloadsCommon.getSummary(window, DownloadsView.kItemCountLimit).addView(
       DownloadsSummary
     );
 
-    if (DownloadIntegration.downloadSpamProtection) {
-      DownloadIntegration.downloadSpamProtection.spamList.addView(
-        DownloadsView
-      );
-    }
+    DownloadIntegration.getDownloadSpamProtection().spamList.addView(
+      DownloadsView
+    );
 
     DownloadsCommon.log(
       "DownloadsView attached - the panel for this window",
       "should now see download items come in."
     );
     DownloadsPanel._attachEventListeners();
     DownloadsCommon.log("DownloadsPanel initialized.");
   },
@@ -197,23 +195,19 @@ var DownloadsPanel = {
 
     DownloadsViewController.terminate();
     DownloadsCommon.getData(window).removeView(DownloadsView);
     DownloadsCommon.getSummary(
       window,
       DownloadsView.kItemCountLimit
     ).removeView(DownloadsSummary);
     this._unattachEventListeners();
-
-    if (DownloadIntegration.downloadSpamProtection) {
-      DownloadIntegration.downloadSpamProtection.spamList.removeView(
-        DownloadsView
-      );
-    }
-
+    DownloadIntegration.getDownloadSpamProtection().spamList.removeView(
+      DownloadsView
+    );
     this._state = this.kStateUninitialized;
 
     DownloadsSummary.active = false;
     DownloadsCommon.log("DownloadsPanel terminated.");
   },
 
   // Panel interface
 
--- a/browser/components/downloads/test/browser/browser_download_spam_protection.js
+++ b/browser/components/downloads/test/browser/browser_download_spam_protection.js
@@ -49,46 +49,53 @@ add_task(async function setup() {
   registerCleanupFunction(async () => {
     Services.prefs.clearUserPref("browser.download.folderList");
     Services.prefs.clearUserPref("browser.download.dir");
     await IOUtils.remove(tempDir.path, { recursive: true });
   });
 });
 
 add_task(async function check_download_spam_ui() {
-  registerCleanupFunction(async () => {
-    BrowserWindowTracker.getTopWindow().DownloadsPanel.hidePanel();
-    DownloadIntegration.downloadSpamProtection.clearDownloadSpam(TEST_URI);
-    let publicList = await Downloads.getList(Downloads.PUBLIC);
-    await publicList.removeFinished();
-    BrowserTestUtils.removeTab(newTab);
-  });
-  let observedBlockedDownloads = 0;
-  let gotAllBlockedDownloads = TestUtils.topicObserved(
-    "blocked-automatic-download",
-    () => {
-      return ++observedBlockedDownloads >= 99;
+  let spamProtection = DownloadIntegration.getDownloadSpamProtection();
+  let oldFunction = spamProtection.observe.bind(spamProtection);
+  let blockedDownloads = PromiseUtils.defer();
+  let counter = 0;
+  let newFunction = async (aSubject, aTopic, URL) => {
+    await oldFunction(aSubject, aTopic, URL);
+    counter++;
+    if (counter == 99) {
+      blockedDownloads.resolve();
     }
-  );
+  };
+  spamProtection.observe = newFunction;
 
   let newTab = await BrowserTestUtils.openNewForegroundTab(
     gBrowser,
     TEST_PATH + "test_spammy_page.html"
   );
 
+  registerCleanupFunction(async () => {
+    spamProtection.observe = oldFunction;
+
+    BrowserWindowTracker.getTopWindow().DownloadsPanel.hidePanel();
+    DownloadIntegration.getDownloadSpamProtection().clearDownloadSpam(TEST_URI);
+    let publicList = await Downloads.getList(Downloads.PUBLIC);
+    await publicList.removeFinished();
+
+    BrowserTestUtils.removeTab(newTab);
+  });
+
   await BrowserTestUtils.synthesizeMouseAtCenter(
     "body",
     {},
     newTab.linkedBrowser
   );
 
-  info("Waiting on all blocked downloads.");
-  await gotAllBlockedDownloads;
+  await blockedDownloads.promise;
 
-  let spamProtection = DownloadIntegration.downloadSpamProtection;
   let spamList = spamProtection.spamList;
   is(
     spamList._downloads[0].blockedDownloadsCount,
     99,
     "99 blocked downloads recorded"
   );
   ok(
     spamList._downloads[0].error.becauseBlockedByReputationCheck,
--- a/toolkit/components/downloads/DownloadIntegration.jsm
+++ b/toolkit/components/downloads/DownloadIntegration.jsm
@@ -162,17 +162,16 @@ const kObserverTopics = [
   "last-pb-context-exited",
   "sleep_notification",
   "suspend_process_notification",
   "wake_notification",
   "resume_process_notification",
   "network:offline-about-to-go-offline",
   "network:offline-status-changed",
   "xpcom-will-shutdown",
-  "blocked-automatic-download",
 ];
 
 /**
  * Maps nsIApplicationReputationService verdicts with the DownloadError ones.
  */
 const kVerdictMap = {
   [Ci.nsIApplicationReputationService.VERDICT_DANGEROUS]:
     Downloads.Error.BLOCK_VERDICT_MALWARE,
@@ -963,21 +962,24 @@ var DownloadIntegration = {
    * Returns the string path for the given directory service location name. This
    * can be overridden by regression tests to return the path of the system
    * temporary directory in all cases.
    */
   _getDirectory(name) {
     return Services.dirsvc.get(name, Ci.nsIFile).path;
   },
   /**
-   * Initializes the DownloadSpamProtection instance.
+   * Returns the DownloadSpamProtection instance.
    * This is used to observe and group multiple automatic downloads.
    */
-  _initializeDownloadSpamProtection() {
-    this.downloadSpamProtection = new DownloadSpamProtection();
+  getDownloadSpamProtection() {
+    if (!this._downloadSpamProtection) {
+      this._downloadSpamProtection = new DownloadSpamProtection();
+    }
+    return this._downloadSpamProtection;
   },
 
   /**
    * Register the downloads interruption observers.
    *
    * @param aList
    *        The public or private downloads list.
    * @param aIsPrivate
@@ -988,16 +990,22 @@ var DownloadIntegration = {
    */
   addListObservers(aList, aIsPrivate) {
     DownloadObserver.registerView(aList, aIsPrivate);
     if (!DownloadObserver.observersAdded) {
       DownloadObserver.observersAdded = true;
       for (let topic of kObserverTopics) {
         Services.obs.addObserver(DownloadObserver, topic);
       }
+      if (AppConstants.MOZ_BUILD_APP == "browser") {
+        Services.obs.addObserver(
+          this.getDownloadSpamProtection(),
+          DownloadSpamProtection.TOPIC
+        );
+      }
     }
     return Promise.resolve();
   },
 
   /**
    * Force a save on _store if it exists. Used to ensure downloads do not
    * persist after being sanitized on Android.
    *
@@ -1217,25 +1225,25 @@ var DownloadObserver = {
       // required services are not available anymore. We can't unregister
       // observers on "quit-application", because this module is also loaded
       // during "make package" automation, and the quit notification is not sent
       // in that execution environment (bug 973637).
       case "xpcom-will-shutdown":
         for (let topic of kObserverTopics) {
           Services.obs.removeObserver(this, topic);
         }
-        break;
-      case "blocked-automatic-download":
         if (
           AppConstants.MOZ_BUILD_APP == "browser" &&
-          !DownloadIntegration.downloadSpamProtection
+          this._downloadSpamProtection
         ) {
-          DownloadIntegration._initializeDownloadSpamProtection();
+          Services.obs.removeObserver(
+            this._downloadSpamProtection,
+            DownloadSpamProtection.TOPIC
+          );
         }
-        DownloadIntegration.downloadSpamProtection.update(aData);
         break;
     }
   },
 
   QueryInterface: ChromeUtils.generateQI(["nsIObserver"]),
 };
 
 /**
--- a/uriloader/exthandler/tests/mochitest/browser_download_spam_permissions.js
+++ b/uriloader/exthandler/tests/mochitest/browser_download_spam_permissions.js
@@ -72,17 +72,17 @@ add_task(async function check_download_s
   };
   Services.obs.addObserver(automaticDownloadObserver, AUTOMATIC_DOWNLOAD_TOPIC);
 
   let newTab = await BrowserTestUtils.openNewForegroundTab(
     gBrowser,
     TEST_PATH + "test_spammy_page.html"
   );
   registerCleanupFunction(async () => {
-    DownloadIntegration.downloadSpamProtection.clearDownloadSpam(TEST_URI);
+    DownloadIntegration.getDownloadSpamProtection().clearDownloadSpam(TEST_URI);
     DownloadsPanel.hidePanel();
     await publicList.removeFinished();
     BrowserTestUtils.removeTab(newTab);
     Services.obs.removeObserver(
       automaticDownloadObserver,
       AUTOMATIC_DOWNLOAD_TOPIC
     );
   });