Bug 1623268 - P4. Rename and reorder variables in ContentBlocking::AllowAccessFor r=timhuang,baku
authorDimi Lee <dlee@mozilla.com>
Thu, 26 Mar 2020 15:12:12 +0000
changeset 520590 a217ec8bc94ed25180c73fd7799a3f543f5e6c98
parent 520589 f89b7ec7b51b8c02cb8a18a002641af5883da42c
child 520591 fa8a9f99515a64b259e53c70b873512328281856
push id37253
push usernerli@mozilla.com
push dateThu, 26 Mar 2020 21:36:52 +0000
treeherdermozilla-central@c644dd16e2cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstimhuang, baku
bugs1623268
milestone76.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 1623268 - P4. Rename and reorder variables in ContentBlocking::AllowAccessFor r=timhuang,baku This patch does the following: - declare local variables right before they are used. - remove unused local variables. - rename local variables to make its purpose more clear Differential Revision: https://phabricator.services.mozilla.com/D67284
toolkit/components/antitracking/ContentBlocking.cpp
--- a/toolkit/components/antitracking/ContentBlocking.cpp
+++ b/toolkit/components/antitracking/ContentBlocking.cpp
@@ -317,33 +317,16 @@ ContentBlocking::AllowAccessFor(
     return StorageAccessGrantPromise::CreateAndResolve(true, __func__);
   }
 
   MOZ_ASSERT(
       behavior == nsICookieService::BEHAVIOR_REJECT_TRACKER ||
       behavior ==
           nsICookieService::BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN);
 
-  nsCOMPtr<nsPIDOMWindowOuter> parentOuter = aParentContext->GetDOMWindow();
-  if (!parentOuter) {
-    LOG(
-        ("No outer window found for our parent window context, bailing out "
-         "early"));
-    return StorageAccessGrantPromise::CreateAndReject(false, __func__);
-  }
-
-  nsCOMPtr<nsPIDOMWindowInner> parentInner =
-      parentOuter->GetCurrentInnerWindow();
-  if (!parentInner) {
-    LOG(
-        ("No inner window found for our parent outer window, bailing out "
-         "early"));
-    return StorageAccessGrantPromise::CreateAndReject(false, __func__);
-  }
-
   // No need to continue when we are already in the allow list.
   bool isInAllowList = false;
   if (NS_FAILED(
           ContentBlockingAllowList::Check(aParentContext, isInAllowList))) {
     return StorageAccessGrantPromise::CreateAndReject(false, __func__);
   }
 
   if (isInAllowList) {
@@ -354,88 +337,90 @@ ContentBlocking::AllowAccessFor(
 
   // Make sure storage access isn't disabled
   if (!isParentTopLevel &&
       Document::StorageAccessSandboxed(aParentContext->GetSandboxFlags())) {
     LOG(("Our document is sandboxed"));
     return StorageAccessGrantPromise::CreateAndReject(false, __func__);
   }
 
+  nsCOMPtr<nsPIDOMWindowOuter> parentOuter = aParentContext->GetDOMWindow();
+  if (!parentOuter) {
+    LOG(
+        ("No outer window found for our parent window context, bailing out "
+         "early"));
+    return StorageAccessGrantPromise::CreateAndReject(false, __func__);
+  }
+
+  nsCOMPtr<nsPIDOMWindowInner> parentInnerWindow =
+      parentOuter->GetCurrentInnerWindow();
+  if (!parentInnerWindow) {
+    LOG(
+        ("No inner window found for our parent outer window, bailing out "
+         "early"));
+    return StorageAccessGrantPromise::CreateAndReject(false, __func__);
+  }
+
   nsCOMPtr<nsIPrincipal> topLevelStoragePrincipal;
   nsAutoCString trackingOrigin;
   nsCOMPtr<nsIPrincipal> trackingPrincipal;
 
-  RefPtr<nsGlobalWindowInner> parentWindow =
-      nsGlobalWindowInner::Cast(parentInner);
-
   LOG(("The current resource is %s-party",
        isParentTopLevel ? "first" : "third"));
 
-  nsresult rv;
   // We are a first party resource.
   if (isParentTopLevel) {
     nsAutoCString origin;
-    rv = aPrincipal->GetAsciiOrigin(origin);
+    nsresult rv = aPrincipal->GetAsciiOrigin(origin);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       LOG(("Can't get the origin from the URI"));
       return StorageAccessGrantPromise::CreateAndReject(false, __func__);
     }
 
     trackingOrigin = origin;
     trackingPrincipal = aPrincipal;
-    topLevelStoragePrincipal = parentWindow->GetPrincipal();
+    topLevelStoragePrincipal =
+        nsGlobalWindowInner::Cast(parentInnerWindow)->GetPrincipal();
     if (NS_WARN_IF(!topLevelStoragePrincipal)) {
       LOG(("Top-level storage area principal not found, bailing out early"));
       return StorageAccessGrantPromise::CreateAndReject(false, __func__);
     }
 
   } else {
     // We should be a 3rd party source.
     // Make sure we are either a third-party tracker or a third-party
     // window (depends upon the cookie bahavior).
     if (behavior == nsICookieService::BEHAVIOR_REJECT_TRACKER &&
-        !nsContentUtils::IsThirdPartyTrackingResourceWindow(parentWindow)) {
+        !nsContentUtils::IsThirdPartyTrackingResourceWindow(
+            parentInnerWindow)) {
       LOG(("Our window isn't a third-party tracking window"));
       return StorageAccessGrantPromise::CreateAndReject(false, __func__);
     } else if (behavior == nsICookieService::
                                BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN &&
-               !nsContentUtils::IsThirdPartyWindowOrChannel(parentWindow,
+               !nsContentUtils::IsThirdPartyWindowOrChannel(parentInnerWindow,
                                                             nullptr, nullptr)) {
       LOG(("Our window isn't a third-party window"));
       return StorageAccessGrantPromise::CreateAndReject(false, __func__);
     }
 
     if (!GetParentPrincipalAndTrackingOrigin(
-            parentWindow,
+            nsGlobalWindowInner::Cast(parentInnerWindow),
             // Don't request the ETP specific behaviour of allowing only
             // singly-nested iframes here, because we are recording an allow
             // permission.
             nsICookieService::BEHAVIOR_ACCEPT,
             getter_AddRefs(topLevelStoragePrincipal), trackingOrigin,
             getter_AddRefs(trackingPrincipal))) {
       LOG(
           ("Error while computing the parent principal and tracking origin, "
            "bailing out early"));
       return StorageAccessGrantPromise::CreateAndReject(false, __func__);
     }
   }
 
-  nsPIDOMWindowOuter* topOuterWindow = aParentContext->Top()->GetDOMWindow();
-  nsGlobalWindowOuter* topWindow = nsGlobalWindowOuter::Cast(topOuterWindow);
-  if (NS_WARN_IF(!topWindow)) {
-    LOG(("No top outer window."));
-    return StorageAccessGrantPromise::CreateAndReject(false, __func__);
-  }
-
-  nsPIDOMWindowInner* topInnerWindow = topWindow->GetCurrentInnerWindow();
-  if (NS_WARN_IF(!topInnerWindow)) {
-    LOG(("No top inner window."));
-    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.
   // Note that if aReason is eOpenerAfterUserInteraction and the
   // trackingPrincipal is not in a blacklist, 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.
   //
@@ -452,51 +437,61 @@ ContentBlocking::AllowAccessFor(
       &isInPrefList);
   if (isInPrefList &&
       !ContentBlockingUserInteraction::Exists(trackingPrincipal)) {
     LOG_PRIN(("Tracking principal (%s) hasn't been interacted with before, "
               "refusing to add a first-party storage permission to access it",
               _spec),
              trackingPrincipal);
     ContentBlockingNotifier::OnDecision(
-        parentInner, ContentBlockingNotifier::BlockingDecision::eBlock,
+        parentInnerWindow, ContentBlockingNotifier::BlockingDecision::eBlock,
         blockReason);
     return StorageAccessGrantPromise::CreateAndReject(false, __func__);
   }
 
-  nsCOMPtr<nsPIDOMWindowOuter> pwin =
-      AntiTrackingUtils::GetTopWindow(parentWindow);
-  if (!pwin) {
+  // Check if we can get top-level outer/inner window when we still
+  // have a chance to report an error.
+  nsCOMPtr<nsPIDOMWindowOuter> topOuterWindow =
+      AntiTrackingUtils::GetTopWindow(parentInnerWindow);
+  if (!topOuterWindow) {
     LOG(("Couldn't get the top window"));
     return StorageAccessGrantPromise::CreateAndReject(false, __func__);
   }
 
+  nsCOMPtr<nsPIDOMWindowInner> topInnerWindow =
+      topOuterWindow->GetCurrentInnerWindow();
+  if (NS_WARN_IF(!topInnerWindow)) {
+    LOG(("No top inner window."));
+    return StorageAccessGrantPromise::CreateAndReject(false, __func__);
+  }
+
   auto storePermission =
-      [pwin, parentWindow, trackingOrigin, trackingPrincipal, topInnerWindow,
-       topLevelStoragePrincipal,
+      [parentInnerWindow, topOuterWindow, topInnerWindow, trackingOrigin,
+       trackingPrincipal, topLevelStoragePrincipal,
        aReason](int aAllowMode) -> RefPtr<StorageAccessGrantPromise> {
     nsAutoCString permissionKey;
     AntiTrackingUtils::CreateStoragePermissionKey(trackingOrigin,
                                                   permissionKey);
 
     // Let's store the permission in the current parent window.
     topInnerWindow->SaveStorageAccessGranted(permissionKey);
 
     // Let's inform the parent window.
-    parentWindow->StorageAccessGranted();
+    nsGlobalWindowInner::Cast(parentInnerWindow)->StorageAccessGranted();
 
     nsIChannel* channel =
-        pwin->GetCurrentInnerWindow()->GetExtantDoc()->GetChannel();
+        topOuterWindow->GetCurrentInnerWindow()->GetExtantDoc()->GetChannel();
 
     ContentBlockingNotifier::OnEvent(
-        pwin, channel, parentWindow->GetExtantDoc()->GetChannel(), false,
-        blockReason, trackingOrigin, Some(aReason));
+        topOuterWindow, channel,
+        parentInnerWindow->GetExtantDoc()->GetChannel(), false, blockReason,
+        trackingOrigin, Some(aReason));
 
     ContentBlockingNotifier::ReportUnblockingToConsole(
-        parentWindow, NS_ConvertUTF8toUTF16(trackingOrigin), aReason);
+        parentInnerWindow, NS_ConvertUTF8toUTF16(trackingOrigin), aReason);
 
     if (XRE_IsParentProcess()) {
       LOG(("Saving the permission: trackingOrigin=%s", trackingOrigin.get()));
       return SaveAccessForOriginOnParentProcess(topLevelStoragePrincipal,
                                                 trackingPrincipal,
                                                 trackingOrigin, aAllowMode)
           ->Then(GetCurrentThreadSerialEventTarget(), __func__,
                  [](ParentAccessGrantPromise::ResolveOrRejectValue&& aValue) {