Bug 1505571 - Don't chcek user-interaction permission when the operation starts from user-interaction, r=ehsan
☠☠ backed out by 29ff944efa40 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Mon, 19 Nov 2018 19:16:24 +0100
changeset 503525 be17a8b7aefa08d2ab6e7967314747073c087512
parent 503524 f1b5651ca4544a45e5ab400bcfbdf63d46ca79d1
child 503526 88af3329ee16b19450f0c4638d4fb39019cdf928
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1505571
milestone65.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 1505571 - Don't chcek user-interaction permission when the operation starts from user-interaction, r=ehsan The permission has been just set, but the content process haven't received it back from the parent process.
dom/base/nsDocument.cpp
dom/base/nsGlobalWindowOuter.cpp
dom/base/nsIDocument.h
toolkit/components/antitracking/AntiTrackingCommon.cpp
toolkit/components/antitracking/AntiTrackingCommon.h
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -12898,17 +12898,17 @@ nsIDocument::SetUserHasInteracted()
 
   mUserHasInteracted = true;
 
   nsCOMPtr<nsILoadInfo> loadInfo = mChannel ? mChannel->GetLoadInfo() : nullptr;
   if (loadInfo) {
     loadInfo->SetDocumentHasUserInteracted(true);
   }
 
-  MaybeAllowStorageForOpener();
+  MaybeAllowStorageForOpenerAfterUserInteraction();
 }
 
 void
 nsIDocument::NotifyUserGestureActivation()
 {
   // Activate this document and all documents up to the top level
   // content document.
   nsIDocument* doc = this;
@@ -12945,17 +12945,17 @@ nsIDocument::SetDocTreeHadPlayRevoked()
 {
   nsIDocument* topLevelDoc = GetTopLevelContentDocument();
   if (topLevelDoc) {
     topLevelDoc->mDocTreeHadPlayRevoked = true;
   }
 }
 
 void
-nsIDocument::MaybeAllowStorageForOpener()
+nsIDocument::MaybeAllowStorageForOpenerAfterUserInteraction()
 {
   if (StaticPrefs::network_cookie_cookieBehavior() !=
         nsICookieService::BEHAVIOR_REJECT_TRACKER) {
     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
@@ -13003,17 +13003,17 @@ nsIDocument::MaybeAllowStorageForOpener(
       !nsContentUtils::IsThirdPartyWindowOrChannel(openerInner, nullptr,
                                                    nullptr)) {
     return;
   }
 
   // We don't care when the asynchronous work finishes here.
   Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(NodePrincipal(),
                                                                      openerInner,
-                                                                     AntiTrackingCommon::eHeuristic);
+                                                                     AntiTrackingCommon::eOpenerAfterUserInteraction);
 }
 
 namespace {
 
 // Documents can stay alive for days. We don't want to update the permission
 // value at any user-interaction, and, using a timer triggered any X seconds
 // should be good enough. 'X' is taken from
 // privacy.userInteraction.document.interval pref.
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -7231,17 +7231,17 @@ nsGlobalWindowOuter::MaybeAllowStorageFo
   }
   nsCOMPtr<nsIPrincipal> principal =
     BasePrincipal::CreateCodebasePrincipal(aURI,
       doc->NodePrincipal()->OriginAttributesRef());
 
   // We don't care when the asynchronous work finishes here.
   Unused << AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor(principal,
                                                                      inner,
-                                                                     AntiTrackingCommon::eHeuristic);
+                                                                     AntiTrackingCommon::eOpener);
 }
 
 //*****************************************************************************
 // nsGlobalWindowOuter: Helper Functions
 //*****************************************************************************
 
 already_AddRefed<nsIDocShellTreeOwner>
 nsGlobalWindowOuter::GetTreeOwner()
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -3977,17 +3977,17 @@ protected:
   void UpdateFrameRequestCallbackSchedulingState(nsIPresShell* aOldShell = nullptr);
 
   // Helper for GetScrollingElement/IsScrollingElement.
   bool IsPotentiallyScrollable(mozilla::dom::HTMLBodyElement* aBody);
 
   // Return the same type parent docuement if exists, or return null.
   nsIDocument* GetSameTypeParentDocument();
 
-  void MaybeAllowStorageForOpener();
+  void MaybeAllowStorageForOpenerAfterUserInteraction();
 
   void MaybeStoreUserInteractionAsPermission();
 
   // Helpers for GetElementsByName.
   static bool MatchNameAttribute(mozilla::dom::Element* aElement,
                                  int32_t aNamespaceID,
                                  nsAtom* aAtom, void* aData);
   static void* UseExistingNameString(nsINode* aRootNode, const nsString* aName);
--- a/toolkit/components/antitracking/AntiTrackingCommon.cpp
+++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp
@@ -327,17 +327,19 @@ ReportUnblockingConsole(nsPIDOMWindowInn
   const char *messageWithSameOrigin = nullptr;
 
   switch (aReason) {
     case AntiTrackingCommon::eStorageAccessAPI:
       messageWithDifferentOrigin = "CookieAllowedForOriginOnTrackerByStorageAccessAPI";
       messageWithSameOrigin = "CookieAllowedForTrackerByStorageAccessAPI";
       break;
 
-    case AntiTrackingCommon::eHeuristic:
+    case AntiTrackingCommon::eOpenerAfterUserInteraction:
+      MOZ_FALLTHROUGH;
+    case AntiTrackingCommon::eOpener:
       messageWithDifferentOrigin = "CookieAllowedForOriginOnTrackerByHeuristic";
       messageWithSameOrigin = "CookieAllowedForTrackerByHeuristic";
       break;
   }
 
   if (aTrackingOrigin == aGrantedOrigin) {
     nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
                                     NS_LITERAL_CSTRING("Content Blocking"),
@@ -482,20 +484,25 @@ AntiTrackingCommon::AddFirstPartyStorage
                                                   getter_AddRefs(topLevelStoragePrincipal),
                                                   trackingOrigin,
                                                   getter_AddRefs(trackingURI),
                                                   getter_AddRefs(trackingPrincipal))) {
     LOG(("Error while computing the parent principal and tracking origin, bailing out early"));
     return StorageAccessGrantPromise::CreateAndReject(false, __func__);
   }
 
-  // We hardcode this block reason since the first-party storage access permission
-  // is granted for the purpose of blocking trackers.
+  // We hardcode this block reason since the first-party storage access
+  // permission is granted for the purpose of blocking trackers.
+  // Note that if aReason is eOpenerAfterUserInteraction, we don't check the
+  // user-interaction state, because it could be that the current process has
+  // just sent the request to store the user-interaction permission into the
+  // parent, without having received the permission itself yet.
   const uint32_t blockReason = nsIWebProgressListener::STATE_COOKIES_BLOCKED_TRACKER;
-  if (!HasUserInteraction(trackingPrincipal)) {
+  if (aReason != eOpenerAfterUserInteraction &&
+      !HasUserInteraction(trackingPrincipal)) {
     LOG_SPEC(("Tracking principal (%s) hasn't been interacted with before, "
               "refusing to add a first-party storage permission to access it",
               _spec), trackingURI);
     NotifyRejection(aParentWindow, blockReason);
     return StorageAccessGrantPromise::CreateAndReject(false, __func__);
   }
 
   nsCOMPtr<nsPIDOMWindowOuter> pwin = GetTopWindow(parentWindow);
@@ -589,17 +596,17 @@ AntiTrackingCommon::SaveFirstPartyStorag
     // permission which won't get persisted to disk.
     expirationType = nsIPermissionManager::EXPIRE_SESSION;
     when = 0;
   }
 
   nsAutoCString type;
   CreatePermissionKey(aTrackingOrigin, aGrantedOrigin, type);
 
-  LOG(("Computed permission key: %s, expiry: %d, proceeding to save in the permission manager",
+  LOG(("Computed permission key: %s, expiry: %u, proceeding to save in the permission manager",
        type.get(), expirationTime));
 
   rv = pm->AddFromPrincipal(aParentPrincipal, type.get(),
                             nsIPermissionManager::ALLOW_ACTION,
                             expirationType, when);
   Unused << NS_WARN_IF(NS_FAILED(rv));
   aResolver(NS_SUCCEEDED(rv));
 
--- a/toolkit/components/antitracking/AntiTrackingCommon.h
+++ b/toolkit/components/antitracking/AntiTrackingCommon.h
@@ -69,17 +69,18 @@ public:
   // This method checks if the principal has the permission to access to the
   // first party storage.
   static bool
   IsFirstPartyStorageAccessGrantedFor(nsIPrincipal* aPrincipal);
 
   enum StorageAccessGrantedReason
   {
     eStorageAccessAPI,
-    eHeuristic,
+    eOpenerAfterUserInteraction,
+    eOpener
   };
 
   // Grant the permission for aOrigin to have access to the first party storage.
   // This method can handle 2 different scenarios:
   // - aParentWindow is a 3rd party context, it opens an aOrigin window and the
   //   user interacts with it. We want to grant the permission at the
   //   combination: top-level + aParentWindow + aOrigin.
   //   Ex: example.net loads an iframe tracker.com, which opens a popup