Bug 1514340 - Part 2: Break out the content blocking related notifications into nsIWebProgressListener.onContentBlockingEvent(); r=baku,johannh

Differential Revision: https://phabricator.services.mozilla.com/D16052

Author: Ehsan Akhgari

accessible/base/DocManager.cpp

@@ -308,6 +308,13 @@ DocManager::OnSecurityChange(nsIWebProgress* aWebProgress, nsIRequest* aRequest,
   return NS_OK;
 }
 
+NS_IMETHODIMP
+DocManager::OnContentBlockingEvent(nsIWebProgress* aWebProgress,
+                                   nsIRequest* aRequest, uint32_t aEvent) {
+  MOZ_ASSERT_UNREACHABLE("notification excluded in AddProgressListener(...)");
+  return NS_OK;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // nsIDOMEventListener
 

browser/base/content/browser-contentblocking.js

@@ -812,7 +812,7 @@ var ContentBlocking = {
     Services.telemetry.getHistogramById("TRACKING_PROTECTION_SHIELD").add(value);
   },
 
-  onSecurityChange(state, webProgress, isSimulated) {
+  onContentBlockingEvent(event, webProgress, isSimulated) {
     let baseURI = this._baseURIForChannelClassifier;
 
     // Don't deal with about:, file: etc.
@@ -831,9 +831,9 @@ var ContentBlocking = {
       // reporting it using the "report breakage" dialog. Under normal circumstances this
       // dialog should only be able to open in the currently selected tab and onSecurityChange
       // runs on tab switch, so we can avoid associating the data with the document directly.
-      blocker.activated = blocker.isBlocking(state);
+      blocker.activated = blocker.isBlocking(event);
       blocker.categoryItem.classList.toggle("blocked", blocker.enabled);
-      let detected = blocker.isDetected(state);
+      let detected = blocker.isDetected(event);
       blocker.categoryItem.hidden = !detected;
       anyDetected = anyDetected || detected;
       anyBlocking = anyBlocking || blocker.activated;

browser/base/content/browser.js

@@ -4898,13 +4898,40 @@ var XULBrowserWindow = {
   // Properties used to cache security state used to update the UI
   _state: null,
   _lastLocation: null,
+  _event: null,
+  _lastLocationForEvent: null,
 
   // This is called in multiple ways:
-  //  1. Due to the nsIWebProgressListener.onSecurityChange notification.
+  //  1. Due to the nsIWebProgressListener.onContentBlockingEvent notification.
   //  2. Called by tabbrowser.xml when updating the current browser.
   //  3. Called directly during this object's initializations.
+  //  4. Due to the nsIWebProgressListener.onLocationChange notification.
   // aRequest will be null always in case 2 and 3, and sometimes in case 1 (for
   // instance, there won't be a request when STATE_BLOCKED_TRACKING_CONTENT is observed).
+  onContentBlockingEvent(aWebProgress, aRequest, aEvent, aIsSimulated) {
+    // Don't need to do anything if the data we use to update the UI hasn't
+    // changed
+    let uri = gBrowser.currentURI;
+    let spec = uri.spec;
+    if (this._event == aEvent &&
+        this._lastLocationForEvent == spec) {
+      return;
+    }
+    this._event = aEvent;
+    this._lastLocationForEvent = spec;
+
+    if (typeof(aIsSimulated) != "boolean" && typeof(aIsSimulated) != "undefined") {
+      throw "onContentBlockingEvent: aIsSimulated receieved an unexpected type";
+    }
+
+    ContentBlocking.onContentBlockingEvent(this._event, aWebProgress, aIsSimulated);
+  },
+
+  // This is called in multiple ways:
+  //  1. Due to the nsIWebProgressListener.onSecurityChange notification.
+  //  2. Called by tabbrowser.xml when updating the current browser.
+  //  3. Called directly during this object's initializations.
+  // aRequest will be null always in case 2 and 3, and sometimes in case 1.
   onSecurityChange(aWebProgress, aRequest, aState, aIsSimulated) {
     // Don't need to do anything if the data we use to update the UI hasn't
     // changed
@@ -4920,10 +4947,6 @@ var XULBrowserWindow = {
     this._state = aState;
     this._lastLocation = spec;
 
-    if (typeof(aIsSimulated) != "boolean" && typeof(aIsSimulated) != "undefined") {
-      throw "onSecurityChange: aIsSimulated receieved an unexpected type";
-    }
-
     // Make sure the "https" part of the URL is striked out or not,
     // depending on the current mixed active content blocking state.
     gURLBar.formatValue();
@@ -4932,7 +4955,6 @@ var XULBrowserWindow = {
       uri = Services.uriFixup.createExposableURI(uri);
     } catch (e) {}
     gIdentityHandler.updateIdentity(this._state, uri);
-    ContentBlocking.onSecurityChange(this._state, aWebProgress, aIsSimulated);
   },
 
   // simulate all change notifications after switching tabs

browser/base/content/tabbrowser.js

@@ -968,10 +968,13 @@ window._gBrowser = {
 
     let securityUI = newBrowser.securityUI;
     if (securityUI) {
+      this._callProgressListeners(null, "onSecurityChange",
+                                  [webProgress, null, securityUI.state],
+                                  true, false);
       // Include the true final argument to indicate that this event is
       // simulated (instead of being observed by the webProgressListener).
-      this._callProgressListeners(null, "onSecurityChange",
-                                  [webProgress, null, securityUI.state, true],
+      this._callProgressListeners(null, "onContentBlockingEvent",
+                                  [webProgress, null, securityUI.contentBlockingEvent, true],
                                   true, false);
     }
 
@@ -1718,10 +1721,14 @@ window._gBrowser = {
     let securityUI = aBrowser.securityUI;
     let state = securityUI ? securityUI.state :
       Ci.nsIWebProgressListener.STATE_IS_INSECURE;
+    this._callProgressListeners(aBrowser, "onSecurityChange",
+                                [aBrowser.webProgress, null, state],
+                                true, false);
+    let event = securityUI ? securityUI.contentBlockingEvent : 0;
     // Include the true final argument to indicate that this event is
     // simulated (instead of being observed by the webProgressListener).
-    this._callProgressListeners(aBrowser, "onSecurityChange",
-                                [aBrowser.webProgress, null, state, true],
+    this._callProgressListeners(aBrowser, "onContentBlockingEvent",
+                                [aBrowser.webProgress, null, event, true],
                                 true, false);
 
     if (aShouldBeRemote) {
@@ -5192,6 +5199,10 @@ class TabProgressListener {
     if (!this.mBlank) {
       this._callProgressListeners("onLocationChange",
                                   [aWebProgress, aRequest, aLocation, aFlags]);
+      // Include the true final argument to indicate that this event is
+      // simulated (instead of being observed by the webProgressListener).
+      this._callProgressListeners("onContentBlockingEvent",
+                                  [aWebProgress, null, 0, true]);
     }
 
     if (topLevel) {
@@ -5215,6 +5226,11 @@ class TabProgressListener {
                                 [aWebProgress, aRequest, aState]);
   }
 
+  onContentBlockingEvent(aWebProgress, aRequest, aEvent) {
+    this._callProgressListeners("onContentBlockingEvent",
+                                [aWebProgress, aRequest, aEvent]);
+  }
+
   onRefreshAttempted(aWebProgress, aURI, aDelay, aSameURI) {
     return this._callProgressListeners("onRefreshAttempted",
                                        [aWebProgress, aURI, aDelay, aSameURI]);

browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js

@@ -69,19 +69,21 @@ async function testTrackingProtectionAnimation(tabbrowser) {
   ok(!ContentBlocking.iconBox.hasAttribute("animate"), "iconBox not animating");
 
   info("Reload tracking cookies tab");
-  securityChanged = waitForSecurityChange(2, tabbrowser.ownerGlobal);
+  securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal);
+  let contentBlockingEvent = waitForContentBlockingEvent(2, tabbrowser.ownerGlobal);
   tabbrowser.reload();
-  await securityChanged;
+  await Promise.all([securityChanged, contentBlockingEvent]);
 
   ok(ContentBlocking.iconBox.hasAttribute("active"), "iconBox active");
   ok(ContentBlocking.iconBox.hasAttribute("animate"), "iconBox animating");
   await BrowserTestUtils.waitForEvent(ContentBlocking.animatedIcon, "animationend");
 
   info("Reload tracking tab");
-  securityChanged = waitForSecurityChange(3, tabbrowser.ownerGlobal);
+  securityChanged = waitForSecurityChange(2, tabbrowser.ownerGlobal);
+  contentBlockingEvent = waitForContentBlockingEvent(3, tabbrowser.ownerGlobal);
   tabbrowser.selectedTab = trackingTab;
   tabbrowser.reload();
-  await securityChanged;
+  await Promise.all([securityChanged, contentBlockingEvent]);
 
   ok(ContentBlocking.iconBox.hasAttribute("active"), "iconBox active");
   ok(ContentBlocking.iconBox.hasAttribute("animate"), "iconBox animating");

browser/base/content/test/trackingUI/browser_trackingUI_fetch.js

@@ -6,13 +6,13 @@ add_task(async function test_fetch() {
   ]});
 
   await BrowserTestUtils.withNewTab({ gBrowser, url: URL }, async function(newTabBrowser) {
-    let securityChange = waitForSecurityChange();
+    let contentBlockingEvent = waitForContentBlockingEvent();
     await ContentTask.spawn(newTabBrowser, null, async function() {
       await content.wrappedJSObject.test_fetch()
                    .then(response => Assert.ok(false, "should have denied the request"))
                    .catch(e => Assert.ok(true, `Caught exception: ${e}`));
     });
-    await securityChange;
+    await contentBlockingEvent;
 
     let ContentBlocking = newTabBrowser.ownerGlobal.ContentBlocking;
     ok(ContentBlocking, "got CB object");

browser/base/content/test/trackingUI/browser_trackingUI_telemetry.js

@@ -54,7 +54,7 @@ add_task(async function testShieldHistogram() {
 
   await promiseTabLoadEvent(tab, TRACKING_PAGE);
   // Note that right now the shield histogram is not measuring what
-  // you might think.  Since onSecurityChange fires twice for a tracking page,
+  // you might think.  Since onContentBlockingEvent fires twice for a tracking page,
   // the total page loads count is double counting, and the shield count
   // (which is meant to measure times when the shield wasn't shown) fires even
   // when tracking elements exist on the page.

browser/base/content/test/trackingUI/head.js

@@ -59,3 +59,23 @@ function waitForSecurityChange(numChanges = 1, win = null) {
     win.gBrowser.addProgressListener(listener);
   });
 }
+
+function waitForContentBlockingEvent(numChanges = 1, win = null) {
+  if (!win) {
+    win = window;
+  }
+  return new Promise(resolve => {
+    let n = 0;
+    let listener = {
+      onContentBlockingEvent() {
+        n = n + 1;
+        info("Received onContentBlockingEvent event " + n + " of " + numChanges);
+        if (n >= numChanges) {
+          win.gBrowser.removeProgressListener(listener);
+          resolve(n);
+        }
+      },
+    };
+    win.gBrowser.addProgressListener(listener);
+  });
+}

browser/components/extensions/parent/ext-tabs.js

@@ -1244,6 +1244,7 @@ this.tabs = class extends ExtensionAPI {
                   onLocationChange(webProgress, request, location, flags) { },
                   onProgressChange(webProgress, request, curSelfProgress, maxSelfProgress, curTotalProgress, maxTotalProgress) { },
                   onSecurityChange(webProgress, request, state) { },
+                  onContentBlockingEvent(webProgress, request, event) { },
                   onStateChange(webProgress, request, flags, status) {
                     if ((flags & Ci.nsIWebProgressListener.STATE_STOP) && (flags & Ci.nsIWebProgressListener.STATE_IS_DOCUMENT)) {
                       resolve(retval == 0 ? "saved" : "replaced");

browser/components/shell/nsMacShellService.cpp

@@ -188,6 +188,13 @@ nsMacShellService::OnSecurityChange(nsIWebProgress* aWebProgress,
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsMacShellService::OnContentBlockingEvent(nsIWebProgress* aWebProgress,
+                                          nsIRequest* aRequest,
+                                          uint32_t aEvent) {
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsMacShellService::OnStateChange(nsIWebProgress* aWebProgress,
                                  nsIRequest* aRequest, uint32_t aStateFlags,