Bug 1937102 - Fix protections UI tests that rely on unpartitioning storage - r=anti-tracking-reviewers,cookie-reviewers,webcompat-reviewers,timhuang,twisniewski

This is where I found and addressed a bug with surplus console messages when we enabled APS

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

browser/base/content/test/protectionsUI/browser_protectionsUI_icon_state.js

@@ -7,8 +7,6 @@
 
 const TP_PREF = "privacy.trackingprotection.enabled";
 const TP_PB_PREF = "privacy.trackingprotection.pbmode.enabled";
-const APS_PREF =
-  "privacy.partition.always_partition_third_party_non_cookie_storage";
 const NCB_PREF = "network.cookie.cookieBehavior";
 const BENIGN_PAGE =
   // eslint-disable-next-line @microsoft/sdl/no-insecure-url
@@ -146,8 +144,6 @@ async function testTrackingProtectionIconState(tabbrowser) {
 }
 
 add_task(async function testNormalBrowsing() {
-  await SpecialPowers.pushPrefEnv({ set: [[APS_PREF, false]] });
-
   await UrlClassifierTestUtils.addTestTrackers();
 
   let gProtectionsHandler = gBrowser.ownerGlobal.gProtectionsHandler;
@@ -179,10 +175,7 @@ add_task(async function testNormalBrowsing() {
 
 add_task(async function testPrivateBrowsing() {
   await SpecialPowers.pushPrefEnv({
-    set: [
-      [APS_PREF, false],
-      ["dom.security.https_first_pbm", false],
-    ],
+    set: [["dom.security.https_first_pbm", false]],
   });
 
   let privateWin = await BrowserTestUtils.openNewBrowserWindow({

browser/base/content/test/protectionsUI/browser_protectionsUI_state.js

@@ -16,8 +16,6 @@
 
 const TP_PREF = "privacy.trackingprotection.enabled";
 const TP_PB_PREF = "privacy.trackingprotection.pbmode.enabled";
-const APS_PREF =
-  "privacy.partition.always_partition_third_party_non_cookie_storage";
 const TPC_PREF = "network.cookie.cookieBehavior";
 const BENIGN_PAGE =
   // eslint-disable-next-line @microsoft/sdl/no-insecure-url
@@ -275,8 +273,6 @@ async function testContentBlocking(tab) {
 }
 
 add_task(async function testNormalBrowsing() {
-  await SpecialPowers.pushPrefEnv({ set: [[APS_PREF, false]] });
-
   await UrlClassifierTestUtils.addTestTrackers();
 
   tabbrowser = gBrowser;
@@ -313,10 +309,7 @@ add_task(async function testNormalBrowsing() {
 
 add_task(async function testPrivateBrowsing() {
   await SpecialPowers.pushPrefEnv({
-    set: [
-      ["dom.security.https_first_pbm", false],
-      [APS_PREF, false],
-    ],
+    set: [["dom.security.https_first_pbm", false]],
   });
   let privateWin = await BrowserTestUtils.openNewBrowserWindow({
     private: true,
@@ -359,8 +352,6 @@ add_task(async function testPrivateBrowsing() {
 add_task(async function testThirdPartyCookies() {
   requestLongerTimeout(3);
 
-  await SpecialPowers.pushPrefEnv({ set: [[APS_PREF, false]] });
-
   await UrlClassifierTestUtils.addTestTrackers();
   gTrackingPageURL = COOKIE_PAGE;
 

browser/components/reportbrokensite/test/browser/browser_antitracking_data_sent.js

@@ -24,7 +24,7 @@ add_task(async function testSendButton() {
   ensureReasonOptional();
 
   const win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
-  const blockedPromise = waitForContentBlockingEvent(4, win);
+  const blockedPromise = waitForContentBlockingEvent(3, win);
   const tab = await openTab(REPORTABLE_PAGE_URL3, win);
   await blockedPromise;
 
@@ -54,7 +54,7 @@ add_task(async function testSendingMoreInfo() {
   ensureSendMoreInfoEnabled();
 
   const win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
-  const blockedPromise = waitForContentBlockingEvent(4, win);
+  const blockedPromise = waitForContentBlockingEvent(3, win);
   const tab = await openTab(REPORTABLE_PAGE_URL3, win);
   await blockedPromise;
 

caps/BasePrincipal.cpp

@@ -856,7 +856,7 @@ BasePrincipal::HasFirstpartyStorageAccess(mozIDOMWindow* aCheckWindow,
     return NS_ERROR_UNEXPECTED;
   }
 
-  *aOutAllowed = ShouldAllowAccessFor(win, uri, aRejectedReason);
+  *aOutAllowed = ShouldAllowAccessFor(win, uri, true, aRejectedReason);
   return NS_OK;
 }
 

dom/base/Document.cpp

@@ -19507,9 +19507,19 @@ nsIPrincipal* Document::EffectiveStoragePrincipal() const {
     return mActiveStoragePrincipal = NodePrincipal();
   }
 
-  StorageAccess storageAccess = StorageAllowedForDocument(this);
-  if (!ShouldPartitionStorage(storageAccess) ||
-      !StoragePartitioningEnabled(storageAccess, cookieJarSettings)) {
+  // We use the lower-level ContentBlocking API here to ensure this
+  // check doesn't send notifications.
+  uint32_t rejectedReason = 0;
+  if (ShouldAllowAccessFor(inner, GetDocumentURI(), false, &rejectedReason)) {
+    return mActiveStoragePrincipal = NodePrincipal();
+  }
+
+  // Let's use the storage principal only if we need to partition the cookie
+  // jar. When the permission is granted, access will be different and the
+  // normal principal will be used.
+  if (ShouldPartitionStorage(rejectedReason) &&
+      !StoragePartitioningEnabled(
+          rejectedReason, const_cast<Document*>(this)->CookieJarSettings())) {
     return mActiveStoragePrincipal = NodePrincipal();
   }
 
@@ -19548,7 +19558,7 @@ nsIPrincipal* Document::EffectiveCookiePrincipal() const {
   // We use the lower-level ContentBlocking API here to ensure this
   // check doesn't send notifications.
   uint32_t rejectedReason = 0;
-  if (ShouldAllowAccessFor(inner, GetDocumentURI(), &rejectedReason)) {
+  if (ShouldAllowAccessFor(inner, GetDocumentURI(), true, &rejectedReason)) {
     return mActiveCookiePrincipal = NodePrincipal();
   }
 

image/ImageCacheKey.cpp

@@ -150,7 +150,7 @@ nsCString ImageCacheKey::GetIsolationKey(Document* aDocument, nsIURI* aURI) {
                                             nullptr)) {
     uint32_t rejectedReason = 0;
     Unused << rejectedReason;
-    return ShouldAllowAccessFor(aDocument->GetInnerWindow(), aURI,
+    return ShouldAllowAccessFor(aDocument->GetInnerWindow(), aURI, true,
                                 &rejectedReason)
                ? ""_ns
                : aDocument->GetBaseDomain();

netwerk/cookie/CookieCommons.cpp

@@ -471,7 +471,7 @@ already_AddRefed<Cookie> CookieCommons::CreateCookieFromDocument(
   if (aDocument->CookieJarSettings()->GetLimitForeignContexts() &&
       !service->HasExistingCookies(baseDomain,
                                    cookiePrincipal->OriginAttributesRef()) &&
-      !ShouldAllowAccessFor(innerWindow, aCookieParser.HostURI(),
+      !ShouldAllowAccessFor(innerWindow, aCookieParser.HostURI(), true,
                             &dummyRejectedReason)) {
     return nullptr;
   }

toolkit/components/antitracking/StorageAccess.cpp

@@ -161,8 +161,8 @@ static StorageAccess InternalStorageAllowedCheck(
   bool disabled = true;
   if (aWindow) {
     nsIURI* documentURI = aURI ? aURI : aWindow->GetDocumentURI();
-    disabled = !documentURI ||
-               !ShouldAllowAccessFor(aWindow, documentURI, &aRejectedReason);
+    disabled = !documentURI || !ShouldAllowAccessFor(aWindow, documentURI, true,
+                                                     &aRejectedReason);
 
     // If the window is a third-party tracker, we should set the rejected reason
     // to partitioned tracker.
@@ -481,7 +481,7 @@ int32_t CookiesBehavior(nsIPrincipal* aPrincipal,
 }
 
 bool ShouldAllowAccessFor(nsPIDOMWindowInner* aWindow, nsIURI* aURI,
-                          uint32_t* aRejectedReason) {
+                          bool aCookies, uint32_t* aRejectedReason) {
   MOZ_ASSERT(aWindow);
   MOZ_ASSERT(aURI);
 
@@ -622,10 +622,13 @@ bool ShouldAllowAccessFor(nsPIDOMWindowInner* aWindow, nsIURI* aURI,
     return false;
   }
 
+  // "Storage access granted" only affects cookie access for third party
+  // documents. So if we are looking if we should allow access for cookies,
+  // then test if that permission is enabled on this document.
   // Document::UsingStorageAccess first checks if storage access granted is
   // cached in the inner window, if no, it then checks the storage permission
   // flag in the channel's loadinfo
-  bool allowed = document->UsingStorageAccess();
+  bool allowed = aCookies && document->UsingStorageAccess();
 
   if (!allowed) {
     *aRejectedReason = blockedReason;

toolkit/components/antitracking/StorageAccess.h

@@ -103,6 +103,9 @@ bool StoragePartitioningEnabled(uint32_t aRejectedReason,
 
 // This method returns true if the URI has first party storage access when
 // loaded inside the passed 3rd party context tracking resource window.
+// If aCookies is true, this considers the permission granted by
+// document.requestStorageAccess. Cookies should be the only website state
+// that changes its accessability based upon that permission.
 // If the window is first party context, please use
 // ApproximateAllowAccessForWithoutChannel();
 //
@@ -118,7 +121,8 @@ bool StoragePartitioningEnabled(uint32_t aRejectedReason,
 // updating the other overloaded functions
 // (and ApproximateAllowAccessForWithoutChannel).
 bool ShouldAllowAccessFor(nsPIDOMWindowInner* a3rdPartyTrackingWindow,
-                          nsIURI* aURI, uint32_t* aRejectedReason);
+                          nsIURI* aURI, bool aCookies,
+                          uint32_t* aRejectedReason);
 
 // Note: you should use ShouldAllowAccessFor() passing the nsIChannel! Use
 // this method _only_ if the channel is not available.  For first party