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 07289a5efee36c5fdeb0fb996b28c185e1654064..249ff9b57eee6f809d4b90ff325e129285cc3aa9 100644 |
--- a/third_party/WebKit/Source/core/dom/Element.cpp |
+++ b/third_party/WebKit/Source/core/dom/Element.cpp |
@@ -1574,8 +1574,13 @@ const AtomicString Element::imageSourceURL() const { |
} |
bool Element::layoutObjectIsNeeded(const ComputedStyle& style) { |
- return style.display() != EDisplay::None && |
- style.display() != EDisplay::Contents; |
+ if (style.display() == EDisplay::None) |
+ return false; |
+ |
+ if (style.display() == EDisplay::Contents) |
+ return isPseudoElement(); |
rune
2017/03/06 10:13:33
Why? Pseudo elements on which display applies can
emilio
2017/03/06 16:52:09
Well, that's definitely not what Gecko does fwiw.
rune
2017/03/07 13:46:01
This case below fails in Gecko. I think that's a b
|
+ |
+ return true; |
} |
LayoutObject* Element::createLayoutObject(const ComputedStyle& style) { |
@@ -1811,7 +1816,6 @@ void Element::detachLayoutTree(const AttachContext& context) { |
bool Element::pseudoStyleCacheIsInvalid(const ComputedStyle* currentStyle, |
ComputedStyle* newStyle) { |
rune
2017/03/06 10:13:33
This method has a terrible name and an incomprehen
emilio
2017/03/06 16:52:09
lol, fun.
|
DCHECK_EQ(currentStyle, computedStyle()); |
- DCHECK(layoutObject()); |
if (!currentStyle) |
return false; |
@@ -1820,16 +1824,34 @@ bool Element::pseudoStyleCacheIsInvalid(const ComputedStyle* currentStyle, |
if (!pseudoStyleCache) |
return false; |
+ if (!layoutObject() && currentStyle->display() != EDisplay::Contents) |
+ return true; |
+ |
+ const ComputedStyle* layoutParentStyle = newStyle; |
+ if (!layoutObject()) { |
+ LayoutObject* parentLayoutObject = |
+ LayoutTreeBuilderTraversal::parentLayoutObject(*this); |
+ DCHECK(parentLayoutObject); |
+ layoutParentStyle = parentLayoutObject->style(); |
+ } |
+ |
size_t cacheSize = pseudoStyleCache->size(); |
for (size_t i = 0; i < cacheSize; ++i) { |
RefPtr<ComputedStyle> newPseudoStyle; |
RefPtr<ComputedStyle> oldPseudoStyle = pseudoStyleCache->at(i); |
PseudoId pseudoId = oldPseudoStyle->styleType(); |
- if (pseudoId == PseudoIdFirstLine || pseudoId == PseudoIdFirstLineInherited) |
+ if (!isPseudoElementAllowed(pseudoId)) |
+ return true; |
rune
2017/03/06 10:13:33
This method is buggy in the first place in that it
|
+ if (pseudoId == PseudoIdBefore || pseudoId == PseudoIdAfter) { |
+ newPseudoStyle = document().ensureStyleResolver().pseudoStyleForElement( |
+ this, PseudoStyleRequest(pseudoId), newStyle, layoutParentStyle); |
+ } else if (pseudoId == PseudoIdFirstLine || |
+ pseudoId == PseudoIdFirstLineInherited) { |
newPseudoStyle = layoutObject()->uncachedFirstLineStyle(newStyle); |
- else |
+ } else { |
newPseudoStyle = layoutObject()->getUncachedPseudoStyle( |
PseudoStyleRequest(pseudoId), newStyle, newStyle); |
+ } |
if (!newPseudoStyle) |
return true; |
if (*oldPseudoStyle != *newPseudoStyle || |
@@ -1839,9 +1861,10 @@ bool Element::pseudoStyleCacheIsInvalid(const ComputedStyle* currentStyle, |
newStyle->setHasPseudoStyle(pseudoId); |
newStyle->addCachedPseudoStyle(newPseudoStyle); |
if (pseudoId == PseudoIdFirstLine || |
- pseudoId == PseudoIdFirstLineInherited) |
+ pseudoId == PseudoIdFirstLineInherited) { |
layoutObject()->firstLineStyleDidChange(*oldPseudoStyle, |
*newPseudoStyle); |
+ } |
return true; |
} |
} |
@@ -2043,9 +2066,14 @@ StyleRecalcChange Element::recalcOwnStyle(StyleRecalcChange change, |
// https://codereview.chromium.org/30453002/ |
layoutObject->setStyleInternal(newStyle.get()); |
} |
- } else if (localChange != NoChange && |
- shouldStoreNonLayoutObjectComputedStyle(*newStyle)) { |
- storeNonLayoutObjectComputedStyle(newStyle); |
+ } else { |
+ if (localChange != NoChange || |
+ pseudoStyleCacheIsInvalid(oldStyle.get(), newStyle.get())) { |
+ if (shouldStoreNonLayoutObjectComputedStyle(*newStyle)) |
+ storeNonLayoutObjectComputedStyle(newStyle); |
+ else if (hasRareData()) |
+ elementRareData()->clearComputedStyle(); |
rune
2017/03/06 10:13:33
Is the clearComputedStyle() here a fix made necess
emilio
2017/03/06 17:03:46
I made it thinking on pseudos. It doesn't fix any
rune
2017/03/07 13:46:01
Acknowledged.
|
+ } |
} |
if (getStyleChangeType() >= SubtreeStyleChange) |
@@ -3197,18 +3225,24 @@ const ComputedStyle* Element::ensureComputedStyle( |
if (!pseudoElementSpecifier) |
return elementStyle; |
- if (ComputedStyle* pseudoElementStyle = |
+ if (ComputedStyle* cached = |
elementStyle->getCachedPseudoStyle(pseudoElementSpecifier)) |
- return pseudoElementStyle; |
+ return cached; |
+ |
+ const ComputedStyle* layoutParentStyle = elementStyle; |
+ if (hasDisplayContentsStyle()) { |
+ LayoutObject* parentLayoutObject = |
+ LayoutTreeBuilderTraversal::parentLayoutObject(*this); |
+ if (parentLayoutObject) |
+ layoutParentStyle = parentLayoutObject->style(); |
+ } |
- // TODO(ecobos): Passing two times elementStyle may be wrong, though we don't |
- // support display: contents elements' pseudo-elements yet, so this is not a |
- // problem for now. |
RefPtr<ComputedStyle> result = |
document().ensureStyleResolver().pseudoStyleForElement( |
- this, PseudoStyleRequest(pseudoElementSpecifier, |
- PseudoStyleRequest::ForComputedStyle), |
- elementStyle, elementStyle); |
+ this, |
+ PseudoStyleRequest(pseudoElementSpecifier, |
+ PseudoStyleRequest::ForComputedStyle), |
+ elementStyle, layoutParentStyle); |
DCHECK(result); |
return elementStyle->addCachedPseudoStyle(result.release()); |
} |
@@ -3292,7 +3326,7 @@ void Element::updatePseudoElement(PseudoId pseudoId, StyleRecalcChange change) { |
// Need to clear the cached style if the PseudoElement wants a recalc so it |
// computes a new style. |
if (element->needsStyleRecalc()) |
- layoutObject()->mutableStyle()->removeCachedPseudoStyle(pseudoId); |
+ mutableComputedStyle()->removeCachedPseudoStyle(pseudoId); |
// PseudoElement styles hang off their parent element's style so if we |
// needed a style recalc we should Force one on the pseudo. FIXME: We |
@@ -3304,9 +3338,9 @@ void Element::updatePseudoElement(PseudoId pseudoId, StyleRecalcChange change) { |
// continuously create and destroy PseudoElements when |
// LayoutObject::isChildAllowed on our parent returns false for the |
// PseudoElement's layoutObject for each style recalc. |
- if (!layoutObject() || |
+ if (!isPseudoElementAllowed(pseudoId) || |
!pseudoElementLayoutObjectIsNeeded( |
- layoutObject()->getCachedPseudoStyle(pseudoId))) |
+ getCachedPseudoStyleOrCache(pseudoId))) |
elementRareData()->setPseudoElement(pseudoId, nullptr); |
} else if (pseudoId == PseudoIdFirstLetter && element && |
change >= UpdatePseudoElements && |
@@ -3343,6 +3377,47 @@ bool Element::updateFirstLetter(Element* element) { |
return false; |
} |
+ComputedStyle* Element::getCachedPseudoStyleOrCache(PseudoId pseudoId) const { |
rune
2017/03/06 10:13:33
I don't understand the name of this method.
emilio
2017/03/06 16:52:09
It explicitly states what getCachedPseudoStyle did
rune
2017/03/07 13:46:01
I read that name as "return the cached pseudo styl
|
+ ComputedStyle* style = mutableComputedStyle(); |
+ if (pseudoId < FirstInternalPseudoId && !style->hasPseudoStyle(pseudoId)) |
+ return nullptr; |
rune
2017/03/07 13:46:01
I don't think we use cache entries for internal ps
|
+ |
+ if (ComputedStyle* cached = style->getCachedPseudoStyle(pseudoId)) |
+ return cached; |
+ RefPtr<ComputedStyle> uncached = getUncachedPseudoStyle(pseudoId); |
+ if (uncached) |
+ return style->addCachedPseudoStyle(uncached.release()); |
+ return nullptr; |
+} |
+ |
+PassRefPtr<ComputedStyle> Element::getUncachedPseudoStyle( |
+ PseudoId pseudoId) const { |
+ if (pseudoId == PseudoIdBefore || pseudoId == PseudoIdAfter) { |
+ LayoutObject* parentLayoutObject = layoutObject(); |
+ if (!parentLayoutObject && hasDisplayContentsStyle()) { |
+ parentLayoutObject = |
+ LayoutTreeBuilderTraversal::parentLayoutObject(*this); |
+ } |
+ if (!parentLayoutObject) |
+ return nullptr; |
+ return document().ensureStyleResolver().pseudoStyleForElement( |
+ const_cast<Element*>(this), PseudoStyleRequest(pseudoId), |
+ computedStyle(), parentLayoutObject->style()); |
+ } |
+ if (!layoutObject()) |
+ return nullptr; |
+ return layoutObject()->getUncachedPseudoStyle(PseudoStyleRequest(pseudoId)); |
+} |
+ |
+// For display: contents elements, we still need to generate ::before and |
+// ::after, but the rest of the pseudo-elements should only be used for elements |
+// with an actual layout object. |
rune
2017/03/06 10:13:33
I was just thinking about this in relation to ::se
emilio
2017/03/06 16:52:09
Note that this works with this patch, because I in
rune
2017/03/07 13:46:01
I find it hard to understand what "allowed" means
|
+bool Element::isPseudoElementAllowed(PseudoId pseudoId) const { |
+ if (hasDisplayContentsStyle()) |
+ return pseudoId == PseudoIdBefore || pseudoId == PseudoIdAfter; |
+ return !!layoutObject(); |
+} |
+ |
void Element::createPseudoElementIfNeeded(PseudoId pseudoId) { |
if (isPseudoElement()) |
return; |