StyleResolver should update RuleSets lazily.

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 869d97d2f2d821f82cb675cb9feee93cbafc8015..2d43a3d733bc24c02d194f40564e3fa39dcee508 100644
--- a/Source/core/css/resolver/StyleResolver.cpp
+++ b/Source/core/css/resolver/StyleResolver.cpp
@@ -57,6 +57,8 @@
 #include "core/css/PageRuleCollector.h"
 #include "core/css/RuleSet.h"
 #include "core/css/StylePropertySet.h"
+#include "core/css/StyleRuleImport.h"
+#include "core/css/StyleSheetContents.h"
 #include "core/css/resolver/AnimatedStyleBuilder.h"
 #include "core/css/resolver/MatchResult.h"
 #include "core/css/resolver/MediaQueryResult.h"
@@ -139,6 +141,7 @@ StyleResolver::StyleResolver(Document& document, bool matchAuthorAndUserStyles)
     , m_fontSelector(CSSFontSelector::create(&document))
     , m_viewportStyleResolver(ViewportStyleResolver::create(&document))
     , m_styleResourceLoader(document.fetcher())
+    , m_needCollectFeatures(false)
 {
     Element* root = document.documentElement();
 
@@ -181,6 +184,84 @@ StyleResolver::StyleResolver(Document& document, bool matchAuthorAndUserStyles)
     styleSheetCollection->appendActiveAuthorStyleSheets(this);
 }
 
+bool rulesHaveViewportOrFontFaceRule(const Vector<RefPtr<StyleRuleBase> >& rules)

[esprehn, 2013/10/25 02:37:58]

These are missing static.

[tasak, 2013/10/25 10:10:10]

Yeah, thanks.
Done.

+{
+    for (unsigned i = 0; i < rules.size(); ++i) {
+        StyleRuleBase* rule = rules[i].get();
+        if (rule->isFontFaceRule() || rule->isViewportRule())
+            return true;
+    }
+    return false;
+}
+
+bool styleSheetHasViewportOrFontFaceRule(StyleSheetContents* sheet)

[esprehn, 2013/10/25 02:37:58]

static

[tasak, 2013/10/25 10:10:10]

Done.

+{
+    if (!sheet)
+        return false;
+
+    if (sheet->importRules().size() > 0) {
+        const Vector<RefPtr<StyleRuleImport> >& importRules = sheet->importRules();
+        for (unsigned i = 0; i < importRules.size(); ++i) {
+            StyleRuleImport* importRule = importRules[i].get();
+            if (styleSheetHasViewportOrFontFaceRule(importRule->styleSheet()))
+                return true;
+        }
+    }
+    return rulesHaveViewportOrFontFaceRule(sheet->childRules());
+}
+
+bool canAppendAuthorStyleSheetsLazily(unsigned firstNew, const Vector<RefPtr<CSSStyleSheet> >& styleSheets)

[esprehn, 2013/10/25 02:37:58]

static

[tasak, 2013/10/25 10:10:10]

Done.

+{
+    unsigned size = styleSheets.size();
+    for (unsigned i = firstNew; i < size; ++i) {
+        if (styleSheetHasViewportOrFontFaceRule(styleSheets[i]->contents()))
+            return false;
+    }
+    return true;
+}
+
+void StyleResolver::lazyAppendAuthorStyleSheets(unsigned firstNew, const Vector<RefPtr<CSSStyleSheet> >& styleSheets)
+{
+    // We cannot lazy apppend stylesheets if the sheets have @viewport and @font-face rules.

[esprehn, 2013/10/25 02:37:58]

extra p in append. Also why can't we lazy append them? We should fix the system
to always allow lazy append (in a future patch)

[tasak, 2013/10/25 10:10:10]

Yeah, I agree that we should solve in a future patch. I added FIXME.

Currently I have no good solutions for @viewport and @font-face rules.

Talking about @viewport rules, I found that the following tests fail:

css3/device-adapt/viewport-height-extend-to-zoom.html
css3/device-adapt/viewport-min-width-extend-to-zoom.html
css3/device-adapt/viewport-width-extend-to-zoom.html

I couldn't find any good place to update viewport style resolver.

Talking about @font-face, I'm afraid that font-loading becomes slow.
I think, "addFontFaceRule" triggers font-loading.
If we delays, we probably need to wait more time until fonts are loaded.

For example, fast/loader/resource-request-callbacks.html fails:

-main frame - css requested 'gw432047.ttf'
 main frame - img requested 'dice.png'
 main frame - img requested 'dice.png'
 main frame - img requested 'not-existing.png'
 main frame - script requested 'js-test-post.js'
+main frame - css requested 'gw432047.ttf'

The test has @font-face rule which needs to load gw432047.ttf.

+    bool canLazyAppend = canAppendAuthorStyleSheetsLazily(firstNew, styleSheets);
+
+    unsigned size = styleSheets.size();
+    for (unsigned i = firstNew; i < size; ++i)
+        m_pendingStyleSheets.add(styleSheets[i].get());
+
+    if (!canLazyAppend)
+        appendPendingAuthorStyleSheets();
+}
+
+void StyleResolver::removePendingAuthorStyleSheets(const Vector<RefPtr<CSSStyleSheet> >& styleSheets)
+{
+    for (unsigned i = i; i < styleSheets.size(); ++i)
+        m_pendingStyleSheets.remove(styleSheets[i].get());
+}
+
+void StyleResolver::appendPendingAuthorStyleSheets()
+{
+    setBuildScopedStyleTreeInDocumentOrder(false);
+    for (ListHashSet<CSSStyleSheet*, 16>::iterator it = m_pendingStyleSheets.begin(); it != m_pendingStyleSheets.end(); ++it) {
+        CSSStyleSheet* cssSheet = *it;
+        ASSERT(!cssSheet->disabled());
+        if (cssSheet->mediaQueries() && !m_medium->eval(cssSheet->mediaQueries(), &m_viewportDependentMediaQueryResults))
+            continue;
+
+        StyleSheetContents* sheet = cssSheet->contents();
+        const ContainerNode* scopingNode = ScopedStyleResolver::scopingNodeFor(cssSheet);
+        if (!scopingNode && cssSheet->ownerNode() && cssSheet->ownerNode()->isInShadowTree())
+            continue;

[esprehn, 2013/10/25 02:37:58]

What's this case for?

[tasak, 2013/10/25 10:10:10]

c.f. fast/dom/shadow/remove-styles-in-shadow-crash-2.html

I would like to explain the test.

Suppose that we have a shadow tree which has more than two style elements, e.g.

div#host
    #shadow-root
        div#A
            style#B
              ...
            style#C
              ...
        ...

Now we are going to remove div#A. In this case, style#B and style#C are also
removed via ChildNodeRemovalNotifier.

So when removing style#B, StyleElement::removedFromDocument is invoked.
StyleEngine removes style#B from active stylesheets, and update active
stylesheets.

But style#C (being destroyed) is still in the active stylesheets.
style#C's inDocument() is still true, ownerNode() is not null, but style#C's
containingShadowRoot() is 0.

So style#C's rules are inserted into document's ruleset. This causes crash in
SelectorChecker.

I would like to refactor "ScopedStyleResolver::scopingNodeFor".
The method returns 0 even if a given StyleSheetContents is invalid. This is very
bad behavior.
(I think, containingShadowRoot() == 0 means that the given StyleSheetContents is
invalid)

+
+        ScopedStyleResolver* resolver = ensureScopedStyleResolver(scopingNode);
+        ASSERT(resolver);
+        resolver->addRulesFromSheet(sheet, *m_medium, this);
+        m_inspectorCSSOMWrappers.collectFromStyleSheetIfNeeded(cssSheet);
+    }
+    m_pendingStyleSheets.clear();
+    finishAppendAuthorStyleSheets();
+}
+
 void StyleResolver::appendAuthorStyleSheets(unsigned firstNew, const Vector<RefPtr<CSSStyleSheet> >& styleSheets)
 {
     // This handles sheets added to the end of the stylesheet list only. In other cases the style resolver
@@ -212,6 +293,17 @@ void StyleResolver::finishAppendAuthorStyleSheets()
         document().renderer()->style()->font().update(fontSelector());
 
     collectViewportRules();
+
+    document().styleEngine()->resetCSSFeatureFlags(ruleFeatureSet());
+}
+
+void StyleResolver::resetRuleFeatures()
+{
+    // Need to recreate RuleFeatureSet.
+    m_features.clear();
+    m_siblingRuleSet.clear();
+    m_uncommonAttributeRuleSet.clear();
+    m_needCollectFeatures = true;
 }
 
 void StyleResolver::resetAuthorStyle(const ContainerNode* scopingNode)
@@ -225,6 +317,7 @@ void StyleResolver::resetAuthorStyle(const ContainerNode* scopingNode)
     m_ruleSets.shadowDistributedRules().reset(scopingNode);
 
     resolver->resetAuthorStyle();
+    resetRuleFeatures();
     if (!scopingNode)
         return;
 
@@ -275,6 +368,7 @@ void StyleResolver::collectFeatures()
 
     m_siblingRuleSet = makeRuleSet(m_features.siblingRules);
     m_uncommonAttributeRuleSet = makeRuleSet(m_features.uncommonAttributeRules);
+    m_needCollectFeatures = false;
 }
 
 bool StyleResolver::hasRulesForId(const AtomicString& id) const
@@ -601,6 +695,8 @@ PassRefPtr<RenderStyle> StyleResolver::styleForElement(Element* element, RenderS
 {
     ASSERT(document().frame());
     ASSERT(documentSettings());
+    ASSERT(!hasPendingAuthorStyleSheets());
+    ASSERT(!m_needCollectFeatures);
 
     // Once an element has a renderer, we don't try to destroy it, since otherwise the renderer
     // will vanish if a style recalc happens during loading.
@@ -697,6 +793,7 @@ PassRefPtr<RenderStyle> StyleResolver::styleForKeyframe(Element* e, const Render
 {
     ASSERT(document().frame());
     ASSERT(documentSettings());
+    ASSERT(!hasPendingAuthorStyleSheets());
 
     if (e == document().documentElement())
         resetDirectionAndWritingModeOnDocument(document());
@@ -1041,6 +1138,7 @@ PassRefPtr<RenderStyle> StyleResolver::pseudoStyleForElement(Element* e, const P
 
 PassRefPtr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
 {
+    ASSERT(!hasPendingAuthorStyleSheets());
     resetDirectionAndWritingModeOnDocument(document());
     StyleResolverState state(document(), document().documentElement()); // m_rootElementStyle will be set to the document style.