Move StyleSheet to the oilpan heap using transition types.

Source/core/css/StyleSheetContents.cpp

Index: Source/core/css/StyleSheetContents.cpp
diff --git a/Source/core/css/StyleSheetContents.cpp b/Source/core/css/StyleSheetContents.cpp
index 034d2269e30885a7477558826597a10a327a5cfb..d70a19152d456be4653fff60cb51c0ea7bcc2702 100644
--- a/Source/core/css/StyleSheetContents.cpp
+++ b/Source/core/css/StyleSheetContents.cpp
@@ -374,8 +374,8 @@ bool StyleSheetContents::loadCompleted() const
         return parentSheet->loadCompleted();
 
     StyleSheetContents* root = rootStyleSheet();
-    for (unsigned i = 0; i < root->m_clients.size(); ++i) {
-        if (!root->m_clients[i]->loadCompleted())
+    for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it) {
+        if (!(*it)->loadCompleted())
             return false;
     }
     return true;
@@ -401,18 +401,23 @@ void StyleSheetContents::checkLoaded()
     if (root->m_clients.isEmpty())
         return;
 
-    Vector<CSSStyleSheet*> clients(root->m_clients);
-    for (unsigned i = 0; i < clients.size(); ++i) {
-        // Avoid |CSSSStyleSheet| and |ownerNode| being deleted by scripts that run via
-        // ScriptableDocumentParser::executeScriptsWaitingForResources().
-        RefPtr<CSSStyleSheet> protectClient(clients[i]);
-
-        if (clients[i]->loadCompleted())
+    // Avoid |CSSSStyleSheet| and |ownerNode| being deleted by scripts that run via

[Mads Ager (chromium), 2014/03/05 11:16:36]

The protection during iteration has changed here. The protection that was going
on here was shaky. It turned out that the last sheet in all layout tests was the
only one that had an owner and therefore the old protection worked. That is
pretty funky. Instead of protecting each element one at a time during the
iteration, we protect them all up front and let them die after the iteration.
That seems saner in any case in the current system and can be removed once we
ship with oilpan.

+    // ScriptableDocumentParser::executeScriptsWaitingForResources().
+    //
+    // FIXME: oilpan: with oilpan we do not need to protect the CSSStyleSheets
+    // explicitly during the iteration. The iterator will make the pointers
+    // strong during iteration so we can just directly iterate root->m_clients.
+    WillBeHeapHashSet<RefPtrWillBeMember<CSSStyleSheet> > protectedClients;

[Erik Corry, 2014/03/05 11:33:29]

I think this can be a vector, then you don't have to rehash and it makes the
iteration a little simpler.

Also you need to revisit the comment.

[Mads Ager (chromium), 2014/03/05 11:36:04]

Good idea, that will simplify. I have replaced it with a Vector with inline
capacity 5 and I then iterate that. Thanks.

+    for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it)
+        protectedClients.add(*it);
+
+    for (WillBeHeapHashSet<RefPtrWillBeMember<CSSStyleSheet> >::iterator it = protectedClients.begin(); it != protectedClients.end(); ++it) {
+        if ((*it)->loadCompleted())
             continue;
 
         // sheetLoaded might be invoked after its owner node is removed from document.
-        if (RefPtr<Node> ownerNode = clients[i]->ownerNode()) {
-            if (clients[i]->sheetLoaded())
+        if (RefPtr<Node> ownerNode = (*it)->ownerNode()) {
+            if ((*it)->sheetLoaded())
                 ownerNode->notifyLoadedSheetAndAllCriticalSubresources(m_didLoadErrorOccur);
         }
     }
@@ -431,8 +436,8 @@ void StyleSheetContents::notifyLoadedSheet(const CSSStyleSheetResource* sheet)
 void StyleSheetContents::startLoadingDynamicSheet()
 {
     StyleSheetContents* root = rootStyleSheet();
-    for (unsigned i = 0; i < root->m_clients.size(); ++i)
-        root->m_clients[i]->startLoadingDynamicSheet();
+    for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it)
+        (*it)->startLoadingDynamicSheet();
 }
 
 StyleSheetContents* StyleSheetContents::rootStyleSheet() const
@@ -457,7 +462,7 @@ Node* StyleSheetContents::singleOwnerNode() const
     if (root->m_clients.isEmpty())
         return 0;
     ASSERT(root->m_clients.size() == 1);
-    return root->m_clients[0]->ownerNode();
+    return (*root->m_clients.begin())->ownerNode();
 }
 
 Document* StyleSheetContents::singleOwnerDocument() const
@@ -519,14 +524,13 @@ StyleSheetContents* StyleSheetContents::parentStyleSheet() const
 void StyleSheetContents::registerClient(CSSStyleSheet* sheet)
 {
     ASSERT(!m_clients.contains(sheet));
-    m_clients.append(sheet);
+    m_clients.add(sheet);
 }
 
 void StyleSheetContents::unregisterClient(CSSStyleSheet* sheet)
 {
-    size_t position = m_clients.find(sheet);
-    ASSERT(position != kNotFound);
-    m_clients.remove(position);
+    ASSERT(m_clients.contains(sheet));
+    m_clients.remove(sheet);
 }
 
 void StyleSheetContents::addedToMemoryCache()
@@ -571,8 +575,8 @@ void StyleSheetContents::clearRuleSet()
 
     // Clearing the ruleSet means we need to recreate the styleResolver data structures.
     // See the StyleResolver calls in ScopedStyleResolver::addRulesFromSheet.
-    for (size_t i = 0; i < m_clients.size(); ++i) {
-        if (Document* document = m_clients[i]->ownerDocument())
+    for (ClientsIterator it = m_clients.begin(); it != m_clients.end(); ++it) {
+        if (Document* document = (*it)->ownerDocument())
             document->styleEngine()->clearResolver();
     }
     m_ruleSet.clear();
@@ -582,8 +586,8 @@ void StyleSheetContents::notifyRemoveFontFaceRule(const StyleRuleFontFace* fontF
 {
     StyleSheetContents* root = rootStyleSheet();
 
-    for (unsigned i = 0; i < root->m_clients.size(); ++i) {
-        if (Node* ownerNode = root->m_clients[0]->ownerNode())
+    for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it) {
+        if (Node* ownerNode = (*it)->ownerNode())
             ownerNode->document().styleEngine()->removeFontFaceRules(WillBeHeapVector<RawPtrWillBeMember<const StyleRuleFontFace> >(1, fontFaceRule));
     }
 }
@@ -620,6 +624,7 @@ void StyleSheetContents::trace(Visitor* visitor)
     visitor->trace(m_ownerRule);
     visitor->trace(m_importRules);
     visitor->trace(m_childRules);
+    visitor->trace(m_clients);
 }
 
 }