Skip to content

GitLab

T
tangled
Commit 435fc62a authored by kochi@chromium.org's avatar kochi@chromium.org
Browse files
Options

Revert r174436 (Reduce the cost of the n^2 loop in collectTreeBoundaryCrossingRules) 

https://src.chromium.org/viewvc/blink?view=rev&revision=174436

r174436 was introduced to improve the performance, but
introduced some regressions for shadow styling.

Added a layout test based on the report in issue 383366
to prevent the same regression again.

BUG=383366, 372083
TEST=pass all layout tests.

Review URL: https://codereview.chromium.org/323423008

git-svn-id: svn://svn.chromium.org/blink/trunk@176099 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 5a629c6c
Hide whitespace changes
Inline Side-by-side
Showing with 28 additions and 15 deletions
third_party/WebKit/LayoutTests/fast/css/content-distributed-nodes-expected.txt 0 → 100644
View file @ 435fc62a
Checking if styles in the nested shadow roots apply properly to distributed elements.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS getComputedStyle(span).color is "rgb(255, 0, 0)"
PASS getComputedStyle(span).backgroundColor is "rgb(0, 128, 0)"
PASS successfullyParsed is true
TEST COMPLETE
red?
third_party/WebKit/LayoutTests/fast/css/content-distributed-nodes.html 0 → 100644
View file @ 435fc62a
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<div id="host">
<div>
<span class="red" id="span">red?</span>
</div>
</div>
<script>
description('Checking if styles in the nested shadow roots apply properly to distributed elements.');
var root = document.querySelector('#host').createShadowRoot();
root.innerHTML = '<div><content></content></div><style>::content .red { color: green; }</style>';
var root2 = root.firstChild.createShadowRoot();
root2.innerHTML = '<style>::content .red { background-color: green; color: red; }</style><content></content>';
var span = document.querySelector('#span');
shouldBeEqualToString('getComputedStyle(span).color', 'rgb(255, 0, 0)');
shouldBeEqualToString('getComputedStyle(span).backgroundColor', 'rgb(0, 128, 0)');
</script>
third_party/WebKit/Source/core/css/TreeBoundaryCrossingRules.cpp
View file @ 435fc62a
...@@ -32,7 +32,6 @@ ...@@ -32,7 +32,6 @@
#include "core/css/ElementRuleCollector.h" #include "core/css/ElementRuleCollector.h"
#include "core/css/RuleFeature.h" #include "core/css/RuleFeature.h"
#include "core/dom/StyleEngine.h" #include "core/dom/StyleEngine.h"
#include "core/dom/shadow/InsertionPoint.h"
#include "core/dom/shadow/ShadowRoot.h" #include "core/dom/shadow/ShadowRoot.h"
namespace WebCore { namespace WebCore {
...@@ -74,8 +73,6 @@ void TreeBoundaryCrossingRules::collectTreeBoundaryCrossingRules(Element* elemen ...@@ -74,8 +73,6 @@ void TreeBoundaryCrossingRules::collectTreeBoundaryCrossingRules(Element* elemen
// When comparing rules declared in inner treescopes, inner's rules win. // When comparing rules declared in inner treescopes, inner's rules win.
CascadeOrder innerCascadeOrder = size(); CascadeOrder innerCascadeOrder = size();
// FIXME: This is n^2 across the number of scoping nodes in the entire document! We need to walk the
// ancestors of |element| instead.
for (DocumentOrderedList::iterator it = m_scopingNodes.begin(); it != m_scopingNodes.end(); ++it) { for (DocumentOrderedList::iterator it = m_scopingNodes.begin(); it != m_scopingNodes.end(); ++it) {
const ContainerNode* scopingNode = toContainerNode(*it); const ContainerNode* scopingNode = toContainerNode(*it);
CSSStyleSheetRuleSubSet* ruleSubSet = m_treeBoundaryCrossingRuleSetMap.get(scopingNode); CSSStyleSheetRuleSubSet* ruleSubSet = m_treeBoundaryCrossingRuleSetMap.get(scopingNode);
...@@ -89,24 +86,12 @@ void TreeBoundaryCrossingRules::collectTreeBoundaryCrossingRules(Element* elemen ...@@ -89,24 +86,12 @@ void TreeBoundaryCrossingRules::collectTreeBoundaryCrossingRules(Element* elemen
scopingNode = toShadowRoot(scopingNode)->host(); scopingNode = toShadowRoot(scopingNode)->host();
} }
// FIXME: This is a terrible hack to avoid doing most of the work in this n^2 loop.
// Instead we should just fix the loop to collect the proper scopes up the tree instead
// of iterating all possible scopes.
const InsertionPoint* insertionPoint = resolveReprojection(element);
if ((insertionPoint && !scopingNode->containsIncludingShadowDOM(insertionPoint))
|| (!insertionPoint && !scopingNode->containsIncludingShadowDOM(element))) {
++innerCascadeOrder;
--outerCascadeOrder;
continue;
}
CascadeOrder cascadeOrder = isInnerTreeScope ? innerCascadeOrder : outerCascadeOrder; CascadeOrder cascadeOrder = isInnerTreeScope ? innerCascadeOrder : outerCascadeOrder;
for (CSSStyleSheetRuleSubSet::iterator it = ruleSubSet->begin(); it != ruleSubSet->end(); ++it) { for (CSSStyleSheetRuleSubSet::iterator it = ruleSubSet->begin(); it != ruleSubSet->end(); ++it) {
CSSStyleSheet* parentStyleSheet = it->first; CSSStyleSheet* parentStyleSheet = it->first;
RuleSet* ruleSet = it->second.get(); RuleSet* ruleSet = it->second.get();
collector.collectMatchingRules(MatchRequest(ruleSet, includeEmptyRules, scopingNode, parentStyleSheet), ruleRange, static_cast<SelectorChecker::BehaviorAtBoundary>(boundaryBehavior), ignoreCascadeScope, cascadeOrder); collector.collectMatchingRules(MatchRequest(ruleSet, includeEmptyRules, scopingNode, parentStyleSheet), ruleRange, static_cast<SelectorChecker::BehaviorAtBoundary>(boundaryBehavior), ignoreCascadeScope, cascadeOrder);
} }
++innerCascadeOrder; ++innerCascadeOrder;
--outerCascadeOrder; --outerCascadeOrder;
} }
......
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