Bug 1590247 - Factor out ScrollbarChange handling. r=dholbert
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 14 Oct 2021 21:19:13 +0000
changeset 595969 1492d117aca49c68e1c6f7791418c93519212137
parent 595968 d41beebcfe89dfac9ce39cdf1f4d6a2819b1e840
child 595970 cb21a032e4ad3b8db96f38cef47b13837fb455e4
push id38880
push usernerli@mozilla.com
push dateFri, 15 Oct 2021 09:50:04 +0000
treeherdermozilla-central@bb0faec6af52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1590247
milestone95.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 1590247 - Factor out ScrollbarChange handling. r=dholbert No behavior change. Differential Revision: https://phabricator.services.mozilla.com/D128370
layout/base/RestyleManager.cpp
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -1251,16 +1251,71 @@ static nsIContent* NextSiblingWhichMayHa
 
 // If |aFrame| is dirty or has dirty children, then we can skip updating
 // overflows since that will happen when it's reflowed.
 static inline bool CanSkipOverflowUpdates(const nsIFrame* aFrame) {
   return aFrame->HasAnyStateBits(NS_FRAME_IS_DIRTY |
                                  NS_FRAME_HAS_DIRTY_CHILDREN);
 }
 
+static inline void MaybeDealWithScrollbarChange(nsStyleChangeData& aData,
+                                                nsPresContext* aPc) {
+  if (!(aData.mHint & nsChangeHint_ScrollbarChange)) {
+    return;
+  }
+  aData.mHint &= ~nsChangeHint_ScrollbarChange;
+  bool doReconstruct = true;  // assume the worst
+
+  // Only bother with this if we're html/body, since:
+  //  (a) It'd be *expensive* to reframe these particular nodes.  They're
+  //      at the root, so reframing would mean rebuilding the world.
+  //  (b) It's often *unnecessary* to reframe for "overflow" changes on
+  //      these particular nodes.  In general, the only reason we reframe
+  //      for "overflow" changes is so we can construct (or destroy) a
+  //      scrollframe & scrollbars -- and the html/body nodes often don't
+  //      need their own scrollframe/scrollbars because they coopt the ones
+  //      on the viewport (which always exist). So depending on whether
+  //      that's happening, we can skip the reframe for these nodes.
+  if (aData.mContent->IsAnyOfHTMLElements(nsGkAtoms::body, nsGkAtoms::html)) {
+    // If the restyled element provided/provides the scrollbar styles for
+    // the viewport before and/or after this restyle, AND it's not coopting
+    // that responsibility from some other element (which would need
+    // reconstruction to make its own scrollframe now), THEN: we don't need
+    // to reconstruct - we can just reflow, because no scrollframe is being
+    // added/removed.
+    nsIContent* prevOverrideNode =
+        aPc->GetViewportScrollStylesOverrideElement();
+    nsIContent* newOverrideNode = aPc->UpdateViewportScrollStylesOverride();
+
+    if (aData.mContent == prevOverrideNode ||
+        aData.mContent == newOverrideNode) {
+      // If we get here, the restyled element provided the scrollbar styles
+      // for viewport before this restyle, OR it will provide them after.
+      if (!prevOverrideNode || !newOverrideNode ||
+          prevOverrideNode == newOverrideNode) {
+        // If we get here, the restyled element is NOT replacing (or being
+        // replaced by) some other element as the viewport's
+        // scrollbar-styles provider. (If it were, we'd potentially need to
+        // reframe to create a dedicated scrollframe for whichever element
+        // is being booted from providing viewport scrollbar styles.)
+        //
+        // Under these conditions, we're OK to assume that this "overflow"
+        // change only impacts the root viewport's scrollframe, which
+        // already exists, so we can simply reflow instead of reframing.
+        aData.mHint |= nsChangeHint_ReflowHintsForScrollbarChange;
+        doReconstruct = false;
+      }
+    }
+  }
+
+  if (doReconstruct) {
+    aData.mHint |= nsChangeHint_ReconstructFrame;
+  }
+}
+
 void RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList) {
   NS_ASSERTION(!nsContentUtils::IsSafeToRunScript(),
                "Someone forgot a script blocker");
 
   // See bug 1378219 comment 9:
   // Recursive calls here are a bit worrying, but apparently do happen in the
   // wild (although not currently in any of our automated tests). Try to get a
   // stack from Nightly/Dev channel to figure out what's going on and whether
@@ -1300,71 +1355,20 @@ void RestyleManager::ProcessRestyledFram
     mDestroyedFrames = MakeUnique<nsTHashSet<const nsIFrame*>>();
   }
 
   AUTO_PROFILER_LABEL("RestyleManager::ProcessRestyledFrames", LAYOUT);
 
   nsPresContext* presContext = PresContext();
   nsCSSFrameConstructor* frameConstructor = presContext->FrameConstructor();
 
-  // Handle nsChangeHint_ScrollbarChange, by either updating the
-  // scrollbars on the viewport, or upgrading the change hint to
-  // frame-reconstruct.
+  // Handle nsChangeHint_ScrollbarChange, by either updating the scrollbars on
+  // the viewport, or upgrading the change hint to frame-reconstruct.
   for (nsStyleChangeData& data : aChangeList) {
-    if (data.mHint & nsChangeHint_ScrollbarChange) {
-      data.mHint &= ~nsChangeHint_ScrollbarChange;
-      bool doReconstruct = true;  // assume the worst
-
-      // Only bother with this if we're html/body, since:
-      //  (a) It'd be *expensive* to reframe these particular nodes.  They're
-      //      at the root, so reframing would mean rebuilding the world.
-      //  (b) It's often *unnecessary* to reframe for "overflow" changes on
-      //      these particular nodes.  In general, the only reason we reframe
-      //      for "overflow" changes is so we can construct (or destroy) a
-      //      scrollframe & scrollbars -- and the html/body nodes often don't
-      //      need their own scrollframe/scrollbars because they coopt the ones
-      //      on the viewport (which always exist). So depending on whether
-      //      that's happening, we can skip the reframe for these nodes.
-      if (data.mContent->IsAnyOfHTMLElements(nsGkAtoms::body,
-                                             nsGkAtoms::html)) {
-        // If the restyled element provided/provides the scrollbar styles for
-        // the viewport before and/or after this restyle, AND it's not coopting
-        // that responsibility from some other element (which would need
-        // reconstruction to make its own scrollframe now), THEN: we don't need
-        // to reconstruct - we can just reflow, because no scrollframe is being
-        // added/removed.
-        nsIContent* prevOverrideNode =
-            presContext->GetViewportScrollStylesOverrideElement();
-        nsIContent* newOverrideNode =
-            presContext->UpdateViewportScrollStylesOverride();
-
-        if (data.mContent == prevOverrideNode ||
-            data.mContent == newOverrideNode) {
-          // If we get here, the restyled element provided the scrollbar styles
-          // for viewport before this restyle, OR it will provide them after.
-          if (!prevOverrideNode || !newOverrideNode ||
-              prevOverrideNode == newOverrideNode) {
-            // If we get here, the restyled element is NOT replacing (or being
-            // replaced by) some other element as the viewport's
-            // scrollbar-styles provider. (If it were, we'd potentially need to
-            // reframe to create a dedicated scrollframe for whichever element
-            // is being booted from providing viewport scrollbar styles.)
-            //
-            // Under these conditions, we're OK to assume that this "overflow"
-            // change only impacts the root viewport's scrollframe, which
-            // already exists, so we can simply reflow instead of reframing.
-            data.mHint |= nsChangeHint_ReflowHintsForScrollbarChange;
-            doReconstruct = false;
-          }
-        }
-      }
-      if (doReconstruct) {
-        data.mHint |= nsChangeHint_ReconstructFrame;
-      }
-    }
+    MaybeDealWithScrollbarChange(data, presContext);
   }
 
   bool didUpdateCursor = false;
 
   for (size_t i = 0; i < aChangeList.Length(); ++i) {
     // Collect and coalesce adjacent siblings for lazy frame construction.
     // Eventually it would be even better to make RecreateFramesForContent
     // accept a range and coalesce all adjacent reconstructs (bug 1344139).