Title: [136559] trunk/Source/WebCore
- Revision
- 136559
- Author
- aba...@webkit.org
- Date
- 2012-12-04 13:46:07 -0800 (Tue, 04 Dec 2012)
Log Message
NodeFilter leaks memory in V8
https://bugs.webkit.org/show_bug.cgi?id=104027
Reviewed by Eric Seidel.
NodeFilter holds a RefPtr to NodeFilterCondition, which holds a
ScopedPersistent to its callback function. If the callback function can
see the NodeFilter wrapper, we have a cycle and a leak.
This patch makes the NodeFilterCondition hold a weak pointer to the
callback function and ties the lifetime of that callback function to
the NodeFilter wrapper (so they live and die together).
* bindings/v8/V8GCController.cpp:
* bindings/v8/V8NodeFilterCondition.cpp:
(WebCore::V8NodeFilterCondition::V8NodeFilterCondition):
(WebCore::V8NodeFilterCondition::weakCallback):
(WebCore):
(WebCore::V8NodeFilterCondition::acceptNode):
* bindings/v8/V8NodeFilterCondition.h:
(WebCore::V8NodeFilterCondition::create):
(V8NodeFilterCondition):
(WebCore::V8NodeFilterCondition::callback):
* dom/NodeFilter.h:
(WebCore::NodeFilter::condition):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (136558 => 136559)
--- trunk/Source/WebCore/ChangeLog 2012-12-04 21:29:22 UTC (rev 136558)
+++ trunk/Source/WebCore/ChangeLog 2012-12-04 21:46:07 UTC (rev 136559)
@@ -1,3 +1,31 @@
+2012-12-04 Adam Barth <aba...@webkit.org>
+
+ NodeFilter leaks memory in V8
+ https://bugs.webkit.org/show_bug.cgi?id=104027
+
+ Reviewed by Eric Seidel.
+
+ NodeFilter holds a RefPtr to NodeFilterCondition, which holds a
+ ScopedPersistent to its callback function. If the callback function can
+ see the NodeFilter wrapper, we have a cycle and a leak.
+
+ This patch makes the NodeFilterCondition hold a weak pointer to the
+ callback function and ties the lifetime of that callback function to
+ the NodeFilter wrapper (so they live and die together).
+
+ * bindings/v8/V8GCController.cpp:
+ * bindings/v8/V8NodeFilterCondition.cpp:
+ (WebCore::V8NodeFilterCondition::V8NodeFilterCondition):
+ (WebCore::V8NodeFilterCondition::weakCallback):
+ (WebCore):
+ (WebCore::V8NodeFilterCondition::acceptNode):
+ * bindings/v8/V8NodeFilterCondition.h:
+ (WebCore::V8NodeFilterCondition::create):
+ (V8NodeFilterCondition):
+ (WebCore::V8NodeFilterCondition::callback):
+ * dom/NodeFilter.h:
+ (WebCore::NodeFilter::condition):
+
2012-12-04 Abhishek Arya <infe...@chromium.org>
Crash in CachedResource::checkNotify due to -webkit-crossfade.
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.cpp (136558 => 136559)
--- trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-12-04 21:29:22 UTC (rev 136558)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-12-04 21:46:07 UTC (rev 136559)
@@ -40,6 +40,8 @@
#include "V8MessagePort.h"
#include "V8MutationObserver.h"
#include "V8Node.h"
+#include "V8NodeFilter.h"
+#include "V8NodeFilterCondition.h"
#include "V8RecursionScope.h"
#include "WrapperTypeInfo.h"
#include <algorithm>
@@ -170,6 +172,7 @@
WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(wrapper);
void* object = toNative(wrapper);
+ // FIXME: Abstract this if cascade into a WrapperTypeInfo function.
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
@@ -185,6 +188,9 @@
for (HashSet<Node*>::iterator it = observedNodes.begin(); it != observedNodes.end(); ++it)
m_grouper.addToGroup(V8GCController::opaqueRootForGC(*it), wrapper);
#endif // ENABLE(MUTATION_OBSERVERS)
+ } else if (V8NodeFilter::info.equals(type)) {
+ NodeFilter* filter = static_cast<NodeFilter*>(object);
+ m_grouper.addToGroup(type->opaqueRootForGC(object, wrapper), static_cast<V8NodeFilterCondition*>(filter->condition())->callback());
} else {
ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
if (activeDOMObject && activeDOMObject->hasPendingActivity())
Modified: trunk/Source/WebCore/bindings/v8/V8NodeFilterCondition.cpp (136558 => 136559)
--- trunk/Source/WebCore/bindings/v8/V8NodeFilterCondition.cpp 2012-12-04 21:29:22 UTC (rev 136558)
+++ trunk/Source/WebCore/bindings/v8/V8NodeFilterCondition.cpp 2012-12-04 21:46:07 UTC (rev 136559)
@@ -40,29 +40,37 @@
namespace WebCore {
-V8NodeFilterCondition::V8NodeFilterCondition(v8::Handle<v8::Value> filter)
- : m_filter(filter)
+V8NodeFilterCondition::V8NodeFilterCondition(v8::Handle<v8::Value> callback)
+ : m_callback(callback)
{
+ m_callback.get().MakeWeak(this, weakCallback);
}
V8NodeFilterCondition::~V8NodeFilterCondition()
{
}
+void V8NodeFilterCondition::weakCallback(v8::Persistent<v8::Value> value, void* context)
+{
+ V8NodeFilterCondition* condition = static_cast<V8NodeFilterCondition*>(context);
+ ASSERT(condition->callback() == value);
+ condition->m_callback.clear();
+}
+
short V8NodeFilterCondition::acceptNode(ScriptState* state, Node* node) const
{
ASSERT(v8::Context::InContext());
- if (!m_filter->IsObject())
+ if (!m_callback->IsObject())
return NodeFilter::FILTER_ACCEPT;
v8::TryCatch exceptionCatcher;
v8::Handle<v8::Function> callback;
- if (m_filter->IsFunction())
- callback = v8::Handle<v8::Function>::Cast(m_filter.get());
+ if (m_callback->IsFunction())
+ callback = v8::Handle<v8::Function>::Cast(m_callback.get());
else {
- v8::Local<v8::Value> value = m_filter->ToObject()->Get(v8::String::New("acceptNode"));
+ v8::Local<v8::Value> value = m_callback->ToObject()->Get(v8::String::New("acceptNode"));
if (!value->IsFunction()) {
throwTypeError("NodeFilter object does not have an acceptNode function");
return NodeFilter::FILTER_REJECT;
Modified: trunk/Source/WebCore/bindings/v8/V8NodeFilterCondition.h (136558 => 136559)
--- trunk/Source/WebCore/bindings/v8/V8NodeFilterCondition.h 2012-12-04 21:29:22 UTC (rev 136558)
+++ trunk/Source/WebCore/bindings/v8/V8NodeFilterCondition.h 2012-12-04 21:46:07 UTC (rev 136559)
@@ -43,19 +43,22 @@
class V8NodeFilterCondition : public NodeFilterCondition {
public:
- static PassRefPtr<V8NodeFilterCondition> create(v8::Handle<v8::Value> filter)
+ static PassRefPtr<V8NodeFilterCondition> create(v8::Handle<v8::Value> callback)
{
- return adoptRef(new V8NodeFilterCondition(filter));
+ return adoptRef(new V8NodeFilterCondition(callback));
}
virtual ~V8NodeFilterCondition();
- virtual short acceptNode(ScriptState*, Node*) const;
+ virtual short acceptNode(ScriptState*, Node*) const OVERRIDE;
+ v8::Persistent<v8::Value> callback() const { return m_callback.get(); }
private:
- explicit V8NodeFilterCondition(v8::Handle<v8::Value> filter);
+ explicit V8NodeFilterCondition(v8::Handle<v8::Value> callback);
- ScopedPersistent<v8::Value> m_filter;
+ static void weakCallback(v8::Persistent<v8::Value>, void*);
+
+ ScopedPersistent<v8::Value> m_callback;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/NodeFilter.h (136558 => 136559)
--- trunk/Source/WebCore/dom/NodeFilter.h 2012-12-04 21:29:22 UTC (rev 136558)
+++ trunk/Source/WebCore/dom/NodeFilter.h 2012-12-04 21:46:07 UTC (rev 136559)
@@ -82,6 +82,7 @@
short acceptNode(Node* node) const { return acceptNode(scriptStateFromNode(mainThreadNormalWorld(), node), node); }
void setCondition(PassRefPtr<NodeFilterCondition> condition) { ASSERT(!m_condition); m_condition = condition; }
+ NodeFilterCondition* condition() const { return m_condition.get(); }
private:
explicit NodeFilter(PassRefPtr<NodeFilterCondition> condition) : m_condition(condition) { }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
