moebius#138: Optimize operations on root of deeply-nested frame tree

https://github.com/MoonchildProductions/moebius/pull/138

dom/base/Element.cpp

@@ -1844,6 +1844,24 @@ Element::UnbindFromTree(bool aDeep, bool aNullParent)
     SetParentIsContent(false);
   }
 
+#ifdef DEBUG
+  // If we can get access to the PresContext, then we sanity-check that
+  // we're not leaving behind a pointer to ourselves as the PresContext's
+  // cached provider of the viewport's scrollbar styles.
+  if (document) {
+    nsIPresShell* presShell = document->GetShell();
+    if (presShell) {
+      nsPresContext* presContext = presShell->GetPresContext();
+      if (presContext) {
+        MOZ_ASSERT(this !=
+                   presContext->GetViewportScrollbarStylesOverrideNode(),
+                   "Leaving behind a raw pointer to this node (as having "
+                   "propagated scrollbar styles) - that's dangerous...");
+      }
+    }
+  }
+#endif
+
   // Ensure that CSS transitions don't continue on an element at a
   // different place in the tree (even if reinserted before next
   // animation refresh).

dom/base/nsDocument.cpp

@@ -2054,10 +2054,17 @@ nsDocument::ResetToURI(nsIURI *aURI, nsILoadGroup *aLoadGroup,
         mFirstChild = content->GetNextSibling();
       }
       mChildren.RemoveChildAt(i);
+      if (content == mCachedRootElement) {
+        // Immediately clear mCachedRootElement, now that it's been removed
+        // from mChildren, so that GetRootElement() will stop returning this
+        // now-stale value.
+        mCachedRootElement = nullptr;
+      }
       nsNodeUtils::ContentRemoved(this, content, i, previousSibling);
       content->UnbindFromTree();
     }
-    mCachedRootElement = nullptr;
+    MOZ_ASSERT(!mCachedRootElement,
+               "After removing all children, there should be no root elem");
   }
   mInUnlinkOrDeletion = oldVal;
 
@@ -3913,8 +3920,18 @@ nsDocument::RemoveChildAt(uint32_t aIndex, bool aNotify)
     DestroyElementMaps();
   }
 
-  doRemoveChildAt(aIndex, aNotify, oldKid, mChildren);
+  // Preemptively clear mCachedRootElement, since we may be about to remove it
+  // from our child list, and we don't want to return this maybe-obsolete value
+  // from any GetRootElement() calls that happen inside of doRemoveChildAt().
+  // (NOTE: for this to be useful, doRemoveChildAt() must NOT trigger any
+  // GetRootElement() calls until after it's removed the child from mChildren.
+  // Any call before that point would restore this soon-to-be-obsolete cached
+  // answer, and our clearing here would be fruitless.)
   mCachedRootElement = nullptr;
+  doRemoveChildAt(aIndex, aNotify, oldKid, mChildren);
+  MOZ_ASSERT(mCachedRootElement != oldKid,
+             "Stale pointer in mCachedRootElement, after we tried to clear it "
+             "(maybe somebody called GetRootElement() too early?)");
 }
 
 void

dom/base/nsINode.cpp

@@ -1907,6 +1907,10 @@ void
 nsINode::doRemoveChildAt(uint32_t aIndex, bool aNotify,
                          nsIContent* aKid, nsAttrAndChildArray& aChildArray)
 {
+  // NOTE: This function must not trigger any calls to
+  // nsIDocument::GetRootElement() calls until *after* it has removed aKid from
+  // aChildArray. Any calls before then could potentially restore a stale
+  // value for our cached root element, per note in nsDocument::RemoveChildAt().
   NS_PRECONDITION(aKid && aKid->GetParentNode() == this &&
                   aKid == GetChildAt(aIndex) &&
                   IndexOf(aKid) == (int32_t)aIndex, "Bogus aKid");

layout/base/RestyleManagerBase.cpp

@@ -154,7 +154,7 @@ RestyleManagerBase::ChangeHintToString(nsChangeHint aHint)
     "NeutralChange", "InvalidateRenderingObservers",
     "ReflowChangesSizeOrPosition", "UpdateComputedBSize",
     "UpdateUsesOpacity", "UpdateBackgroundPosition",
-    "AddOrRemoveTransform"
+    "AddOrRemoveTransform", "CSSOverflowChange",
   };
   static_assert(nsChangeHint_AllHints == (1 << ArrayLength(names)) - 1,
                 "Name list doesn't match change hints.");
@@ -1070,6 +1070,67 @@ RestyleManagerBase::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
   FramePropertyTable* propTable = presContext->PropertyTable();
   nsCSSFrameConstructor* frameConstructor = presContext->FrameConstructor();
 
+  // Handle nsChangeHint_CSSOverflowChange, by either updating the
+  // scrollbars on the viewport, or upgrading the change hint to frame-reconstruct.
+  for (nsStyleChangeData& data : aChangeList) {
+    if (data.mHint & nsChangeHint_CSSOverflowChange) {
+      data.mHint &= ~nsChangeHint_CSSOverflowChange;
+      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->GetViewportScrollbarStylesOverrideNode();
+        nsIContent* newOverrideNode =
+          presContext->UpdateViewportScrollbarStylesOverride();
+
+        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.
+            // When requesting this reflow, we send the exact same change hints
+            // that "width" and "height" would send (since conceptually,
+            // adding/removing scrollbars is like changing the available
+            // space).
+            data.mHint |= (nsChangeHint_ReflowHintsForISizeChange |
+                           nsChangeHint_ReflowHintsForBSizeChange);
+            doReconstruct = false;
+          }
+        }
+      }
+      if (doReconstruct) {
+        data.mHint |= nsChangeHint_ReconstructFrame;
+      }
+    }
+  }
+
   // Make sure to not rebuild quote or counter lists while we're
   // processing restyles
   frameConstructor->BeginUpdate();

layout/base/nsCSSFrameConstructor.cpp

@@ -8246,11 +8246,19 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent*  aContainer,
     *aDestroyedFramesFor = aChild;
   }
 
+  nsPresContext* presContext = mPresShell->GetPresContext();
+  MOZ_ASSERT(presContext, "Our presShell should have a valid presContext");
+
   if (aChild->IsHTMLElement(nsGkAtoms::body) ||
       (!aContainer && aChild->IsElement())) {
-    // This might be the element we propagated viewport scrollbar
-    // styles from.  Recompute those.
-    mPresShell->GetPresContext()->UpdateViewportScrollbarStylesOverride();
+    // We might be removing the element that we propagated viewport scrollbar
+    // styles from.  Recompute those. (This clause covers two of the three
+    // possible scrollbar-propagation sources: the <body> [as aChild or a
+    // descendant] and the root node. The other possible scrollbar-propagation
+    // source is a fullscreen element, and we have code elsewhere to update
+    // scrollbars after fullscreen elements are removed -- specifically, it's
+    // part of the fullscreen cleanup code called by Element::UnbindFromTree.)
+    presContext->UpdateViewportScrollbarStylesOverride();
   }
 
   // XXXldb Do we need to re-resolve style to handle the CSS2 + combinator and
@@ -8316,7 +8324,6 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent*  aContainer,
     ClearDisplayContentsIn(aChild, aContainer);
   }
 
-  nsPresContext* presContext = mPresShell->GetPresContext();
 #ifdef MOZ_XUL
   if (NotifyListBoxBody(presContext, aContainer, aChild, aOldNextSibling,
                         childFrame, CONTENT_REMOVED)) {

layout/base/nsChangeHint.h

@@ -217,6 +217,16 @@ enum nsChangeHint {
    */
   nsChangeHint_AddOrRemoveTransform = 1 << 27,
 
+  /**
+   * Indicates that the overflow-x and/or overflow-y property changed.
+   *
+   * In most cases, this is equivalent to nsChangeHint_ReconstructFrame. But
+   * in some special cases where the change is really targeting the viewport's
+   * scrollframe, this is instead equivalent to nsChangeHint_AllReflowHints
+   * (because the viewport always has an associated scrollframe).
+   */
+  nsChangeHint_CSSOverflowChange = 1 << 28,
+
   // IMPORTANT NOTE: When adding new hints, consider whether you need
   // to add them to NS_HintsNotHandledForDescendantsIn() below. Please
   // also add them to RestyleManager::ChangeHintToString and modify
@@ -225,7 +235,7 @@ enum nsChangeHint {
   /**
    * Dummy hint value for all hints. It exists for compile time check.
    */
-  nsChangeHint_AllHints = (1 << 28) - 1,
+  nsChangeHint_AllHints = (1 << 29) - 1,
 };
 
 // Redefine these operators to return nothing. This will catch any use
@@ -306,6 +316,7 @@ inline nsChangeHint operator^=(nsChangeHint& aLeft, nsChangeHint aRight)
           nsChangeHint_UpdatePostTransformOverflow | \
           nsChangeHint_UpdateParentOverflow | \
           nsChangeHint_ChildrenOnlyTransform | \
+          nsChangeHint_CSSOverflowChange | \
           nsChangeHint_RecomputePosition | \
           nsChangeHint_UpdateContainingBlock | \
           nsChangeHint_AddOrRemoveTransform | \
@@ -374,6 +385,48 @@ inline nsChangeHint NS_HintsNotHandledForDescendantsIn(nsChangeHint aChangeHint)
                nsChangeHint_ClearAncestorIntrinsics |   \
                nsChangeHint_ClearDescendantIntrinsics | \
                nsChangeHint_NeedDirtyReflow)
+
+// Below are the change hints that we send for ISize & BSize changes.
+// Each is similar to nsChangeHint_AllReflowHints with a few changes.
+
+// * For an ISize change, we send nsChangeHint_AllReflowHints, with two bits
+// excluded: nsChangeHint_ClearDescendantIntrinsics (because an ancestor's
+// inline-size change can't affect descendant intrinsic sizes), and
+// nsChangeHint_NeedDirtyReflow (because ISize changes don't need to *force*
+// all descendants to reflow).
+#define nsChangeHint_ReflowHintsForISizeChange            \
+  nsChangeHint(nsChangeHint_AllReflowHints &              \
+               ~(nsChangeHint_ClearDescendantIntrinsics | \
+                 nsChangeHint_NeedDirtyReflow))
+
+// * For a BSize change, we send almost the same hints as for ISize changes,
+// with one extra: nsChangeHint_UpdateComputedBSize.  We need this hint because
+// BSize changes CAN affect descendant intrinsic sizes, due to replaced
+// elements with percentage BSizes in descendants which also have percentage
+// BSizes. nsChangeHint_UpdateComputedBSize clears intrinsic sizes for frames
+// that have such replaced elements. (We could instead send
+// nsChangeHint_ClearDescendantIntrinsics, but that's broader than we need.)
+//
+// NOTE: You might think that BSize changes could exclude
+// nsChangeHint_ClearAncestorIntrinsics (which is inline-axis specific), but we
+// do need to send it, to clear cached results from CSS Flex measuring reflows.
+#define nsChangeHint_ReflowHintsForBSizeChange            \
+  nsChangeHint((nsChangeHint_AllReflowHints |             \
+                nsChangeHint_UpdateComputedBSize) &       \
+               ~(nsChangeHint_ClearDescendantIntrinsics | \
+                 nsChangeHint_NeedDirtyReflow))
+
+// * For changes to the float area of an already-floated element, we need all
+// reflow hints, but not the ones that apply to descendants.
+// Our descendants aren't impacted when our float area only changes
+// placement but not size/shape. (e.g. if we change which side we float to).
+// But our ancestors/siblings are potentially impacted, so we need to send
+// the non-descendant reflow hints.
+#define nsChangeHint_ReflowHintsForFloatAreaChange            \
+  nsChangeHint(nsChangeHint_AllReflowHints &              \
+               ~(nsChangeHint_ClearDescendantIntrinsics | \
+                 nsChangeHint_NeedDirtyReflow))
+
 #define NS_STYLE_HINT_REFLOW \
   nsChangeHint(NS_STYLE_HINT_VISUAL | nsChangeHint_AllReflowHints)
 

layout/base/nsPresContext.cpp

@@ -208,6 +208,7 @@ nsPresContext::nsPresContext(nsIDocument* aDocument, nsPresContextType aType)
     mTextZoom(1.0), mFullZoom(1.0), mOverrideDPPX(0.0),
     mLastFontInflationScreenSize(gfxSize(-1.0, -1.0)),
     mPageSize(-1, -1), mPPScale(1.0f),
+    mViewportScrollbarOverrideNode(nullptr),
     mViewportStyleScrollbar(NS_STYLE_OVERFLOW_AUTO, NS_STYLE_OVERFLOW_AUTO),
     mImageAnimationModePref(imgIContainer::kNormalAnimMode),
     mAllInvalidated(false),
@@ -1423,10 +1424,10 @@ nsPresContext::UpdateViewportScrollbarStylesOverride()
   // Start off with our default styles, and then update them as needed.
   mViewportStyleScrollbar = ScrollbarStyles(NS_STYLE_OVERFLOW_AUTO,
                                             NS_STYLE_OVERFLOW_AUTO);
-  nsIContent* propagatedFrom = nullptr;
+  mViewportScrollbarOverrideNode = nullptr;
   // Don't propagate the scrollbar state in printing or print preview.
   if (!IsPaginated()) {
-    propagatedFrom =
+    mViewportScrollbarOverrideNode =
       GetPropagatedScrollbarStylesForViewport(this, &mViewportStyleScrollbar);
   }
 
@@ -1438,13 +1439,13 @@ nsPresContext::UpdateViewportScrollbarStylesOverride()
     // the styles are from, so that the state of those elements is not
     // affected across fullscreen change.
     if (fullscreenElement != document->GetRootElement() &&
-        fullscreenElement != propagatedFrom) {
+        fullscreenElement != mViewportScrollbarOverrideNode) {
       mViewportStyleScrollbar = ScrollbarStyles(NS_STYLE_OVERFLOW_HIDDEN,
                                                 NS_STYLE_OVERFLOW_HIDDEN);
     }
   }
 
-  return propagatedFrom;
+  return mViewportScrollbarOverrideNode;
 }
 
 bool

layout/base/nsPresContext.h

@@ -719,7 +719,18 @@ public:
    *         it was propagated from.
    */
   nsIContent* UpdateViewportScrollbarStylesOverride();
-  const ScrollbarStyles& GetViewportScrollbarStylesOverride()
+
+  /**
+   * Returns the cached result from the last call to
+   * UpdateViewportScrollbarStylesOverride() -- i.e. return the node
+   * whose scrollbar styles we have propagated to the viewport (or nullptr if
+   * there is no such node).
+   */
+  nsIContent* GetViewportScrollbarStylesOverrideNode() const {
+    return mViewportScrollbarOverrideNode;
+  }
+
+  const ScrollbarStyles& GetViewportScrollbarStylesOverride() const
   {
     return mViewportStyleScrollbar;
   }
@@ -1310,7 +1321,16 @@ protected:
 
   nscolor               mBodyTextColor;
 
+  // This is a non-owning pointer. May be null. If non-null, it's guaranteed
+  // to be pointing to a node that's still alive, because we'll reset it in
+  // UpdateViewportScrollbarStylesOverride() as part of the cleanup code
+  // when this node is removed from the document. (For <body> and the root node,
+  // this call happens in nsCSSFrameConstructor::ContentRemoved(). For
+  // fullscreen elements, it happens in the fullscreen-specific cleanup
+  // invoked by Element::UnbindFromTree().)
+  nsIContent* MOZ_NON_OWNING_REF mViewportScrollbarOverrideNode;
   ScrollbarStyles       mViewportStyleScrollbar;
+
   uint8_t               mFocusRingWidth;
 
   bool mExistThrottledUpdates;

layout/reftests/scrolling/propagated-overflow-style-1-ref.html

@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>
+    Reference case with body and html *independently* scrollable.
+  </title>
+  <style>
+    html {
+      overflow: scroll;
+    }
+    body {
+      overflow: scroll;
+    }
+  </style>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-1a.html

@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>
+    Testcase with body and html *independently* scrollable,
+    with body's "overflow" set dynamically.
+  </title>
+  <style>
+    html {
+      overflow: scroll;
+    }
+  </style>
+  <script>
+    function doTest() {
+      document.body.style.overflow = "scroll";
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", doTest);
+  </script>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-1b.html

@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>
+    Testcase with body and html *independently* scrollable,
+    with html's "overflow" set dynamically.
+  </title>
+  <style>
+    body {
+      overflow: scroll;
+    }
+  </style>
+  <script>
+    function doTest() {
+      document.documentElement.style.overflow = "scroll";
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", doTest);
+  </script>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-1c.html

@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>
+    Testcase with body and html *independently* scrollable,
+    with both html & body's "overflow" set dynamically.
+  </title>
+  <script>
+    function doTest() {
+      document.documentElement.style.overflow = "scroll";
+      document.body.style.overflow = "scroll";
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", doTest);
+  </script>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-2-ref.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>
+    Reference case with the root viewport scrollable, via styles on html node.
+  </title>
+  <style>
+    html {
+      overflow: scroll;
+    }
+  </style>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-2a.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>
+    Testcase with only one of [html,body] being scrollable,
+    after body's "overflow" is reset dynamically.
+  </title>
+  <style>
+    html {
+      overflow: scroll;
+    }
+    body {
+      overflow: scroll;
+    }
+  </style>
+  <script>
+    function doTest() {
+      document.body.style.overflow = "visible";
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", doTest);
+  </script>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-2b.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>
+    Testcase with only one of [html,body] being scrollable,
+    after html's "overflow" is reset dynamically.
+  </title>
+  <style>
+    html {
+      overflow: scroll;
+    }
+    body {
+      overflow: scroll;
+    }
+  </style>
+  <script>
+    function doTest() {
+      document.documentElement.style.overflow = "visible";
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", doTest);
+  </script>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-2c.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>
+    Testcase with only one of [html,body] being scrollable,
+    with their "overflow" styles being dynamically swapped.
+  </title>
+  <style>
+    html {
+      overflow: scroll;
+    }
+  </style>
+  <script>
+    function doTest() {
+      document.documentElement.style.overflow = "visible";
+      document.body.style.overflow = "scroll";
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", doTest);
+  </script>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-2d.html

@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+  <title>
+    Testcase with only one of [html,body] being scrollable,
+    with their "overflow" styles being dynamically swapped.
+  </title>
+  <style>
+    body {
+      overflow: scroll;
+    }
+  </style>
+  <script>
+    function doTest() {
+      document.documentElement.style.overflow = "scroll";
+      document.body.style.overflow = "visible";
+      document.documentElement.removeAttribute("class");
+    }
+    window.addEventListener("MozReftestInvalidate", doTest);
+  </script>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/propagated-overflow-style-2e.html

@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>
+    Testcase with the root viewport scrollable, via styles on body node.
+  </title>
+  <style>
+    body {
+      overflow: scroll;
+    }
+  </style>
+</head>
+<body>
+</body>
+</html>

layout/reftests/scrolling/reftest.list

@@ -85,3 +85,13 @@ fuzzy-if(asyncPan&&!layersGPUAccelerated,102,2420) == frame-scrolling-attr-2.htm
 == fractional-scroll-area.html?top=0.4&outerBottom=99.6&innerBottom=200.4&scrollBefore=999 fractional-scroll-area.html?top=0&outerBottom=100&innerBottom=200&scrollBefore=999
 == fractional-scroll-area.html?top=0.4&outerBottom=100.4&innerBottom=200.4&scrollBefore=999 fractional-scroll-area.html?top=0&outerBottom=100&innerBottom=200&scrollBefore=999
 != fractional-scroll-area-invalidation.html about:blank
+
+# Tests for "overflow" styles that may be propagated to the viewport:
+== propagated-overflow-style-1a.html propagated-overflow-style-1-ref.html
+== propagated-overflow-style-1b.html propagated-overflow-style-1-ref.html
+== propagated-overflow-style-1c.html propagated-overflow-style-1-ref.html
+== propagated-overflow-style-2a.html propagated-overflow-style-2-ref.html
+== propagated-overflow-style-2b.html propagated-overflow-style-2-ref.html
+== propagated-overflow-style-2c.html propagated-overflow-style-2-ref.html
+== propagated-overflow-style-2d.html propagated-overflow-style-2-ref.html
+== propagated-overflow-style-2e.html propagated-overflow-style-2-ref.html

layout/style/nsStyleStruct.cpp

@@ -1629,28 +1629,11 @@ nsStylePosition::CalcDifference(const nsStylePosition& aNewData,
   if (aOldStyleVisibility) {
     bool isVertical = WritingMode(aOldStyleVisibility).IsVertical();
     if (isVertical ? widthChanged : heightChanged) {
-      // Block-size changes can affect descendant intrinsic sizes due to
-      // replaced elements with percentage bsizes in descendants which
-      // also have percentage bsizes. This is handled via
-      // nsChangeHint_UpdateComputedBSize which clears intrinsic sizes
-      // for frames that have such replaced elements.
-      //
-      // We need to use nsChangeHint_ClearAncestorIntrinsics for
-      // block-size changes so we clear results of cached CSS Flex
-      // measuring reflows.
-      hint |= nsChangeHint_NeedReflow |
-              nsChangeHint_UpdateComputedBSize |
-              nsChangeHint_ReflowChangesSizeOrPosition |
-              nsChangeHint_ClearAncestorIntrinsics;
+      hint |= nsChangeHint_ReflowHintsForBSizeChange;
     }
 
     if (isVertical ? heightChanged : widthChanged) {
-      // None of our inline-size differences can affect descendant
-      // intrinsic sizes and none of them need to force children to
-      // reflow.
-      hint |= nsChangeHint_AllReflowHints &
-              ~(nsChangeHint_ClearDescendantIntrinsics |
-                nsChangeHint_NeedDirtyReflow);
+      hint |= nsChangeHint_ReflowHintsForISizeChange;
     }
   } else {
     if (widthChanged || heightChanged) {
@@ -3263,8 +3246,6 @@ nsStyleDisplay::CalcDifference(const nsStyleDisplay& aNewData) const
       || mDisplay != aNewData.mDisplay
       || mContain != aNewData.mContain
       || (mFloat == StyleFloat::None) != (aNewData.mFloat == StyleFloat::None)
-      || mOverflowX != aNewData.mOverflowX
-      || mOverflowY != aNewData.mOverflowY
       || mScrollBehavior != aNewData.mScrollBehavior
       || mScrollSnapTypeX != aNewData.mScrollSnapTypeX
       || mScrollSnapTypeY != aNewData.mScrollSnapTypeY
@@ -3276,6 +3257,11 @@ nsStyleDisplay::CalcDifference(const nsStyleDisplay& aNewData) const
     hint |= nsChangeHint_ReconstructFrame;
   }
 
+  if (mOverflowX != aNewData.mOverflowX
+      || mOverflowY != aNewData.mOverflowY) {
+    hint |= nsChangeHint_CSSOverflowChange;
+  }
+
   /* Note: When mScrollBehavior, mScrollSnapTypeX, mScrollSnapTypeY,
    * mScrollSnapPointsX, mScrollSnapPointsY, or mScrollSnapDestination are
    * changed, nsChangeHint_NeutralChange is not sufficient to enter

layout/style/test/mochitest.ini

@@ -295,6 +295,7 @@ skip-if = toolkit == 'android'
 [test_variables.html]
 support-files = support/external-variable-url.css
 [test_video_object_fit.html]
+[test_viewport_scrollbar_causing_reflow.html]
 [test_viewport_units.html]
 [test_visited_image_loading.html]
 skip-if = toolkit == 'android' #TIMED_OUT

layout/style/test/test_dynamic_change_causing_reflow.html

@@ -95,6 +95,90 @@ const gTestcases = [
     expectReflow: true,
   },
 
+  // * Changing 'overflow' on <body> should cause reflow,
+  // but not frame reconstruction
+  {
+    elem: document.body,
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: hidden",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: scroll",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: hidden",
+    afterStyle:  "overflow: auto",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: hidden",
+    afterStyle:  "overflow: scroll",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: hidden",
+    afterStyle:  "overflow: visible",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: auto",
+    afterStyle:  "overflow: hidden",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.body,
+    beforeStyle: "overflow: visible",
+    afterStyle:  "overflow: hidden",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+
+  // * Changing 'overflow' on <html> should cause reflow,
+  // but not frame reconstruction
+  {
+    elem: document.documentElement,
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: auto",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+  {
+    elem: document.documentElement,
+    beforeStyle: "overflow: visible",
+    afterStyle:  "overflow: auto",
+    expectConstruction: false,
+    expectReflow: true,
+  },
+
+  // * Setting 'overflow' on arbitrary node should cause reflow as well as
+  // frame reconstruction
+  {
+    /* beforeStyle: implicitly 'overflow:visible' */
+    afterStyle:  "overflow: auto",
+    expectConstruction: true,
+    expectReflow: true,
+  },
+  {
+    beforeStyle: "overflow: auto",
+    afterStyle:  "overflow: visible",
+    expectConstruction: true,
+    expectReflow: true,
+  },
+
   // * Changing 'display' should cause frame construction and reflow.
   {
     beforeStyle: "display: inline",
@@ -135,23 +219,34 @@ function runOneTest(aTestcase)
     return;
   }
 
+  // Figure out which element we'll be tweaking (defaulting to gElem)
+  let elem = aTestcase.elem ?
+    aTestcase.elem : gElem;
+
+  // Verify that 'style' attribute is unset (avoid causing ourselves trouble):
+  if (elem.hasAttribute("style")) {
+    ok(false,
+       "test element has 'style' attribute already set! We're going to stomp " +
+       "on whatever's there when we clean up...");
+  }
+
   // Set the "before" style, and compose the first part of the message
   // to be used in our "is"/"isnot" invocations:
   let msgPrefix = "Changing style ";
   if (aTestcase.beforeStyle) {
-    gElem.setAttribute("style", aTestcase.beforeStyle);
+    elem.setAttribute("style", aTestcase.beforeStyle);
     msgPrefix += "from '" + aTestcase.beforeStyle + "' ";
   }
-  msgPrefix += "to '" + aTestcase.afterStyle + "' ";
+  msgPrefix += "on " + elem.nodeName + " ";
 
   // Establish initial counts:
-  let unusedVal = gElem.offsetHeight; // flush layout
+  let unusedVal = elem.offsetHeight; // flush layout
   let origFramesConstructed = gUtils.framesConstructed;
   let origFramesReflowed = gUtils.framesReflowed;
 
   // Make the change and flush:
-  gElem.setAttribute("style", aTestcase.afterStyle);
-  unusedVal = gElem.offsetHeight; // flush layout
+  elem.setAttribute("style", aTestcase.afterStyle);
+  unusedVal = elem.offsetHeight; // flush layout
 
   // Make our is/isnot assertions about whether things should have changed:
   checkFinalCount(gUtils.framesConstructed, origFramesConstructed,
@@ -162,7 +257,7 @@ function runOneTest(aTestcase)
                   "reflow");
 
   // Clean up!
-  gElem.removeAttribute("style");
+  elem.removeAttribute("style");
 }
 
 gTestcases.forEach(runOneTest);

layout/style/test/test_viewport_scrollbar_causing_reflow.html

@@ -0,0 +1,125 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1367568
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1367568</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=">Mozilla Bug 1367568</a>
+<div id="content">
+  <!-- Some fixed-width divs that we shouldn't have to reflow when the viewport
+       changes: -->
+  <div style="width: 100px">
+    fixed-width
+    <div>(child)</div>
+  </div>
+  <div style="position: absolute; width: 150px">
+    abs-fixed-width
+    <div>(child)</div>
+  </div>
+</div>
+<pre id="test">
+<script type="application/javascript">
+"use strict";
+
+/** Test for Bug 1367568 **/
+
+/**
+ * This test verifies that "overflow" changes on the <body> don't cause
+ * an unnecessarily large amount of reflow.
+ */
+
+// Vars used in setStyleAndMeasure that we really only have to look up once:
+const gUtils = SpecialPowers.getDOMWindowUtils(window);
+
+function setStyleAndMeasure(initialStyle, finalStyle) {
+  is(document.body.style.length, 0,
+     "Bug in test - body should start with empty style");
+  let unusedVal = document.body.offsetHeight; // flush layout
+  let constructCount = gUtils.framesConstructed;
+
+  document.body.style = initialStyle;
+  unusedVal = document.body.offsetHeight; // flush layout
+  let reflowCountBeforeTweak = gUtils.framesReflowed;
+
+  document.body.style = finalStyle;
+  unusedVal = document.body.offsetHeight; // flush layout
+  let reflowCountAfterTweak = gUtils.framesReflowed;
+
+  // Clean up:
+  document.body.style = "";
+
+  is(gUtils.framesConstructed, constructCount,
+     "Style tweak shouldn't have triggered frame construction");
+
+  // ...and return the delta:
+  return reflowCountAfterTweak - reflowCountBeforeTweak;
+}
+
+function main() {
+  // First, we sanity-check that our measurement make sense -- if we leave
+  // styles unchanged, we should measure no frames being reflowed:
+  let count = setStyleAndMeasure("width: 50px; height: 80px",
+                                 "width: 50px; height: 80px");
+  is(count, 0,
+     "Shouldn't reflow anything when we leave 'width' & 'height' unchanged");
+
+  // Now: see how many frames are reflowed when the "width" & "height" change.
+  // We'll use this as the reference when measuring reflow counts for various
+  // changes to "overflow" below.
+  count = setStyleAndMeasure("width: 50px; height: 80px",
+                             "width: 90px; height: 60px");
+  ok(count > 0,
+     "Should reflow some frames when 'width' & 'height' change");
+
+  // Expected maximum number of frames reflowed for "overflow" changes
+  // (+2 is to allow for reflowing scrollbars themselves):
+  const expectedMax = count + 2;
+
+  // Shared ending for messages in all ok() checks below:
+  const messageSuffix =
+    " shouldn't be greater than count for tweaking width/height on body (" +
+    expectedMax + ")";
+
+  // OK, here is where the relevant tests actually begin!!
+  // See how many frames we reflow for various tweaks to "overflow" on
+  // the body -- we expect the count to be no larger than |expectedMax|.
+  count = setStyleAndMeasure("", "overflow: scroll");
+  ok(count <= expectedMax,
+     "Reflow count when setting 'overflow: scroll' on body (" + count + ")" +
+     messageSuffix);
+
+  count = setStyleAndMeasure("", "overflow: hidden");
+  ok(count <= expectedMax,
+     "Reflow count when setting 'overflow: hidden' on body (" + count + ")" +
+     messageSuffix);
+
+  // Test removal of "overflow: scroll":
+  count = setStyleAndMeasure("overflow: scroll", "");
+  ok(count <= expectedMax,
+     "Reflow count when removing 'overflow: scroll' from body (" + count + ")" +
+     messageSuffix);
+
+  count = setStyleAndMeasure("overflow: hidden", "");
+  ok(count <= expectedMax,
+     "Reflow count when removing 'overflow: hidden' from body (" + count + ")" +
+     messageSuffix);
+
+  // Test change between two non-'visible' overflow values:
+  count = setStyleAndMeasure("overflow: scroll", "overflow: hidden");
+  ok(count <= expectedMax,
+     "Reflow count when changing 'overflow' on body (" + count + ")" +
+     messageSuffix);
+}
+
+main();
+
+</script>
+</pre>
+</body>
+</html>