Bug 1601786 - Remove the "allow on any site" option from the storage access API prompt; r=baku

Differential Revision: https://phabricator.services.mozilla.com/D56246 --HG-- extra : moz-landing-system : lando
2019-12-18 21:31:35 +00:00 · 2019-12-18 21:31:35 +00:00 · 5d2fded7b8
--- a/browser/locales/en-US/chrome/browser/browser.properties
+++ b/browser/locales/en-US/chrome/browser/browser.properties
@ -1027,8 +1027,6 @@ panel.back = Back

 storageAccess.Allow.label = Allow Access
 storageAccess.Allow.accesskey = A
-storageAccess.AllowOnAnySite.label = Allow access on any site
-storageAccess.AllowOnAnySite.accesskey = w
 storageAccess.DontAllow.label = Block Access
 storageAccess.DontAllow.accesskey = B
 # LOCALIZATION NOTE (storageAccess2.message):
--- a/browser/modules/PermissionUI.jsm
+++ b/browser/modules/PermissionUI.jsm
@ -1336,18 +1336,6 @@ StorageAccessPermissionPrompt.prototype = {
          self.allow({ "storage-access": "allow" });
        },
      },
-      {
-        label: gBrowserBundle.GetStringFromName(
-          "storageAccess.AllowOnAnySite.label"
-        ),
-        accessKey: gBrowserBundle.GetStringFromName(
-          "storageAccess.AllowOnAnySite.accesskey"
-        ),
-        action: Ci.nsIPermissionManager.ALLOW_ACTION,
-        callback(state) {
-          self.allow({ "storage-access": "allow-on-any-site" });
-        },
-      },
    ];
  },

--- a/browser/modules/test/browser/browser_PermissionUI_prompts.js
+++ b/browser/modules/test/browser/browser_PermissionUI_prompts.js
@ -110,15 +110,9 @@ async function testPrompt(Prompt) {
        Prompt == PermissionUI.DesktopNotificationPermissionPrompt;
      let isPersistentStoragePrompt =
        Prompt == PermissionUI.PersistentStoragePermissionPrompt;
-      let isStorageAccessPrompt =
-        Prompt == PermissionUI.StorageAccessPermissionPrompt;

      let expectedSecondaryActionsCount =
-        isNotificationPrompt ||
-        isPersistentStoragePrompt ||
-        isStorageAccessPrompt
-          ? 2
-          : 1;
+        isNotificationPrompt || isPersistentStoragePrompt ? 2 : 1;
      Assert.equal(
        notification.secondaryActions.length,
        expectedSecondaryActionsCount,
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@ -15417,12 +15417,6 @@ already_AddRefed<Promise> Document::RequestStorageAccess(ErrorResult& aRv) {
                      Telemetry::LABELS_STORAGE_ACCESS_API_UI::Allow);
                  p->Resolve(AntiTrackingCommon::eAllow, __func__);
                },
-                // Allow on any site
-                [p] {
-                  Telemetry::AccumulateCategorical(
-                      Telemetry::LABELS_STORAGE_ACCESS_API_UI::AllowOnAnySite);
-                  p->Resolve(AntiTrackingCommon::eAllowOnAnySite, __func__);
-                },
                // Block
                [p] {
                  Telemetry::AccumulateCategorical(
@ -15432,17 +15426,6 @@ already_AddRefed<Promise> Document::RequestStorageAccess(ErrorResult& aRv) {

        typedef ContentPermissionRequestBase::PromptResult PromptResult;
        PromptResult pr = sapr->CheckPromptPrefs();
-        bool onAnySite = false;
-        if (pr == PromptResult::Pending) {
-          // Also check our custom pref for the "Allow on any site" case
-          if (Preferences::GetBool("dom.storage_access.prompt.testing",
-                                   false) &&
-              Preferences::GetBool(
-                  "dom.storage_access.prompt.testing.allowonanysite", false)) {
-            pr = PromptResult::Granted;
-            onAnySite = true;
-          }
-        }

        if (pr == PromptResult::Pending) {
          // We're about to show a prompt, record the request attempt
@ -15452,7 +15435,7 @@ already_AddRefed<Promise> Document::RequestStorageAccess(ErrorResult& aRv) {

        self->AutomaticStorageAccessCanBeGranted()->Then(
            GetCurrentThreadSerialEventTarget(), __func__,
-            [p, pr, sapr, inner, onAnySite](
+            [p, pr, sapr, inner](
                const AutomaticStorageAccessGrantPromise::ResolveOrRejectValue&
                    aValue) -> void {
              // Make a copy because we can't modified copy-captured lambda
@ -15479,9 +15462,7 @@ already_AddRefed<Promise> Document::RequestStorageAccess(ErrorResult& aRv) {
                if (pr2 == PromptResult::Granted) {
                  AntiTrackingCommon::StorageAccessPromptChoices choice =
                      AntiTrackingCommon::eAllow;
-                  if (onAnySite) {
-                    choice = AntiTrackingCommon::eAllowOnAnySite;
-                  } else if (autoGrant) {
+                  if (autoGrant) {
                    choice = AntiTrackingCommon::eAllowAutoGrant;
                  }
                  if (!autoGrant) {
--- a/dom/base/StorageAccessPermissionRequest.cpp
+++ b/dom/base/StorageAccessPermissionRequest.cpp
@ -20,14 +20,11 @@ NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED_0(StorageAccessPermissionRequest,

 StorageAccessPermissionRequest::StorageAccessPermissionRequest(
    nsPIDOMWindowInner* aWindow, nsIPrincipal* aNodePrincipal,
-    AllowCallback&& aAllowCallback,
-    AllowAnySiteCallback&& aAllowAnySiteCallback,
-    CancelCallback&& aCancelCallback)
+    AllowCallback&& aAllowCallback, CancelCallback&& aCancelCallback)
    : ContentPermissionRequestBase(aNodePrincipal, aWindow,
                                   NS_LITERAL_CSTRING("dom.storage_access"),
                                   NS_LITERAL_CSTRING("storage-access")),
      mAllowCallback(std::move(aAllowCallback)),
-      mAllowAnySiteCallback(std::move(aAllowAnySiteCallback)),
      mCancelCallback(std::move(aCancelCallback)),
      mCallbackCalled(false) {
  mPermissionRequests.AppendElement(
@ -58,11 +55,7 @@ StorageAccessPermissionRequest::Allow(JS::HandleValue aChoices) {

  if (!mCallbackCalled) {
    mCallbackCalled = true;
-    if (choices.Length() == 1 &&
-        choices[0].choice().EqualsLiteral("allow-on-any-site")) {
-      mAllowAnySiteCallback();
-    } else if (choices.Length() == 1 &&
-               choices[0].choice().EqualsLiteral("allow")) {
+    if (choices.Length() == 1 && choices[0].choice().EqualsLiteral("allow")) {
      mAllowCallback();
    }
  }
@ -103,10 +96,9 @@ StorageAccessPermissionRequest::MaybeDelayAutomaticGrants() {
 }

 already_AddRefed<StorageAccessPermissionRequest>
-StorageAccessPermissionRequest::Create(
-    nsPIDOMWindowInner* aWindow, AllowCallback&& aAllowCallback,
-    AllowAnySiteCallback&& aAllowAnySiteCallback,
-    CancelCallback&& aCancelCallback) {
+StorageAccessPermissionRequest::Create(nsPIDOMWindowInner* aWindow,
+                                       AllowCallback&& aAllowCallback,
+                                       CancelCallback&& aCancelCallback) {
  if (!aWindow) {
    return nullptr;
  }
@ -115,9 +107,9 @@ StorageAccessPermissionRequest::Create(
    return nullptr;
  }
  RefPtr<StorageAccessPermissionRequest> request =
-      new StorageAccessPermissionRequest(
-          aWindow, win->GetPrincipal(), std::move(aAllowCallback),
-          std::move(aAllowAnySiteCallback), std::move(aCancelCallback));
+      new StorageAccessPermissionRequest(aWindow, win->GetPrincipal(),
+                                         std::move(aAllowCallback),
+                                         std::move(aCancelCallback));
  return request.forget();
 }

--- a/dom/base/StorageAccessPermissionRequest.h
+++ b/dom/base/StorageAccessPermissionRequest.h
@ -29,12 +29,10 @@ class StorageAccessPermissionRequest final
  NS_IMETHOD Allow(JS::HandleValue choices) override;

  typedef std::function<void()> AllowCallback;
-  typedef std::function<void()> AllowAnySiteCallback;
  typedef std::function<void()> CancelCallback;

  static already_AddRefed<StorageAccessPermissionRequest> Create(
      nsPIDOMWindowInner* aWindow, AllowCallback&& aAllowCallback,
-      AllowAnySiteCallback&& aAllowAnySiteCallback,
      CancelCallback&& aCancelCallback);

  typedef MozPromise<bool, bool, true> AutoGrantDelayPromise;
@ -44,14 +42,12 @@ class StorageAccessPermissionRequest final
  StorageAccessPermissionRequest(nsPIDOMWindowInner* aWindow,
                                 nsIPrincipal* aNodePrincipal,
                                 AllowCallback&& aAllowCallback,
-                                 AllowAnySiteCallback&& aAllowAnySiteCallback,
                                 CancelCallback&& aCancelCallback);
  ~StorageAccessPermissionRequest();

  unsigned CalculateSimulatedDelay();

  AllowCallback mAllowCallback;
-  AllowAnySiteCallback mAllowAnySiteCallback;
  CancelCallback mCancelCallback;
  nsTArray<PermissionRequest> mPermissionRequests;
  bool mCallbackCalled;
--- a/toolkit/components/antitracking/AntiTrackingCommon.cpp
+++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp
@ -1097,9 +1097,7 @@ AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(
                        aValue) {
                   if (aValue.IsResolve()) {
                     return StorageAccessGrantPromise::CreateAndResolve(
-                         NS_SUCCEEDED(aValue.ResolveValue()) ? eAllowOnAnySite
-                                                             : eAllow,
-                         __func__);
+                         eAllow, __func__);
                   }
                   return StorageAccessGrantPromise::CreateAndReject(false,
                                                                     __func__);
@ -1153,8 +1151,7 @@ AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(
    nsIPrincipal* aParentPrincipal, nsIPrincipal* aTrackingPrincipal,
    const nsCString& aTrackingOrigin, int aAllowMode) {
  MOZ_ASSERT(XRE_IsParentProcess());
-  MOZ_ASSERT(aAllowMode == eAllow || aAllowMode == eAllowAutoGrant ||
-             aAllowMode == eAllowOnAnySite);
+  MOZ_ASSERT(aAllowMode == eAllow || aAllowMode == eAllowAutoGrant);

  if (!aParentPrincipal || !aTrackingPrincipal) {
    LOG(("Invalid input arguments passed"));
@ -1189,34 +1186,8 @@ AntiTrackingCommon::SaveFirstPartyStorageAccessGrantedForOriginOnParentProcess(
      StaticPrefs::privacy_restrict3rdpartystorage_expiration() * 1000;
  int64_t when = (PR_Now() / PR_USEC_PER_MSEC) + expirationTime;

-  nsresult rv;
-  if (aAllowMode == eAllowOnAnySite) {
-    uint32_t privateBrowsingId = 0;
-    rv = aTrackingPrincipal->GetPrivateBrowsingId(&privateBrowsingId);
-    if (!NS_WARN_IF(NS_FAILED(rv)) && privateBrowsingId > 0) {
-      // If we are coming from a private window, make sure to store a
-      // session-only permission which won't get persisted to disk.
-      expirationType = nsIPermissionManager::EXPIRE_SESSION;
-      when = 0;
-    }
-
-    LOG(
-        ("Setting 'any site' permission expiry: %u, proceeding to save in the "
-         "permission manager",
-         expirationTime));
-
-    rv = permManager->AddFromPrincipal(
-        aTrackingPrincipal, NS_LITERAL_CSTRING("cookie"),
-        nsICookiePermission::ACCESS_ALLOW, expirationType, when);
-    Unused << NS_WARN_IF(NS_FAILED(rv));
-  }
-
-  // We must grant the storage permission also if we allow it for any site
-  // because the setting 'cookie' permission is not applied to existing
-  // documents (See CookieSettings documentation).
-
  uint32_t privateBrowsingId = 0;
-  rv = aParentPrincipal->GetPrivateBrowsingId(&privateBrowsingId);
+  nsresult rv = aParentPrincipal->GetPrivateBrowsingId(&privateBrowsingId);
  if ((!NS_WARN_IF(NS_FAILED(rv)) && privateBrowsingId > 0) ||
      (aAllowMode == eAllowAutoGrant)) {
    // If we are coming from a private window or are automatically granting a
--- a/toolkit/components/antitracking/AntiTrackingCommon.h
+++ b/toolkit/components/antitracking/AntiTrackingCommon.h
@ -76,7 +76,7 @@ class AntiTrackingCommon final {
    eOpenerAfterUserInteraction,
    eOpener
  };
-  enum StorageAccessPromptChoices { eAllow, eAllowAutoGrant, eAllowOnAnySite };
+  enum StorageAccessPromptChoices { eAllow, eAllowAutoGrant };

  // Grant the permission for aOrigin to have access to the first party storage.
  // This method can handle 2 different scenarios:
--- a/toolkit/components/antitracking/test/browser/browser_storageAccessDoorHanger.js
+++ b/toolkit/components/antitracking/test/browser/browser_storageAccessDoorHanger.js
@ -6,7 +6,6 @@ Services.scriptloader.loadSubScript(CHROME_BASE + "head.js", this);

 const BLOCK = 0;
 const ALLOW = 1;
-const ALLOW_ON_ANY_SITE = 2;

 async function testDoorHanger(
  choice,
@ -129,12 +128,6 @@ async function testDoorHanger(
            .type.startsWith("3rdPartyStorage^") &&
          subject.principal.origin == new URL(topPage).origin &&
          data == "added";
-      } else if (choice == ALLOW_ON_ANY_SITE) {
-        result =
-          subject &&
-          subject.QueryInterface(Ci.nsIPermission).type == "cookie" &&
-          subject.principal.origin == "https://tracking.example.org" &&
-          data == "added";
      }
      return result;
    });
@ -169,8 +162,6 @@ async function testDoorHanger(
      }
    } else if (choice == ALLOW) {
      await clickSecondaryAction(choice - 1);
-    } else if (choice == ALLOW_ON_ANY_SITE) {
-      await clickSecondaryAction(choice - 1);
    }
    if (choice != BLOCK) {
      await permChanged;
@ -290,14 +281,6 @@ async function runRound(topPage, showPrompt, maxConcurrent) {
    await preparePermissionsFromOtherSites(topPage);
    await testDoorHanger(ALLOW, showPrompt, false, topPage, maxConcurrent);
    await cleanUp();
-    await preparePermissionsFromOtherSites(topPage);
-    await testDoorHanger(
-      ALLOW_ON_ANY_SITE,
-      showPrompt,
-      false,
-      topPage,
-      maxConcurrent
-    );
  } else {
    await preparePermissionsFromOtherSites(topPage);
    await testDoorHanger(ALLOW, showPrompt, false, topPage, maxConcurrent);