Bug 1677324 - Keep painting the of the old remote frame when going into the bfcache until the child unsuppresses painting. r=nika

This should pretty much match our non-fission behavior. I found the
original approach of keeping the frameloader from nsSubDocumentFrame
quite hard to get green on try unfortunately, because among other
things, the new frameloader wouldn't get the right viewport sizes /
position / etc.

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

dom/base/nsFrameLoaderOwner.cpp

@@ -5,6 +5,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsFrameLoaderOwner.h"
+#include "mozilla/dom/BrowserParent.h"
 #include "nsFrameLoader.h"
 #include "nsFocusManager.h"
 #include "nsNetUtil.h"
@@ -114,6 +115,10 @@ void nsFrameLoaderOwner::ChangeRemotenessCommon(
   doc->BlockOnload();
   auto cleanup = MakeScopeExit([&]() { doc->UnblockOnload(false); });
 
+  // If we store the previous nsFrameLoader in the bfcache, this will be filled
+  // with the SessionHistoryEntry which now owns the frame.
+  RefPtr<SessionHistoryEntry> bfcacheEntry;
+
   {
     // Introduce a script blocker to ensure no JS is executed during the
     // nsFrameLoader teardown & recreation process. Unload listeners will be run
@@ -133,16 +138,16 @@ void nsFrameLoaderOwner::ChangeRemotenessCommon(
 
       MOZ_ASSERT_IF(aOptions.mTryUseBFCache, aOptions.mReplaceBrowsingContext);
       if (aOptions.mTryUseBFCache && bc) {
-        SessionHistoryEntry* she =
-            bc->Canonical()->GetActiveSessionHistoryEntry();
-        bool useBFCache = she && she == aOptions.mActiveSessionHistoryEntry &&
-                          !she->GetFrameLoader();
+        bfcacheEntry = bc->Canonical()->GetActiveSessionHistoryEntry();
+        bool useBFCache = bfcacheEntry &&
+                          bfcacheEntry == aOptions.mActiveSessionHistoryEntry &&
+                          !bfcacheEntry->GetFrameLoader();
         if (useBFCache) {
           MOZ_LOG(gSHIPBFCacheLog, LogLevel::Debug,
                   ("nsFrameLoaderOwner::ChangeRemotenessCommon: store the old "
                    "page in bfcache"));
           Unused << bc->SetIsInBFCache(true);
-          she->SetFrameLoader(mFrameLoader);
+          bfcacheEntry->SetFrameLoader(mFrameLoader);
           // Session history owns now the frameloader.
           mFrameLoader = nullptr;
         }
@@ -177,14 +182,35 @@ void nsFrameLoaderOwner::ChangeRemotenessCommon(
     }
   }
 
-  ChangeFrameLoaderCommon(owner);
+  // Now that we have a new FrameLoader, we'll eventually need to reset
+  // nsSubDocumentFrame to use the new one. We can delay doing this if we're
+  // keeping our old frameloader around in the BFCache and the new frame hasn't
+  // presented yet to continue painting the previous document.
+  //
+  // We may not have a `BrowserParent` yet, so if `IsRemoteFrame` is true, and
+  // `browserParent` is null, we know it hasn't painted yet.
+  bool retainPaint = true;
+  auto* browserParent = BrowserParent::GetFrom(mFrameLoader);
+  if (!bfcacheEntry || !mFrameLoader->IsRemoteFrame()) {
+    MOZ_LOG(gSHIPBFCacheLog, LogLevel::Debug,
+            ("Previous frameLoader not entering BFCache - immediately "
+             "resetting nsSubDocumentFrame (bfcacheEntry=%p, isRemoteFrame=%d, "
+             "browserParent=%p)",
+             bfcacheEntry.get(), mFrameLoader->IsRemoteFrame(), browserParent));
+    retainPaint = false;
+  }
+
+  ChangeFrameLoaderCommon(owner, retainPaint);
 }
 
-void nsFrameLoaderOwner::ChangeFrameLoaderCommon(Element* aOwner) {
+void nsFrameLoaderOwner::ChangeFrameLoaderCommon(Element* aOwner,
+                                                 bool aRetainPaint) {
   // Now that we've got a new FrameLoader, we need to reset our
   // nsSubDocumentFrame to use the new FrameLoader.
   if (nsSubDocumentFrame* ourFrame = do_QueryFrame(aOwner->GetPrimaryFrame())) {
-    ourFrame->ResetFrameLoader(nsSubDocumentFrame::RetainPaintData::No);
+    auto retain = aRetainPaint ? nsSubDocumentFrame::RetainPaintData::Yes
+                               : nsSubDocumentFrame::RetainPaintData::No;
+    ourFrame->ResetFrameLoader(retain);
   }
 
   // If the element is focused, or the current mouse over target then
@@ -326,7 +352,7 @@ void nsFrameLoaderOwner::ReplaceFrameLoader(nsFrameLoader* aNewFrameLoader) {
   }
 
   RefPtr<Element> owner = do_QueryObject(this);
-  ChangeFrameLoaderCommon(owner);
+  ChangeFrameLoaderCommon(owner, /* aRetainPaint = */ false);
 }
 
 void nsFrameLoaderOwner::AttachFrameLoader(nsFrameLoader* aFrameLoader) {

dom/base/nsFrameLoaderOwner.h

@@ -109,7 +109,8 @@ class nsFrameLoaderOwner : public nsISupports {
       mozilla::dom::BrowsingContextGroup* aGroup,
       std::function<void()>& aFrameLoaderInit, mozilla::ErrorResult& aRv);
 
-  void ChangeFrameLoaderCommon(mozilla::dom::Element* aOwner);
+  void ChangeFrameLoaderCommon(mozilla::dom::Element* aOwner,
+                               bool aRetainPaint);
 
  protected:
   virtual ~nsFrameLoaderOwner() = default;

dom/ipc/BrowserParent.cpp

@@ -132,6 +132,7 @@
 #include "nsISecureBrowserUI.h"
 #include "nsIXULRuntime.h"
 #include "VsyncSource.h"
+#include "nsSubDocumentFrame.h"
 
 #ifdef XP_WIN
 #  include "FxRWindowManager.h"
@@ -162,6 +163,8 @@ using namespace mozilla::gfx;
 using mozilla::LazyLogModule;
 using mozilla::Unused;
 
+extern mozilla::LazyLogModule gSHIPBFCacheLog;
+
 LazyLogModule gBrowserFocusLog("BrowserFocus");
 
 #define LOGBROWSERFOCUS(args) \
@@ -646,6 +649,18 @@ void BrowserParent::Destroy() {
   mMarkedDestroying = true;
 }
 
+mozilla::ipc::IPCResult BrowserParent::RecvDidUnsuppressPainting() {
+  if (!mFrameElement) {
+    return IPC_OK();
+  }
+  nsSubDocumentFrame* subdocFrame =
+      do_QueryFrame(mFrameElement->GetPrimaryFrame());
+  if (subdocFrame && subdocFrame->HasRetainedPaintData()) {
+    subdocFrame->ClearRetainedPaintData();
+  }
+  return IPC_OK();
+}
+
 mozilla::ipc::IPCResult BrowserParent::RecvEnsureLayersConnected(
     CompositorOptions* aCompositorOptions) {
   if (mRemoteLayerTreeOwner.IsInitialized()) {
@@ -663,10 +678,19 @@ void BrowserParent::ActorDestroy(ActorDestroyReason why) {
   ContentProcessManager::GetSingleton()->UnregisterRemoteFrame(mTabId);
 
   if (mRemoteLayerTreeOwner.IsInitialized()) {
+    auto layersId = mRemoteLayerTreeOwner.GetLayersId();
+    if (mFrameElement) {
+      nsSubDocumentFrame* f = do_QueryFrame(mFrameElement->GetPrimaryFrame());
+      if (f && f->HasRetainedPaintData() &&
+          f->GetRemotePaintData().mLayersId == layersId) {
+        f->ClearRetainedPaintData();
+      }
+    }
+
     // It's important to unmap layers after the remote browser has been
     // destroyed, otherwise it may still send messages to the compositor which
     // will reject them, causing assertions.
-    RemoveBrowserParentFromTable(mRemoteLayerTreeOwner.GetLayersId());
+    RemoveBrowserParentFromTable(layersId);
     mRemoteLayerTreeOwner.Destroy();
   }
 
@@ -3581,15 +3605,15 @@ void BrowserParent::LayerTreeUpdate(const LayersObserverEpoch& aEpoch,
     return;
   }
 
-  RefPtr<EventTarget> target = mFrameElement;
-  if (!target) {
+  RefPtr<Element> frameElement = mFrameElement;
+  if (!frameElement) {
     NS_WARNING("Could not locate target for layer tree message.");
     return;
   }
 
   mHasLayers = aActive;
 
-  RefPtr<Event> event = NS_NewDOMEvent(mFrameElement, nullptr, nullptr);
+  RefPtr<Event> event = NS_NewDOMEvent(frameElement, nullptr, nullptr);
   if (aActive) {
     mHasPresented = true;
     event->InitEvent(u"MozLayerTreeReady"_ns, true, false);
@@ -3598,7 +3622,7 @@ void BrowserParent::LayerTreeUpdate(const LayersObserverEpoch& aEpoch,
   }
   event->SetTrusted(true);
   event->WidgetEventPtr()->mFlags.mOnlyChromeDispatch = true;
-  mFrameElement->DispatchEvent(*event);
+  frameElement->DispatchEvent(*event);
 }
 
 mozilla::ipc::IPCResult BrowserParent::RecvPaintWhileInterruptingJSNoOp(

dom/ipc/BrowserParent.h

@@ -255,6 +255,7 @@ class BrowserParent final : public PBrowserParent,
 
   void AddWindowListeners();
 
+  mozilla::ipc::IPCResult RecvDidUnsuppressPainting();
   mozilla::ipc::IPCResult RecvMoveFocus(const bool& aForward,
                                         const bool& aForDocumentNavigation);
 

dom/ipc/PBrowser.ipdl

@@ -223,7 +223,8 @@ parent:
      */
     sync DispatchFocusToTopLevelWindow();
 
-parent:
+    async DidUnsuppressPainting();
+
     /**
      * When child sends this message, parent should move focus to
      * the next or previous focusable element or document.

layout/base/PresShell.cpp

@@ -3884,12 +3884,17 @@ void PresShell::UnsuppressAndInvalidate() {
   ScheduleBeforeFirstPaint();
 
   mPaintingSuppressed = false;
-  nsIFrame* rootFrame = mFrameConstructor->GetRootFrame();
-  if (rootFrame) {
+  if (nsIFrame* rootFrame = mFrameConstructor->GetRootFrame()) {
     // let's assume that outline on a root frame is not supported
     rootFrame->InvalidateFrame();
   }
 
+  if (mPresContext->IsRootContentDocumentCrossProcess()) {
+    if (auto* bc = BrowserChild::GetFrom(mDocument->GetDocShell())) {
+      bc->SendDidUnsuppressPainting();
+    }
+  }
+
   // now that painting is unsuppressed, focus may be set on the document
   if (nsPIDOMWindowOuter* win = mDocument->GetWindow()) {
     win->SetReadyForFocus();

layout/generic/nsSubDocumentFrame.h

@@ -147,6 +147,7 @@ class nsSubDocumentFrame final : public nsAtomicContainerFrame,
   };
 
   RemoteFramePaintData GetRemotePaintData() const;
+  bool HasRetainedPaintData() const { return mRetainedRemoteFrame.isSome(); }
 
  protected:
   friend class AsyncFrameInit;