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..77969051ed2552f1d011dd0d4618d678a01a2347 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,25 @@ 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
+    // ScriptableDocumentParser::executeScriptsWaitingForResources(). Also protect

[haraken, 2014/03/05 14:21:22]

FYI, I'm planning to make a change that moves
ScriptableDocumentParser::executeScriptsWaitingForResources() to the next event
loop (to fix another protection bug). It will enable us to remove the
protection.

[Mads Ager (chromium), 2014/03/05 14:51:16]

The protection here is not only because of executeScriptsWaitingForResources. It
is also because the protectedClients[i]->sheetLoaded() call on line 422 can lead
to CSSStyleSheets reaching ref count zero. That was the problem when I landed
this change the first time.

+    // the |CSSStyleSheet| from being deleted during iteration via the |sheetLoaded|
+    // method.
+    //
+    // 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.
+    WillBeHeapVector<RefPtrWillBeMember<CSSStyleSheet>, 5> protectedClients;
+    for (ClientsIterator it = root->m_clients.begin(); it != root->m_clients.end(); ++it)
+        protectedClients.append(*it);
+
+    for (unsigned i = 0; i < protectedClients.size(); ++i) {
+        if (protectedClients[i]->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 = protectedClients[i]->ownerNode()) {
+            if (protectedClients[i]->sheetLoaded())
                 ownerNode->notifyLoadedSheetAndAllCriticalSubresources(m_didLoadErrorOccur);
         }
     }
@@ -431,8 +438,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 +464,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 +526,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 +577,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 +588,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 +626,7 @@ void StyleSheetContents::trace(Visitor* visitor)
     visitor->trace(m_ownerRule);
     visitor->trace(m_importRules);
     visitor->trace(m_childRules);
+    visitor->trace(m_clients);
 }
 
 }