Bug 1511303 - Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre. r=baku, a=RyanVM
authorEhsan Akhgari <ehsan@mozilla.com>
Tue, 18 Dec 2018 23:20:30 +0000
changeset 509117 fef8ee3bdc39ed3171fc133f9d4215dd7228540f
parent 509116 ef357d2c21405cd5c4de8129c81c6a000b651a0c
child 509118 4f9b811b0f2aa917536f1a0419c5e9a2dcd801e9
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, RyanVM
bugs1511303
milestone65.0
Bug 1511303 - Ensure that visiting a page with an iframe does not cause an entry to be injected in the Cookies subpanel in the Control Centre. r=baku, a=RyanVM Differential Revision: https://phabricator.services.mozilla.com/D14776
browser/base/content/test/general/browser_alltabslistener.js
browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js
browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js
browser/base/content/test/trackingUI/browser_trackingUI_state.js
dom/serviceworkers/ServiceWorkerInterceptController.cpp
--- a/browser/base/content/test/general/browser_alltabslistener.js
+++ b/browser/base/content/test/general/browser_alltabslistener.js
@@ -146,56 +146,52 @@ function runTest(browser, url, next) {
 
 function startTest1() {
   info("\nTest 1");
   gBrowser.addProgressListener(gFrontProgressListener);
   gBrowser.addTabsProgressListener(gAllProgressListener);
 
   gAllNotifications = [
     "onStateChange",
-    "onSecurityChange",
     "onLocationChange",
     "onSecurityChange",
     "onStateChange",
   ];
   gFrontNotifications = gAllNotifications;
   runTest(gForegroundBrowser, "http://example.org" + gTestPage, startTest2);
 }
 
 function startTest2() {
   info("\nTest 2");
   gAllNotifications = [
     "onStateChange",
-    "onSecurityChange",
     "onLocationChange",
     "onSecurityChange",
     "onStateChange",
   ];
   gFrontNotifications = gAllNotifications;
   runTest(gForegroundBrowser, "https://example.com" + gTestPage, startTest3);
 }
 
 function startTest3() {
   info("\nTest 3");
   gAllNotifications = [
     "onStateChange",
-    "onSecurityChange",
     "onLocationChange",
     "onSecurityChange",
     "onStateChange",
   ];
   gFrontNotifications = [];
   runTest(gBackgroundBrowser, "http://example.org" + gTestPage, startTest4);
 }
 
 function startTest4() {
   info("\nTest 4");
   gAllNotifications = [
     "onStateChange",
-    "onSecurityChange",
     "onLocationChange",
     "onSecurityChange",
     "onStateChange",
   ];
   gFrontNotifications = [];
   runTest(gBackgroundBrowser, "https://example.com" + gTestPage, startTest5);
 }
 
@@ -206,30 +202,28 @@ function startTest5() {
   [gForegroundTab, gBackgroundTab] = [gBackgroundTab, gForegroundTab];
   // Avoid the onLocationChange this will fire
   gBrowser.removeProgressListener(gFrontProgressListener);
   gBrowser.selectedTab = gForegroundTab;
   gBrowser.addProgressListener(gFrontProgressListener);
 
   gAllNotifications = [
     "onStateChange",
-    "onSecurityChange",
     "onLocationChange",
     "onSecurityChange",
     "onStateChange",
   ];
   gFrontNotifications = gAllNotifications;
   runTest(gForegroundBrowser, "http://example.org" + gTestPage, startTest6);
 }
 
 function startTest6() {
   info("\nTest 6");
   gAllNotifications = [
     "onStateChange",
-    "onSecurityChange",
     "onLocationChange",
     "onSecurityChange",
     "onStateChange",
   ];
   gFrontNotifications = [];
   runTest(gBackgroundBrowser, "http://example.org" + gTestPage, finishTest);
 }
 
--- a/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js
+++ b/browser/base/content/test/siteIdentity/browser_ignore_same_page_navigation.js
@@ -26,16 +26,16 @@ add_task(async function() {
     };
     browser.addProgressListener(progressListener, Ci.nsIWebProgress.NOTIFY_ALL);
 
     let uri = getRootDirectory(gTestPath).replace("chrome://mochitests/content",
                                                   "https://example.com") + "dummy_page.html";
     BrowserTestUtils.loadURI(browser, uri);
     await BrowserTestUtils.browserLoaded(browser, false, uri);
     is(onLocationChangeCount, 1, "should have 1 onLocationChange event");
-    is(onSecurityChangeCount, 2, "should have 2 onSecurityChange event");
+    is(onSecurityChangeCount, 1, "should have 1 onSecurityChange event");
     await ContentTask.spawn(browser, null, async () => {
       content.history.pushState({}, "", "https://example.com");
     });
     is(onLocationChangeCount, 2, "should have 2 onLocationChange events");
-    is(onSecurityChangeCount, 2, "should still have only 2 onSecurityChange event");
+    is(onSecurityChangeCount, 1, "should still have only 1 onSecurityChange event");
   });
 });
--- a/browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js
+++ b/browser/base/content/test/trackingUI/browser_trackingUI_animation_2.js
@@ -64,26 +64,26 @@ async function testTrackingProtectionAni
   securityChanged = waitForSecurityChange(1, tabbrowser.ownerGlobal);
   tabbrowser.selectedTab = trackingCookiesTab;
   await securityChanged;
 
   ok(ContentBlocking.iconBox.hasAttribute("active"), "iconBox active");
   ok(!ContentBlocking.iconBox.hasAttribute("animate"), "iconBox not animating");
 
   info("Reload tracking cookies tab");
-  securityChanged = waitForSecurityChange(3, tabbrowser.ownerGlobal);
+  securityChanged = waitForSecurityChange(2, tabbrowser.ownerGlobal);
   tabbrowser.reload();
   await securityChanged;
 
   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(4, tabbrowser.ownerGlobal);
+  securityChanged = waitForSecurityChange(3, tabbrowser.ownerGlobal);
   tabbrowser.selectedTab = trackingTab;
   tabbrowser.reload();
   await securityChanged;
 
   ok(ContentBlocking.iconBox.hasAttribute("active"), "iconBox active");
   ok(ContentBlocking.iconBox.hasAttribute("animate"), "iconBox animating");
   await BrowserTestUtils.waitForEvent(ContentBlocking.animatedIcon, "animationend");
 
--- a/browser/base/content/test/trackingUI/browser_trackingUI_state.js
+++ b/browser/base/content/test/trackingUI/browser_trackingUI_state.js
@@ -130,18 +130,23 @@ function testTrackingPage(window) {
        "unblockButton is" + (blockedByTP ? "" : " not") + " visible");
   }
 
   ok(hidden("#identity-popup-content-blocking-not-detected"), "blocking not detected label is hidden");
   ok(!hidden("#identity-popup-content-blocking-detected"), "blocking detected label is visible");
 
   ok(!hidden("#identity-popup-content-blocking-category-tracking-protection"),
     "Showing trackers category");
-  ok(!hidden("#identity-popup-content-blocking-category-cookies"),
-    "Showing cookie restrictions category");
+  if (gTrackingPageURL == COOKIE_PAGE) {
+    ok(!hidden("#identity-popup-content-blocking-category-cookies"),
+      "Showing cookie restrictions category");
+  } else {
+    ok(hidden("#identity-popup-content-blocking-category-cookies"),
+      "Not showing cookie restrictions category");
+  }
 }
 
 function testTrackingPageUnblocked(blockedByTP, window) {
   info("Tracking content must be white-listed and not blocked");
   ok(ContentBlocking.content.hasAttribute("detected"), "trackers are detected");
   ok(ContentBlocking.content.hasAttribute("hasException"), "content shows exception");
 
   ok(!ContentBlocking.iconBox.hasAttribute("active"), "shield is not active");
@@ -153,18 +158,23 @@ function testTrackingPageUnblocked(block
   ok(!hidden("#tracking-action-block"), "blockButton is visible");
   ok(hidden("#tracking-action-unblock"), "unblockButton is hidden");
 
   ok(hidden("#identity-popup-content-blocking-not-detected"), "blocking not detected label is hidden");
   ok(!hidden("#identity-popup-content-blocking-detected"), "blocking detected label is visible");
 
   ok(!hidden("#identity-popup-content-blocking-category-tracking-protection"),
     "Showing trackers category");
-  ok(!hidden("#identity-popup-content-blocking-category-cookies"),
-    "Showing cookie restrictions category");
+  if (gTrackingPageURL == COOKIE_PAGE) {
+    ok(!hidden("#identity-popup-content-blocking-category-cookies"),
+      "Showing cookie restrictions category");
+  } else {
+    ok(hidden("#identity-popup-content-blocking-category-cookies"),
+      "Not showing cookie restrictions category");
+  }
 }
 
 async function testContentBlocking(tab) {
   info("Testing with Tracking Protection ENABLED.");
 
   info("Load a test page not containing tracking elements");
   await promiseTabLoadEvent(tab, BENIGN_PAGE);
   testBenignPage();
--- a/dom/serviceworkers/ServiceWorkerInterceptController.cpp
+++ b/dom/serviceworkers/ServiceWorkerInterceptController.cpp
@@ -33,32 +33,35 @@ ServiceWorkerInterceptController::Should
   // window.
   if (!nsContentUtils::IsNonSubresourceRequest(aChannel)) {
     const Maybe<ServiceWorkerDescriptor>& controller =
         loadInfo->GetController();
     *aShouldIntercept = controller.isSome();
     return NS_OK;
   }
 
+  nsCOMPtr<nsIPrincipal> principal = BasePrincipal::CreateCodebasePrincipal(
+      aURI, loadInfo->GetOriginAttributes());
+
+  // First check with the ServiceWorkerManager for a matching service worker.
+  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+  if (!swm || !swm->IsAvailable(principal, aURI)) {
+    return NS_OK;
+  }
+
+  // Then check to see if we are allowed to control the window.
+  // It is important to check for the availability of the service worker first
+  // to avoid showing warnings about the use of third-party cookies in the UI
+  // unnecessarily when no service worker is being accessed.
   if (nsContentUtils::StorageAllowedForChannel(aChannel) !=
       nsContentUtils::StorageAccess::eAllow) {
     return NS_OK;
   }
 
-  nsCOMPtr<nsIPrincipal> principal = BasePrincipal::CreateCodebasePrincipal(
-      aURI, loadInfo->GetOriginAttributes());
-
-  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-  if (!swm) {
-    return NS_OK;
-  }
-
-  // We're allowed to control a window, so check with the ServiceWorkerManager
-  // for a matching service worker.
-  *aShouldIntercept = swm->IsAvailable(principal, aURI);
+  *aShouldIntercept = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ServiceWorkerInterceptController::ChannelIntercepted(
     nsIInterceptedChannel* aChannel) {
   // Note, do not cancel the interception here.  The caller will try to
   // ResetInterception() on error.