- Revision
- 133169
- Author
- aba...@webkit.org
- Date
- 2012-11-01 07:52:07 -0700 (Thu, 01 Nov 2012)
Log Message
[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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (133168 => 133169)
--- trunk/Source/WebCore/ChangeLog 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/ChangeLog 2012-11-01 14:52:07 UTC (rev 133169)
@@ -1,3 +1,35 @@
+2012-11-01 Adam Barth <aba...@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 <senorbla...@chromium.org>
Unreviewed, rolling out r133143.
Modified: trunk/Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h (133168 => 133169)
--- trunk/Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/bindings/v8/IntrusiveDOMWrapperMap.h 2012-11-01 14:52:07 UTC (rev 133169)
@@ -45,7 +45,7 @@
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);
}
Modified: trunk/Source/WebCore/bindings/v8/ScriptProfiler.cpp (133168 => 133169)
--- trunk/Source/WebCore/bindings/v8/ScriptProfiler.cpp 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/bindings/v8/ScriptProfiler.cpp 2012-11-01 14:52:07 UTC (rev 133169)
@@ -165,7 +165,7 @@
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 @@
void ScriptProfiler::initialize()
{
- v8::HeapProfiler::DefineWrapperClass(v8DOMSubtreeClassId, &retainedDOMInfo);
+ v8::HeapProfiler::DefineWrapperClass(v8DOMNodeClassId, &retainedDOMInfo);
}
void ScriptProfiler::visitNodeWrappers(WrappedNodeVisitor* visitor)
Modified: trunk/Source/WebCore/bindings/v8/V8DOMMap.cpp (133168 => 133169)
--- trunk/Source/WebCore/bindings/v8/V8DOMMap.cpp 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/bindings/v8/V8DOMMap.cpp 2012-11-01 14:52:07 UTC (rev 133169)
@@ -69,7 +69,7 @@
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());
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp (133168 => 133169)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.cpp 2012-11-01 14:52:07 UTC (rev 133169)
@@ -72,7 +72,7 @@
{
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;
}
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h (133168 => 133169)
--- trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWrapper.h 2012-11-01 14:52:07 UTC (rev 133169)
@@ -131,6 +131,7 @@
{
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;
}
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.cpp (133168 => 133169)
--- trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-11-01 14:52:07 UTC (rev 133169)
@@ -132,40 +132,6 @@
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 @@
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;
- WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(wrapper);
+ ASSERT(V8DOMWrapper::maybeDOMWrapper(value));
- ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
- if (activeDOMObject && activeDOMObject->hasPendingActivity())
- m_grouper->keepAlive(wrapper);
+ if (value.IsIndependent())
+ return;
- m_grouper->addToGroup(V8GCController::opaqueRootForGC(node), wrapper);
+ 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());
+
+ Node* node = static_cast<Node*>(object);
+
+ if (node->hasEventListeners())
+ addImplicitReferencesForNodeWithEventListeners(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,15 +248,10 @@
v8::HandleScope scope;
- WrapperGrouper grouper;
+ WrapperVisitor visitor;
+ v8::V8::VisitHandlesWithClassIds(&visitor);
+ visitor.notifyFinished();
- NodeVisitor nodeVisitor(&grouper);
- ObjectVisitor objectVisitor(&grouper);
- visitAllDOMNodes(&nodeVisitor);
- visitDOMObjects(&objectVisitor);
-
- grouper.apply();
-
V8PerIsolateData* data = ""
data->stringCache()->clearOnGC();
}
Modified: trunk/Source/WebCore/bindings/v8/WrapperTypeInfo.h (133168 => 133169)
--- trunk/Source/WebCore/bindings/v8/WrapperTypeInfo.h 2012-11-01 14:44:52 UTC (rev 133168)
+++ trunk/Source/WebCore/bindings/v8/WrapperTypeInfo.h 2012-11-01 14:52:07 UTC (rev 133169)
@@ -42,7 +42,8 @@
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*);
