Commit 4953b70f authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Commit Bot

[Squad] Re-add null check for layout parent.

Because of issue 831568, we may end up trying to generate pseudo
elements for element outside the flat tree if we did a getComputedStyle
for all non-flat-tree ancestors and then triggered style invalidation of
the pseudo element's originating element.

This is not a new issue, but [1] boldly tried to make the if-test for a
layout parent a DCHECK. This CL is effectively reverting that change.

This should ultimately be fixed by fixing 831568 and re-introduce DCHECK
instead of the if-test.

[1] https://crrev.com/86f6eefc70ed2aa46969754e207198bd6917ee33

Bug: 862098, 831568
Change-Id: Id10fdbe28592e492ef6903b82d2fc47bff930ea4
Reviewed-on: https://chromium-review.googlesource.com/1146729Reviewed-by: default avatarAnders Ruud <andruud@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577245}
parent dec3452b
...@@ -1455,4 +1455,21 @@ TEST_F(StyleEngineTest, ShadowRootStyleRecalcCrash) { ...@@ -1455,4 +1455,21 @@ TEST_F(StyleEngineTest, ShadowRootStyleRecalcCrash) {
GetDocument().View()->UpdateAllLifecyclePhases(); GetDocument().View()->UpdateAllLifecyclePhases();
} }
TEST_F(StyleEngineTest, GetComputedStyleOutsideFlatTreeCrash) {
GetDocument().body()->SetInnerHTMLFromString(R"HTML(
<style>
body, div { display: contents }
div::before { display: contents; content: "" }
</style>
<div id=inner></div>
)HTML");
GetDocument().documentElement()->CreateV0ShadowRootForTesting();
GetDocument().View()->UpdateAllLifecyclePhases();
GetDocument().body()->EnsureComputedStyle();
GetDocument().getElementById("inner")->SetInlineStyleProperty(
CSSPropertyColor, "blue");
GetDocument().View()->UpdateAllLifecyclePhases();
}
} // namespace blink } // namespace blink
...@@ -4143,9 +4143,14 @@ scoped_refptr<ComputedStyle> Element::StyleForPseudoElement( ...@@ -4143,9 +4143,14 @@ scoped_refptr<ComputedStyle> Element::StyleForPseudoElement(
if (is_before_or_after) { if (is_before_or_after) {
const ComputedStyle* layout_parent_style = style; const ComputedStyle* layout_parent_style = style;
if (style->Display() == EDisplay::kContents) { if (style->Display() == EDisplay::kContents) {
Node* layout_parent = LayoutTreeBuilderTraversal::LayoutParent(*this); // TODO(futhark@chromium.org): Calling getComputedStyle for elements
DCHECK(layout_parent); // outside the flat tree should return empty styles, but currently we do
layout_parent_style = layout_parent->GetComputedStyle(); // not. See issue https://crbug.com/831568. We can replace the if-test
// with DCHECK(layout_parent) when that issue is fixed.
if (Node* layout_parent =
LayoutTreeBuilderTraversal::LayoutParent(*this)) {
layout_parent_style = layout_parent->GetComputedStyle();
}
} }
return GetDocument().EnsureStyleResolver().PseudoStyleForElement( return GetDocument().EnsureStyleResolver().PseudoStyleForElement(
this, request, style, layout_parent_style); this, request, style, layout_parent_style);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment