- Revision
- 128125
- Author
- [email protected]
- Date
- 2012-09-10 16:22:10 -0700 (Mon, 10 Sep 2012)
Log Message
[V8] V8AuxiliaryContext used by IDB leaks memory
https://bugs.webkit.org/show_bug.cgi?id=96317
Patch by Adam Barth <[email protected]> on 2012-09-10
Reviewed by Tony Chang.
Source/WebCore:
Before this patch, we leaked m_auxiliaryContext on V8PerIsolateData
because no one ever called Dispose to balance the call to
V8::Context::New. This patch uses ScopedPersistent to call Dispose
automatically.
Also, I've deleted the V8AuxiliaryContext because it just reinvents
V8::Context::Scope.
* bindings/v8/IDBBindingUtilities.cpp:
(WebCore::createIDBKeyFromSerializedValueAndKeyPath):
(WebCore::injectIDBKeyIntoSerializedValue):
* bindings/v8/V8PerIsolateData.cpp:
(WebCore::V8PerIsolateData::ensureAuxiliaryContext):
(WebCore):
* bindings/v8/V8PerIsolateData.h:
(V8PerIsolateData):
* bindings/v8/V8Utilities.cpp:
* bindings/v8/V8Utilities.h:
Source/WebKit/chromium:
Call the V8 APIs directly instead of using a helper class.
* tests/IDBBindingUtilitiesTest.cpp:
(WebCore::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (128124 => 128125)
--- trunk/Source/WebCore/ChangeLog 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebCore/ChangeLog 2012-09-10 23:22:10 UTC (rev 128125)
@@ -1,5 +1,31 @@
2012-09-10 Adam Barth <[email protected]>
+ [V8] V8AuxiliaryContext used by IDB leaks memory
+ https://bugs.webkit.org/show_bug.cgi?id=96317
+
+ Reviewed by Tony Chang.
+
+ Before this patch, we leaked m_auxiliaryContext on V8PerIsolateData
+ because no one ever called Dispose to balance the call to
+ V8::Context::New. This patch uses ScopedPersistent to call Dispose
+ automatically.
+
+ Also, I've deleted the V8AuxiliaryContext because it just reinvents
+ V8::Context::Scope.
+
+ * bindings/v8/IDBBindingUtilities.cpp:
+ (WebCore::createIDBKeyFromSerializedValueAndKeyPath):
+ (WebCore::injectIDBKeyIntoSerializedValue):
+ * bindings/v8/V8PerIsolateData.cpp:
+ (WebCore::V8PerIsolateData::ensureAuxiliaryContext):
+ (WebCore):
+ * bindings/v8/V8PerIsolateData.h:
+ (V8PerIsolateData):
+ * bindings/v8/V8Utilities.cpp:
+ * bindings/v8/V8Utilities.h:
+
+2012-09-10 Adam Barth <[email protected]>
+
[V8] currentDocument doesn't need to handle the case where there is no v8::Context
https://bugs.webkit.org/show_bug.cgi?id=96312
Modified: trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp (128124 => 128125)
--- trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp 2012-09-10 23:22:10 UTC (rev 128125)
@@ -162,7 +162,9 @@
RefPtr<SerializedScriptValue> value = prpValue;
- V8AuxiliaryContext context;
+ v8::HandleScope handleScope;
+ v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
+
v8::Handle<v8::Value> v8Value(value->deserialize());
v8::Handle<v8::Value> v8Key(getNthValueOnKeyPath(v8Value, keyPathElements, keyPathElements.size()));
if (v8Key.IsEmpty())
@@ -207,7 +209,9 @@
if (!keyPathElements.size())
return 0;
- V8AuxiliaryContext context;
+ v8::HandleScope handleScope;
+ v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
+
v8::Handle<v8::Value> v8Value(value->deserialize());
v8::Handle<v8::Value> parent(ensureNthValueOnKeyPath(v8Value, keyPathElements, keyPathElements.size() - 1));
if (parent.IsEmpty())
Modified: trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp (128124 => 128125)
--- trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp 2012-09-10 23:22:10 UTC (rev 128125)
@@ -107,6 +107,13 @@
}
#endif
+v8::Handle<v8::Context> V8PerIsolateData::ensureAuxiliaryContext()
+{
+ if (m_auxiliaryContext.isEmpty())
+ m_auxiliaryContext.adopt(v8::Context::New());
+ return m_auxiliaryContext.get();
+}
+
v8::Handle<v8::Value> V8PerIsolateData::constructorOfToString(const v8::Arguments& args)
{
// The DOM constructors' toString functions grab the current toString
Modified: trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h (128124 => 128125)
--- trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h 2012-09-10 23:22:10 UTC (rev 128125)
@@ -26,6 +26,7 @@
#ifndef V8PerIsolateData_h
#define V8PerIsolateData_h
+#include "ScopedPersistent.h"
#include <v8.h>
#include <wtf/HashMap.h>
#include <wtf/OwnPtr.h>
@@ -91,7 +92,7 @@
DOMDataList& allStores() { return m_domDataList; }
V8HiddenPropertyName* hiddenPropertyName() { return m_hiddenPropertyName.get(); }
- v8::Persistent<v8::Context>& auxiliaryContext() { return m_auxiliaryContext; }
+ v8::Handle<v8::Context> ensureAuxiliaryContext();
void registerDOMDataStore(DOMDataStore* domDataStore)
{
@@ -151,7 +152,7 @@
DOMDataStore* m_domDataStore;
OwnPtr<V8HiddenPropertyName> m_hiddenPropertyName;
- v8::Persistent<v8::Context> m_auxiliaryContext;
+ ScopedPersistent<v8::Context> m_auxiliaryContext;
bool m_constructorMode;
friend class ConstructorMode;
Modified: trunk/Source/WebCore/bindings/v8/V8Utilities.cpp (128124 => 128125)
--- trunk/Source/WebCore/bindings/v8/V8Utilities.cpp 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebCore/bindings/v8/V8Utilities.cpp 2012-09-10 23:22:10 UTC (rev 128125)
@@ -48,24 +48,6 @@
namespace WebCore {
-V8AuxiliaryContext::V8AuxiliaryContext()
-{
- auxiliaryContext()->Enter();
-}
-
-V8AuxiliaryContext::~V8AuxiliaryContext()
-{
- auxiliaryContext()->Exit();
-}
-
-v8::Persistent<v8::Context>& V8AuxiliaryContext::auxiliaryContext()
-{
- v8::Persistent<v8::Context>& context = V8PerIsolateData::current()->auxiliaryContext();
- if (context.IsEmpty())
- context = v8::Context::New();
- return context;
-}
-
// Use an array to hold dependents. It works like a ref-counted scheme.
// A value can be added more than once to the DOM object.
void createHiddenDependency(v8::Handle<v8::Object> object, v8::Local<v8::Value> value, int cacheIndex)
Modified: trunk/Source/WebCore/bindings/v8/V8Utilities.h (128124 => 128125)
--- trunk/Source/WebCore/bindings/v8/V8Utilities.h 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebCore/bindings/v8/V8Utilities.h 2012-09-10 23:22:10 UTC (rev 128125)
@@ -62,15 +62,6 @@
typedef unsigned CallbackAllowedValueFlags;
- class V8AuxiliaryContext {
- public:
- V8AuxiliaryContext();
- virtual ~V8AuxiliaryContext();
- private:
- v8::HandleScope m_handleScope;
- static v8::Persistent<v8::Context>& auxiliaryContext();
- };
-
typedef WTF::Vector<RefPtr<MessagePort>, 1> MessagePortArray;
typedef WTF::Vector<RefPtr<ArrayBuffer>, 1> ArrayBufferArray;
Modified: trunk/Source/WebKit/chromium/ChangeLog (128124 => 128125)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-09-10 23:22:10 UTC (rev 128125)
@@ -1,5 +1,17 @@
2012-09-10 Adam Barth <[email protected]>
+ [V8] V8AuxiliaryContext used by IDB leaks memory
+ https://bugs.webkit.org/show_bug.cgi?id=96317
+
+ Reviewed by Tony Chang.
+
+ Call the V8 APIs directly instead of using a helper class.
+
+ * tests/IDBBindingUtilitiesTest.cpp:
+ (WebCore::TEST):
+
+2012-09-10 Adam Barth <[email protected]>
+
[V8] Inline firstFrame and currentFrame into their unique callers
https://bugs.webkit.org/show_bug.cgi?id=96309
Modified: trunk/Source/WebKit/chromium/tests/IDBBindingUtilitiesTest.cpp (128124 => 128125)
--- trunk/Source/WebKit/chromium/tests/IDBBindingUtilitiesTest.cpp 2012-09-10 23:21:53 UTC (rev 128124)
+++ trunk/Source/WebKit/chromium/tests/IDBBindingUtilitiesTest.cpp 2012-09-10 23:22:10 UTC (rev 128125)
@@ -28,6 +28,7 @@
#include "IDBKey.h"
#include "IDBKeyPath.h"
#include "SerializedScriptValue.h"
+#include "V8PerIsolateData.h"
#include "V8Utilities.h"
#include <gtest/gtest.h>
@@ -91,7 +92,9 @@
TEST(IDBKeyFromValueAndKeyPathTest, TopLevelPropertyStringValue)
{
- V8AuxiliaryContext v8context;
+ v8::HandleScope handleScope;
+ v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
+
v8::Local<v8::Object> object = v8::Object::New();
object->Set(v8::String::New("foo"), v8::String::New("zoo"));
@@ -103,7 +106,9 @@
TEST(IDBKeyFromValueAndKeyPathTest, TopLevelPropertyNumberValue)
{
- V8AuxiliaryContext v8context;
+ v8::HandleScope handleScope;
+ v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
+
v8::Local<v8::Object> object = v8::Object::New();
object->Set(v8::String::New("foo"), v8::Number::New(456));
@@ -115,7 +120,9 @@
TEST(IDBKeyFromValueAndKeyPathTest, SubProperty)
{
- V8AuxiliaryContext v8context;
+ v8::HandleScope handleScope;
+ v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
+
v8::Local<v8::Object> object = v8::Object::New();
v8::Local<v8::Object> subProperty = v8::Object::New();
subProperty->Set(v8::String::New("bar"), v8::String::New("zee"));
@@ -129,7 +136,9 @@
TEST(InjectIDBKeyTest, TopLevelPropertyStringValue)
{
- V8AuxiliaryContext v8context;
+ v8::HandleScope handleScope;
+ v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
+
v8::Local<v8::Object> object = v8::Object::New();
object->Set(v8::String::New("foo"), v8::String::New("zoo"));
@@ -141,7 +150,9 @@
TEST(InjectIDBKeyTest, SubProperty)
{
- V8AuxiliaryContext v8context;
+ v8::HandleScope handleScope;
+ v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
+
v8::Local<v8::Object> object = v8::Object::New();
v8::Local<v8::Object> subProperty = v8::Object::New();
subProperty->Set(v8::String::New("bar"), v8::String::New("zee"));
