Modified: trunk/ChangeLog (144993 => 144994)
--- trunk/ChangeLog 2013-03-06 23:25:19 UTC (rev 144993)
+++ trunk/ChangeLog 2013-03-06 23:56:27 UTC (rev 144994)
@@ -1,3 +1,12 @@
+2013-03-06 Adam Klein <ad...@chromium.org>
+
+ [V8] Use implicit references instead of object groups to keep registered MutationObservers alive
+ https://bugs.webkit.org/show_bug.cgi?id=111382
+
+ Reviewed by Adam Barth.
+
+ * ManualTests/mutation-observer-leaks-nodes.html: Added.
+
2013-03-06 Gustavo Noronha Silva <g...@gnome.org>
Build fix. Fixes problems building code that uses deprecated functions from GTK+ 2,
Added: trunk/ManualTests/mutation-observer-leaks-nodes.html (0 => 144994)
--- trunk/ManualTests/mutation-observer-leaks-nodes.html (rev 0)
+++ trunk/ManualTests/mutation-observer-leaks-nodes.html 2013-03-06 23:56:27 UTC (rev 144994)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<body>
+<script>
+testRunner.dumpAsText();
+var count = 100;
+var observer = new MutationObserver(function(){});
+for (var i = 0; i < count; i++) {
+ var span = document.createElement('span');
+ observer.observe(span, {attributes:true});
+};
+GCController.collect();
+</script>
+<p>Number of leaked nodes reported by DRT should be less than 100</p>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (144993 => 144994)
--- trunk/Source/WebCore/ChangeLog 2013-03-06 23:25:19 UTC (rev 144993)
+++ trunk/Source/WebCore/ChangeLog 2013-03-06 23:56:27 UTC (rev 144994)
@@ -1,3 +1,33 @@
+2013-03-06 Adam Klein <ad...@chromium.org>
+
+ [V8] Use implicit references instead of object groups to keep registered MutationObservers alive
+ https://bugs.webkit.org/show_bug.cgi?id=111382
+
+ Reviewed by Adam Barth.
+
+ Two-phase approach to implicit references: after grouping objects
+ together, add an implicit reference between each registered node's
+ group and the MutationObserver's group (which includes wrappers from
+ all worlds).
+
+ Also changed many uses of v8::Value to v8::Object where we know we're
+ dealing with Object and the V8 API expects them.
+
+ Test: ManualTests/mutation-observer-leaks-nodes.html
+
+ * bindings/v8/V8GCController.cpp:
+ (WebCore::ImplicitConnection::ImplicitConnection):
+ (WebCore::ImplicitConnection::wrapper):
+ (ImplicitConnection):
+ (WebCore::ImplicitReference::ImplicitReference): Wrapper class holding a parent who should have an implicit reference to a child.
+ (ImplicitReference):
+ (WebCore::operator<): Needed for std::sort() call to avoid the overhead of using a HashMap
+ (WebCore::WrapperGrouper::addObjectWrapperToGroup):
+ (WebCore::WrapperGrouper::addNodeWrapperToGroup):
+ (WebCore::WrapperGrouper::addImplicitReference):
+ (WrapperGrouper):
+ (WebCore::WrapperGrouper::apply):
+
2013-03-06 Ankur Taly <at...@google.com>
Modify log method in V8DOMActivityLogger so that the apiName and
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.cpp (144993 => 144994)
--- trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2013-03-06 23:25:19 UTC (rev 144993)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2013-03-06 23:56:27 UTC (rev 144994)
@@ -49,13 +49,13 @@
class ImplicitConnection {
public:
- ImplicitConnection(void* root, v8::Persistent<v8::Value> wrapper)
+ ImplicitConnection(void* root, v8::Persistent<v8::Object> wrapper)
: m_root(root)
, m_wrapper(wrapper)
, m_rootNode(0)
{
}
- ImplicitConnection(Node* root, v8::Persistent<v8::Value> wrapper)
+ ImplicitConnection(Node* root, v8::Persistent<v8::Object> wrapper)
: m_root(root)
, m_wrapper(wrapper)
, m_rootNode(root)
@@ -63,7 +63,7 @@
}
void* root() const { return m_root; }
- v8::Persistent<v8::Value> wrapper() const { return m_wrapper; }
+ v8::Persistent<v8::Object> wrapper() const { return m_wrapper; }
PassOwnPtr<RetainedObjectInfo> retainedObjectInfo()
{
@@ -74,7 +74,7 @@
private:
void* m_root;
- v8::Persistent<v8::Value> m_wrapper;
+ v8::Persistent<v8::Object> m_wrapper;
Node* m_rootNode;
};
@@ -83,6 +83,22 @@
return left.root() < right.root();
}
+struct ImplicitReference {
+ ImplicitReference(void* parent, v8::Persistent<v8::Object> child)
+ : parent(parent)
+ , child(child)
+ {
+ }
+
+ void* parent;
+ v8::Persistent<v8::Object> child;
+};
+
+bool operator<(const ImplicitReference& left, const ImplicitReference& right)
+{
+ return left.parent < right.parent;
+}
+
class WrapperGrouper {
public:
WrapperGrouper()
@@ -90,16 +106,22 @@
m_liveObjects.append(V8PerIsolateData::current()->ensureLiveRoot());
}
- void addObjectWrapperToGroup(void* root, v8::Persistent<v8::Value> wrapper)
+ void addObjectWrapperToGroup(void* root, v8::Persistent<v8::Object> wrapper)
{
m_connections.append(ImplicitConnection(root, wrapper));
}
- void addNodeWrapperToGroup(Node* root, v8::Persistent<v8::Value> wrapper)
+ void addNodeWrapperToGroup(Node* root, v8::Persistent<v8::Object> wrapper)
{
m_connections.append(ImplicitConnection(root, wrapper));
}
+ void addImplicitReference(void* parent, v8::Persistent<v8::Object> child)
+ {
+ m_references.append(ImplicitReference(parent, child));
+ m_rootGroupMap.add(parent, v8::Persistent<v8::Object>());
+ }
+
void keepAlive(v8::Persistent<v8::Value> wrapper)
{
m_liveObjects.append(wrapper);
@@ -115,6 +137,7 @@
size_t i = 0;
while (i < m_connections.size()) {
void* root = m_connections[i].root();
+ v8::Persistent<v8::Object> groupRepresentativeWrapper = m_connections[i].wrapper();
OwnPtr<RetainedObjectInfo> retainedObjectInfo = m_connections[i].retainedObjectInfo();
do {
@@ -124,13 +147,37 @@
if (group.size() > 1)
v8::V8::AddObjectGroup(group.data(), group.size(), retainedObjectInfo.leakPtr());
+ HashMap<void*, v8::Persistent<v8::Object> >::iterator iter = m_rootGroupMap.find(root);
+ if (iter != m_rootGroupMap.end())
+ iter->value = groupRepresentativeWrapper;
+
group.shrink(0);
}
+
+ std::sort(m_references.begin(), m_references.end());
+ i = 0;
+ while (i < m_references.size()) {
+ void* parent = m_references[i].parent;
+ v8::Persistent<v8::Object> parentWrapper = m_rootGroupMap.get(parent);
+ if (parentWrapper.IsEmpty()) {
+ ++i;
+ continue;
+ }
+
+ Vector<v8::Persistent<v8::Value> > children;
+ do {
+ children.append(m_references[i++].child);
+ } while (i < m_references.size() && parent == m_references[i].parent);
+
+ v8::V8::AddImplicitReferences(parentWrapper, children.data(), children.size());
+ }
}
private:
Vector<v8::Persistent<v8::Value> > m_liveObjects;
Vector<ImplicitConnection> m_connections;
+ Vector<ImplicitReference> m_references;
+ HashMap<void*, v8::Persistent<v8::Object> > m_rootGroupMap;
};
// FIXME: This should use opaque GC roots.
@@ -316,7 +363,7 @@
MutationObserver* observer = static_cast<MutationObserver*>(object);
HashSet<Node*> observedNodes = observer->getObservedNodes();
for (HashSet<Node*>::iterator it = observedNodes.begin(); it != observedNodes.end(); ++it)
- m_grouper.addObjectWrapperToGroup(V8GCController::opaqueRootForGC(*it, m_isolate), wrapper);
+ m_grouper.addImplicitReference(V8GCController::opaqueRootForGC(*it, m_isolate), wrapper);
} else {
ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
if (activeDOMObject && activeDOMObject->hasPendingActivity())
