Bug 1672479: Fix beforeunload handling in location.refresh under SHiP. r=peterv

Per spec, whenever a "beforeunload" event handler which would affect a
`location.reload()` call exists, it must be called before the `reload()` call
returns. If a handler requests to block the navigation and we choose to
display a confirmation prompt, that must also be displayed before the call
returns.

With session history in parent, though, that currently does not happen,
because `location.reload()` triggers an async IPC call to the parent process,
and only attempts the actual reload (and thus beforeunload dispatch and
prompting) once it returns, which is too late to affect the caller.

This patch changes the handling in this case to manually perform permit unload
checks before taking an async code path. This still leaves the opportunity for
session history handlers in the parent to cancel the load asynchronously, but
that doesn't violate any spec-defined behavior.

Differential Revision: https://phabricator.services.mozilla.com/D94354

docshell/base/nsDocShell.cpp

@@ -3993,10 +3993,19 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
     bool forceReload = IsForceReloadType(loadType);
     if (!XRE_IsParentProcess()) {
       RefPtr<nsDocShell> docShell(this);
+      nsCOMPtr<nsIContentViewer> cv(mContentViewer);
+
+      bool okToUnload = true;
+      MOZ_TRY(cv->PermitUnload(&okToUnload));
+      if (!okToUnload) {
+        return NS_OK;
+      }
+
       RefPtr<Document> doc(GetDocument());
       RefPtr<BrowsingContext> browsingContext(mBrowsingContext);
       nsCOMPtr<nsIURI> currentURI(mCurrentURI);
       nsCOMPtr<nsIReferrerInfo> referrerInfo(mReferrerInfo);
+
       ContentChild::GetSingleton()->SendNotifyOnHistoryReload(
           mBrowsingContext, forceReload,
           [docShell, doc, loadType, browsingContext, currentURI, referrerInfo](
@@ -4016,16 +4025,18 @@ nsDocShell::Reload(uint32_t aReloadFlags) {
               MOZ_LOG(
                   gSHLog, LogLevel::Debug,
                   ("nsDocShell %p Reload - LoadHistoryEntry", docShell.get()));
+              loadState.ref()->SetNotifiedBeforeUnloadListeners(true);
               docShell->LoadHistoryEntry(loadState.ref(), loadType,
                                          reloadingActiveEntry.ref());
             } else {
               MOZ_LOG(gSHLog, LogLevel::Debug,
                       ("nsDocShell %p ReloadDocument", docShell.get()));
               ReloadDocument(docShell, doc, loadType, browsingContext,
-                             currentURI, referrerInfo);
+                             currentURI, referrerInfo,
+                             /* aNotifiedBeforeUnloadListeners */ true);
             }
           },
-          [](ResponseRejectReason) {});
+          [](mozilla::ipc::ResponseRejectReason) {});
     } else {
       // Parent process
       bool canReload = false;
@@ -4079,7 +4090,8 @@ nsresult nsDocShell::ReloadDocument(nsDocShell* aDocShell, Document* aDocument,
                                     uint32_t aLoadType,
                                     BrowsingContext* aBrowsingContext,
                                     nsIURI* aCurrentURI,
-                                    nsIReferrerInfo* aReferrerInfo) {
+                                    nsIReferrerInfo* aReferrerInfo,
+                                    bool aNotifiedBeforeUnloadListeners) {
   if (!aDocument) {
     return NS_OK;
   }
@@ -4152,6 +4164,7 @@ nsresult nsDocShell::ReloadDocument(nsDocShell* aDocShell, Document* aDocument,
   loadState->SetBaseURI(baseURI);
   loadState->SetHasValidUserGestureActivation(
       context && context->HasValidTransientUserGestureActivation());
+  loadState->SetNotifiedBeforeUnloadListeners(aNotifiedBeforeUnloadListeners);
   return aDocShell->InternalLoad(loadState);
 }
 
@@ -9194,7 +9207,8 @@ nsresult nsDocShell::InternalLoad(nsDocShellLoadState* aLoadState,
   }
   // Check if the page doesn't want to be unloaded. The javascript:
   // protocol handler deals with this for javascript: URLs.
-  if (!isJavaScript && isNotDownload && mContentViewer) {
+  if (!isJavaScript && isNotDownload &&
+      !aLoadState->NotifiedBeforeUnloadListeners() && mContentViewer) {
     bool okToUnload;
     rv = mContentViewer->PermitUnload(&okToUnload);
 

docshell/base/nsDocShell.h

@@ -645,10 +645,14 @@ class nsDocShell final : public nsDocLoader,
   void SetHistoryEntryAndUpdateBC(const mozilla::Maybe<nsISHEntry*>& aLSHE,
                                   const mozilla::Maybe<nsISHEntry*>& aOSHE);
 
+  // If aNotifiedBeforeUnloadListeners is true, "beforeunload" event listeners
+  // were notified by the caller and given the chance to abort the navigation,
+  // and should not be notified again.
   static nsresult ReloadDocument(
       nsDocShell* aDocShell, mozilla::dom::Document* aDocument,
       uint32_t aLoadType, mozilla::dom::BrowsingContext* aBrowsingContext,
-      nsIURI* aCurrentURI, nsIReferrerInfo* aReferrerInfo);
+      nsIURI* aCurrentURI, nsIReferrerInfo* aReferrerInfo,
+      bool aNotifiedBeforeUnloadListeners = false);
 
   //
   // URI Load

docshell/base/nsDocShellLoadState.cpp

@@ -38,7 +38,8 @@ nsDocShellLoadState::nsDocShellLoadState(nsIURI* aURI)
 
 nsDocShellLoadState::nsDocShellLoadState(
     const DocShellLoadStateInit& aLoadState)
-    : mLoadIdentifier(aLoadState.LoadIdentifier()) {
+    : mNotifiedBeforeUnloadListeners(false),
+      mLoadIdentifier(aLoadState.LoadIdentifier()) {
   MOZ_ASSERT(aLoadState.URI(), "Cannot create a LoadState with a null URI!");
   mResultPrincipalURI = aLoadState.ResultPrincipalURI();
   mResultPrincipalURIIsSome = aLoadState.ResultPrincipalURIIsSome();
@@ -94,6 +95,7 @@ nsDocShellLoadState::nsDocShellLoadState(const nsDocShellLoadState& aOther)
       mLoadReplace(aOther.mLoadReplace),
       mInheritPrincipal(aOther.mInheritPrincipal),
       mPrincipalIsExplicit(aOther.mPrincipalIsExplicit),
+      mNotifiedBeforeUnloadListeners(aOther.mNotifiedBeforeUnloadListeners),
       mPrincipalToInherit(aOther.mPrincipalToInherit),
       mPartitionedPrincipalToInherit(aOther.mPartitionedPrincipalToInherit),
       mForceAllowDataURI(aOther.mForceAllowDataURI),
@@ -133,6 +135,7 @@ nsDocShellLoadState::nsDocShellLoadState(nsIURI* aURI, uint64_t aLoadIdentifier)
       mLoadReplace(false),
       mInheritPrincipal(false),
       mPrincipalIsExplicit(false),
+      mNotifiedBeforeUnloadListeners(false),
       mForceAllowDataURI(false),
       mOriginalFrameSrc(false),
       mIsFormSubmission(false),
@@ -465,6 +468,15 @@ void nsDocShellLoadState::SetPrincipalIsExplicit(bool aPrincipalIsExplicit) {
   mPrincipalIsExplicit = aPrincipalIsExplicit;
 }
 
+bool nsDocShellLoadState::NotifiedBeforeUnloadListeners() const {
+  return mNotifiedBeforeUnloadListeners;
+}
+
+void nsDocShellLoadState::SetNotifiedBeforeUnloadListeners(
+    bool aNotifiedBeforeUnloadListeners) {
+  mNotifiedBeforeUnloadListeners = aNotifiedBeforeUnloadListeners;
+}
+
 bool nsDocShellLoadState::ForceAllowDataURI() const {
   return mForceAllowDataURI;
 }

docshell/base/nsDocShellLoadState.h

@@ -120,6 +120,14 @@ class nsDocShellLoadState final {
 
   void SetPrincipalIsExplicit(bool aPrincipalIsExplicit);
 
+
+  // If true, "beforeunload" event listeners were notified by the creater of the
+  // LoadState and given the chance to abort the navigation, and should not be
+  // notified again.
+  bool NotifiedBeforeUnloadListeners() const;
+
+  void SetNotifiedBeforeUnloadListeners(bool aNotifiedBeforeUnloadListeners);
+
   bool ForceAllowDataURI() const;
 
   void SetForceAllowDataURI(bool aForceAllowDataURI);
@@ -360,6 +368,8 @@ class nsDocShellLoadState final {
   // for a content docshell the load fails.
   bool mPrincipalIsExplicit;
 
+  bool mNotifiedBeforeUnloadListeners;
+
   // Principal we're inheriting. If null, this means the principal should be
   // inherited from the current document. If set to NullPrincipal, the channel
   // will fill in principal information later in the load. See internal comments

docshell/test/browser/browser.ini

@@ -133,7 +133,6 @@ support-files =
 [browser_loadURI_postdata.js]
 [browser_multiple_pushState.js]
 [browser_onbeforeunload.js]
-skip-if = fission
 [browser_onbeforeunload_navigation.js]
 skip-if = (os == 'win' && !debug) # bug 1300351
 [browser_onunload_stop.js]

docshell/test/browser/browser_onbeforeunload.js

@@ -2,7 +2,7 @@
 
 // We need to test a lot of permutations here, and there isn't any sensible way
 // to split them up or run them faster.
-requestLongerTimeout(2);
+requestLongerTimeout(4);
 
 const BASE_URL = "http://mochi.test:8888/browser/docshell/test/browser/";
 
@@ -123,7 +123,7 @@ function* generatePermutations(depth) {
   }
 }
 
-const PERMUTATIONS = Array.from(generatePermutations(3));
+const PERMUTATIONS = Array.from(generatePermutations(4));
 
 const FRAMES = [
   { process: 0 },