WIP: Move StyleEngine::m_activeTreeScopes to TreeScope::m_childTreeScopesWithActiveStyleSheets

Source/core/dom/StyleEngine.cpp

Index: Source/core/dom/StyleEngine.cpp
diff --git a/Source/core/dom/StyleEngine.cpp b/Source/core/dom/StyleEngine.cpp
index 6788ad0e364aaf4e997eedbf224e9ab7bb6cecec..9d2381eb067c284ede1b322d9fd42f3049f88742 100644
--- a/Source/core/dom/StyleEngine.cpp
+++ b/Source/core/dom/StyleEngine.cpp
@@ -100,7 +100,6 @@ void StyleEngine::detachFromDocument()
     m_fontSelector.clear();
     m_resolver.clear();
     m_styleSheetCollectionMap.clear();
-    m_activeTreeScopes.clear();
 }
 #endif
 
@@ -114,66 +113,6 @@ inline Document* StyleEngine::master()
     return import->master();
 }
 
-void StyleEngine::OrderedTreeScopeSet::insert(TreeScope* treeScope)
-{
-    if (m_treeScopes.isEmpty()) {
-        m_treeScopes.append(treeScope);
-        m_hash.add(treeScope);
-        return;
-    }
-    if (m_hash.contains(treeScope))
-        return;
-
-    int end = m_treeScopes.size() - 1;
-    int start = 0;
-    int position = 0;
-    unsigned result = 0;
-
-    while (start <= end) {
-        position = (start + end) / 2;
-        result = m_treeScopes[position]->comparePosition(*treeScope);
-
-        if (result & Node::DOCUMENT_POSITION_PRECEDING) {
-            end = position - 1;
-        } else {
-            ASSERT(result & Node::DOCUMENT_POSITION_FOLLOWING);
-            start = position + 1;
-        }
-    }
-
-    if (result & Node::DOCUMENT_POSITION_FOLLOWING) {
-        ++position;
-        ASSERT(static_cast<size_t>(position) == m_treeScopes.size() || (m_treeScopes[position]->comparePosition(*treeScope) & Node::DOCUMENT_POSITION_PRECEDING));
-    }
-    m_treeScopes.insert(position, treeScope);
-    m_hash.add(treeScope);
-
-#if ENABLE(ASSERT)
-    // Check whether m_treeScopes is sorted in document order or not.
-    for (unsigned i = 0; i < m_treeScopes.size() - 1; ++i) {
-        unsigned result = m_treeScopes[i]->comparePosition(*m_treeScopes[i + 1]);
-        ASSERT(result & Node::DOCUMENT_POSITION_FOLLOWING);
-    }
-#endif
-}
-
-void StyleEngine::OrderedTreeScopeSet::remove(TreeScope* treeScope)
-{
-    if (!m_hash.contains(treeScope))
-        return;
-    size_t position = m_treeScopes.find(treeScope);
-    m_treeScopes.remove(position);
-    m_hash.remove(treeScope);
-}
-
-DEFINE_TRACE(StyleEngine::OrderedTreeScopeSet)
-{
-#if ENABLE(OILPAN)
-    visitor->trace(m_treeScopes);
-    visitor->trace(m_hash);
-#endif
-}
-
 TreeScopeStyleSheetCollection* StyleEngine::ensureStyleSheetCollectionFor(TreeScope& treeScope)
 {
     if (treeScope == m_document)
@@ -287,7 +226,7 @@ void StyleEngine::addStyleSheetCandidateNode(Node* node, bool createdByParser)
 
     markTreeScopeDirty(treeScope);
     if (treeScope != m_document)
-        m_activeTreeScopes.insert(&treeScope);
+        treeScope.insertToActiveStyleSheetsTraversal();
 }
 
 void StyleEngine::removeStyleSheetCandidateNode(Node* node)
@@ -336,22 +275,18 @@ bool StyleEngine::shouldUpdateShadowTreeStyleSheetCollection(StyleResolverUpdate
     return !m_dirtyTreeScopes.isEmpty() || updateMode == FullStyleUpdate;
 }
 
-void StyleEngine::clearMediaQueryRuleSetOnTreeScopeStyleSheets(UnorderedTreeScopeSet::iterator begin, UnorderedTreeScopeSet::iterator end)
+void StyleEngine::clearMediaQueryRuleSetStyleSheets()
 {
-    for (UnorderedTreeScopeSet::iterator it = begin; it != end; ++it) {
-        TreeScope& treeScope = **it;
-        ASSERT(treeScope != m_document);
-        ShadowTreeStyleSheetCollection* collection = static_cast<ShadowTreeStyleSheetCollection*>(styleSheetCollectionFor(treeScope));
+    for (TreeScope* treeScope : TreeScope::TreeScopesWithActiveStyleSheetsTraversal(document())) {
+        TreeScopeStyleSheetCollection* collection = styleSheetCollectionFor(*treeScope);
+        ASSERT(collection);
+        collection->clearMediaQueryRuleSetStyleSheets();
+    }
+    for (TreeScope* treeScope : m_dirtyTreeScopes) {
+        TreeScopeStyleSheetCollection* collection = styleSheetCollectionFor(*treeScope);
         ASSERT(collection);
         collection->clearMediaQueryRuleSetStyleSheets();
     }
-}
-
-void StyleEngine::clearMediaQueryRuleSetStyleSheets()
-{
-    documentStyleSheetCollection()->clearMediaQueryRuleSetStyleSheets();
-    clearMediaQueryRuleSetOnTreeScopeStyleSheets(m_activeTreeScopes.beginUnordered(), m_activeTreeScopes.endUnordered());
-    clearMediaQueryRuleSetOnTreeScopeStyleSheets(m_dirtyTreeScopes.begin(), m_dirtyTreeScopes.end());
 }
 
 void StyleEngine::updateStyleSheetsInImport(DocumentStyleSheetCollector& parentCollector)
@@ -363,14 +298,15 @@ void StyleEngine::updateStyleSheetsInImport(DocumentStyleSheetCollector& parentC
     documentStyleSheetCollection()->swapSheetsForSheetList(sheetsForList);
 }
 
-void StyleEngine::updateActiveStyleSheetsInShadow(StyleResolverUpdateMode updateMode, TreeScope* treeScope, UnorderedTreeScopeSet& treeScopesRemoved)
+void StyleEngine::updateActiveStyleSheetsInShadow(StyleResolverUpdateMode updateMode, TreeScope* treeScope, TreeScope::UnorderedTreeScopeSet& treeScopesRemoved)
 {
     ASSERT(treeScope != m_document);
     ShadowTreeStyleSheetCollection* collection = static_cast<ShadowTreeStyleSheetCollection*>(styleSheetCollectionFor(*treeScope));
     ASSERT(collection);
     collection->updateActiveStyleSheets(*this, updateMode);
     if (!collection->hasStyleSheetCandidateNodes()) {
-        treeScopesRemoved.add(treeScope);
+        if (treeScope->isInActiveStyleSheetsTraversal() && !treeScope->hasChildTreeScopesWithActiveStyleSheets())

[kochi, 2015/05/26 01:52:34]

What is this addition?

[kojii, 2015/05/26 05:21:58]

Before the patch, removing treeScopes that are not in m_activeTreeScopes is
simply ignored.

With this patch, TreeScope::removeFromActiveStyleSheetsTraversal has
ASSERT(m_inActiveStyleSheetsTraversal). Another change is that treeScopes
without active stylesheet but has children with active stylesheet need to be
included.

+            treeScopesRemoved.add(treeScope);
         // When removing TreeScope from ActiveTreeScopes,
         // its resolver should be destroyed by invoking resetAuthorStyle.
         ASSERT(!treeScope->scopedStyleResolver());
@@ -389,18 +325,17 @@ void StyleEngine::updateActiveStyleSheets(StyleResolverUpdateMode updateMode)
         documentStyleSheetCollection()->updateActiveStyleSheets(*this, updateMode);
 
     if (shouldUpdateShadowTreeStyleSheetCollection(updateMode)) {
-        UnorderedTreeScopeSet treeScopesRemoved;
-
+        TreeScope::UnorderedTreeScopeSet treeScopesRemoved;
         if (updateMode == FullStyleUpdate) {
-            for (unsigned i = 0; i < m_activeTreeScopes.size(); ++i)
-                updateActiveStyleSheetsInShadow(updateMode, m_activeTreeScopes[i], treeScopesRemoved);
+            for (TreeScope* treeScope : TreeScope::TreeScopesWithActiveStyleSheetsTraversal(document().childTreeScopesWithActiveStyleSheets()))
+                updateActiveStyleSheetsInShadow(updateMode, treeScope, treeScopesRemoved);
         } else {
-            for (UnorderedTreeScopeSet::iterator it = m_dirtyTreeScopes.begin(); it != m_dirtyTreeScopes.end(); ++it) {
-                updateActiveStyleSheetsInShadow(updateMode, *it, treeScopesRemoved);
+            for (TreeScope* treeScope : m_dirtyTreeScopes) {
+                updateActiveStyleSheetsInShadow(updateMode, treeScope, treeScopesRemoved);
             }
         }
-        for (UnorderedTreeScopeSet::iterator it = treeScopesRemoved.begin(); it != treeScopesRemoved.end(); ++it)
-            m_activeTreeScopes.remove(*it);
+        for (TreeScope* removed : treeScopesRemoved)
+            removed->removeFromActiveStyleSheetsTraversal();

[kochi, 2015/05/26 01:52:34]

what removed points after returning from the function?

[kojii, 2015/05/26 05:21:58]

This does not destruct the pointer, just remove from the traversal list. Along
with another feedback from you, I've renamed the method to make it clearer.

     }
 
     InspectorInstrumentation::activeStyleSheetsUpdated(m_document);
@@ -410,17 +345,14 @@ void StyleEngine::updateActiveStyleSheets(StyleResolverUpdateMode updateMode)
     m_documentScopeDirty = false;
 }
 
-const WillBeHeapVector<RefPtrWillBeMember<CSSStyleSheet>> StyleEngine::activeStyleSheetsForInspector() const
+const WillBeHeapVector<RefPtrWillBeMember<CSSStyleSheet>> StyleEngine::activeStyleSheetsForInspector()
 {
-    if (m_activeTreeScopes.isEmpty())
+    if (!document().hasChildTreeScopesWithActiveStyleSheets())
         return documentStyleSheetCollection()->activeAuthorStyleSheets();
 
     WillBeHeapVector<RefPtrWillBeMember<CSSStyleSheet>> activeStyleSheets;
-
-    activeStyleSheets.appendVector(documentStyleSheetCollection()->activeAuthorStyleSheets());
-    for (unsigned i = 0; i < m_activeTreeScopes.size(); ++i) {
-        TreeScope* treeScope = const_cast<TreeScope*>(m_activeTreeScopes[i]);
-        if (TreeScopeStyleSheetCollection* collection = m_styleSheetCollectionMap.get(treeScope))
+    for (TreeScope* treeScope : TreeScope::TreeScopesWithActiveStyleSheetsTraversal(document())) {
+        if (TreeScopeStyleSheetCollection* collection = styleSheetCollectionFor(*treeScope))
             activeStyleSheets.appendVector(collection->activeAuthorStyleSheets());
     }
 
@@ -433,7 +365,6 @@ const WillBeHeapVector<RefPtrWillBeMember<CSSStyleSheet>> StyleEngine::activeSty
 void StyleEngine::didRemoveShadowRoot(ShadowRoot* shadowRoot)
 {
     m_styleSheetCollectionMap.remove(shadowRoot);
-    m_activeTreeScopes.remove(shadowRoot);
     m_dirtyTreeScopes.remove(shadowRoot);
 }
 
@@ -446,7 +377,6 @@ void StyleEngine::shadowRootRemovedFromDocument(ShadowRoot* shadowRoot)
             styleResolver->removePendingAuthorStyleSheets(collection->activeAuthorStyleSheets());
     }
     m_styleSheetCollectionMap.remove(shadowRoot);
-    m_activeTreeScopes.remove(shadowRoot);
     m_dirtyTreeScopes.remove(shadowRoot);
 }
 
@@ -454,9 +384,8 @@ void StyleEngine::appendActiveAuthorStyleSheets()
 {
     ASSERT(isMaster());
 
-    m_resolver->appendAuthorStyleSheets(documentStyleSheetCollection()->activeAuthorStyleSheets());
-    for (unsigned i = 0; i < m_activeTreeScopes.size(); ++i) {
-        if (TreeScopeStyleSheetCollection* collection = m_styleSheetCollectionMap.get(m_activeTreeScopes[i]))
+    for (TreeScope* treeScope : TreeScope::TreeScopesWithActiveStyleSheetsTraversal(document())) {
+        if (TreeScopeStyleSheetCollection* collection = styleSheetCollectionFor(*treeScope))
             m_resolver->appendAuthorStyleSheets(collection->activeAuthorStyleSheets());
     }
     m_resolver->finishAppendAuthorStyleSheets();
@@ -483,7 +412,6 @@ void StyleEngine::clearResolver()
     ASSERT(!document().inStyleRecalc());
     ASSERT(isMaster() || !m_resolver);
 
-    document().clearScopedStyleResolver();

[kochi, 2015/05/26 01:52:34]

Where did this go?

[kojii, 2015/05/26 05:21:58]

The loop below traverses document() and its descendants, so we no longer need to
call document() separately and then m_activeTreeScopes.

     // StyleEngine::shadowRootRemovedFromDocument removes not-in-document
     // treescopes from activeTreeScopes. StyleEngine::didRemoveShadowRoot
     // removes treescopes which are being destroyed from activeTreeScopes.
@@ -491,8 +419,8 @@ void StyleEngine::clearResolver()
     // just removed from document. If document is destroyed before invoking
     // updateActiveStyleSheets, the treescope has a scopedStyleResolver which
     // has destroyed StyleSheetContents.
-    for (UnorderedTreeScopeSet::iterator it = m_activeTreeScopes.beginUnordered(); it != m_activeTreeScopes.endUnordered(); ++it)
-        (*it)->clearScopedStyleResolver();
+    for (TreeScope* treeScope : TreeScope::TreeScopesWithActiveStyleSheetsTraversal(document()))
+        treeScope->clearScopedStyleResolver();
 
     m_resolver.clear();
 }
@@ -658,10 +586,7 @@ void StyleEngine::removeSheet(StyleSheetContents* contents)
 void StyleEngine::collectScopedStyleFeaturesTo(RuleFeatureSet& features) const
 {
     HashSet<const StyleSheetContents*> visitedSharedStyleSheetContents;
-    if (document().scopedStyleResolver())
-        document().scopedStyleResolver()->collectFeaturesTo(features, visitedSharedStyleSheetContents);
-    for (unsigned i = 0; i < m_activeTreeScopes.size(); ++i) {
-        TreeScope* treeScope = const_cast<TreeScope*>(m_activeTreeScopes[i]);
+    for (TreeScope* treeScope : TreeScope::TreeScopesWithActiveStyleSheetsTraversal(document())) {
         // When creating StyleResolver, dirty treescopes might not be processed.
         // So some active treescopes might not have a scoped style resolver.
         // In this case, we should skip collectFeatures for the treescopes without
@@ -790,7 +715,6 @@ DEFINE_TRACE(StyleEngine)
     visitor->trace(m_resolver);
     visitor->trace(m_styleInvalidator);
     visitor->trace(m_dirtyTreeScopes);
-    visitor->trace(m_activeTreeScopes);
     visitor->trace(m_fontSelector);
     visitor->trace(m_textToSheetCache);
     visitor->trace(m_sheetToTextCache);