2010-12-13 Anton Muhin <antonm@chromium.org>
authorantonm@chromium.org <antonm@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Dec 2010 13:34:00 +0000 (13:34 +0000)
committerantonm@chromium.org <antonm@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Dec 2010 13:34:00 +0000 (13:34 +0000)
        Reviewed by Pavel Feldman.

        [v8] More CSS wrappers GC work: keep document's style sheets, style sheet and css rule lists' items in proper object groups
        https://bugs.webkit.org/show_bug.cgi?id=50828

        * bindings/v8/DOMData.h:
        (WebCore::DOMData::WrapperMapObjectRemover::visitDOMWrapper):
        (WebCore::DOMData::removeObjectsFromWrapperMap):
        * bindings/v8/DOMDataStore.h:
        (WebCore::ChunkedTable::visit):
        (WebCore::ChunkedTable::visitEntries):
        (WebCore::DOMDataStore::IntrusiveDOMWrapperMap::visit):
        (WebCore::DOMDataStore::IntrusiveDOMWrapperMap::ChunkedTableTraits::visit):
        * bindings/v8/V8DOMMap.cpp:
        (WebCore::removeAllDOMObjectsInCurrentThread):
        (WebCore::visitDOMNodesInCurrentThread):
        (WebCore::visitDOMObjectsInCurrentThread):
        (WebCore::visitActiveDOMObjectsInCurrentThread):
        (WebCore::visitDOMSVGElementInstancesInCurrentThread):
        * bindings/v8/V8DOMMap.h:
        (WebCore::WeakReferenceMap::visit):
        * bindings/v8/V8GCController.cpp:
        (WebCore::DOMObjectVisitor::visitDOMWrapper):
        (WebCore::EnsureWeakDOMNodeVisitor::visitDOMWrapper):
        (WebCore::GCPrologueVisitor::visitDOMWrapper):
        (WebCore::NodeGrouperVisitor::visitDOMWrapper):
        (WebCore::NodeGrouperVisitor::applyGrouping):
        (WebCore::DOMObjectGrouperVisitor::visitDOMWrapper):
        (WebCore::GCEpilogueVisitor::visitDOMWrapper):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@73907 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bindings/v8/DOMData.h
WebCore/bindings/v8/DOMDataStore.h
WebCore/bindings/v8/V8DOMMap.cpp
WebCore/bindings/v8/V8DOMMap.h
WebCore/bindings/v8/V8GCController.cpp

index 4623e4e..ab5ffc2 100644 (file)
@@ -1,3 +1,35 @@
+2010-12-13  Anton Muhin  <antonm@chromium.org>
+
+        Reviewed by Pavel Feldman.
+
+        [v8] More CSS wrappers GC work: keep document's style sheets, style sheet and css rule lists' items in proper object groups
+        https://bugs.webkit.org/show_bug.cgi?id=50828
+
+        * bindings/v8/DOMData.h:
+        (WebCore::DOMData::WrapperMapObjectRemover::visitDOMWrapper):
+        (WebCore::DOMData::removeObjectsFromWrapperMap):
+        * bindings/v8/DOMDataStore.h:
+        (WebCore::ChunkedTable::visit):
+        (WebCore::ChunkedTable::visitEntries):
+        (WebCore::DOMDataStore::IntrusiveDOMWrapperMap::visit):
+        (WebCore::DOMDataStore::IntrusiveDOMWrapperMap::ChunkedTableTraits::visit):
+        * bindings/v8/V8DOMMap.cpp:
+        (WebCore::removeAllDOMObjectsInCurrentThread):
+        (WebCore::visitDOMNodesInCurrentThread):
+        (WebCore::visitDOMObjectsInCurrentThread):
+        (WebCore::visitActiveDOMObjectsInCurrentThread):
+        (WebCore::visitDOMSVGElementInstancesInCurrentThread):
+        * bindings/v8/V8DOMMap.h:
+        (WebCore::WeakReferenceMap::visit):
+        * bindings/v8/V8GCController.cpp:
+        (WebCore::DOMObjectVisitor::visitDOMWrapper):
+        (WebCore::EnsureWeakDOMNodeVisitor::visitDOMWrapper):
+        (WebCore::GCPrologueVisitor::visitDOMWrapper):
+        (WebCore::NodeGrouperVisitor::visitDOMWrapper):
+        (WebCore::NodeGrouperVisitor::applyGrouping):
+        (WebCore::DOMObjectGrouperVisitor::visitDOMWrapper):
+        (WebCore::GCEpilogueVisitor::visitDOMWrapper):
+
 2010-12-13  Zoltan Herczeg  <zherczeg@webkit.org>
 
         Unreviewed build fix for r73894.
index c8b4a41..78edf92 100644 (file)
@@ -55,7 +55,7 @@ namespace WebCore {
         static void handleWeakObject(DOMDataStore::DOMWrapperMapType, v8::Persistent<v8::Object>, T* domObject);
 
         template<typename T>
-        static void removeObjectsFromWrapperMap(AbstractWeakReferenceMap<T, v8::Object>& domMap);
+        static void removeObjectsFromWrapperMap(DOMDataStore* store, AbstractWeakReferenceMap<T, v8::Object>& domMap);
 
         ThreadIdentifier owningThread() const { return m_owningThread; }
 
@@ -65,7 +65,7 @@ namespace WebCore {
         template<typename T>
         class WrapperMapObjectRemover : public WeakReferenceMap<T, v8::Object>::Visitor {
         public:
-            virtual void visitDOMWrapper(T* domObject, v8::Persistent<v8::Object> v8Object)
+            virtual void visitDOMWrapper(DOMDataStore* store, T* domObject, v8::Persistent<v8::Object> v8Object)
             {
                 WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(v8Object);
                 derefObject(type, domObject);
@@ -102,10 +102,10 @@ namespace WebCore {
     }
 
     template<typename T>
-    void DOMData::removeObjectsFromWrapperMap(AbstractWeakReferenceMap<T, v8::Object>& domMap)
+    void DOMData::removeObjectsFromWrapperMap(DOMDataStore* store, AbstractWeakReferenceMap<T, v8::Object>& domMap)
     {
         WrapperMapObjectRemover<T> remover;
-        domMap.visit(&remover);
+        domMap.visit(store, &remover);
         domMap.clear();
     }
 
index 3758e23..a1051dd 100644 (file)
@@ -106,14 +106,14 @@ namespace WebCore {
             m_last = m_current + CHUNK_SIZE;
         }
 
-        void visit(typename Traits::Visitor* visitor)
+        void visit(DOMDataStore* store, typename Traits::Visitor* visitor)
         {
             if (!m_chunks)
                 return;
 
-            visitEntries(m_chunks->m_entries, m_current, visitor);
+            visitEntries(store, m_chunks->m_entries, m_current, visitor);
             for (Chunk* chunk = m_chunks->m_previous; chunk; chunk = chunk->m_previous)
-                visitEntries(chunk->m_entries, chunk->m_entries + CHUNK_SIZE, visitor);
+                visitEntries(store, chunk->m_entries, chunk->m_entries + CHUNK_SIZE, visitor);
         }
 
       private:
@@ -129,10 +129,10 @@ namespace WebCore {
                 Traits::clear(entry);
         }
 
-        static void visitEntries(T* first, T* last, typename Traits::Visitor* visitor)
+        static void visitEntries(DOMDataStore* store, T* first, T* last, typename Traits::Visitor* visitor)
         {
             for (T* entry = first; entry < last; entry++)
-                Traits::visit(entry, visitor);
+                Traits::visit(store, entry, visitor);
         }
 
         Chunk* m_chunks;
@@ -185,9 +185,9 @@ namespace WebCore {
                 return obj->wrapper();
             }
 
-            virtual void visit(Visitor* visitor)
+            virtual void visit(DOMDataStore* store, Visitor* visitor)
             {
-                m_table.visit(visitor);
+                m_table.visit(store, visitor);
             }
 
             virtual bool removeIfPresent(Node* key, v8::Persistent<v8::Data> value);
@@ -220,12 +220,12 @@ namespace WebCore {
                     entry->Dispose();
                 }
 
-                static void visit(v8::Persistent<v8::Object>* entry, Visitor* visitor)
+                static void visit(DOMDataStore* store, v8::Persistent<v8::Object>* entry, Visitor* visitor)
                 {
                     Node* node = V8Node::toNative(*entry);
                     ASSERT(node->wrapper() == entry);
 
-                    visitor->visitDOMWrapper(node, *entry);
+                    visitor->visitDOMWrapper(store, node, *entry);
                 }
             };
 
index e1ac2c6..40d1dc3 100644 (file)
@@ -90,24 +90,26 @@ DOMWrapperMap<SVGElementInstance>& getDOMSVGElementInstanceMap()
 
 void removeAllDOMObjectsInCurrentThread()
 {
+    DOMDataStore& store = getDOMDataStore();
+
     v8::HandleScope scope;
 
     // The DOM objects with the following types only exist on the main thread.
     if (WTF::isMainThread()) {
         // Remove all DOM nodes.
-        DOMData::removeObjectsFromWrapperMap<Node>(getDOMNodeMap());
+        DOMData::removeObjectsFromWrapperMap<Node>(&store, store.domNodeMap());
 
 #if ENABLE(SVG)
         // Remove all SVG element instances in the wrapper map.
-        DOMData::removeObjectsFromWrapperMap<SVGElementInstance>(getDOMSVGElementInstanceMap());
+        DOMData::removeObjectsFromWrapperMap<SVGElementInstance>(&store, store.domSvgElementInstanceMap());
 #endif
     }
 
     // Remove all DOM objects in the wrapper map.
-    DOMData::removeObjectsFromWrapperMap<void>(getDOMObjectMap());
+    DOMData::removeObjectsFromWrapperMap<void>(&store, store.domObjectMap());
 
     // Remove all active DOM objects in the wrapper map.
-    DOMData::removeObjectsFromWrapperMap<void>(getActiveDOMObjectMap());
+    DOMData::removeObjectsFromWrapperMap<void>(&store, store.activeDomObjectMap());
 }
 
 void visitDOMNodesInCurrentThread(DOMWrapperMap<Node>::Visitor* visitor)
@@ -121,7 +123,7 @@ void visitDOMNodesInCurrentThread(DOMWrapperMap<Node>::Visitor* visitor)
         if (!store->domData()->owningThread() == WTF::currentThread())
             continue;
 
-        store->domNodeMap().visit(visitor);
+        store->domNodeMap().visit(store, visitor);
     }
 }
 
@@ -136,7 +138,7 @@ void visitDOMObjectsInCurrentThread(DOMWrapperMap<void>::Visitor* visitor)
         if (!store->domData()->owningThread() == WTF::currentThread())
             continue;
 
-        store->domObjectMap().visit(visitor);
+        store->domObjectMap().visit(store, visitor);
     }
 }
 
@@ -151,7 +153,7 @@ void visitActiveDOMObjectsInCurrentThread(DOMWrapperMap<void>::Visitor* visitor)
         if (!store->domData()->owningThread() == WTF::currentThread())
             continue;
 
-        store->activeDomObjectMap().visit(visitor);
+        store->activeDomObjectMap().visit(store, visitor);
     }
 }
 
@@ -168,7 +170,7 @@ void visitDOMSVGElementInstancesInCurrentThread(DOMWrapperMap<SVGElementInstance
         if (!store->domData()->owningThread() == WTF::currentThread())
             continue;
 
-        store->domSvgElementInstanceMap().visit(visitor);
+        store->domSvgElementInstanceMap().visit(store, visitor);
     }
 }
 
index 7f95b50..c2b5a9d 100644 (file)
@@ -36,6 +36,7 @@
 #include <v8.h>
 
 namespace WebCore {
+    class DOMDataStore;
     class Node;
 #if ENABLE(SVG)
     class SVGElementInstance;
@@ -50,7 +51,7 @@ namespace WebCore {
         public:
             virtual void startMap() { }
             virtual void endMap() { }
-            virtual void visitDOMWrapper(KeyType* key, v8::Persistent<ValueType> object) = 0;
+            virtual void visitDOMWrapper(DOMDataStore* store, KeyType* key, v8::Persistent<ValueType> object) = 0;
         protected:
             virtual ~Visitor() { }
         };
@@ -58,7 +59,7 @@ namespace WebCore {
         virtual v8::Persistent<ValueType> get(KeyType* obj) = 0;
         virtual void set(KeyType* obj, v8::Persistent<ValueType> wrapper) = 0;
         virtual bool contains(KeyType* obj) = 0;
-        virtual void visit(Visitor* visitor) = 0;
+        virtual void visit(DOMDataStore* store, Visitor* visitor) = 0;
         virtual bool removeIfPresent(KeyType* key, v8::Persistent<v8::Data> value) = 0;
         virtual void clear() = 0;
 
@@ -122,12 +123,12 @@ namespace WebCore {
 
         bool contains(KeyType* obj) { return m_map.contains(obj); }
 
-        virtual void visit(typename Parent::Visitor* visitor)
+        virtual void visit(DOMDataStore* store, typename Parent::Visitor* visitor)
         {
             visitor->startMap();
             typename HashMap<KeyType*, ValueType*>::iterator it = m_map.begin();
             for (; it != m_map.end(); ++it)
-                visitor->visitDOMWrapper(it->first, v8::Persistent<ValueType>(it->second));
+                visitor->visitDOMWrapper(store, it->first, v8::Persistent<ValueType>(it->second));
             visitor->endMap();
         }
 
index 68494ff..07da7e5 100644 (file)
 #include "V8CSSFontFaceRule.h"
 #include "V8CSSImportRule.h"
 #include "V8CSSMediaRule.h"
+#include "V8CSSRuleList.h"
 #include "V8CSSStyleRule.h"
 #include "V8CSSStyleSheet.h"
 #include "V8DOMMap.h"
 #include "V8MessagePort.h"
 #include "V8Proxy.h"
+#include "V8StyleSheetList.h"
 #include "WrapperTypeInfo.h"
 
 #include <algorithm>
@@ -135,7 +137,7 @@ static void enumerateDOMObjectMap(DOMObjectMap& wrapperMap)
 
 class DOMObjectVisitor : public DOMWrapperMap<void>::Visitor {
 public:
-    void visitDOMWrapper(void* object, v8::Persistent<v8::Object> wrapper)
+    void visitDOMWrapper(DOMDataStore* store, void* object, v8::Persistent<v8::Object> wrapper)
     {
         WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(wrapper);
         UNUSED_PARAM(type);
@@ -145,7 +147,7 @@ public:
 
 class EnsureWeakDOMNodeVisitor : public DOMWrapperMap<Node>::Visitor {
 public:
-    void visitDOMWrapper(Node* object, v8::Persistent<v8::Object> wrapper)
+    void visitDOMWrapper(DOMDataStore* store, Node* object, v8::Persistent<v8::Object> wrapper)
     {
         UNUSED_PARAM(object);
         ASSERT(wrapper.IsWeak());
@@ -193,7 +195,7 @@ void V8GCController::gcUnprotect(void* domObject)
 
 class GCPrologueVisitor : public DOMWrapperMap<void>::Visitor {
 public:
-    void visitDOMWrapper(void* object, v8::Persistent<v8::Object> wrapper)
+    void visitDOMWrapper(DOMDataStore* store, void* object, v8::Persistent<v8::Object> wrapper)
     {
         WrapperTypeInfo* typeInfo = V8DOMWrapper::domWrapperType(wrapper);  
 
@@ -287,7 +289,7 @@ public:
         // FIXME: grouper_.reserveCapacity(node_map.size());  ?
     }
 
-    void visitDOMWrapper(Node* node, v8::Persistent<v8::Object> wrapper)
+    void visitDOMWrapper(DOMDataStore* store, Node* node, v8::Persistent<v8::Object> wrapper)
     {
         // If the node is in document, put it in the ownerDocument's object group.
         //
@@ -320,28 +322,30 @@ public:
             groupId = reinterpret_cast<uintptr_t>(root);
         }
         m_grouper.append(GrouperItem(groupId, wrapper));
-    }
-
-    void applyGrouping()
-    {
-        /* FIXME: Re-enabled this code to avoid GCing these wrappers!
-                      Currently this depends on looking up the wrapper
-                      during a GC, but we don't know which isolated world
-                      we're in, so it's unclear which map to look in...
 
         // If the node is styled and there is a wrapper for the inline
         // style declaration, we need to keep that style declaration
         // wrapper alive as well, so we add it to the object group.
         if (node->isStyledElement()) {
-          StyledElement* element = reinterpret_cast<StyledElement*>(node);
-          CSSStyleDeclaration* style = element->inlineStyleDecl();
-          if (style != NULL) {
-            wrapper = getDOMObjectMap().get(style);
+            StyledElement* element = reinterpret_cast<StyledElement*>(node);
+            CSSStyleDeclaration* style = element->inlineStyleDecl();
+            if (style) {
+                wrapper = store->domObjectMap().get(style);
+                if (!wrapper.IsEmpty())
+                    m_grouper.append(GrouperItem(groupId, wrapper));
+            }
+        }
+
+        if (node->isDocumentNode()) {
+            Document* document = reinterpret_cast<Document*>(node);
+            wrapper = store->domObjectMap().get(document->styleSheets());
             if (!wrapper.IsEmpty())
-              group.append(wrapper);
-          }
+                m_grouper.append(GrouperItem(groupId, wrapper));
         }
-        */
+    }
+
+    void applyGrouping()
+    {
         makeV8ObjectGroups(m_grouper);
     }
 
@@ -365,34 +369,54 @@ public:
         makeV8ObjectGroups(m_grouper);
     }
 
-    void visitDOMWrapper(void* object, v8::Persistent<v8::Object> wrapper)
+    void visitDOMWrapper(DOMDataStore* store, void* object, v8::Persistent<v8::Object> wrapper)
     {
         WrapperTypeInfo* typeInfo = V8DOMWrapper::domWrapperType(wrapper);
-        // FIXME: extend WrapperTypeInfo with isStyle to simplify the check below.
+        // FIXME: extend WrapperTypeInfo with isStyle to simplify the check below or consider
+        // adding a virtual method to WrapperTypeInfo which would know how to group objects.
         // FIXME: check if there are other StyleBase wrappers we should care of.
-        if (!V8CSSStyleSheet::info.equals(typeInfo)
-            && !V8CSSCharsetRule::info.equals(typeInfo)
-            && !V8CSSFontFaceRule::info.equals(typeInfo)
-            && !V8CSSStyleRule::info.equals(typeInfo)
-            && !V8CSSImportRule::info.equals(typeInfo)
-            && !V8CSSMediaRule::info.equals(typeInfo)) {
-            return;
-        }
-        StyleBase* styleBase = static_cast<StyleBase*>(object);
-
-        // We put the whole tree of style elements into a single object group.
-        // To achieve that we group elements by the roots of their trees.
-        StyleBase* root = styleBase;
-        ASSERT(root);
-        while (true) {
-          StyleBase* parent = root->parent();
-          if (!parent)
-              break;
-          root = parent;
+        if (V8CSSStyleSheet::info.equals(typeInfo)
+            || V8CSSCharsetRule::info.equals(typeInfo)
+            || V8CSSFontFaceRule::info.equals(typeInfo)
+            || V8CSSStyleRule::info.equals(typeInfo)
+            || V8CSSImportRule::info.equals(typeInfo)
+            || V8CSSMediaRule::info.equals(typeInfo)) {
+            StyleBase* styleBase = static_cast<StyleBase*>(object);
+
+            // We put the whole tree of style elements into a single object group.
+            // To achieve that we group elements by the roots of their trees.
+            StyleBase* root = styleBase;
+            ASSERT(root);
+            while (true) {
+                StyleBase* parent = root->parent();
+                if (!parent)
+                    break;
+                root = parent;
+            }
+            // Group id is an address of the root.
+            uintptr_t groupId = reinterpret_cast<uintptr_t>(root);
+            m_grouper.append(GrouperItem(groupId, wrapper));
+        } else if (V8StyleSheetList::info.equals(typeInfo)) {
+            StyleSheetList* styleSheetList = static_cast<StyleSheetList*>(object);
+            uintptr_t groupId = reinterpret_cast<uintptr_t>(styleSheetList);
+            m_grouper.append(GrouperItem(groupId, wrapper));
+            for (unsigned i = 0; i < styleSheetList->length(); i++) {
+                StyleSheet* styleSheet = styleSheetList->item(i);
+                wrapper = store->domObjectMap().get(styleSheet);
+                if (!wrapper.IsEmpty())
+                    m_grouper.append(GrouperItem(groupId, wrapper));
+            }
+        } else if (V8CSSRuleList::info.equals(typeInfo)) {
+            CSSRuleList* cssRuleList = static_cast<CSSRuleList*>(object);
+            uintptr_t groupId = reinterpret_cast<uintptr_t>(cssRuleList);
+            m_grouper.append(GrouperItem(groupId, wrapper));
+            for (unsigned i = 0; i < cssRuleList->length(); i++) {
+                CSSRule* cssRule = cssRuleList->item(i);
+                wrapper = store->domObjectMap().get(cssRule);
+                if (!wrapper.IsEmpty())
+                    m_grouper.append(GrouperItem(groupId, wrapper));
+            }
         }
-        // Group id is an address of the root.
-        uintptr_t groupId = reinterpret_cast<uintptr_t>(root);
-        m_grouper.append(GrouperItem(groupId, wrapper));
     }
 
 private:
@@ -429,7 +453,7 @@ void V8GCController::gcPrologue()
 
 class GCEpilogueVisitor : public DOMWrapperMap<void>::Visitor {
 public:
-    void visitDOMWrapper(void* object, v8::Persistent<v8::Object> wrapper)
+    void visitDOMWrapper(DOMDataStore* store, void* object, v8::Persistent<v8::Object> wrapper)
     {
         WrapperTypeInfo* typeInfo = V8DOMWrapper::domWrapperType(wrapper);
         if (V8MessagePort::info.equals(typeInfo)) {