Removed add/removeScopedStyleResolver from StyleEngine

Source/core/css/resolver/StyleResolver.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 2004-2005 Allan Sandfeld Jensen (kde@carewolf.com)
  * Copyright (C) 2006, 2007 Nicholas Shanks (webkit@nickshanks.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2007 Alexey Proskuryakov <ap@webkit.org>
  * Copyright (C) 2007, 2008 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (c) 2011, Code Aurora Forum. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
@@ skipping 163 matching lines @@
 void StyleResolver::appendCSSStyleSheet(CSSStyleSheet& cssSheet)
 {
     ASSERT(!cssSheet.disabled());
     if (cssSheet.mediaQueries() && !m_medium->eval(cssSheet.mediaQueries(), &m_viewportDependentMediaQueryResults))
         return;
 
     TreeScope* treeScope = ScopedStyleResolver::treeScopeFor(document(), &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);
 }
 
 void StyleResolver::addRulesFromSheet(CSSStyleSheet& cssSheet, TreeScope* treeScope, unsigned index)
 {
     StyleSheetContents* sheet = cssSheet.contents();
     AddRuleFlags addRuleFlags = document().securityOrigin()->canRequest(sheet->baseURL()) ? RuleHasDocumentSecurityOrigin : RuleHasNoSpecialState;
     const RuleSet& ruleSet = sheet->ensureRuleSet(*m_medium, addRuleFlags);
 
     addMediaQueryResults(ruleSet.viewportDependentMediaQueryResults());
@@ skipping 34 matching lines @@
     // Need to recreate RuleFeatureSet.
     m_features.clear();
     m_siblingRuleSet.clear();
     m_uncommonAttributeRuleSet.clear();
     m_needCollectFeatures = true;
 }
 
 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);
 
     // FIXME(BUG 72461): We don't add @font-face rules of scoped style sheets for the moment.
     if (scope.isDocumentNode()) {
         const WillBeHeapVector<RawPtrWillBeMember<StyleRuleFontFace> > fontFaceRules = authorRules.fontFaceRules();
         for (unsigned i = 0; i < fontFaceRules.size(); ++i)
             addFontFaceRule(m_document, document().styleEngine()->fontSelector(), fontFaceRules[i]);
         if (fontFaceRules.size())
             invalidateMatchedPropertiesCache();
     }
 }
 
 void StyleResolver::resetAuthorStyle(TreeScope& treeScope)
 {
     ScopedStyleResolver* resolver = treeScope.scopedStyleResolver();
     if (!resolver)
         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();
 }
 
 static PassOwnPtrWillBeRawPtr<RuleSet> makeRuleSet(const WillBeHeapVector<RuleFeature>& rules)
 {
     size_t size = rules.size();
     if (!size)
         return nullptr;
     OwnPtrWillBeRawPtr<RuleSet> ruleSet = RuleSet::create();
     for (size_t i = 0; i < size; ++i)
@@ skipping 110 matching lines @@
 
     m_treeBoundaryCrossingRules.collectTreeBoundaryCrossingRules(element, collector, includeEmptyRules);
     collector.sortAndTransferMatchedRules();
 }
 
 void StyleResolver::matchAuthorRules(Element* element, ElementRuleCollector& collector, bool includeEmptyRules)
 {
     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;
     }
 
     WillBeHeapVector<RawPtrWillBeMember<ScopedStyleResolver>, 8> resolvers;
     resolveScopedStyles(element, resolvers);
 
     WillBeHeapVector<RawPtrWillBeMember<ScopedStyleResolver>, 8> resolversInShadowTree;
     collectScopedResolversForHostedShadowTrees(element, resolversInShadowTree);
     if (!resolversInShadowTree.isEmpty()) {
         matchAuthorRulesForShadowHost(element, collector, includeEmptyRules, resolvers, resolversInShadowTree);
         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);
     collector.sortAndTransferMatchedRules();
 }
 
 void StyleResolver::matchUARules(ElementRuleCollector& collector)
 {
     collector.setMatchingUARules(true);
 
@@ skipping 1175 matching lines @@
     visitor->trace(m_uncommonAttributeRuleSet);
     visitor->trace(m_watchedSelectorsRules);
     visitor->trace(m_treeBoundaryCrossingRules);
     visitor->trace(m_styleSharingLists);
     visitor->trace(m_pendingStyleSheets);
     visitor->trace(m_document);
 #endif
 }
 
 } // namespace blink