Changeset 128125 in webkit


Ignore:
Timestamp:
Sep 10, 2012 4:22:10 PM (12 years ago)
Author:
abarth@webkit.org
Message:

[V8] V8AuxiliaryContext used by IDB leaks memory
​https://bugs.webkit.org/show_bug.cgi?id=96317

Patch by Adam Barth <​abarth@chromium.org> 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):

Location:
trunk/Source
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r128124 r128125  
     12012-09-10  Adam Barth  <abarth@chromium.org>
     2
     3        [V8] V8AuxiliaryContext used by IDB leaks memory
     4        https://bugs.webkit.org/show_bug.cgi?id=96317
     5
     6        Reviewed by Tony Chang.
     7
     8        Before this patch, we leaked m_auxiliaryContext on V8PerIsolateData
     9        because no one ever called Dispose to balance the call to
     10        V8::Context::New. This patch uses ScopedPersistent to call Dispose
     11        automatically.
     12
     13        Also, I've deleted the V8AuxiliaryContext because it just reinvents
     14        V8::Context::Scope.
     15
     16        * bindings/v8/IDBBindingUtilities.cpp:
     17        (WebCore::createIDBKeyFromSerializedValueAndKeyPath):
     18        (WebCore::injectIDBKeyIntoSerializedValue):
     19        * bindings/v8/V8PerIsolateData.cpp:
     20        (WebCore::V8PerIsolateData::ensureAuxiliaryContext):
     21        (WebCore):
     22        * bindings/v8/V8PerIsolateData.h:
     23        (V8PerIsolateData):
     24        * bindings/v8/V8Utilities.cpp:
     25        * bindings/v8/V8Utilities.h:
     26
    1272012-09-10  Adam Barth  <abarth@chromium.org>
    228

  • trunk/Source/WebCore/bindings/v8/IDBBindingUtilities.cpp

    r125481 r128125  
    163163    RefPtr<SerializedScriptValue> value = prpValue;
    164164
    165     V8AuxiliaryContext context;
     165    v8::HandleScope handleScope;
     166    v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
     167
    166168    v8::Handle<v8::Value> v8Value(value->deserialize());
    167169    v8::Handle<v8::Value> v8Key(getNthValueOnKeyPath(v8Value, keyPathElements, keyPathElements.size()));
    … …  
    208210        return 0;
    209211
    210     V8AuxiliaryContext context;
     212    v8::HandleScope handleScope;
     213    v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
     214
    211215    v8::Handle<v8::Value> v8Value(value->deserialize());
    212216    v8::Handle<v8::Value> parent(ensureNthValueOnKeyPath(v8Value, keyPathElements, keyPathElements.size() - 1));

  • trunk/Source/WebCore/bindings/v8/V8PerIsolateData.cpp

    r127718 r128125  
    108108#endif
    109109
     110v8::Handle<v8::Context> V8PerIsolateData::ensureAuxiliaryContext()
     111{
     112    if (m_auxiliaryContext.isEmpty())
     113        m_auxiliaryContext.adopt(v8::Context::New());
     114    return m_auxiliaryContext.get();
     115}
     116
    110117v8::Handle<v8::Value> V8PerIsolateData::constructorOfToString(const v8::Arguments& args)
    111118{

  • trunk/Source/WebCore/bindings/v8/V8PerIsolateData.h

    r127718 r128125  
    2727#define V8PerIsolateData_h
    2828
     29#include "ScopedPersistent.h"
    2930#include <v8.h>
    3031#include <wtf/HashMap.h>
    … …  
    9293
    9394    V8HiddenPropertyName* hiddenPropertyName() { return m_hiddenPropertyName.get(); }
    94     v8::Persistent<v8::Context>& auxiliaryContext() { return m_auxiliaryContext; }
     95    v8::Handle<v8::Context> ensureAuxiliaryContext();
    9596
    9697    void registerDOMDataStore(DOMDataStore* domDataStore)
    … …  
    152153
    153154    OwnPtr<V8HiddenPropertyName> m_hiddenPropertyName;
    154     v8::Persistent<v8::Context> m_auxiliaryContext;
     155    ScopedPersistent<v8::Context> m_auxiliaryContext;
    155156
    156157    bool m_constructorMode;

  • trunk/Source/WebCore/bindings/v8/V8Utilities.cpp

    r126506 r128125  
    4848
    4949namespace WebCore {
    50 
    51 V8AuxiliaryContext::V8AuxiliaryContext()
    52 {
    53     auxiliaryContext()->Enter();
    54 }
    55 
    56 V8AuxiliaryContext::~V8AuxiliaryContext()
    57 {
    58     auxiliaryContext()->Exit();
    59 }
    60 
    61 v8::Persistent<v8::Context>& V8AuxiliaryContext::auxiliaryContext()
    62 {
    63     v8::Persistent<v8::Context>& context = V8PerIsolateData::current()->auxiliaryContext();
    64     if (context.IsEmpty())
    65         context = v8::Context::New();
    66     return context;
    67 }
    6850
    6951// Use an array to hold dependents. It works like a ref-counted scheme.

  • trunk/Source/WebCore/bindings/v8/V8Utilities.h

    r125478 r128125  
    6363    typedef unsigned CallbackAllowedValueFlags;
    6464
    65     class V8AuxiliaryContext {
    66     public:
    67         V8AuxiliaryContext();
    68         virtual ~V8AuxiliaryContext();
    69     private:
    70         v8::HandleScope m_handleScope;
    71         static v8::Persistent<v8::Context>& auxiliaryContext();
    72     };
    73 
    7465    typedef WTF::Vector<RefPtr<MessagePort>, 1> MessagePortArray;
    7566    typedef WTF::Vector<RefPtr<ArrayBuffer>, 1> ArrayBufferArray;

  • trunk/Source/WebKit/chromium/ChangeLog

    r128123 r128125  
     12012-09-10  Adam Barth  <abarth@chromium.org>
     2
     3        [V8] V8AuxiliaryContext used by IDB leaks memory
     4        https://bugs.webkit.org/show_bug.cgi?id=96317
     5
     6        Reviewed by Tony Chang.
     7
     8        Call the V8 APIs directly instead of using a helper class.
     9
     10        * tests/IDBBindingUtilitiesTest.cpp:
     11        (WebCore::TEST):
     12
    1132012-09-10  Adam Barth  <abarth@chromium.org>
    214

  • trunk/Source/WebKit/chromium/tests/IDBBindingUtilitiesTest.cpp

    r117817 r128125  
    2929#include "IDBKeyPath.h"
    3030#include "SerializedScriptValue.h"
     31#include "V8PerIsolateData.h"
    3132#include "V8Utilities.h"
    3233
    … …  
    9293TEST(IDBKeyFromValueAndKeyPathTest, TopLevelPropertyStringValue)
    9394{
    94     V8AuxiliaryContext v8context;
     95    v8::HandleScope handleScope;
     96    v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
     97
    9598    v8::Local<v8::Object> object = v8::Object::New();
    9699    object->Set(v8::String::New("foo"), v8::String::New("zoo"));
    … …  
    104107TEST(IDBKeyFromValueAndKeyPathTest, TopLevelPropertyNumberValue)
    105108{
    106     V8AuxiliaryContext v8context;
     109    v8::HandleScope handleScope;
     110    v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
     111
    107112    v8::Local<v8::Object> object = v8::Object::New();
    108113    object->Set(v8::String::New("foo"), v8::Number::New(456));
    … …  
    116121TEST(IDBKeyFromValueAndKeyPathTest, SubProperty)
    117122{
    118     V8AuxiliaryContext v8context;
     123    v8::HandleScope handleScope;
     124    v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
     125
    119126    v8::Local<v8::Object> object = v8::Object::New();
    120127    v8::Local<v8::Object> subProperty = v8::Object::New();
    … …  
    130137TEST(InjectIDBKeyTest, TopLevelPropertyStringValue)
    131138{
    132     V8AuxiliaryContext v8context;
     139    v8::HandleScope handleScope;
     140    v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
     141
    133142    v8::Local<v8::Object> object = v8::Object::New();
    134143    object->Set(v8::String::New("foo"), v8::String::New("zoo"));
    … …  
    142151TEST(InjectIDBKeyTest, SubProperty)
    143152{
    144     V8AuxiliaryContext v8context;
     153    v8::HandleScope handleScope;
     154    v8::Context::Scope scope(V8PerIsolateData::current()->ensureAuxiliaryContext());
     155
    145156    v8::Local<v8::Object> object = v8::Object::New();
    146157    v8::Local<v8::Object> subProperty = v8::Object::New();
Note: See TracChangeset for help on using the changeset viewer.