Bug 1357461: Ensure not to increment the restyle generation if we haven't restyled after all.
This can happen with content attribute or state changes that end up not
generating any hint.
In particular, in the media_queries_dynamic.html case, the iframe resize was
toggling a few scrollbar attributes, which made us never pass the "didn't
restyle" test, even though the test really passed.
I'll probably need to add a workaround to assume we use viewport units, so
probably won't pass for long.
MozReview-Commit-ID: 2oEfic5yaOy
--- a/layout/base/ServoRestyleManager.cpp
+++ b/layout/base/ServoRestyleManager.cpp
@@ -278,17 +278,17 @@ UpdateFramePseudoElementStyles(nsIFrame*
UpdateBlockFramePseudoElements(static_cast<nsBlockFrame*>(aFrame),
aStyleSet,
aChangeList);
}
UpdateBackdropIfNeeded(aFrame, aStyleSet, aChangeList);
}
-void
+bool
ServoRestyleManager::ProcessPostTraversal(Element* aElement,
nsStyleContext* aParentContext,
ServoStyleSet* aStyleSet,
nsStyleChangeList& aChangeList)
{
nsIFrame* styleFrame = nsLayoutUtils::GetStyleFrame(aElement);
// Grab the change hint from Servo.
@@ -314,17 +314,17 @@ ServoRestyleManager::ProcessPostTraversa
// If our change hint is reconstruct, we delegate to the frame constructor,
// which consumes the new style and expects the old style to be on the frame.
//
// XXXbholley: We should teach the frame constructor how to clear the dirty
// descendants bit to avoid the traversal here.
if (changeHint & nsChangeHint_ReconstructFrame) {
ClearRestyleStateFromSubtree(aElement);
- return;
+ return true;
}
// TODO(emilio): We could avoid some refcount traffic here, specially in the
// ServoComputedValues case, which uses atomic refcounting.
//
// Hold the old style context alive, because it could become a dangling
// pointer during the replacement. In practice it's not a huge deal (on
// GetNextContinuationWithSameStyle the pointer is not dereferenced, only
@@ -409,62 +409,70 @@ ServoRestyleManager::ProcessPostTraversa
AddLayerChangesForAnimation(styleFrame, aElement, aChangeList);
}
const bool descendantsNeedFrames =
aElement->HasFlag(NODE_DESCENDANTS_NEED_FRAMES);
const bool traverseElementChildren =
aElement->HasDirtyDescendantsForServo() || descendantsNeedFrames;
const bool traverseTextChildren = recreateContext || descendantsNeedFrames;
+ bool recreatedAnyContext = recreateContext;
if (traverseElementChildren || traverseTextChildren) {
nsStyleContext* upToDateContext =
recreateContext ? newContext : oldStyleContext;
StyleChildrenIterator it(aElement);
TextPostTraversalState textState(
*upToDateContext, *aStyleSet, displayContentsNode && recreateContext);
for (nsIContent* n = it.GetNextChild(); n; n = it.GetNextChild()) {
if (traverseElementChildren && n->IsElement()) {
- ProcessPostTraversal(n->AsElement(), upToDateContext,
- aStyleSet, aChangeList);
+ recreatedAnyContext |=
+ ProcessPostTraversal(n->AsElement(), upToDateContext,
+ aStyleSet, aChangeList);
} else if (traverseTextChildren && n->IsNodeOfType(nsINode::eTEXT)) {
- ProcessPostTraversalForText(n, aChangeList, textState);
+ recreatedAnyContext |=
+ ProcessPostTraversalForText(n, aChangeList, textState);
}
}
}
aElement->UnsetHasDirtyDescendantsForServo();
aElement->UnsetFlags(NODE_DESCENDANTS_NEED_FRAMES);
+ return recreatedAnyContext;
}
-void
+bool
ServoRestyleManager::ProcessPostTraversalForText(
nsIContent* aTextNode,
nsStyleChangeList& aChangeList,
TextPostTraversalState& aPostTraversalState)
{
// Handle lazy frame construction.
if (aTextNode->HasFlag(NODE_NEEDS_FRAME)) {
aChangeList.AppendChange(nullptr, aTextNode, nsChangeHint_ReconstructFrame);
- return;
+ return true;
}
// Handle restyle.
nsIFrame* primaryFrame = aTextNode->GetPrimaryFrame();
- if (primaryFrame) {
- RefPtr<nsStyleContext> oldStyleContext = primaryFrame->StyleContext();
- nsStyleContext& newContext = aPostTraversalState.ComputeStyle(aTextNode);
- aPostTraversalState.ComputeHintIfNeeded(
- aTextNode, primaryFrame, newContext, aChangeList);
+ if (!primaryFrame) {
+ return false;
+ }
- for (nsIFrame* f = primaryFrame; f;
- f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
- f->SetStyleContext(&newContext);
- }
+ RefPtr<nsStyleContext> oldStyleContext = primaryFrame->StyleContext();
+ nsStyleContext& newContext = aPostTraversalState.ComputeStyle(aTextNode);
+ aPostTraversalState.ComputeHintIfNeeded(
+ aTextNode, primaryFrame, newContext, aChangeList);
+
+ for (nsIFrame* f = primaryFrame; f;
+ f = GetNextContinuationWithSameStyle(f, oldStyleContext)) {
+ f->SetStyleContext(&newContext);
}
+
+ return true;
}
void
ServoRestyleManager::ClearSnapshots()
{
for (auto iter = mSnapshots.Iter(); !iter.Done(); iter.Next()) {
iter.Key()->UnsetFlags(ELEMENT_HAS_SNAPSHOT | ELEMENT_HANDLED_SNAPSHOT);
iter.Remove();
@@ -574,18 +582,20 @@ ServoRestyleManager::DoProcessPendingRes
if (!animationOnly) {
ClearSnapshots();
}
// Recreate style contexts, and queue up change hints (which also handle
// lazy frame construction).
nsStyleChangeList currentChanges(StyleBackendType::Servo);
DocumentStyleRootIterator iter(doc);
+ bool anyStyleChanged = false;
while (Element* root = iter.GetNextStyleRoot()) {
- ProcessPostTraversal(root, nullptr, styleSet, currentChanges);
+ anyStyleChanged |=
+ ProcessPostTraversal(root, nullptr, styleSet, currentChanges);
}
// Process the change hints.
//
// Unfortunately, the frame constructor can generate new change hints while
// processing existing ones. We redirect those into a secondary queue and
// iterate until there's nothing left.
ReentrantChangeList newChanges;
@@ -604,17 +614,28 @@ ServoRestyleManager::DoProcessPendingRes
}
currentChanges.AppendChange(change.mContent->GetPrimaryFrame(),
change.mContent, change.mHint);
}
newChanges.Clear();
}
mReentrantChanges = nullptr;
- IncrementRestyleGeneration();
+
+ if (anyStyleChanged) {
+ // Maybe no styles changed when:
+ //
+ // * Only explicit change hints were posted in the first place.
+ // * When an attribute or state change in the content happens not to need
+ // a restyle after all.
+ //
+ // In any case, we don't need to increment the restyle generation in that
+ // case.
+ IncrementRestyleGeneration();
+ }
}
FlushOverflowChangedTracker();
if (!animationOnly) {
ClearSnapshots();
styleSet->AssertTreeIsClean();
mHaveNonAnimationRestyles = false;
--- a/layout/base/ServoRestyleManager.h
+++ b/layout/base/ServoRestyleManager.h
@@ -111,25 +111,31 @@ public:
protected:
~ServoRestyleManager() override
{
MOZ_ASSERT(!mReentrantChanges);
}
private:
/**
- * Performs post-Servo-traversal processing on this element and its descendants.
+ * Performs post-Servo-traversal processing on this element and its
+ * descendants.
+ *
+ * Returns whether any style did actually change. There may be cases where we
+ * didn't need to change any style after all, for example, when a content
+ * attribute changes that happens not to have any effect on the style of that
+ * element or any descendant or sibling.
*/
- void ProcessPostTraversal(Element* aElement,
+ bool ProcessPostTraversal(Element* aElement,
nsStyleContext* aParentContext,
ServoStyleSet* aStyleSet,
nsStyleChangeList& aChangeList);
struct TextPostTraversalState;
- void ProcessPostTraversalForText(nsIContent* aTextNode,
+ bool ProcessPostTraversalForText(nsIContent* aTextNode,
nsStyleChangeList& aChangeList,
TextPostTraversalState& aState);
inline ServoStyleSet* StyleSet() const
{
MOZ_ASSERT(PresContext()->StyleSet()->IsServo(),
"ServoRestyleManager should only be used with a Servo-flavored "
"style backend");