Backed out changeset 462c7102e2db (bug 1502240) for failures related to nsICookieService CLOSED TREE
authorCiure Andrei <aciure@mozilla.com>
Fri, 02 Nov 2018 19:24:40 +0200
changeset 444186 ae1c0d2ca42868e1a0fa32f4c8e4bea9a7352bda
parent 444185 a9b82d8f9001249c64c13e740a7f7622a7fe5572
child 444187 e20ad5ce00a7d6525640df2baf401ab9b898684f
push id72240
push useraciure@mozilla.com
push dateFri, 02 Nov 2018 17:25:35 +0000
treeherderautoland@ae1c0d2ca428 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1502240
milestone65.0a1
backs out462c7102e2dbac877e81c8065e1719aada935dcc
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
Backed out changeset 462c7102e2db (bug 1502240) for failures related to nsICookieService CLOSED TREE
netwerk/cookie/nsCookieService.cpp
toolkit/components/antitracking/AntiTrackingCommon.cpp
toolkit/components/antitracking/test/browser/head.js
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -4278,40 +4278,40 @@ nsCookieService::CheckPrefs(nsICookiePer
   // access to the first-party cookie jar.
   if (aIsForeign && aIsTrackingResource && !aFirstPartyStorageAccessGranted &&
       aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_TRACKER) {
       COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "cookies are disabled in trackers");
       *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_TRACKER;
       return STATUS_REJECTED;
   }
 
-  // check default prefs.
-  // Check aFirstPartyStorageAccessGranted when checking aCookieBehavior
-  // so that we take things such as the content blocking allow list into account.
-  if (aCookieBehavior == nsICookieService::BEHAVIOR_REJECT &&
-      !aFirstPartyStorageAccessGranted) {
+  // check default prefs
+  if (aCookieBehavior == nsICookieService::BEHAVIOR_REJECT) {
     COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "cookies are disabled");
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL;
     return STATUS_REJECTED;
   }
 
   // check if cookie is foreign
   if (aIsForeign) {
+    // Check aFirstPartyStorageAccessGranted when rejecting all third-party cookies,
+    // so that we take things such as the content blocking allow list into account.
     if (aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN &&
         !aFirstPartyStorageAccessGranted) {
       COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
       *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
       return STATUS_REJECTED;
     }
 
-    if (aCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN &&
-        !aFirstPartyStorageAccessGranted && aNumOfCookies == 0) {
-      COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
-      *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
-      return STATUS_REJECTED;
+    if (aCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
+      if (aNumOfCookies == 0) {
+        COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party");
+        *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
+        return STATUS_REJECTED;
+      }
     }
 
     MOZ_ASSERT(aCookieBehavior == nsICookieService::BEHAVIOR_ACCEPT ||
                aCookieBehavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN ||
                // But with permission granted.
                aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
                aCookieBehavior == nsICookieService::BEHAVIOR_REJECT_TRACKER);
 
--- a/toolkit/components/antitracking/AntiTrackingCommon.cpp
+++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ -641,45 +641,52 @@ AntiTrackingCommon::IsFirstPartyStorageA
   }
 
   int32_t behavior = CookiesBehavior(toplevelPrincipal);
   if (behavior == nsICookieService::BEHAVIOR_ACCEPT) {
     LOG(("The cookie behavior pref mandates accepting all cookies!"));
     return true;
   }
 
-  if (CheckContentBlockingAllowList(aWindow)) {
-    return true;
-  }
-
   if (behavior == nsICookieService::BEHAVIOR_REJECT) {
     LOG(("The cookie behavior pref mandates rejecting all cookies!"));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL;
     return false;
   }
 
   // Let's check if this is a 3rd party context.
   if (!nsContentUtils::IsThirdPartyWindowOrChannel(aWindow, nullptr, aURI)) {
     LOG(("Our window isn't a third-party window"));
     return true;
   }
 
+  if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN) {
+    if (CheckContentBlockingAllowList(aWindow)) {
+      LOG(("Allowing access even though our behavior is reject foreign"));
+      return true;
+    }
+  }
+
   if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
       behavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
     // XXX For non-cookie forms of storage, we handle BEHAVIOR_LIMIT_FOREIGN by
     // simply rejecting the request to use the storage. In the future, if we
     // change the meaning of BEHAVIOR_LIMIT_FOREIGN to be one which makes sense
     // for non-cookie storage types, this may change.
     LOG(("Nothing more to do due to the behavior code %d", int(behavior)));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
     return false;
   }
 
   MOZ_ASSERT(behavior == nsICookieService::BEHAVIOR_REJECT_TRACKER);
 
+  if (CheckContentBlockingAllowList(aWindow)) {
+    return true;
+  }
+
   if (!nsContentUtils::IsTrackingResourceWindow(aWindow)) {
     LOG(("Our window isn't a tracking window"));
     return true;
   }
 
   nsCOMPtr<nsIPrincipal> parentPrincipal;
   nsAutoCString trackingOrigin;
   if (!GetParentPrincipalAndTrackingOrigin(nsGlobalWindowInner::Cast(aWindow),
@@ -815,20 +822,16 @@ AntiTrackingCommon::IsFirstPartyStorageA
   }
 
   int32_t behavior = CookiesBehavior(toplevelPrincipal);
   if (behavior == nsICookieService::BEHAVIOR_ACCEPT) {
     LOG(("The cookie behavior pref mandates accepting all cookies!"));
     return true;
   }
 
-  if (CheckContentBlockingAllowList(aChannel)) {
-    return true;
-  }
-
   if (behavior == nsICookieService::BEHAVIOR_REJECT) {
     LOG(("The cookie behavior pref mandates rejecting all cookies!"));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL;
     return false;
   }
 
   nsCOMPtr<mozIThirdPartyUtil> thirdPartyUtil = services::GetThirdPartyUtil();
   if (!thirdPartyUtil) {
@@ -844,29 +847,40 @@ AntiTrackingCommon::IsFirstPartyStorageA
   // Be careful to check the return value of IsThirdPartyChannel, since
   // IsThirdPartyChannel() will fail if the channel's loading principal is the
   // system principal...
   if (NS_SUCCEEDED(rv) && !thirdParty) {
     LOG(("Our channel isn't a third-party channel"));
     return true;
   }
 
+  if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN) {
+    if (CheckContentBlockingAllowList(aChannel)) {
+      LOG(("Allowing access even though our behavior is reject foreign"));
+      return true;
+    }
+  }
+
   if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN ||
       behavior == nsICookieService::BEHAVIOR_LIMIT_FOREIGN) {
     // XXX For non-cookie forms of storage, we handle BEHAVIOR_LIMIT_FOREIGN by
     // simply rejecting the request to use the storage. In the future, if we
     // change the meaning of BEHAVIOR_LIMIT_FOREIGN to be one which makes sense
     // for non-cookie storage types, this may change.
     LOG(("Nothing more to do due to the behavior code %d", int(behavior)));
     *aRejectedReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN;
     return false;
   }
 
   MOZ_ASSERT(behavior == nsICookieService::BEHAVIOR_REJECT_TRACKER);
 
+  if (CheckContentBlockingAllowList(aChannel)) {
+    return true;
+  }
+
   // Not a tracker.
   if (!aChannel->GetIsTrackingResource()) {
     LOG(("Our channel isn't a tracking channel"));
     return true;
   }
 
   nsIPrincipal* parentPrincipal = loadInfo->TopLevelStorageAreaPrincipal();
   if (!parentPrincipal) {
--- a/toolkit/components/antitracking/test/browser/head.js
+++ b/toolkit/components/antitracking/test/browser/head.js
@@ -10,17 +10,16 @@ const TEST_EMBEDDER_PAGE = TEST_DOMAIN +
 const TEST_POPUP_PAGE = TEST_DOMAIN + TEST_PATH + "popup.html";
 const TEST_3RD_PARTY_PAGE = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdParty.html";
 const TEST_3RD_PARTY_PAGE_WO = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartyWO.html";
 const TEST_3RD_PARTY_PAGE_UI = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartyUI.html";
 const TEST_3RD_PARTY_PAGE_WITH_SVG = TEST_3RD_PARTY_DOMAIN + TEST_PATH + "3rdPartySVG.html";
 const TEST_4TH_PARTY_PAGE = TEST_4TH_PARTY_DOMAIN + TEST_PATH + "3rdParty.html";
 
 const BEHAVIOR_ACCEPT         = Ci.nsICookieService.BEHAVIOR_ACCEPT;
-const BEHAVIOR_LIMIT_FOREIGN  = Ci.nsICookieService.BEHAVIOR_LIMIT_FOREIGN;
 const BEHAVIOR_REJECT_FOREIGN = Ci.nsICookieService.BEHAVIOR_REJECT_FOREIGN;
 const BEHAVIOR_REJECT_TRACKER = Ci.nsICookieService.BEHAVIOR_REJECT_TRACKER;
 
 var gFeatures = undefined;
 
 let {UrlClassifierTestUtils} = ChromeUtils.import("resource://testing-common/UrlClassifierTestUtils.jsm", {});
 
 requestLongerTimeout(5);
@@ -140,31 +139,16 @@ this.AntiTracking = {
           iframeSandbox,
           accessRemoval: null, // only passed with non-blocking callback
           callbackAfterRemoval: null,
         });
         this._createCleanupTask(cleanupFunction);
 
         this._createTask({
           name,
-          cookieBehavior: BEHAVIOR_LIMIT_FOREIGN,
-          blockingByContentBlockingRTUI: true,
-          allowList: true,
-          callback: callbackNonTracking,
-          extraPrefs: [],
-          expectedBlockingNotifications: false,
-          runInPrivateWindow,
-          iframeSandbox,
-          accessRemoval: null, // only passed with non-blocking callback
-          callbackAfterRemoval: null,
-        });
-        this._createCleanupTask(cleanupFunction);
-
-        this._createTask({
-          name,
           cookieBehavior: BEHAVIOR_REJECT_FOREIGN,
           blockingByContentBlockingRTUI: true,
           allowList: true,
           callback: callbackNonTracking,
           extraPrefs: [],
           expectedBlockingNotifications: false,
           runInPrivateWindow,
           iframeSandbox,
@@ -266,18 +250,17 @@ this.AntiTracking = {
       await AntiTracking._setupTest(win, options.cookieBehavior,
                                     options.blockingByContentBlockingRTUI,
                                     options.extraPrefs);
 
       let cookieBlocked = 0;
       let listener = {
         onSecurityChange(webProgress, request, oldState, state,
                          contentBlockingLogJSON) {
-          if ((state & Ci.nsIWebProgressListener.STATE_COOKIES_BLOCKED_TRACKER) ||
-              (state & Ci.nsIWebProgressListener.STATE_COOKIES_BLOCKED_FOREIGN)) {
+          if (state & Ci.nsIWebProgressListener.STATE_COOKIES_BLOCKED_TRACKER) {
             ++cookieBlocked;
           }
           let contentBlockingLog = {};
           try {
             contentBlockingLog = JSON.parse(contentBlockingLogJSON);
           } catch (e) {
           }