Bug 1491061 - Part 1: Make Disable Protection honour both the Content Blocking UI pref and the pref controlling whether Third-Party Cookies section appears under Content Blocking UI; r=baku
☠☠ backed out by dc31941ced58 ☠ ☠
authorEhsan Akhgari <ehsan@mozilla.com>
Fri, 14 Sep 2018 12:44:16 -0400
changeset 436799 43552fcae4a4bb4dccb7488b061b31c4927f858d
parent 436798 0fb2ac9ad5ec112fd128c648cca4b4b6d37f7c0a
child 436800 494e23ba027e71fe4e498384ed4e2871ff4bb041
push id34660
push userbtara@mozilla.com
push dateMon, 17 Sep 2018 21:58:52 +0000
treeherdermozilla-central@87a95e1b7ec6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1491061
milestone64.0a1
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
Bug 1491061 - Part 1: Make Disable Protection honour both the Content Blocking UI pref and the pref controlling whether Third-Party Cookies section appears under Content Blocking UI; r=baku Differential Revision: https://phabricator.services.mozilla.com/D5887
browser/app/profile/firefox.js
dom/base/nsDocument.cpp
dom/base/nsGlobalWindowOuter.cpp
dom/workers/RuntimeService.cpp
modules/libpref/init/StaticPrefList.h
toolkit/components/antitracking/AntiTrackingCommon.cpp
toolkit/components/antitracking/AntiTrackingCommon.h
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1494,23 +1494,16 @@ pref("toolkit.telemetry.hybridContent.en
 pref("browser.ping-centre.telemetry", true);
 pref("browser.ping-centre.log", false);
 pref("browser.ping-centre.staging.endpoint", "https://onyx_tiles.stage.mozaws.net/v3/links/ping-centre");
 pref("browser.ping-centre.production.endpoint", "https://tiles.services.mozilla.com/v3/links/ping-centre");
 
 // Enable GMP support in the addon manager.
 pref("media.gmp-provider.enabled", true);
 
-// Enable the new Content Blocking UI only on Nightly.
-#ifdef NIGHTLY_BUILD
-pref("browser.contentblocking.ui.enabled", true);
-#else
-pref("browser.contentblocking.ui.enabled", false);
-#endif
-
 // Define a set of default features for the Content Blocking UI
 pref("browser.contentblocking.fastblock.ui.enabled", true);
 pref("browser.contentblocking.fastblock.control-center.ui.enabled", true);
 pref("browser.contentblocking.trackingprotection.ui.enabled", true);
 pref("browser.contentblocking.trackingprotection.control-center.ui.enabled", true);
 pref("browser.contentblocking.rejecttrackers.ui.enabled", true);
 pref("browser.contentblocking.rejecttrackers.ui.recommended", true);
 pref("browser.contentblocking.rejecttrackers.control-center.ui.enabled", true);
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -12827,17 +12827,17 @@ nsIDocument::SetDocTreeHadPlayRevoked()
   }
 }
 
 void
 nsIDocument::MaybeAllowStorageForOpener()
 {
   if (StaticPrefs::network_cookie_cookieBehavior() !=
         nsICookieService::BEHAVIOR_REJECT_TRACKER ||
-      !StaticPrefs::browser_contentblocking_enabled()) {
+      !AntiTrackingCommon::ShouldHonorContentBlockingCookieRestrictions()) {
     return;
   }
 
   // This will probably change for project fission, but currently this document
   // and the opener are on the same process. In the future, we should make this
   // part async.
 
   nsPIDOMWindowInner* inner = GetInnerWindow();
@@ -13680,17 +13680,17 @@ nsIDocument::RequestStorageAccess(mozill
     // If the document is in PB mode, it doesn't have access to its persistent
     // cookie jar, so reject the promise here.
     promise->MaybeRejectWithUndefined();
     return promise.forget();
   }
 
   bool granted = true;
   bool isTrackingWindow = false;
-  if (StaticPrefs::browser_contentblocking_enabled() &&
+  if (AntiTrackingCommon::ShouldHonorContentBlockingCookieRestrictions() &&
       StaticPrefs::network_cookie_cookieBehavior() ==
         nsICookieService::BEHAVIOR_REJECT_TRACKER) {
     // Only do something special for third-party tracking content.
     if (nsContentUtils::StorageDisabledByAntiTracking(this, nullptr)) {
       // Note: If this has returned true, the top-level document is guaranteed
       // to not be on the Content Blocking allow list.
       DebugOnly<bool> isOnAllowList = false;
       MOZ_ASSERT_IF(NS_SUCCEEDED(AntiTrackingCommon::IsOnContentBlockingAllowList(
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -2016,17 +2016,17 @@ nsGlobalWindowOuter::SetNewDocument(nsID
   // If we have a recorded interesting Large-Allocation header status, report it
   // to the newly attached document.
   ReportLargeAllocStatus();
   mLargeAllocStatus = LargeAllocStatus::NONE;
 
   mHasStorageAccess = false;
   nsIURI* uri = aDocument->GetDocumentURI();
   if (newInnerWindow) {
-    if (StaticPrefs::browser_contentblocking_enabled() &&
+    if (AntiTrackingCommon::ShouldHonorContentBlockingCookieRestrictions() &&
         StaticPrefs::network_cookie_cookieBehavior() ==
           nsICookieService::BEHAVIOR_REJECT_TRACKER &&
         nsContentUtils::IsThirdPartyWindowOrChannel(newInnerWindow, nullptr,
                                                     uri) &&
         nsContentUtils::IsTrackingResourceWindow(newInnerWindow)) {
       // Grant storage access by default if the first-party storage access
       // permission has been granted already.
       // Don't notify in this case, since we would be notifying the user needlessly.
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -26,16 +26,17 @@
 #include "nsPIDOMWindow.h"
 
 #include <algorithm>
 #include "mozilla/ipc/BackgroundChild.h"
 #include "GeckoProfiler.h"
 #include "jsfriendapi.h"
 #include "js/LocaleSensitive.h"
 #include "mozilla/AbstractThread.h"
+#include "mozilla/AntiTrackingCommon.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/AsyncEventDispatcher.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/CycleCollectedJSContext.h"
 #include "mozilla/CycleCollectedJSRuntime.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/dom/asmjscache/AsmJSCache.h"
@@ -2267,17 +2268,17 @@ RuntimeService::ResumeWorkersForWindow(n
 
 void
 RuntimeService::PropagateFirstPartyStorageAccessGranted(nsPIDOMWindowInner* aWindow)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aWindow);
   MOZ_ASSERT(StaticPrefs::network_cookie_cookieBehavior() ==
                nsICookieService::BEHAVIOR_REJECT_TRACKER &&
-             StaticPrefs::browser_contentblocking_enabled());
+             AntiTrackingCommon::ShouldHonorContentBlockingCookieRestrictions());
 
   nsTArray<WorkerPrivate*> workers;
   GetWorkersForWindow(aWindow, workers);
 
   for (uint32_t index = 0; index < workers.Length(); index++) {
     workers[index]->PropagateFirstPartyStorageAccessGranted();
   }
 }
@@ -2879,17 +2880,17 @@ ResumeWorkersForWindow(nsPIDOMWindowInne
 }
 
 void
 PropagateFirstPartyStorageAccessGrantedToWorkers(nsPIDOMWindowInner* aWindow)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(StaticPrefs::network_cookie_cookieBehavior() ==
                nsICookieService::BEHAVIOR_REJECT_TRACKER &&
-             StaticPrefs::browser_contentblocking_enabled());
+             AntiTrackingCommon::ShouldHonorContentBlockingCookieRestrictions());
 
   RuntimeService* runtime = RuntimeService::GetService();
   if (runtime) {
     runtime->PropagateFirstPartyStorageAccessGranted(aWindow);
   }
 }
 
 WorkerPrivate*
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -1473,16 +1473,37 @@ PREF("preferences.allow.omt-write", bool
 // Turning this off may disable some protections.  Please do not turn this pref off without
 // realizing the implications of what you're doing.
 VARCACHE_PREF(
   "browser.contentblocking.enabled",
    browser_contentblocking_enabled,
   bool, true
 )
 
+// Whether Content Blocking UI has been enabled.
+// Enable the new Content Blocking UI only on Nightly.
+#ifdef NIGHTLY_BUILD
+# define PREF_VALUE true
+#else
+# define PREF_VALUE false
+#endif
+VARCACHE_PREF(
+  "browser.contentblocking.ui.enabled",
+   browser_contentblocking_ui_enabled,
+  bool, PREF_VALUE
+)
+#undef PREF_VALUE
+
+// Whether Content Blocking Third-Party Cookies UI has been enabled.
+VARCACHE_PREF(
+  "browser.contentblocking.rejecttrackers.ui.enabled",
+   browser_contentblocking_rejecttrackers_ui_enabled,
+  bool, false
+)
+
 // Whether FastBlock has been enabled.
 VARCACHE_PREF(
   "browser.fastblock.enabled",
   browser_fastblock_enabled,
   bool, false
 )
 
 // Anti-tracking permission expiration
--- a/toolkit/components/antitracking/AntiTrackingCommon.cpp
+++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ -198,17 +198,17 @@ ReportBlockingToConsole(nsPIDOMWindowOut
 {
   MOZ_ASSERT(aWindow && aChannel);
   MOZ_ASSERT(aRejectedReason == nsIWebProgressListener::STATE_COOKIES_BLOCKED_BY_PERMISSION ||
              aRejectedReason == nsIWebProgressListener::STATE_COOKIES_BLOCKED_TRACKER ||
              aRejectedReason == nsIWebProgressListener::STATE_COOKIES_BLOCKED_ALL ||
              aRejectedReason == nsIWebProgressListener::STATE_COOKIES_BLOCKED_FOREIGN ||
              aRejectedReason == nsIWebProgressListener::STATE_BLOCKED_SLOW_TRACKING_CONTENT);
 
-  if (!StaticPrefs::browser_contentblocking_enabled()) {
+  if (!AntiTrackingCommon::ShouldHonorContentBlockingCookieRestrictions()) {
     return;
   }
 
   nsCOMPtr<nsIDocShell> docShell = aWindow->GetDocShell();
   if (NS_WARN_IF(!docShell)) {
     return;
   }
 
@@ -318,16 +318,24 @@ ReportUnblockingConsole(nsPIDOMWindowInn
                                     nsContentUtils::eNECKO_PROPERTIES,
                                     messageWithDifferentOrigin,
                                     params, 3);
   }
 }
 
 } // anonymous
 
+/* static */ bool
+AntiTrackingCommon::ShouldHonorContentBlockingCookieRestrictions()
+{
+  return StaticPrefs::browser_contentblocking_enabled() &&
+         StaticPrefs::browser_contentblocking_ui_enabled() &&
+         StaticPrefs::browser_contentblocking_rejecttrackers_ui_enabled();
+}
+
 /* static */ RefPtr<AntiTrackingCommon::StorageAccessGrantPromise>
 AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(const nsAString& aOrigin,
                                                          nsPIDOMWindowInner* aParentWindow,
                                                          StorageAccessGrantedReason aReason)
 {
   MOZ_ASSERT(aParentWindow);
 
   LOG(("Adding a first-party storage exception for %s...",
@@ -335,17 +343,17 @@ AntiTrackingCommon::AddFirstPartyStorage
 
   if (StaticPrefs::network_cookie_cookieBehavior() !=
         nsICookieService::BEHAVIOR_REJECT_TRACKER) {
     LOG(("Disabled by network.cookie.cookieBehavior pref (%d), bailing out early",
          StaticPrefs::network_cookie_cookieBehavior()));
     return StorageAccessGrantPromise::CreateAndResolve(true, __func__);
   }
 
-  if (!StaticPrefs::browser_contentblocking_enabled()) {
+  if (!ShouldHonorContentBlockingCookieRestrictions()) {
     LOG(("The content blocking pref has been disabled, bail out early"));
     return StorageAccessGrantPromise::CreateAndResolve(true, __func__);
   }
 
   if (CheckContentBlockingAllowList(aParentWindow)) {
     return StorageAccessGrantPromise::CreateAndResolve(true, __func__);
   }
 
@@ -535,17 +543,17 @@ AntiTrackingCommon::IsFirstPartyStorageA
   // 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) {
     // Now, we have to also honour the Content Blocking pref.
-    if (!StaticPrefs::browser_contentblocking_enabled()) {
+    if (!ShouldHonorContentBlockingCookieRestrictions()) {
       LOG(("The content blocking pref has been disabled, bail out early by "
            "by pretending our window isn't a third-party window"));
       return true;
     }
 
     if (CheckContentBlockingAllowList(aWindow)) {
       LOG(("Allowing access even though our behavior is reject foreign"));
       return true;
@@ -561,17 +569,17 @@ AntiTrackingCommon::IsFirstPartyStorageA
     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);
 
   // Now, we have to also honour the Content Blocking pref.
-  if (!StaticPrefs::browser_contentblocking_enabled()) {
+  if (!ShouldHonorContentBlockingCookieRestrictions()) {
     LOG(("The content blocking pref has been disabled, bail out early by "
          "by pretending our window isn't a tracking window"));
     return true;
   }
 
   if (CheckContentBlockingAllowList(aWindow)) {
     return true;
   }
@@ -742,17 +750,17 @@ AntiTrackingCommon::IsFirstPartyStorageA
   // system principal...
   if (NS_SUCCEEDED(rv) && !thirdParty) {
     LOG(("Our channel isn't a third-party channel"));
     return true;
   }
 
   if (behavior == nsICookieService::BEHAVIOR_REJECT_FOREIGN) {
     // Now, we have to also honour the Content Blocking pref.
-    if (!StaticPrefs::browser_contentblocking_enabled()) {
+    if (!ShouldHonorContentBlockingCookieRestrictions()) {
       LOG(("The content blocking pref has been disabled, bail out early by "
            "by pretending our window isn't a third-party window"));
       return true;
     }
 
     if (CheckContentBlockingAllowList(aChannel)) {
       LOG(("Allowing access even though our behavior is reject foreign"));
       return true;
@@ -768,17 +776,17 @@ AntiTrackingCommon::IsFirstPartyStorageA
     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);
 
   // Now, we have to also honour the Content Blocking pref.
-  if (!StaticPrefs::browser_contentblocking_enabled()) {
+  if (!ShouldHonorContentBlockingCookieRestrictions()) {
     LOG(("The content blocking pref has been disabled, bail out early by "
          "pretending our channel isn't a tracking channel"));
     return true;
   }
 
   if (CheckContentBlockingAllowList(aChannel)) {
     return true;
   }
@@ -890,17 +898,17 @@ AntiTrackingCommon::MaybeIsFirstPartySto
   if (StaticPrefs::network_cookie_cookieBehavior() !=
         nsICookieService::BEHAVIOR_REJECT_TRACKER) {
     LOG(("Disabled by the pref (%d), bail out early",
          StaticPrefs::network_cookie_cookieBehavior()));
     return true;
   }
 
   // Now, we have to also honour the Content Blocking pref.
-  if (!StaticPrefs::browser_contentblocking_enabled()) {
+  if (!ShouldHonorContentBlockingCookieRestrictions()) {
     LOG(("The content blocking pref has been disabled, bail out early"));
     return true;
   }
 
   if (CheckContentBlockingAllowList(aFirstPartyWindow)) {
     return true;
   }
 
--- a/toolkit/components/antitracking/AntiTrackingCommon.h
+++ b/toolkit/components/antitracking/AntiTrackingCommon.h
@@ -24,16 +24,29 @@ class AntiTrackingCommon final
 public:
   // Normally we would include PContentParent.h here and use the
   // ipc::FirstPartyStorageAccessGrantedForOriginResolver type which maps to
   // the same underlying type, but that results in Windows compilation errors,
   // so we use the underlying type to avoid the #include here.
   typedef std::function<void(const bool&)>
     FirstPartyStorageAccessGrantedForOriginResolver;
 
+  // This function should be called to determine whether we need to honour the
+  // content blocking cookie restrictions.  It takes into account whether
+  // content blocking itself is active, and also whether the UI for it is being
+  // shown to the user.  The reason we make this depend on whether the UI is being
+  // shown is to avoid confusing scenarios where the user's privacy choices will
+  // be overridden by the invisible prefs that cannot be controlled in the UI.
+  //
+  // Please note that this function doesn't perform any special checks on _what_
+  // kind of restrictions the consumer is expected to follow.  The consumer is
+  // still responsible to perform further checks to determine that.
+  static bool
+  ShouldHonorContentBlockingCookieRestrictions();
+
   // This method returns true if the URI has first party storage access when
   // loaded inside the passed 3rd party context tracking resource window.
   // If the window is first party context, please use
   // MaybeIsFirstPartyStorageAccessGrantedFor();
   //
   // aRejectedReason could be set to one of these values if passed and if the
   // storage permission is not granted:
   //  * nsIWebProgressListener::STATE_COOKIES_BLOCKED_BY_PERMISSION