--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -1800,16 +1800,34 @@ Element::UnbindFromTree(bool aDeep, bool
RefPtr<nsINode> p;
p.swap(mParent);
} else {
mParent = nullptr;
}
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).
// We need to delete the properties while we're still in document
// (if we were in document).
// FIXME (Bug 522599): Need a test for this.
if (MayHaveAnimations()) {
DeleteProperty(nsGkAtoms::transitionsOfBeforeProperty);
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -449,17 +449,17 @@ RestyleManager::ChangeHintToString(nsCha
"UpdateTransformLayer", "ReconstructFrame", "UpdateOverflow",
"UpdateSubtreeOverflow", "UpdatePostTransformOverflow",
"UpdateParentOverflow",
"ChildrenOnlyTransform", "RecomputePosition", "UpdateContainingBlock",
"BorderStyleNoneChange", "UpdateTextPath", "SchedulePaint",
"NeutralChange", "InvalidateRenderingObservers",
"ReflowChangesSizeOrPosition", "UpdateComputedBSize",
"UpdateUsesOpacity", "UpdateBackgroundPosition",
- "AddOrRemoveTransform"
+ "AddOrRemoveTransform", "CSSOverflowChange",
};
static_assert(nsChangeHint_AllHints == (1 << ArrayLength(names)) - 1,
"Name list doesn't match change hints.");
uint32_t hint = aHint & ((1 << ArrayLength(names)) - 1);
uint32_t rest = aHint & ~((1 << ArrayLength(names)) - 1);
if ((hint & NS_STYLE_HINT_REFLOW) == NS_STYLE_HINT_REFLOW) {
result.AppendLiteral("NS_STYLE_HINT_REFLOW");
hint = hint & ~NS_STYLE_HINT_REFLOW;
@@ -1343,16 +1343,72 @@ RestyleManager::ProcessRestyledFrames(ns
PROFILER_LABEL("RestyleManager", "ProcessRestyledFrames",
js::ProfileEntry::Category::CSS);
nsPresContext* presContext = PresContext();
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.
+ data.mHint |= nsChangeHint_AllReflowHints;
+ doReconstruct = false;
+ }
+ }
+ }
+ if (doReconstruct) {
+ data.mHint |= nsChangeHint_ReconstructFrame;
+ }
+ }
+ }
+
// Make sure to not rebuild quote or counter lists while we're
// processing restyles
frameConstructor->BeginUpdate();
// Mark frames so that we skip frames that die along the way, bug 123049.
// A frame can be in the list multiple times with different hints. Further
// optmization is possible if nsStyleChangeList::AppendChange could coalesce
for (const nsStyleChangeData& data : aChangeList) {
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -8488,18 +8488,23 @@ nsCSSFrameConstructor::ContentRemoved(ns
ServoRestyleManager::ClearServoDataFromSubtree(aChild->AsElement());
}
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.
+ // 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
// the :empty pseudo-class?
#ifdef DEBUG
if (gNoisyContentUpdates) {
--- a/layout/base/nsChangeHint.h
+++ b/layout/base/nsChangeHint.h
@@ -217,32 +217,42 @@ enum nsChangeHint : uint32_t {
nsChangeHint_UpdateBackgroundPosition = 1 << 26,
/**
* Indicates that a frame has changed to or from having the CSS
* transform property set.
*/
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 a new hint, you will need to add it to
// one of:
//
// * nsChangeHint_Hints_NeverHandledForDescendants
// * nsChangeHint_Hints_AlwaysHandledForDescendants
// * nsChangeHint_Hints_SometimesHandledForDescendants
//
// and you also may need to handle it in NS_HintsNotHandledForDescendantsIn.
//
// Please also add it to RestyleManager::ChangeHintToString and
// modify nsChangeHint_AllHints below accordingly.
/**
* 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
// of these operators on hints. We should not be using these operators
// on nsChangeHints
inline void operator<(nsChangeHint s1, nsChangeHint s2) {}
inline void operator>(nsChangeHint s1, nsChangeHint s2) {}
inline void operator!=(nsChangeHint s1, nsChangeHint s2) {}
@@ -321,16 +331,17 @@ inline nsChangeHint operator^=(nsChangeH
nsChangeHint_UpdateSubtreeOverflow | \
nsChangeHint_UpdateTextPath \
)
// The change hints that are never handled for descendants.
#define nsChangeHint_Hints_NeverHandledForDescendants ( \
nsChangeHint_BorderStyleNoneChange | \
nsChangeHint_ChildrenOnlyTransform | \
+ nsChangeHint_CSSOverflowChange | \
nsChangeHint_InvalidateRenderingObservers | \
nsChangeHint_RecomputePosition | \
nsChangeHint_UpdateBackgroundPosition | \
nsChangeHint_UpdateComputedBSize | \
nsChangeHint_UpdateContainingBlock | \
nsChangeHint_UpdateEffects | \
nsChangeHint_UpdateOpacityLayer | \
nsChangeHint_UpdateOverflow | \
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -225,16 +225,17 @@ nsPresContext::nsPresContext(nsIDocument
mDefaultColor(NS_RGBA(0,0,0,0)),
mBackgroundColor(NS_RGB(0xFF, 0xFF, 0xFF)),
mLinkColor(NS_RGB(0x00, 0x00, 0xEE)),
mActiveLinkColor(NS_RGB(0xEE, 0x00, 0x00)),
mVisitedLinkColor(NS_RGB(0x55, 0x1A, 0x8B)),
mFocusBackgroundColor(mBackgroundColor),
mFocusTextColor(mDefaultColor),
mBodyTextColor(mDefaultColor),
+ mViewportScrollbarOverrideNode(nullptr),
mViewportStyleScrollbar(NS_STYLE_OVERFLOW_AUTO, NS_STYLE_OVERFLOW_AUTO),
mFocusRingWidth(1),
mExistThrottledUpdates(false),
// mImageAnimationMode is initialised below, in constructor body
mImageAnimationModePref(imgIContainer::kNormalAnimMode),
mInterruptChecksToSkip(0),
mElementsRestyled(0),
mFramesConstructed(0),
@@ -1490,38 +1491,37 @@ GetPropagatedScrollbarStylesForViewport(
}
nsIContent*
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);
}
nsIDocument* document = Document();
if (Element* fullscreenElement = document->GetFullscreenElement()) {
// If the document is in fullscreen, but the fullscreen element is
// not the root element, we should explicitly suppress the scrollbar
// here. Note that, we still need to return the original element
// 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
nsPresContext::ElementWouldPropagateScrollbarStyles(Element* aElement)
{
MOZ_ASSERT(IsPaginated(), "Should only be called on paginated contexts");
if (aElement->GetParent() && !aElement->IsHTMLElement(nsGkAtoms::body)) {
// We certainly won't be propagating from this element.
--- a/layout/base/nsPresContext.h
+++ b/layout/base/nsPresContext.h
@@ -732,17 +732,28 @@ public:
* return the element that we took the overflow from (which should then be
* treated as "overflow: visible"), and we store the overflow style here.
* If the document is in fullscreen, and the fullscreen element is not the
* root, the scrollbar of viewport will be suppressed.
* @return if scroll was propagated from some content node, the content node
* 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;
}
/**
* Check whether the given element would propagate its scrollbar styles to the
* viewport in non-paginated mode. Must only be called if IsPaginated().
*/
@@ -1380,17 +1391,26 @@ protected:
nscolor mActiveLinkColor;
nscolor mVisitedLinkColor;
nscolor mFocusBackgroundColor;
nscolor mFocusTextColor;
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;
uint16_t mImageAnimationMode;
uint16_t mImageAnimationModePref;
// Most documents will only use one (or very few) language groups. Rather
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -3483,29 +3483,32 @@ nsStyleDisplay::CalcDifference(const nsS
{
nsChangeHint hint = nsChangeHint(0);
if (!DefinitelyEqualURIsAndPrincipal(mBinding.ForceGet(), aNewData.mBinding.ForceGet())
|| mPosition != aNewData.mPosition
|| 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
|| mScrollSnapPointsX != aNewData.mScrollSnapPointsX
|| mScrollSnapPointsY != aNewData.mScrollSnapPointsY
|| mScrollSnapDestination != aNewData.mScrollSnapDestination
|| mTopLayer != aNewData.mTopLayer
|| mResize != aNewData.mResize) {
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
* nsCSSFrameConstructor::PropagateScrollToViewport. By using the same hint
* as used when the overflow css property changes,
* nsChangeHint_ReconstructFrame, PropagateScrollToViewport will be called.
*
* The scroll-behavior css property is not expected to change often (the
--- a/layout/style/test/test_dynamic_change_causing_reflow.html
+++ b/layout/style/test/test_dynamic_change_causing_reflow.html
@@ -90,16 +90,100 @@ const gTestcases = [
// * Changing 'height' should cause reflow, but not frame construction.
{
beforeStyle: "height: 10px",
afterStyle: "height: 15px",
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",
afterStyle: "display: table",
expectConstruction: true,
expectReflow: true,
},
@@ -130,44 +214,56 @@ const gElem = document.getElementById("c
function runOneTest(aTestcase)
{
// sanity-check that we have the one main thing we need:
if (!aTestcase.afterStyle) {
ok(false, "testcase is missing an 'afterStyle' to change to");
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,
aTestcase.expectConstruction, msgPrefix,
"frame construction");
checkFinalCount(gUtils.framesReflowed, origFramesReflowed,
aTestcase.expectReflow, msgPrefix,
"reflow");
// Clean up!
- gElem.removeAttribute("style");
+ elem.removeAttribute("style");
}
gTestcases.forEach(runOneTest);
</script>
</pre>
</body>
</html>