Commit 044af02e authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Chromium LUCI CQ

Use flat tree ancestors for selector filter

SelectorFilterParentScopes are added to the stack in the flat tree
order, but the SelectorFilter were still assuming shadow-including
ancestors constituted the filter stack.

This fixes the performance issue in 1159206 by correctly rejecting the
expensive indirect adjacent selector based on the bloom filter.

Bug: 1159206
Change-Id: Iddfeb78b78567de8fa19be8c923219fa738ff303
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2595379Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837985}
parent 074fb6d1
...@@ -3,20 +3,21 @@ ...@@ -3,20 +3,21 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "third_party/blink/renderer/core/css/resolver/selector_filter_parent_scope.h" #include "third_party/blink/renderer/core/css/resolver/selector_filter_parent_scope.h"
#include "third_party/blink/renderer/core/dom/flat_tree_traversal.h"
namespace blink { namespace blink {
SelectorFilterParentScope* SelectorFilterParentScope::current_scope_ = nullptr; SelectorFilterParentScope* SelectorFilterParentScope::current_scope_ = nullptr;
void SelectorFilterParentScope::PushAncestors(Element& element) { void SelectorFilterParentScope::PushAncestors(Element& element) {
if (Element* ancestor = element.ParentOrShadowHostElement()) { if (Element* ancestor = FlatTreeTraversal::ParentElement(element)) {
PushAncestors(*ancestor); PushAncestors(*ancestor);
resolver_->GetSelectorFilter().PushParent(*ancestor); resolver_->GetSelectorFilter().PushParent(*ancestor);
} }
} }
void SelectorFilterParentScope::PopAncestors(Element& element) { void SelectorFilterParentScope::PopAncestors(Element& element) {
if (Element* ancestor = element.ParentOrShadowHostElement()) { if (Element* ancestor = FlatTreeTraversal::ParentElement(element)) {
resolver_->GetSelectorFilter().PopParent(*ancestor); resolver_->GetSelectorFilter().PopParent(*ancestor);
PopAncestors(*ancestor); PopAncestors(*ancestor);
} }
......
...@@ -32,6 +32,7 @@ ...@@ -32,6 +32,7 @@
#include "third_party/blink/renderer/core/css/css_selector.h" #include "third_party/blink/renderer/core/css/css_selector.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/flat_tree_traversal.h"
namespace blink { namespace blink {
...@@ -65,8 +66,9 @@ static inline void CollectElementIdentifierHashes( ...@@ -65,8 +66,9 @@ static inline void CollectElementIdentifierHashes(
void SelectorFilter::PushParentStackFrame(Element& parent) { void SelectorFilter::PushParentStackFrame(Element& parent) {
DCHECK(ancestor_identifier_filter_); DCHECK(ancestor_identifier_filter_);
DCHECK(parent_stack_.IsEmpty() || DCHECK(parent_stack_.IsEmpty() ||
parent_stack_.back().element == parent.ParentOrShadowHostElement()); parent_stack_.back().element ==
DCHECK(!parent_stack_.IsEmpty() || !parent.ParentOrShadowHostElement()); FlatTreeTraversal::ParentElement(parent));
DCHECK(!parent_stack_.IsEmpty() || !FlatTreeTraversal::ParentElement(parent));
parent_stack_.push_back(ParentStackFrame(parent)); parent_stack_.push_back(ParentStackFrame(parent));
ParentStackFrame& parent_frame = parent_stack_.back(); ParentStackFrame& parent_frame = parent_stack_.back();
// Mix tags, class names and ids into some sort of weird bouillabaisse. // Mix tags, class names and ids into some sort of weird bouillabaisse.
...@@ -106,7 +108,7 @@ void SelectorFilter::PushParent(Element& parent) { ...@@ -106,7 +108,7 @@ void SelectorFilter::PushParent(Element& parent) {
DCHECK(ancestor_identifier_filter_); DCHECK(ancestor_identifier_filter_);
// We may get invoked for some random elements in some wacky cases during // We may get invoked for some random elements in some wacky cases during
// style resolve. Pause maintaining the stack in this case. // style resolve. Pause maintaining the stack in this case.
if (parent_stack_.back().element != parent.ParentOrShadowHostElement()) if (parent_stack_.back().element != FlatTreeTraversal::ParentElement(parent))
return; return;
PushParentStackFrame(parent); PushParentStackFrame(parent);
} }
......
...@@ -2006,20 +2006,20 @@ void StyleEngine::UpdateStyleAndLayoutTreeForContainer(Element& container) { ...@@ -2006,20 +2006,20 @@ void StyleEngine::UpdateStyleAndLayoutTreeForContainer(Element& container) {
void StyleEngine::RecalcStyle(StyleRecalcChange change) { void StyleEngine::RecalcStyle(StyleRecalcChange change) {
DCHECK(GetDocument().documentElement()); DCHECK(GetDocument().documentElement());
Element* root_element = &style_recalc_root_.RootElement(); Element& root_element = style_recalc_root_.RootElement();
Element* parent = root_element->ParentOrShadowHostElement(); Element* parent = FlatTreeTraversal::ParentElement(root_element);
SelectorFilterRootScope filter_scope(parent); SelectorFilterRootScope filter_scope(parent);
root_element->RecalcStyle(change); root_element.RecalcStyle(change);
for (ContainerNode* ancestor = root_element->GetStyleRecalcParent(); ancestor; for (ContainerNode* ancestor = root_element.GetStyleRecalcParent(); ancestor;
ancestor = ancestor->GetStyleRecalcParent()) { ancestor = ancestor->GetStyleRecalcParent()) {
if (auto* ancestor_element = DynamicTo<Element>(ancestor)) if (auto* ancestor_element = DynamicTo<Element>(ancestor))
ancestor_element->RecalcStyleForTraversalRootAncestor(); ancestor_element->RecalcStyleForTraversalRootAncestor();
ancestor->ClearChildNeedsStyleRecalc(); ancestor->ClearChildNeedsStyleRecalc();
} }
style_recalc_root_.Clear(); style_recalc_root_.Clear();
if (!parent || IsA<HTMLBodyElement>(*root_element)) if (!parent || IsA<HTMLBodyElement>(root_element))
PropagateWritingModeAndDirectionToHTMLRoot(); PropagateWritingModeAndDirectionToHTMLRoot();
} }
......
...@@ -3836,4 +3836,45 @@ TEST_F(StyleEngineTest, MarkStyleDirtyFromContainerRecalc) { ...@@ -3836,4 +3836,45 @@ TEST_F(StyleEngineTest, MarkStyleDirtyFromContainerRecalc) {
EXPECT_NE(old_inner_style, new_inner_style); EXPECT_NE(old_inner_style, new_inner_style);
} }
TEST_F(StyleEngineTest, FastRejectForHostChild) {
GetDocument().body()->setInnerHTML(R"HTML(
<style>
.notfound span {
color: pink;
}
</style>
<div id="host">
<span id="slotted"></span>
</div>
)HTML");
Element* host = GetDocument().getElementById("host");
ASSERT_TRUE(host);
ShadowRoot& shadow_root =
host->AttachShadowRootInternal(ShadowRootType::kOpen);
shadow_root.setInnerHTML(R"HTML(
<slot></slot>
)HTML");
UpdateAllLifecyclePhases();
StyleEngine& engine = GetStyleEngine();
// If the Stats() were already enabled, we would not start with 0 counts.
EXPECT_FALSE(engine.Stats());
engine.SetStatsEnabled(true);
StyleResolverStats* stats = engine.Stats();
ASSERT_TRUE(stats);
EXPECT_EQ(0u, stats->rules_fast_rejected);
Element* span = GetDocument().getElementById("slotted");
ASSERT_TRUE(span);
span->SetInlineStyleProperty(CSSPropertyID::kColor, "green");
GetDocument().Lifecycle().AdvanceTo(DocumentLifecycle::kInStyleRecalc);
GetStyleEngine().RecalcStyle();
// Should fast reject ".notfound span"
EXPECT_EQ(1u, stats->rules_fast_rejected);
}
} // namespace blink } // namespace blink
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