Bug 1543786 - Ensure that we revoke a top frame's storage access when it is navigated away; r=baku
authorEhsan Akhgari <ehsan@mozilla.com>
Wed, 17 Apr 2019 15:01:53 +0000
changeset 469854 2418f3bd2940a5d824ae841690dad8102a20ff39
parent 469853 46235152b24e33c652c4e94430897369bbb2090b
child 469855 a7af13fee7b22d8a56d75bb72fc1191792dab23d
push id35883
push userbtara@mozilla.com
push dateWed, 17 Apr 2019 21:47:29 +0000
treeherdermozilla-central@02b89c29412b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1543786
milestone68.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 1543786 - Ensure that we revoke a top frame's storage access when it is navigated away; r=baku Differential Revision: https://phabricator.services.mozilla.com/D27155
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
dom/base/nsFrameLoader.cpp
dom/ipc/TabChild.cpp
toolkit/components/antitracking/test/browser/antitracking_head.js
toolkit/components/antitracking/test/browser/browser.ini
toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -2664,26 +2664,42 @@ nsresult nsDocShell::SetDocLoaderParent(
   if (parentURIListener) {
     mContentListener->SetParentContentListener(parentURIListener);
   }
 
   // Our parent has changed. Recompute scriptability.
   RecomputeCanExecuteScripts();
 
   // Inform windows when they're being removed from their parent.
-  if (!aParent && mScriptGlobal) {
-    mScriptGlobal->ParentWindowChanged();
+  if (!aParent) {
+    MaybeClearStorageAccessFlag();
   }
 
   NS_ASSERTION(mInheritPrivateBrowsingId || wasPrivate == UsePrivateBrowsing(),
                "Private browsing state changed while inheritance was disabled");
 
   return NS_OK;
 }
 
+void nsDocShell::MaybeClearStorageAccessFlag() {
+  if (mScriptGlobal) {
+    // Tell our window that the parent has now changed.
+    mScriptGlobal->ParentWindowChanged();
+
+    // Tell all of our children about the change recursively as well.
+    nsTObserverArray<nsDocLoader*>::ForwardIterator iter(mChildList);
+    while (iter.HasMore()) {
+      nsCOMPtr<nsIDocShell> child = do_QueryObject(iter.GetNext());
+      if (child) {
+        static_cast<nsDocShell*>(child.get())->MaybeClearStorageAccessFlag();
+      }
+    }
+  }
+}
+
 NS_IMETHODIMP
 nsDocShell::GetSameTypeParent(nsIDocShellTreeItem** aParent) {
   NS_ENSURE_ARG_POINTER(aParent);
   *aParent = nullptr;
 
   if (nsIDocShell::GetIsMozBrowser()) {
     return NS_OK;
   }
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -396,16 +396,19 @@ class nsDocShell final : public nsDocLoa
   /**
    * Loads the given URI. See comments on nsDocShellLoadState members for more
    * information on information used. aDocShell and aRequest come from
    * onLinkClickSync, which is triggered during form submission.
    */
   nsresult InternalLoad(nsDocShellLoadState* aLoadState,
                         nsIDocShell** aDocShell, nsIRequest** aRequest);
 
+  // Clear the document's storage access flag if needed.
+  void MaybeClearStorageAccessFlag();
+
  private:  // member functions
   friend class nsDSURIContentListener;
   friend class FramingChecker;
   friend class OnLinkClickEvent;
   friend class nsIDocShell;
   friend class mozilla::dom::BrowsingContext;
 
   // It is necessary to allow adding a timeline marker wherever a docshell
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -572,16 +572,22 @@ nsresult nsFrameLoader::ReallyStartLoadi
     if (!mRemoteBrowserShown) {
       // This can fail if it's too early to show the frame, we will retry later.
       Unused << ShowRemoteFrame(ScreenIntSize(0, 0));
     }
 
     return NS_OK;
   }
 
+  if (GetDocShell()) {
+    // If we already have a docshell, ensure that the docshell's storage access
+    // flag is cleared.
+    GetDocShell()->MaybeClearStorageAccessFlag();
+  }
+
   nsresult rv = MaybeCreateDocShell();
   if (NS_FAILED(rv)) {
     return rv;
   }
   MOZ_ASSERT(GetDocShell(),
              "MaybeCreateDocShell succeeded with a null docShell");
 
   // If we have a pending switch, just resume our load.
@@ -1103,16 +1109,18 @@ void nsFrameLoader::Hide() {
     mHideCalled = true;
     return;
   }
 
   if (!GetDocShell()) {
     return;
   }
 
+  GetDocShell()->MaybeClearStorageAccessFlag();
+
   nsCOMPtr<nsIContentViewer> contentViewer;
   GetDocShell()->GetContentViewer(getter_AddRefs(contentViewer));
   if (contentViewer) contentViewer->SetSticky(false);
 
   RefPtr<nsDocShell> baseWin = GetDocShell();
   baseWin->SetVisibility(false);
   baseWin->SetParentWidget(nullptr);
 }
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1047,24 +1047,29 @@ mozilla::ipc::IPCResult TabChild::RecvLo
   }
 
   LoadURIOptions loadURIOptions;
   loadURIOptions.mTriggeringPrincipal = nsContentUtils::GetSystemPrincipal();
   loadURIOptions.mLoadFlags =
       nsIWebNavigation::LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP |
       nsIWebNavigation::LOAD_FLAGS_DISALLOW_INHERIT_PRINCIPAL;
 
-  nsresult rv =
-      WebNavigation()->LoadURI(NS_ConvertUTF8toUTF16(aURI), loadURIOptions);
+  nsIWebNavigation* webNav = WebNavigation();
+  nsresult rv = webNav->LoadURI(NS_ConvertUTF8toUTF16(aURI), loadURIOptions);
   if (NS_FAILED(rv)) {
     NS_WARNING(
         "WebNavigation()->LoadURI failed. Eating exception, what else can I "
         "do?");
   }
 
+  nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation());
+  if (docShell) {
+    nsDocShell::Cast(docShell)->MaybeClearStorageAccessFlag();
+  }
+
   CrashReporter::AnnotateCrashReport(CrashReporter::Annotation::URL, aURI);
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult TabChild::RecvResumeLoad(
     const uint64_t& aPendingSwitchID, const ShowInfo& aInfo) {
   if (!mDidLoadURLInit) {
--- a/toolkit/components/antitracking/test/browser/antitracking_head.js
+++ b/toolkit/components/antitracking/test/browser/antitracking_head.js
@@ -330,17 +330,17 @@ this.AntiTracking = {
                                   options.blockingByContentBlockingRTUI &&
                                   !options.allowList;
       let thirdPartyPage;
       if (typeof options.thirdPartyPage == "string") {
         thirdPartyPage = options.thirdPartyPage;
       } else {
         thirdPartyPage = TEST_3RD_PARTY_PAGE;
       }
-      await ContentTask.spawn(browser,
+      let id = await ContentTask.spawn(browser,
                               { page: thirdPartyPage,
                                 nextPage: TEST_4TH_PARTY_PAGE,
                                 callback: options.callback.toString(),
                                 callbackAfterRemoval: options.callbackAfterRemoval ?
                                   options.callbackAfterRemoval.toString() : null,
                                 accessRemoval: options.accessRemoval,
                                 iframeSandbox: options.iframeSandbox,
                                 allowList: options.allowList,
@@ -415,24 +415,67 @@ this.AntiTracking = {
                   return;
                 }
 
                 ok(false, "Unknown message");
               });
 
               ifr.src = obj.nextPage;
             });
+          case "navigate-topframe":
+            // pass-through
             break;
           default:
             ok(false, "Unexpected accessRemoval code passed: " + obj.accessRemoval);
             break;
           }
         }
+
+        return id;
       });
 
+      if (doAccessRemovalChecks &&
+          options.accessRemoval == "navigate-topframe") {
+        await BrowserTestUtils.loadURI(browser, TEST_4TH_PARTY_PAGE);
+        await BrowserTestUtils.browserLoaded(browser);
+
+        let pageshow = BrowserTestUtils.waitForContentEvent(tab.linkedBrowser, "pageshow");
+        gBrowser.goBack();
+        await pageshow;
+
+        await ContentTask.spawn(browser,
+                                { id,
+                                  callbackAfterRemoval: options.callbackAfterRemoval ?
+                                    options.callbackAfterRemoval.toString() : null,
+                                },
+                                async function(obj) {
+          let ifr = content.document.getElementById(obj.id);
+          ifr.contentWindow.postMessage(obj.callbackAfterRemoval, "*");
+
+          content.addEventListener("message", function msg(event) {
+            if (event.data.type == "finish") {
+              content.removeEventListener("message", msg);
+              return;
+            }
+
+            if (event.data.type == "ok") {
+              ok(event.data.what, event.data.msg);
+              return;
+            }
+
+            if (event.data.type == "info") {
+              info(event.data.msg);
+              return;
+            }
+
+            ok(false, "Unknown message");
+          });
+        });
+      }
+
       if (options.allowList) {
         info("Enabling content blocking for this page");
         win.ContentBlocking.enableForCurrentPage();
 
         // The previous function reloads the browser, so wait for it to load again!
         await BrowserTestUtils.browserLoaded(browser);
       }
 
--- a/toolkit/components/antitracking/test/browser/browser.ini
+++ b/toolkit/components/antitracking/test/browser/browser.ini
@@ -74,16 +74,18 @@ support-files = tracker.js
 [browser_userInteraction.js]
 [browser_storageAccessDoorHanger.js]
 [browser_storageAccessPrivateWindow.js]
 skip-if = serviceworker_e10s
 [browser_storageAccessPromiseRejectHandlerUserInteraction.js]
 [browser_storageAccessPromiseResolveHandlerUserInteraction.js]
 [browser_storageAccessRemovalNavigateSubframe.js]
 skip-if = serviceworker_e10s
+[browser_storageAccessRemovalNavigateTopframe.js]
+skip-if = serviceworker_e10s
 [browser_storageAccessSandboxed.js]
 skip-if = serviceworker_e10s
 [browser_storageAccessWithHeuristics.js]
 [browser_allowPermissionForTracker.js]
 [browser_denyPermissionForTracker.js]
 [browser_localStorageEvents.js]
 support-files = localStorage.html
 [browser_partitionedLocalStorage.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/antitracking/test/browser/browser_storageAccessRemovalNavigateTopframe.js
@@ -0,0 +1,42 @@
+/* import-globals-from antitracking_head.js */
+
+AntiTracking.runTest("Storage Access is removed when topframe navigates",
+  // blocking callback
+  async _ => {
+    /* import-globals-from storageAccessAPIHelpers.js */
+    await noStorageAccessInitially();
+  },
+
+  // non-blocking callback
+  async _ => {
+    /* import-globals-from storageAccessAPIHelpers.js */
+    if (allowListed) {
+      await hasStorageAccessInitially();
+    } else {
+      await noStorageAccessInitially();
+    }
+
+    /* import-globals-from storageAccessAPIHelpers.js */
+    let [threw, rejected] = await callRequestStorageAccess();
+    ok(!threw, "requestStorageAccess should not throw");
+    ok(!rejected, "requestStorageAccess should be available");
+  },
+  // cleanup function
+  async _ => {
+    await new Promise(resolve => {
+      Services.clearData.deleteData(Ci.nsIClearDataService.CLEAR_ALL, value => resolve());
+    });
+  },
+  null, // extra prefs
+  false, // no window open test
+  false, // no user-interaction test
+  0, // no blocking notifications
+  false, // run in normal window
+  null, // no iframe sandbox
+  "navigate-topframe", // access removal type
+  // after-removal callback
+  async _ => {
+    /* import-globals-from storageAccessAPIHelpers.js */
+    await noStorageAccessInitially();
+  }
+);