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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 2727853002: [css-display] Support display: contents pseudo-elements.
Patch Set: [css-display] Support display: contents pseudo-elements. Created 3 years, 10 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
« no previous file with comments | « third_party/WebKit/Source/core/dom/Element.h ('k') | third_party/WebKit/Source/core/dom/PseudoElement.cpp » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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;
« no previous file with comments | « third_party/WebKit/Source/core/dom/Element.h ('k') | third_party/WebKit/Source/core/dom/PseudoElement.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698