Bug 1863246 - Make the page that enters BFCache not asking the parent process to update the active browsing context r=peterv,dom-core

Currently, when a page enters BFCache, it updates the parent process
for the active BC; however, the page that is about to show will do the
same. These two operations are triggered in different processes with
different active id, they are racy and problematic.

This patch fixes the above issue by not updating the parent process
when a page enters BFCache.

This only applies to BFCacheInParent is enabled.

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

docshell/test/navigation/browser.toml

@@ -26,3 +26,5 @@ skip-if = ["!sessionHistoryInParent"]
 
 ["browser_test_simultaneous_normal_and_history_loads.js"]
 skip-if = ["!sessionHistoryInParent"] # The test is for the new session history
+
+["browser_bfcache_activebc.js"]

docshell/test/navigation/browser_bfcache_activebc.js

@@ -0,0 +1,81 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const FocusManager = Services.focus;
+
+const SITE_URL_1 =
+  getRootDirectory(gTestPath).replace(
+    "chrome://mochitests/content",
+    "https://example.com"
+  ) + "empty.html";
+
+const SITE_URL_2 =
+  getRootDirectory(gTestPath).replace(
+    "chrome://mochitests/content",
+    "http://127.0.0.1:8888"
+  ) + "empty.html";
+
+// Test ensures that when a page goes to BFCache, it won't
+// accidentally update the active browsing context to null to
+// the parent process.
+add_task(async function () {
+  // Load Site 1
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, SITE_URL_1);
+
+  BrowserTestUtils.startLoadingURIString(tab.linkedBrowser, SITE_URL_2);
+  // Navigated to Site 2 in the same tab
+  await BrowserTestUtils.browserLoaded(tab.linkedBrowser);
+
+  const pageNavigatedBackToSite1 = BrowserTestUtils.waitForContentEvent(
+    tab.linkedBrowser,
+    "pageshow"
+  );
+  tab.linkedBrowser.goBack();
+  // Navigated site 1 by going back
+  await pageNavigatedBackToSite1;
+
+  const pageHideForSite1Run = SpecialPowers.spawn(tab.linkedBrowser, [], () => {
+    return new Promise(r => {
+      content.addEventListener("pagehide", function () {
+        const start = Date.now();
+        // block the main thread for 2 seconds.
+        while (Date.now() - start < 2000) {
+          r();
+        }
+      });
+    });
+  });
+
+  let pageNavigatedBackToSite2 = BrowserTestUtils.waitForContentEvent(
+    tab.linkedBrowser,
+    "pageshow"
+  );
+
+  // Navigate to site 2 again by going forward.
+  //
+  // In a buggy Firefox build, this navigation would trigger
+  // two activeBrowsingContextInChrome updates. One from
+  // site 1 to set it to nullptr, and one from the site 2
+  // to itself.
+  tab.linkedBrowser.goForward();
+
+  await pageNavigatedBackToSite2;
+  // Forcefully to make site 1 to update activeBrowsingContextInChrome
+  // after site 2.
+  await pageHideForSite1Run;
+
+  // Give the parent process some opportunities to update
+  // the activeBrowsingContextInChrome via IPC.
+  await new Promise(r => {
+    /* eslint-disable mozilla/no-arbitrary-setTimeout */
+    setTimeout(r, 2000);
+  });
+
+  Assert.ok(
+    !!FocusManager.activeContentBrowsingContext,
+    "active browsing context in content should be non-null"
+  );
+  BrowserTestUtils.removeTab(tab);
+});

dom/base/nsFocusManager.cpp

@@ -42,6 +42,7 @@
 #include "nsRange.h"
 #include "nsFrameLoaderOwner.h"
 #include "nsQueryObject.h"
+#include "nsIXULRuntime.h"
 
 #include "mozilla/AccessibleCaretEventHub.h"
 #include "mozilla/ContentEvents.h"
@@ -395,6 +396,16 @@ nsFocusManager::GetFocusedContentBrowsingContext(
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsFocusManager::GetActiveContentBrowsingContext(
+    BrowsingContext** aBrowsingContext) {
+  MOZ_DIAGNOSTIC_ASSERT(
+      XRE_IsParentProcess(),
+      "We only have use cases for this in the parent process");
+  NS_IF_ADDREF(*aBrowsingContext = GetActiveBrowsingContextInChrome());
+  return NS_OK;
+}
+
 nsresult nsFocusManager::SetFocusedWindowWithCallerType(
     mozIDOMWindowProxy* aWindowToFocus, CallerType aCallerType) {
   LOGFOCUS(("<<SetFocusedWindow begin>>"));
@@ -734,7 +745,8 @@ void nsFocusManager::WindowRaised(mozIDOMWindowProxy* aWindow,
   if (XRE_IsParentProcess()) {
     mActiveWindow = window;
   } else if (bc->IsTop()) {
-    SetActiveBrowsingContextInContent(bc, aActionId);
+    SetActiveBrowsingContextInContent(bc, aActionId,
+                                      false /* aIsEnteringBFCache */);
   }
 
   // ensure that the window is enabled and visible
@@ -843,7 +855,8 @@ void nsFocusManager::WindowLowered(mozIDOMWindowProxy* aWindow,
   } else {
     BrowsingContext* bc = window->GetBrowsingContext();
     if (bc == bc->Top()) {
-      SetActiveBrowsingContextInContent(nullptr, aActionId);
+      SetActiveBrowsingContextInContent(nullptr, aActionId,
+                                        false /* aIsEnteringBFCache */);
     }
   }
 
@@ -1032,7 +1045,7 @@ void nsFocusManager::WindowShown(mozIDOMWindowProxy* aWindow,
 }
 
 void nsFocusManager::WindowHidden(mozIDOMWindowProxy* aWindow,
-                                  uint64_t aActionId) {
+                                  uint64_t aActionId, bool aIsEnteringBFCache) {
   // if there is no window or it is not the same or an ancestor of the
   // currently focused window, just return, as the current focus will not
   // be affected.
@@ -1193,7 +1206,7 @@ void nsFocusManager::WindowHidden(mozIDOMWindowProxy* aWindow,
       mActiveBrowsingContextInContent ==
           docShellBeingHidden->GetBrowsingContext() &&
       mActiveBrowsingContextInContent->GetIsInBFCache()) {
-    SetActiveBrowsingContextInContent(nullptr, aActionId);
+    SetActiveBrowsingContextInContent(nullptr, aActionId, aIsEnteringBFCache);
   }
 
   // if the window being hidden is an ancestor of the focused window, adjust
@@ -5165,7 +5178,8 @@ void nsFocusManager::BrowsingContextDetached(BrowsingContext* aContext) {
 }
 
 void nsFocusManager::SetActiveBrowsingContextInContent(
-    mozilla::dom::BrowsingContext* aContext, uint64_t aActionId) {
+    mozilla::dom::BrowsingContext* aContext, uint64_t aActionId,
+    bool aIsEnteringBFCache) {
   MOZ_ASSERT(!XRE_IsParentProcess());
   MOZ_ASSERT(!aContext || aContext->IsInProcess());
   mozilla::dom::ContentChild* contentChild =
@@ -5184,7 +5198,12 @@ void nsFocusManager::SetActiveBrowsingContextInContent(
   if (aContext != mActiveBrowsingContextInContent) {
     if (aContext) {
       contentChild->SendSetActiveBrowsingContext(aContext, aActionId);
-    } else if (mActiveBrowsingContextInContent) {
+    } else if (mActiveBrowsingContextInContent &&
+               !(BFCacheInParent() && aIsEnteringBFCache)) {
+      // No need to tell the parent process to update the active browsing
+      // context to null if we are entering BFCache, because the browsing
+      // context that is about to show will update it.
+      //
       // We want to sync this over only if this isn't happening
       // due to the active BrowsingContext switching processes,
       // in which case the BrowserChild has already marked itself

dom/base/nsFocusManager.h

@@ -259,7 +259,8 @@ class nsFocusManager final : public nsIFocusManager,
    * longer accept focus.
    */
   MOZ_CAN_RUN_SCRIPT void WindowHidden(mozIDOMWindowProxy* aWindow,
-                                       uint64_t aActionId);
+                                       uint64_t aActionId,
+                                       bool aIsEnteringBFCache);
 
   /**
    * Fire any events that have been delayed due to synchronized actions.
@@ -826,7 +827,8 @@ class nsFocusManager final : public nsIFocusManager,
   // Sets the BrowsingContext corresponding to top-level Web content
   // in the frontmost tab if focus is in Web content.
   void SetActiveBrowsingContextInContent(
-      mozilla::dom::BrowsingContext* aContext, uint64_t aActionId);
+      mozilla::dom::BrowsingContext* aContext, uint64_t aActionId,
+      bool aIsEnteringBFCache);
 
   // Content-only
   // Receives notification of another process setting the top-level Web

dom/base/nsGlobalWindowInner.cpp

@@ -4409,14 +4409,15 @@ void nsGlobalWindowInner::SetReadyForFocus() {
   }
 }
 
-void nsGlobalWindowInner::PageHidden() {
+void nsGlobalWindowInner::PageHidden(bool aIsEnteringBFCacheInParent) {
   // the window is being hidden, so tell the focus manager that the frame is
   // no longer valid. Use the persisted field to determine if the document
   // is being destroyed.
 
   if (RefPtr<nsFocusManager> fm = nsFocusManager::GetFocusManager()) {
     nsCOMPtr<nsPIDOMWindowOuter> outerWindow = GetOuterWindow();
-    fm->WindowHidden(outerWindow, nsFocusManager::GenerateFocusActionId());
+    fm->WindowHidden(outerWindow, nsFocusManager::GenerateFocusActionId(),
+                     aIsEnteringBFCacheInParent);
   }
 
   mNeedsFocus = true;

dom/base/nsGlobalWindowInner.h

@@ -467,7 +467,8 @@ class nsGlobalWindowInner final : public mozilla::dom::EventTarget,
 
   virtual bool TakeFocus(bool aFocus, uint32_t aFocusMethod) override;
   MOZ_CAN_RUN_SCRIPT_BOUNDARY virtual void SetReadyForFocus() override;
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY virtual void PageHidden() override;
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY virtual void PageHidden(
+      bool aIsEnteringBFCache) override;
   virtual nsresult DispatchAsyncHashchange(nsIURI* aOldURI,
                                            nsIURI* aNewURI) override;
   virtual nsresult DispatchSyncPopState() override;

dom/base/nsGlobalWindowOuter.cpp

@@ -6621,8 +6621,8 @@ void nsGlobalWindowOuter::SetReadyForFocus() {
   FORWARD_TO_INNER_VOID(SetReadyForFocus, ());
 }
 
-void nsGlobalWindowOuter::PageHidden() {
-  FORWARD_TO_INNER_VOID(PageHidden, ());
+void nsGlobalWindowOuter::PageHidden(bool aIsEnteringBFCacheInParent) {
+  FORWARD_TO_INNER_VOID(PageHidden, (aIsEnteringBFCacheInParent));
 }
 
 already_AddRefed<nsICSSDeclaration>

dom/base/nsGlobalWindowOuter.h

@@ -434,7 +434,7 @@ class nsGlobalWindowOuter final : public mozilla::dom::EventTarget,
 
   virtual bool TakeFocus(bool aFocus, uint32_t aFocusMethod) override;
   virtual void SetReadyForFocus() override;
-  virtual void PageHidden() override;
+  virtual void PageHidden(bool aIsEnteringBFCacheInParent) override;
 
   /**
    * Set a arguments for this window. This will be set on the window

dom/base/nsPIDOMWindow.h

@@ -569,7 +569,7 @@ class nsPIDOMWindowInner : public mozIDOMWindow {
    * Indicates that the page in the window has been hidden. This is used to
    * reset the focus state.
    */
-  virtual void PageHidden() = 0;
+  virtual void PageHidden(bool aIsEnteringBFCacheInParent) = 0;
 
   /**
    * Instructs this window to asynchronously dispatch a hashchange event.  This
@@ -1057,7 +1057,7 @@ class nsPIDOMWindowOuter : public mozIDOMWindowProxy {
    * Indicates that the page in the window has been hidden. This is used to
    * reset the focus state.
    */
-  virtual void PageHidden() = 0;
+  virtual void PageHidden(bool aIsEnteringBFCacheInParent) = 0;
 
   /**
    * Return the window id of this window

dom/interfaces/base/nsIFocusManager.idl

@@ -57,6 +57,12 @@ interface nsIFocusManager : nsISupports
    */
   readonly attribute BrowsingContext activeBrowsingContext;
 
+  /**
+   * Parent-process only: The chrome process notion of content's active
+   * browsing context.
+   */
+  readonly attribute BrowsingContext activeContentBrowsingContext;
+
   /**
    * The child window within the activeWindow that is focused. This will
    * always be activeWindow, a child window of activeWindow or null if no

layout/base/nsDocumentViewer.cpp

@@ -1353,7 +1353,7 @@ nsDocumentViewer::PageHide(bool aIsUnload) {
   // inform the window so that the focus state is reset.
   NS_ENSURE_STATE(mDocument);
   nsPIDOMWindowOuter* window = mDocument->GetWindow();
-  if (window) window->PageHidden();
+  if (window) window->PageHidden(!aIsUnload);
 
   if (aIsUnload) {
     // if Destroy() was called during OnPageHide(), mDocument is nullptr.