Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(182)

Unified Diff: third_party/WebKit/Source/core/dom/Element.cpp

Issue 2473743003: Call Element::rebuildLayoutTree from Document::updateStyle directly (Closed)
Patch Set: Final patch - make sure to retain information needed to determine whether to call detachLayoutTree() Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/dom/Element.cpp
diff --git a/third_party/WebKit/Source/core/dom/Element.cpp b/third_party/WebKit/Source/core/dom/Element.cpp
index ff3aeb0b7f2622666fb366d0edf7fca9159bb403..29bbf92a73fafe343332a430e09b7f2592ec6f4a 100644
--- a/third_party/WebKit/Source/core/dom/Element.cpp
+++ b/third_party/WebKit/Source/core/dom/Element.cpp
@@ -1895,16 +1895,24 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling) {
elementAnimations->setAnimationStyleChange(false);
}
}
- if (parentComputedStyle())
+ if (parentComputedStyle()) {
change = recalcOwnStyle(change, nextTextSibling);
- clearNeedsStyleRecalc();
- clearNeedsReattachLayoutTree();
+ } else {
+ // In case we don't perform recalcOwnStyle we will never clear the
+ // NeedsReattachLayoutTree flag which is set on the creation of each
+ // Node. Clear that here.
+ clearNeedsReattachLayoutTree();
+ }
+ // If you are going to reattachLayoutTree you need to retain this flag
+ // to determine if detachLayoutTree() needs to happen.
+ if (!needsReattachLayoutTree())
+ clearNeedsStyleRecalc();
}
- // If we reattached we don't need to recalc the style of our descendants
- // anymore.
- if ((change >= UpdatePseudoElements && change < Reattach) ||
- childNeedsStyleRecalc()) {
+ // If we are going to reattach we don't need to recalc the style of
+ // our descendants anymore.
+ if (change < Reattach &&
+ (change >= UpdatePseudoElements || childNeedsStyleRecalc())) {
SelectorFilterParentScope filterScope(*this);
StyleSharingDepthScope sharingScope(*this);
@@ -1913,8 +1921,14 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling) {
if (change > UpdatePseudoElements || childNeedsStyleRecalc()) {
for (ShadowRoot* root = youngestShadowRoot(); root;
root = root->olderShadowRoot()) {
- if (root->shouldCallRecalcStyle(change))
+ if (root->shouldCallRecalcStyle(change)) {
root->recalcStyle(change);
+ } else {
+ // In case we don't perform recalcStyle we will never clear the
+ // NeedsReattachLayoutTree flag which is set on the creation of each
+ // Node. Clear that here.
+ root->clearNeedsReattachLayoutTree();
esprehn 2017/02/02 08:37:09 I wonder how this can happen, how do we get here w
nainar 2017/02/02 23:13:06 Yup this is redundant code and never executed. Rem
+ }
}
recalcDescendantStyles(change);
}
@@ -1930,7 +1944,6 @@ void Element::recalcStyle(StyleRecalcChange change, Text* nextTextSibling) {
childNeedsStyleRecalc() ? Force : change);
clearChildNeedsStyleRecalc();
- clearChildNeedsReattachLayoutTree();
}
if (hasCustomStyleCallbacks())
@@ -1990,7 +2003,7 @@ StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change,
styleReattachData.nextTextSibling = nextTextSibling;
document().addStyleReattachData(*this, styleReattachData);
setNeedsReattachLayoutTree();
- return rebuildLayoutTree();
+ return Reattach;
}
DCHECK(oldStyle);
@@ -2031,23 +2044,31 @@ StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change,
return localChange;
}
-StyleRecalcChange Element::rebuildLayoutTree() {
+void Element::rebuildLayoutTree() {
DCHECK(inActiveDocument());
+ DCHECK(parentNode());
+
StyleReattachData styleReattachData = document().getStyleReattachData(*this);
AttachContext reattachContext;
reattachContext.resolvedStyle = styleReattachData.computedStyle.get();
bool layoutObjectWillChange = needsAttach() || layoutObject();
- // We are calling Element::rebuildLayoutTree() from inside
- // Element::recalcOwnStyle where we set the NeedsReattachLayoutTree
- // flag - so needsReattachLayoutTree() should always be true.
- DCHECK(parentNode());
- DCHECK(parentNode()->childNeedsReattachLayoutTree());
- DCHECK(needsReattachLayoutTree());
- reattachLayoutTree(reattachContext);
- // Since needsReattachLayoutTree() is always true we go into
- // reattachLayoutTree() which reattaches all the descendant
- // sub-trees. At this point no child should need reattaching.
+ if (needsReattachLayoutTree()) {
+ reattachLayoutTree(reattachContext);
+ } else if (childNeedsReattachLayoutTree()) {
+ DCHECK(!needsReattachLayoutTree());
+ SelectorFilterParentScope filterScope(*this);
+ StyleSharingDepthScope sharingScope(*this);
+ reattachPseudoElementLayoutTree(PseudoIdBefore);
+ rebuildShadowRootLayoutTree();
+ rebuildChildrenLayoutTrees();
+ reattachPseudoElementLayoutTree(PseudoIdAfter);
+ reattachPseudoElementLayoutTree(PseudoIdBackdrop);
+ reattachPseudoElementLayoutTree(PseudoIdFirstLetter);
+ }
+ DCHECK(!needsStyleRecalc());
+ DCHECK(!childNeedsStyleRecalc());
+ DCHECK(!needsReattachLayoutTree());
DCHECK(!childNeedsReattachLayoutTree());
if (layoutObjectWillChange || layoutObject()) {
@@ -2056,9 +2077,25 @@ StyleRecalcChange Element::rebuildLayoutTree() {
// or store it.
// The choice is between increased time and increased memory complexity.
reattachWhitespaceSiblingsIfNeeded(styleReattachData.nextTextSibling);
- return Reattach;
}
- return ReattachNoLayoutObject;
+}
+
+void Element::rebuildShadowRootLayoutTree() {
+ for (ShadowRoot* root = youngestShadowRoot(); root;
+ root = root->olderShadowRoot()) {
+ if (root->needsReattachLayoutTree() || root->childNeedsReattachLayoutTree())
+ root->rebuildLayoutTree();
+ }
+}
+
+void Element::reattachPseudoElementLayoutTree(PseudoId pseudoId) {
+ if (PseudoElement* element = pseudoElement(pseudoId)) {
+ if (element->needsReattachLayoutTree() ||
+ element->childNeedsReattachLayoutTree())
+ element->rebuildLayoutTree();
+ } else {
+ createPseudoElementIfNeeded(pseudoId);
+ }
}
void Element::updateCallbackSelectors(const ComputedStyle* oldStyle,
@@ -3208,7 +3245,6 @@ void Element::cancelFocusAppearanceUpdate() {
}
void Element::updatePseudoElement(PseudoId pseudoId, StyleRecalcChange change) {
- DCHECK(!needsStyleRecalc());
PseudoElement* element = pseudoElement(pseudoId);
if (element && (change == UpdatePseudoElements ||

Powered by Google App Engine
This is Rietveld 408576698