Removed add/removeScopedStyleResolver from StyleEngine

Source/core/css/resolver/StyleResolver.cpp

Index: Source/core/css/resolver/StyleResolver.cpp
diff --git a/Source/core/css/resolver/StyleResolver.cpp b/Source/core/css/resolver/StyleResolver.cpp
index cd4f778df1e55209db6b091cf0ee98cfd6374952..3fff8aa63c2a0d8d1a031bf3e9a73903a2f2b4da 100644
--- a/Source/core/css/resolver/StyleResolver.cpp
+++ b/Source/core/css/resolver/StyleResolver.cpp
@@ -181,10 +181,7 @@ void StyleResolver::appendCSSStyleSheet(CSSStyleSheet& cssSheet)
     if (!treeScope)
         return;
 
-    ScopedStyleResolver& resolver = treeScope->ensureScopedStyleResolver();
-    document().styleEngine()->addScopedStyleResolver(&resolver);
-    unsigned index = resolver.appendCSSStyleSheet(&cssSheet);
-
+    unsigned index = treeScope->ensureScopedStyleResolver().appendCSSStyleSheet(&cssSheet);
     addRulesFromSheet(cssSheet, treeScope, index);
 }
 
@@ -239,10 +236,9 @@ void StyleResolver::resetRuleFeatures()
 void StyleResolver::processScopedRules(const RuleSet& authorRules, CSSStyleSheet* parentStyleSheet, unsigned parentIndex, ContainerNode& scope)
 {
     const WillBeHeapVector<RawPtrWillBeMember<StyleRuleKeyframes> > keyframesRules = authorRules.keyframesRules();
-    ScopedStyleResolver* resolver = &scope.treeScope().ensureScopedStyleResolver();
-    document().styleEngine()->addScopedStyleResolver(resolver);
+    ScopedStyleResolver& resolver = scope.treeScope().ensureScopedStyleResolver();
     for (unsigned i = 0; i < keyframesRules.size(); ++i)
-        resolver->addKeyframeStyle(keyframesRules[i]);
+        resolver.addKeyframeStyle(keyframesRules[i]);
 
     m_treeBoundaryCrossingRules.addTreeBoundaryCrossingRules(authorRules, parentStyleSheet, parentIndex, scope);
 
@@ -263,14 +259,12 @@ void StyleResolver::resetAuthorStyle(TreeScope& treeScope)
         return;
 
     m_treeBoundaryCrossingRules.reset(&treeScope.rootNode());
-
     resolver->resetAuthorStyle();
     resetRuleFeatures();
     if (treeScope.rootNode().isDocumentNode())
         return;
 
     // resolver is going to be freed below.
-    document().styleEngine()->removeScopedStyleResolver(resolver);
     treeScope.clearScopedStyleResolver();
 }
 
@@ -401,8 +395,13 @@ void StyleResolver::matchAuthorRules(Element* element, ElementRuleCollector& col
     collector.clearMatchedRules();
     collector.matchedResult().ranges.lastAuthorRule = collector.matchedResult().matchedProperties.size() - 1;
 
-    if (document().styleEngine()->hasOnlyScopedResolverForDocument()) {
-        document().scopedStyleResolver()->collectMatchingAuthorRules(collector, includeEmptyRules, ignoreCascadeScope);
+    if (document().styleEngine()->onlyDocumentHasStyles()) {
+        ScopedStyleResolver* resolver = document().scopedStyleResolver();
+        if (!resolver)
+            return;
+        // If we have no resolver for some document, the document has no styles.

[kochi, 2014/11/26 07:28:37]

nit: s/for some document/for a document/ ?

[tasak, 2014/11/26 08:46:32]

Done.

+        // We don't need to see any rules (including treeboundary crossing ones).
+        resolver->collectMatchingAuthorRules(collector, includeEmptyRules, ignoreCascadeScope);
         m_treeBoundaryCrossingRules.collectTreeBoundaryCrossingRules(element, collector, includeEmptyRules);
         collector.sortAndTransferMatchedRules();
         return;
@@ -418,15 +417,14 @@ void StyleResolver::matchAuthorRules(Element* element, ElementRuleCollector& col
         return;
     }
 
-    if (resolvers.isEmpty())
-        return;
-
-    CascadeScope cascadeScope = 0;
-    CascadeOrder cascadeOrder = resolvers.size();
-    for (unsigned i = 0; i < resolvers.size(); ++i, --cascadeOrder) {
-        ScopedStyleResolver* resolver = resolvers.at(i);
-        // FIXME: Need to clarify how to treat style scoped.
-        resolver->collectMatchingAuthorRules(collector, includeEmptyRules, cascadeScope++, resolver->treeScope() == element->treeScope() && resolver->treeScope().rootNode().isShadowRoot() ? 0 : cascadeOrder);
+    if (!resolvers.isEmpty()) {

[kochi, 2014/11/26 07:28:37]

Why is this change needed?

[tasak, 2014/11/26 08:46:33]

Because this patch doesn't ensure document's scoped style resolver.

We should always create document's scoped style resolver in
StyleEngine::createResolver?

(The reason why we need the above "if(!resolver)" is the same.)

[kochi, 2014/11/26 09:12:07]

The difference is for the case resolvers.isEmpty(), line 430-431
will be executed.

So if resolvers.isEmpty() is true but there are shadow roots,
line 430 may collect some rules?

In that case, do we expect any regression or change about style
application?

[tasak, 2014/11/26 09:40:57]

Yes.
Yes. The following tests fail:
  fast/dom/shadow/content-pseudo-element-dynamic-attribute-change.html [
ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-for-shadow-element.html [
ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-match-all.html [ ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-match-descendant.html [
ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-nested.html [ ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-relative-selector.html [
ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-reprojection.html [ ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-specifiers-in-left-side.html [
ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-support-selector.html [
ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-used-in-selector-list.html [
ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element-with-any.html [ ImageOnlyFailure ]
  fast/dom/shadow/content-pseudo-element.html [ ImageOnlyFailure ]
 
fast/dom/shadow/no-style-sharing-with-uncommon-attribute-and-pseudo-content.html
[ ImageOnlyFailure ]
  fast/dom/shadow/style-sharing-with-content-and-host.html [ ImageOnlyFailure ]

+        CascadeScope cascadeScope = 0;
+        CascadeOrder cascadeOrder = resolvers.size();
+        for (unsigned i = 0; i < resolvers.size(); ++i, --cascadeOrder) {
+            ScopedStyleResolver* resolver = resolvers.at(i);
+            // FIXME: Need to clarify how to treat style scoped.
+            resolver->collectMatchingAuthorRules(collector, includeEmptyRules, cascadeScope++, resolver->treeScope() == element->treeScope() && resolver->treeScope().rootNode().isShadowRoot() ? 0 : cascadeOrder);
+        }
     }
 
     m_treeBoundaryCrossingRules.collectTreeBoundaryCrossingRules(element, collector, includeEmptyRules);