[V8] Unify the V8GCController visitors
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2012 14:52:07 +0000 (14:52 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2012 14:52:07 +0000 (14:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100897

Reviewed by Eric Seidel.

After this patch, we use a single visitor for all DOM wrappers,
regardless of type. We also visit all the wrappers in one pass by
calling v8::V8::VisitHandlesWithClassIds directly rather than via
visitAllDOMNodes.

This patch also introduces a wrapper class ID for non-Node DOM objects.
Previously, only DOM nodes had a class ID.

* bindings/v8/IntrusiveDOMWrapperMap.h:
* bindings/v8/ScriptProfiler.cpp:
(WebCore::retainedDOMInfo):
(WebCore::ScriptProfiler::initialize):
* bindings/v8/V8DOMMap.cpp:
(WebCore::visitAllDOMNodes):
* bindings/v8/V8DOMWrapper.cpp:
(WebCore::V8DOMWrapper::setJSWrapperForDOMNode):
* bindings/v8/V8DOMWrapper.h:
(WebCore::V8DOMWrapper::setJSWrapperForDOMObject):
* bindings/v8/V8GCController.cpp:
(WebCore::GCHandleVisitor::notifyFinished):
(GCHandleVisitor):
(WebCore::V8GCController::majorGCPrologue):
* bindings/v8/WrapperTypeInfo.h:
(WebCore):

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

Source/WebCore/ChangeLog
Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h
Source/WebCore/bindings/v8/ScriptProfiler.cpp
Source/WebCore/bindings/v8/V8DOMMap.cpp
Source/WebCore/bindings/v8/V8DOMWrapper.cpp
Source/WebCore/bindings/v8/V8DOMWrapper.h
Source/WebCore/bindings/v8/V8GCController.cpp
Source/WebCore/bindings/v8/WrapperTypeInfo.h

index fb14c98..9ccd4fa 100644 (file)
@@ -1,3 +1,35 @@
+2012-11-01  Adam Barth  <abarth@webkit.org>
+
+        [V8] Unify the V8GCController visitors
+        https://bugs.webkit.org/show_bug.cgi?id=100897
+
+        Reviewed by Eric Seidel.
+
+        After this patch, we use a single visitor for all DOM wrappers,
+        regardless of type. We also visit all the wrappers in one pass by
+        calling v8::V8::VisitHandlesWithClassIds directly rather than via
+        visitAllDOMNodes.
+
+        This patch also introduces a wrapper class ID for non-Node DOM objects.
+        Previously, only DOM nodes had a class ID.
+
+        * bindings/v8/IntrusiveDOMWrapperMap.h:
+        * bindings/v8/ScriptProfiler.cpp:
+        (WebCore::retainedDOMInfo):
+        (WebCore::ScriptProfiler::initialize):
+        * bindings/v8/V8DOMMap.cpp:
+        (WebCore::visitAllDOMNodes):
+        * bindings/v8/V8DOMWrapper.cpp:
+        (WebCore::V8DOMWrapper::setJSWrapperForDOMNode):
+        * bindings/v8/V8DOMWrapper.h:
+        (WebCore::V8DOMWrapper::setJSWrapperForDOMObject):
+        * bindings/v8/V8GCController.cpp:
+        (WebCore::GCHandleVisitor::notifyFinished):
+        (GCHandleVisitor):
+        (WebCore::V8GCController::majorGCPrologue):
+        * bindings/v8/WrapperTypeInfo.h:
+        (WebCore):
+
 2012-11-01  Stephen White  <senorblanco@chromium.org>
 
         Unreviewed, rolling out r133143.
index f465b16..dccfd10 100644 (file)
@@ -45,7 +45,7 @@ public:
     virtual void set(Node* node, v8::Persistent<v8::Object> wrapper) OVERRIDE
     {
         ASSERT(node && node->wrapper().IsEmpty());
-        ASSERT(wrapper.WrapperClassId() == v8DOMSubtreeClassId);
+        ASSERT(wrapper.WrapperClassId() == v8DOMNodeClassId);
         node->setWrapper(wrapper);
         wrapper.MakeWeak(node, weakCallback);
     }
index 1b7191c..78e7e07 100644 (file)
@@ -165,7 +165,7 @@ PassRefPtr<ScriptHeapSnapshot> ScriptProfiler::takeHeapSnapshot(const String& ti
 
 static v8::RetainedObjectInfo* retainedDOMInfo(uint16_t classId, v8::Handle<v8::Value> wrapper)
 {
-    ASSERT(classId == v8DOMSubtreeClassId);
+    ASSERT(classId == v8DOMNodeClassId);
     if (!wrapper->IsObject())
         return 0;
     Node* node = V8Node::toNative(wrapper.As<v8::Object>());
@@ -174,7 +174,7 @@ static v8::RetainedObjectInfo* retainedDOMInfo(uint16_t classId, v8::Handle<v8::
 
 void ScriptProfiler::initialize()
 {
-    v8::HeapProfiler::DefineWrapperClass(v8DOMSubtreeClassId, &retainedDOMInfo);
+    v8::HeapProfiler::DefineWrapperClass(v8DOMNodeClassId, &retainedDOMInfo);
 }
 
 void ScriptProfiler::visitNodeWrappers(WrappedNodeVisitor* visitor)
index f661dd9..ac4cd8d 100644 (file)
@@ -69,7 +69,7 @@ void visitAllDOMNodes(NodeWrapperVisitor* visitor)
 
         virtual void VisitPersistentHandle(v8::Persistent<v8::Value> value, uint16_t classId)
         {
-            if (classId != v8DOMSubtreeClassId)
+            if (classId != v8DOMNodeClassId)
                 return;
             ASSERT(V8Node::HasInstance(value));
             ASSERT(value->IsObject());
index b68a876..783cabf 100644 (file)
@@ -72,7 +72,7 @@ v8::Persistent<v8::Object> V8DOMWrapper::setJSWrapperForDOMNode(PassRefPtr<Node>
 {
     v8::Persistent<v8::Object> wrapperHandle = v8::Persistent<v8::Object>::New(wrapper);
     ASSERT(maybeDOMWrapper(wrapperHandle));
-    wrapperHandle.SetWrapperClassId(v8DOMSubtreeClassId);
+    wrapperHandle.SetWrapperClassId(v8DOMNodeClassId);
     getDOMNodeMap(isolate).set(node.leakRef(), wrapperHandle);
     return wrapperHandle;
 }
index a52923b..721d295 100644 (file)
@@ -131,6 +131,7 @@ namespace WebCore {
     {
         v8::Persistent<v8::Object> wrapperHandle = v8::Persistent<v8::Object>::New(wrapper);
         ASSERT(maybeDOMWrapper(wrapperHandle));
+        wrapperHandle.SetWrapperClassId(v8DOMObjectClassId);
         getDOMObjectMap(isolate).set(object.leakRef(), wrapperHandle);
         return wrapperHandle;
     }
index ff7dfbc..5646b74 100644 (file)
@@ -132,40 +132,6 @@ private:
     Vector<ImplicitConnection> m_connections;
 };
 
-class ObjectVisitor : public DOMWrapperVisitor<void> {
-public:
-    explicit ObjectVisitor(WrapperGrouper* grouper)
-        : m_grouper(grouper)
-    {
-    }
-
-    void visitDOMWrapper(DOMDataStore*, void* object, v8::Persistent<v8::Object> wrapper)
-    {
-        if (wrapper.IsIndependent())
-            return;
-
-        WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(wrapper);
-
-        if (V8MessagePort::info.equals(type)) {
-            // Mark each port as in-use if it's entangled. For simplicity's sake,
-            // we assume all ports are remotely entangled, since the Chromium port
-            // implementation can't tell the difference.
-            MessagePort* port = static_cast<MessagePort*>(object);
-            if (port->isEntangled() || port->hasPendingActivity())
-                m_grouper->keepAlive(wrapper);
-        } else {
-            ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
-            if (activeDOMObject && activeDOMObject->hasPendingActivity())
-                m_grouper->keepAlive(wrapper);
-        }
-
-        m_grouper->addToGroup(type->opaqueRootForGC(object, wrapper), wrapper);
-    }
-
-private:
-    WrapperGrouper* m_grouper;
-};
-
 // FIXME: This should use opaque GC roots.
 static void addImplicitReferencesForNodeWithEventListeners(Node* node, v8::Persistent<v8::Object> wrapper)
 {
@@ -207,29 +173,61 @@ void* V8GCController::opaqueRootForGC(Node* node)
     return node;
 }
 
-class NodeVisitor : public NodeWrapperVisitor {
+class WrapperVisitor : public v8::PersistentHandleVisitor {
 public:
-    explicit NodeVisitor(WrapperGrouper* grouper)
-        : m_grouper(grouper)
+    virtual void VisitPersistentHandle(v8::Persistent<v8::Value> value, uint16_t classId) OVERRIDE
     {
-    }
+        ASSERT(value->IsObject());
+        v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
 
-    void visitNodeWrapper(Node* node, v8::Persistent<v8::Object> wrapper)
-    {
-        if (node->hasEventListeners())
-            addImplicitReferencesForNodeWithEventListeners(node, wrapper);
+        if (classId != v8DOMNodeClassId && classId != v8DOMObjectClassId)
+            return;
+
+        ASSERT(V8DOMWrapper::maybeDOMWrapper(value));
+
+        if (value.IsIndependent())
+            return;
+
+        WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(wrapper);
+        void* object = toNative(wrapper);
+
+        if (V8MessagePort::info.equals(type)) {
+            // Mark each port as in-use if it's entangled. For simplicity's sake,
+            // we assume all ports are remotely entangled, since the Chromium port
+            // implementation can't tell the difference.
+            MessagePort* port = static_cast<MessagePort*>(object);
+            if (port->isEntangled() || port->hasPendingActivity())
+                m_grouper.keepAlive(wrapper);
+        } else {
+            ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
+            if (activeDOMObject && activeDOMObject->hasPendingActivity())
+                m_grouper.keepAlive(wrapper);
+        }
+
+        if (classId == v8DOMNodeClassId) {
+            ASSERT(V8Node::HasInstance(wrapper));
+            ASSERT(!wrapper.IsIndependent());
 
-        WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(wrapper);  
+            Node* node = static_cast<Node*>(object);
 
-        ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
-        if (activeDOMObject && activeDOMObject->hasPendingActivity())
-            m_grouper->keepAlive(wrapper);
+            if (node->hasEventListeners())
+                addImplicitReferencesForNodeWithEventListeners(node, wrapper);
 
-        m_grouper->addToGroup(V8GCController::opaqueRootForGC(node), wrapper);
+            m_grouper.addToGroup(V8GCController::opaqueRootForGC(node), wrapper);
+        } else if (classId == v8DOMObjectClassId) {
+            m_grouper.addToGroup(type->opaqueRootForGC(object, wrapper), wrapper);
+        } else {
+            ASSERT_NOT_REACHED();
+        }
+    }
+
+    void notifyFinished()
+    {
+        m_grouper.apply();
     }
 
 private:
-    WrapperGrouper* m_grouper;
+    WrapperGrouper m_grouper;
 };
 
 void V8GCController::gcPrologue(v8::GCType type, v8::GCCallbackFlags flags)
@@ -250,14 +248,9 @@ void V8GCController::majorGCPrologue()
 
     v8::HandleScope scope;
 
-    WrapperGrouper grouper;
-
-    NodeVisitor nodeVisitor(&grouper);
-    ObjectVisitor objectVisitor(&grouper);
-    visitAllDOMNodes(&nodeVisitor);
-    visitDOMObjects(&objectVisitor);
-
-    grouper.apply();
+    WrapperVisitor visitor;
+    v8::V8::VisitHandlesWithClassIds(&visitor);
+    visitor.notifyFinished();
 
     V8PerIsolateData* data = V8PerIsolateData::current();
     data->stringCache()->clearOnGC();
index 27c7ef6..0cb4b95 100644 (file)
@@ -42,7 +42,8 @@ namespace WebCore {
     static const int v8DOMWrapperObjectIndex = 1;
     static const int v8DefaultWrapperInternalFieldCount = 2;
 
-    static const uint16_t v8DOMSubtreeClassId = 1;
+    static const uint16_t v8DOMNodeClassId = 1;
+    static const uint16_t v8DOMObjectClassId = 2;
 
     typedef v8::Persistent<v8::FunctionTemplate> (*GetTemplateFunction)();
     typedef void (*DerefObjectFunction)(void*);