Bug 1717983 - Improve PresShell active flag handling. r=nika

This moves the logic of whether a pres shell should be active to a
single place to make it sane to reason about, and fixes the
subdocument propagation when a BrowserChild becomes visible.

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

docshell/base/nsDocShell.cpp

@@ -4721,9 +4721,9 @@ nsDocShell::GetVisibility(bool* aVisibility) {
 }
 
 void nsDocShell::ActivenessMaybeChanged() {
-  bool isActive = mBrowsingContext->IsActive();
+  const bool isActive = mBrowsingContext->IsActive();
   if (RefPtr<PresShell> presShell = GetPresShell()) {
-    presShell->SetIsActive(isActive);
+    presShell->ActivenessMaybeChanged();
   }
 
   // Tell the window about it
@@ -7990,7 +7990,6 @@ nsresult nsDocShell::SetupNewViewer(nsIContentViewer* aNewViewer,
   }
 
   nscolor bgcolor = NS_RGBA(0, 0, 0, 0);
-  bool isActive = false;
   // Ensure that the content viewer is destroyed *after* the GC - bug 71515
   nsCOMPtr<nsIContentViewer> contentViewer = mContentViewer;
   if (contentViewer) {
@@ -8002,7 +8001,6 @@ nsresult nsDocShell::SetupNewViewer(nsIContentViewer* aNewViewer,
     // presentation shell, so we can use it for the next document.
     if (PresShell* presShell = contentViewer->GetPresShell()) {
       bgcolor = presShell->GetCanvasBackground();
-      isActive = presShell->IsActive();
     }
 
     contentViewer->Close(mSavingOldViewer ? mOSHE.get() : nullptr);
@@ -8047,9 +8045,7 @@ nsresult nsDocShell::SetupNewViewer(nsIContentViewer* aNewViewer,
   // pres shell. This improves page load continuity.
   if (RefPtr<PresShell> presShell = mContentViewer->GetPresShell()) {
     presShell->SetCanvasBackground(bgcolor);
-    if (isActive) {
+    presShell->ActivenessMaybeChanged();
-      presShell->SetIsActive(isActive);
-    }
   }
 
   // XXX: It looks like the LayoutState gets restored again in Embed()

dom/ipc/BrowserChild.cpp

@@ -2960,23 +2960,20 @@ void BrowserChild::MakeVisible() {
     mPuppetWidget->Show(true);
   }
 
-  nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation());
+  if (nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation())) {
-  if (!docShell) {
+    // The browser / tab-switcher is responsible of fixing the browsingContext
-    return;
+    // state up explicitly via SetDocShellIsActive, which propagates to children
-  }
+    // automatically.
-
+    //
-  // The browser / tab-switcher is responsible of fixing the browsingContext
+    // We need it not to be observable, as this used via RecvRenderLayers and co.,
-  // state up explicitly via SetDocShellIsActive, which propagates to children
+    // for stuff like async tab warming.
-  // automatically.
+    //
-  //
+    // We don't want to go through the docshell because we don't want to change
-  // We need it not to be observable, as this used via RecvRenderLayers and co.,
+    // the visibility state of the document, which has side effects like firing
-  // for stuff like async tab warming.
+    // events to content, unblocking media playback, unthrottling timeouts...
-  //
+    if (RefPtr<PresShell> presShell = docShell->GetPresShell()) {
-  // We don't want to go through the docshell because we don't want to change
+      presShell->ActivenessMaybeChanged();
-  // the visibility state of the document, which has side effects like firing
+    }
-  // events to content, unblocking media playback, unthrottling timeouts...
-  if (RefPtr<PresShell> presShell = docShell->GetPresShell()) {
-    presShell->SetIsActive(true);
   }
 }
 
@@ -2990,24 +2987,22 @@ void BrowserChild::MakeHidden() {
   // setting up a layer manager. We should skip clearing cached layers
   // in that case, since doing so might accidentally put is into
   // BasicLayers mode.
-  if (mPuppetWidget && mPuppetWidget->HasLayerManager()) {
+  if (mPuppetWidget) {
-    ClearCachedResources();
+    if (mPuppetWidget->HasLayerManager()) {
+      ClearCachedResources();
+    }
+    mPuppetWidget->Show(false);
   }
 
   if (nsCOMPtr<nsIDocShell> docShell = do_GetInterface(WebNavigation())) {
-    // We don't use
+    // We don't use BrowserChildBase::GetPresShell() here because that would
-    // BrowserChildBase::GetPresShell() here because that would create a content
+    // create a content viewer if one doesn't exist yet. Creating a content
-    // viewer if one doesn't exist yet. Creating a content viewer can cause JS
+    // viewer can cause JS to run, which we want to avoid.
-    // to run, which we want to avoid. nsIDocShell::GetPresShell returns null if
+    // nsIDocShell::GetPresShell returns null if no content viewer exists yet.
-    // no content viewer exists yet.
     if (RefPtr<PresShell> presShell = docShell->GetPresShell()) {
-      presShell->SetIsActive(false);
+      presShell->ActivenessMaybeChanged();
     }
   }
-
-  if (mPuppetWidget) {
-    mPuppetWidget->Show(false);
-  }
 }
 
 NS_IMETHODIMP

layout/base/PresShell.cpp

@@ -1032,7 +1032,7 @@ void PresShell::Init(nsPresContext* aPresContext, nsViewManager* aViewManager) {
   }
 
   // Get our activeness from the docShell.
-  QueryIsActive();
+  ActivenessMaybeChanged();
 
   // Setup our font inflation preferences.
   mFontSizeInflationEmPerLine = StaticPrefs::font_size_inflation_emPerLine();
@@ -9309,7 +9309,7 @@ void PresShell::Thaw(bool aIncludeSubDocuments) {
 
   // Get the activeness of our presshell, as this might have changed
   // while we were in the bfcache
-  QueryIsActive();
+  ActivenessMaybeChanged();
 
   // We're now unfrozen
   mFrozen = false;
@@ -10763,12 +10763,15 @@ nsAccessibilityService* PresShell::GetAccessibilityService() {
 
 #endif  // #ifdef ACCESSIBILITY
 
-// Asks our docshell whether we're active.
+void PresShell::ActivenessMaybeChanged() {
-void PresShell::QueryIsActive() {
+  if (!mDocument) {
-  Document* doc = mDocument;
-  if (!doc) {
     return;
   }
+  SetIsActive(ShouldBeActive());
+}
+
+bool PresShell::ShouldBeActive() const {
+  Document* doc = mDocument;
   if (Document* displayDoc = doc->GetDisplayDocument()) {
     // Ok, we're an external resource document -- we need to use our display
     // document's docshell to determine "IsActive" status, since we lack
@@ -10778,25 +10781,33 @@ void PresShell::QueryIsActive() {
     doc = displayDoc;
   }
 
-  if (BrowsingContext* bc = doc->GetBrowsingContext()) {
+  Document* root = nsContentUtils::GetInProcessSubtreeRootDocument(doc);
-    // Even though in theory the docshell here could be "Inactive and
+  if (auto* browserChild = BrowserChild::GetFrom(root->GetDocShell())) {
-    // Foreground", thus implying aIsHidden=false for SetIsActive(), this is a
+    // We might want to activate a tab even though the browsing-context is not
-    // newly created PresShell so we'd like to invalidate anyway upon being made
+    // active if the BrowserChild is considered visible. This serves two
-    // active to ensure that the contents get painted.
+    // purposes:
-    auto* browserChild = BrowserChild::GetFrom(doc->GetDocShell());
+    //
-    const bool hiddenInRemoteFrame = browserChild &&
+    //  * For top-level tabs, we use this for tab warming. The browsing-context
-                                     !browserChild->IsTopLevel() &&
+    //    might still be inactive, but we want to activate the pres shell and
-                                     !browserChild->IsVisible();
+    //    the refresh driver.
-    SetIsActive(bc->IsActive() && !hiddenInRemoteFrame);
+    //
+    //  * For oop iframes, we do want to throttle them if they're not visible.
+    //
+    // TODO(emilio): Consider unifying the in-process vs. fission iframe
+    // throttling code (in-process throttling for non-visible iframes lives
+    // right now in Document::ShouldThrottleFrameRequests(), but that only
+    // throttles rAF).
+    return browserChild->IsVisible();
   }
+
+  BrowsingContext* bc = doc->GetBrowsingContext();
+  return bc && bc->IsActive();
 }
 
-nsresult PresShell::SetIsActive(bool aIsActive) {
+void PresShell::SetIsActive(bool aIsActive) {
   MOZ_ASSERT(mDocument, "should only be called with a document");
 
-#if defined(MOZ_WIDGET_ANDROID)
   const bool changed = mIsActive != aIsActive;
-#endif
 
   mIsActive = aIsActive;
 
@@ -10806,17 +10817,24 @@ nsresult PresShell::SetIsActive(bool aIsActive) {
     presContext->RefreshDriver()->SetThrottled(!mIsActive);
   }
 
-  {
+  if (changed) {
-    // Propagate state-change to my resource documents' PresShells
+    // Propagate state-change to my resource documents' PresShells and other
-    auto recurse = [aIsActive](Document& aResourceDoc) {
+    // subdocuments.
-      if (PresShell* presShell = aResourceDoc.GetPresShell()) {
+    //
+    // Note that it is fine to not propagate to fission iframes. Those will
+    // become active / inactive as needed as a result of they getting painted /
+    // not painted eventually.
+    auto recurse = [aIsActive](Document& aSubDoc) {
+      if (PresShell* presShell = aSubDoc.GetPresShell()) {
         presShell->SetIsActive(aIsActive);
       }
       return CallState::Continue;
     };
     mDocument->EnumerateExternalResources(recurse);
+    mDocument->EnumerateSubDocuments(recurse);
   }
-  nsresult rv = UpdateImageLockingState();
+
+  UpdateImageLockingState();
 #ifdef ACCESSIBILITY
   if (aIsActive) {
     if (nsAccessibilityService* accService =
@@ -10841,8 +10859,6 @@ nsresult PresShell::SetIsActive(bool aIsActive) {
       rootFrame->SchedulePaint();
     }
   }
-
-  return rv;
 }
 
 RefPtr<MobileViewportManager> PresShell::GetMobileViewportManager() const {
@@ -10952,11 +10968,11 @@ bool PresShell::UsesMobileViewportSizing() const {
  * Determines the current image locking state. Called when one of the
  * dependent factors changes.
  */
-nsresult PresShell::UpdateImageLockingState() {
+void PresShell::UpdateImageLockingState() {
   // We're locked if we're both thawed and active.
   bool locked = !mFrozen && mIsActive;
 
-  nsresult rv = mDocument->ImageTracker()->SetLockingState(locked);
+  mDocument->ImageTracker()->SetLockingState(locked);
 
   if (locked) {
     // Request decodes for visible image frames; we want to start decoding as
@@ -10967,8 +10983,6 @@ nsresult PresShell::UpdateImageLockingState() {
       }
     }
   }
-
-  return rv;
 }
 
 PresShell* PresShell::GetRootPresShell() const {

layout/base/PresShell.h

@@ -881,9 +881,8 @@ class PresShell final : public nsStubDocumentObserver,
     return mObservesMutationsForPrint;
   }
 
-  nsresult SetIsActive(bool aIsActive);
+  void ActivenessMaybeChanged();
-
+  bool IsActive() const { return mIsActive; }
-  bool IsActive() { return mIsActive; }
 
   /**
    * Keep track of how many times this presshell has been rendered to
@@ -1692,6 +1691,10 @@ class PresShell final : public nsStubDocumentObserver,
  private:
   ~PresShell();
 
+  void SetIsActive(bool aIsActive);
+  bool ShouldBeActive() const;
+
+
   /**
    * Refresh observer management.
    */
@@ -1936,8 +1939,7 @@ class PresShell final : public nsStubDocumentObserver,
   };
   MOZ_CAN_RUN_SCRIPT void ProcessSynthMouseMoveEvent(bool aFromScroll);
 
-  void QueryIsActive();
+  void UpdateImageLockingState();
-  nsresult UpdateImageLockingState();
 
   already_AddRefed<PresShell> GetParentPresShellForEventHandling();