Changeset 147391 in webkit


Ignore:
Timestamp:
Apr 1, 2013 11:23:54 PM (11 years ago)
Author:
esprehn@chromium.org
Message:

Make all v8 weak callbacks type safe
https://bugs.webkit.org/show_bug.cgi?id=111802

Reviewed by Adam Barth.

Make all v8 handle weak callbacks typesafe by adding a new class WeakHandleListner
that uses templates to generate the callback proxies that have the correct type
arguments. Now getting the arguments wrong will fail compilation.

No new tests, no change in behavior.

  • bindings/v8/DOMDataStore.h:

(WebCore::::callback):

  • bindings/v8/DOMWrapperMap.h:

(WebCore::DOMWrapperMap::DOMWrapperMap):
(WebCore::DOMWrapperMap::set):
(WebCore::DOMWrapperMap::reportMemoryUsage):
(DOMWrapperMap):

  • bindings/v8/DOMWrapperWorld.cpp:

(WebCore::::callback):
(WebCore::DOMWrapperWorld::makeContextWeak):

  • bindings/v8/ScriptState.cpp:

(WebCore::::callback):
(WebCore::ScriptState::ScriptState):

  • bindings/v8/ScriptState.h:

(ScriptState):

  • bindings/v8/ScriptWrappable.h:

(ScriptWrappable):
(WebCore::ScriptWrappable::setWrapper):
(WebCore::::callback):

  • bindings/v8/V8AbstractEventListener.cpp:

(WebCore::::callback):
(WebCore::V8AbstractEventListener::setListenerObject):

  • bindings/v8/V8AbstractEventListener.h:

(V8AbstractEventListener):

  • bindings/v8/V8MutationCallback.cpp:

(WebCore::::callback):
(WebCore::V8MutationCallback::V8MutationCallback):

  • bindings/v8/V8MutationCallback.h:

(V8MutationCallback):

  • bindings/v8/V8NPObject.cpp:

(V8NPTemplateMap):
(WebCore::V8NPTemplateMap::set):
(WebCore::::callback):
(WebCore::staticNPObjectMap):

  • bindings/v8/V8Utilities.h:

(WeakHandleListener):
(WebCore::WeakHandleListener::makeWeak):
(WebCore::WeakHandleListener::WeakHandleListener):
(WebCore::WeakHandleListener::invokeWeakCallback):

  • bindings/v8/V8ValueCache.cpp:

(WebCore::::callback):
(WebCore::StringCache::v8ExternalStringSlow):

  • bindings/v8/custom/V8InjectedScriptManager.cpp:

(WebCore::::callback):
(WebCore::createInjectedScriptHostV8Wrapper):

Location:
trunk/Source/WebCore
Files:
15 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r147389 r147391  
     12013-04-01  Elliott Sprehn  <esprehn@chromium.org>
     2
     3        Make all v8 weak callbacks type safe
     4        https://bugs.webkit.org/show_bug.cgi?id=111802
     5
     6        Reviewed by Adam Barth.
     7
     8        Make all v8 handle weak callbacks typesafe by adding a new class WeakHandleListner
     9        that uses templates to generate the callback proxies that have the correct type
     10        arguments. Now getting the arguments wrong will fail compilation.
     11
     12        No new tests, no change in behavior.
     13
     14        * bindings/v8/DOMDataStore.h:
     15        (WebCore::::callback):
     16        * bindings/v8/DOMWrapperMap.h:
     17        (WebCore::DOMWrapperMap::DOMWrapperMap):
     18        (WebCore::DOMWrapperMap::set):
     19        (WebCore::DOMWrapperMap::reportMemoryUsage):
     20        (DOMWrapperMap):
     21        * bindings/v8/DOMWrapperWorld.cpp:
     22        (WebCore::::callback):
     23        (WebCore::DOMWrapperWorld::makeContextWeak):
     24        * bindings/v8/ScriptState.cpp:
     25        (WebCore::::callback):
     26        (WebCore::ScriptState::ScriptState):
     27        * bindings/v8/ScriptState.h:
     28        (ScriptState):
     29        * bindings/v8/ScriptWrappable.h:
     30        (ScriptWrappable):
     31        (WebCore::ScriptWrappable::setWrapper):
     32        (WebCore::::callback):
     33        * bindings/v8/V8AbstractEventListener.cpp:
     34        (WebCore::::callback):
     35        (WebCore::V8AbstractEventListener::setListenerObject):
     36        * bindings/v8/V8AbstractEventListener.h:
     37        (V8AbstractEventListener):
     38        * bindings/v8/V8MutationCallback.cpp:
     39        (WebCore::::callback):
     40        (WebCore::V8MutationCallback::V8MutationCallback):
     41        * bindings/v8/V8MutationCallback.h:
     42        (V8MutationCallback):
     43        * bindings/v8/V8NPObject.cpp:
     44        (V8NPTemplateMap):
     45        (WebCore::V8NPTemplateMap::set):
     46        (WebCore::::callback):
     47        (WebCore::staticNPObjectMap):
     48        * bindings/v8/V8Utilities.h:
     49        (WeakHandleListener):
     50        (WebCore::WeakHandleListener::makeWeak):
     51        (WebCore::WeakHandleListener::WeakHandleListener):
     52        (WebCore::WeakHandleListener::invokeWeakCallback):
     53        * bindings/v8/V8ValueCache.cpp:
     54        (WebCore::::callback):
     55        (WebCore::StringCache::v8ExternalStringSlow):
     56        * bindings/v8/custom/V8InjectedScriptManager.cpp:
     57        (WebCore::::callback):
     58        (WebCore::createInjectedScriptHostV8Wrapper):
     59
    1602013-04-01  Tien-Ren Chen  <trchen@chromium.org>
    261
  • trunk/Source/WebCore/bindings/v8/DOMDataStore.h

    r146357 r147391  
    171171};
    172172
     173template<>
     174inline void WeakHandleListener<DOMWrapperMap<void> >::callback(v8::Isolate* isolate, v8::Persistent<v8::Value> value, DOMWrapperMap<void>* map)
     175{
     176    ASSERT(value->IsObject());
     177    v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
     178    WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
     179    ASSERT(type->derefObjectFunction);
     180    void* key = static_cast<void*>(toNative(wrapper));
     181    map->removeAndDispose(key, wrapper, isolate);
     182    type->derefObject(key);
     183}
     184
    173185} // namespace WebCore
    174186
  • trunk/Source/WebCore/bindings/v8/DOMWrapperMap.h

    r142250 r147391  
    3232#define DOMWrapperMap_h
    3333
     34#include "V8Utilities.h"
    3435#include "WebCoreMemoryInstrumentation.h"
    3536#include "WrapperTypeInfo.h"
     
    4546    typedef HashMap<KeyType*, v8::Persistent<v8::Object> > MapType;
    4647
    47     explicit DOMWrapperMap(v8::Isolate* isolate, v8::NearDeathCallback callback = &defaultWeakCallback)
    48         : m_callback(callback)
    49         , m_isolate(isolate)
     48    explicit DOMWrapperMap(v8::Isolate* isolate)
     49        : m_isolate(isolate)
    5050    {
    5151    }
     
    6262        v8::Persistent<v8::Object> persistent = v8::Persistent<v8::Object>::New(m_isolate, wrapper);
    6363        configuration.configureWrapper(persistent, m_isolate);
    64         persistent.MakeWeak(m_isolate, this, m_callback);
     64        WeakHandleListener<DOMWrapperMap<KeyType> >::makeWeak(m_isolate, persistent, this);
    6565        m_map.set(key, persistent);
    6666    }
     
    8181        MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::Binding);
    8282        info.addMember(m_map, "map");
    83         info.ignoreMember(m_callback);
    8483    }
    8584
     
    9695
    9796private:
    98     static void defaultWeakCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> value, void* context)
    99     {
    100         DOMWrapperMap<KeyType>* map = static_cast<DOMWrapperMap<KeyType>*>(context);
    101         ASSERT(value->IsObject());
    102         v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
    103         WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
    104         ASSERT(type->derefObjectFunction);
    105         KeyType* key = static_cast<KeyType*>(toNative(wrapper));
    106         map->removeAndDispose(key, wrapper, isolate);
    107         type->derefObject(key);
    108     }
    109 
    110     v8::NearDeathCallback m_callback;
    11197    v8::Isolate* m_isolate;
    11298    MapType m_map;
  • trunk/Source/WebCore/bindings/v8/DOMWrapperWorld.cpp

    r144617 r147391  
    8585}
    8686
    87 static void isolatedWorldWeakCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> object, void* parameter)
     87template<>
     88void WeakHandleListener<DOMWrapperWorld>::callback(v8::Isolate* isolate, v8::Persistent<v8::Value> object, DOMWrapperWorld* world)
    8889{
    8990    object.Dispose(isolate);
    9091    object.Clear();
    91     static_cast<DOMWrapperWorld*>(parameter)->deref();
     92    world->deref();
    9293}
    9394
     
    9798    ASSERT(isolatedWorld(context) == this);
    9899    v8::Isolate* isolate = context->GetIsolate();
    99     v8::Persistent<v8::Context>::New(isolate, context).MakeWeak(isolate, this, isolatedWorldWeakCallback);
     100    v8::Persistent<v8::Context> persistent = v8::Persistent<v8::Context>::New(isolate, context);
     101    WeakHandleListener<DOMWrapperWorld>::makeWeak(isolate, persistent, this);
    100102    // Matching deref is in weak callback.
    101103    this->ref();
  • trunk/Source/WebCore/bindings/v8/ScriptState.cpp

    r141771 r147391  
    4646namespace WebCore {
    4747
     48template<>
     49void WeakHandleListener<ScriptState>::callback(v8::Isolate* isolate, v8::Persistent<v8::Value> object, ScriptState* scriptState)
     50{
     51    delete scriptState;
     52}
     53
    4854ScriptState::ScriptState(v8::Handle<v8::Context> context)
    4955    : m_context(context)
    5056{
    51     m_context.get().MakeWeak(context->GetIsolate(), this, &ScriptState::weakReferenceCallback);
     57    WeakHandleListener<ScriptState>::makeWeak(context->GetIsolate(), m_context.get(), this);
    5258}
    5359
     
    8995    ASSERT(!context.IsEmpty());
    9096    return ScriptState::forContext(context);
    91 }
    92 
    93 void ScriptState::weakReferenceCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> object, void* parameter)
    94 {
    95     ScriptState* scriptState = static_cast<ScriptState*>(parameter);
    96     delete scriptState;
    9797}
    9898
  • trunk/Source/WebCore/bindings/v8/ScriptState.h

    r141771 r147391  
    3434#include "DOMWrapperWorld.h"
    3535#include "ScopedPersistent.h"
     36#include "V8Utilities.h"
    3637#include <v8.h>
    3738#include <wtf/Noncopyable.h>
     
    8182private:
    8283    friend ScriptState* mainWorldScriptState(Frame*);
     84    friend class WeakHandleListener<ScriptState>;
    8385    explicit ScriptState(v8::Handle<v8::Context>);
    8486
  • trunk/Source/WebCore/bindings/v8/ScriptWrappable.h

    r143737 r147391  
    3232#define ScriptWrappable_h
    3333
     34#include "V8Utilities.h"
    3435#include "WebCoreMemoryInstrumentation.h"
    3536#include "WrapperTypeInfo.h"
     
    3940
    4041class ScriptWrappable {
     42    friend class WeakHandleListener<ScriptWrappable>;
    4143public:
    4244    ScriptWrappable() { }
     
    5254        v8::Persistent<v8::Object> persistent = v8::Persistent<v8::Object>::New(isolate, wrapper);
    5355        configuration.configureWrapper(persistent, isolate);
    54         persistent.MakeWeak(isolate, this, weakCallback);
     56        WeakHandleListener<ScriptWrappable>::makeWeak(isolate, persistent, this);
    5557        m_maskedWrapper = maskOrUnmaskPointer(*persistent);
    5658    }
     
    8183        return reinterpret_cast<v8::Object*>((objectPointer ^ randomMask) & (!objectPointer - 1)); // Preserve null without branching.
    8284    }
     85};
    8386
    84     static void weakCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> value, void* context)
    85     {
    86         ScriptWrappable* key = static_cast<ScriptWrappable*>(context);
    87         ASSERT(value->IsObject());
    88         v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
    89         ASSERT(key->wrapper() == wrapper);
     87template<>
     88inline void WeakHandleListener<ScriptWrappable>::callback(v8::Isolate* isolate, v8::Persistent<v8::Value> value, ScriptWrappable* key)
     89{
     90    ASSERT(value->IsObject());
     91    v8::Persistent<v8::Object> wrapper = v8::Persistent<v8::Object>::Cast(value);
     92    ASSERT(key->wrapper() == wrapper);
    9093
    91         // Note: |object| might not be equal to |key|, e.g., if ScriptWrappable isn't a left-most base class.
    92         void* object = toNative(wrapper);
    93         WrapperTypeInfo* info = toWrapperTypeInfo(wrapper);
    94         ASSERT(info->derefObjectFunction);
     94    // Note: |object| might not be equal to |key|, e.g., if ScriptWrappable isn't a left-most base class.
     95    void* object = toNative(wrapper);
     96    WrapperTypeInfo* info = toWrapperTypeInfo(wrapper);
     97    ASSERT(info->derefObjectFunction);
    9598
    96         key->disposeWrapper(value, isolate);
    97         // FIXME: I noticed that 50%~ of minor GC cycle times can be consumed
    98         // inside key->deref(), which causes Node destructions. We should
    99         // make Node destructions incremental.
    100         info->derefObject(object);
    101     }
    102 };
     99    key->disposeWrapper(value, isolate);
     100    // FIXME: I noticed that 50%~ of minor GC cycle times can be consumed
     101    // inside key->deref(), which causes Node destructions. We should
     102    // make Node destructions incremental.
     103    info->derefObject(object);
     104}
    103105
    104106} // namespace WebCore
  • trunk/Source/WebCore/bindings/v8/V8AbstractEventListener.cpp

    r142940 r147391  
    4343#include "V8EventTarget.h"
    4444#include "V8HiddenPropertyName.h"
    45 #include "V8Utilities.h"
    4645#include "WorkerContext.h"
    4746
    4847namespace WebCore {
    4948
    50 void V8AbstractEventListener::weakEventListenerCallback(v8::Isolate* isolate, v8::Persistent<v8::Value>, void* parameter)
     49template<>
     50void WeakHandleListener<V8AbstractEventListener>::callback(v8::Isolate*, v8::Persistent<v8::Value>, V8AbstractEventListener* listener)
    5151{
    52     V8AbstractEventListener* listener = static_cast<V8AbstractEventListener*>(parameter);
    5352    listener->m_listener.clear();
    5453}
     
    107106{
    108107    m_listener.set(listener);
    109     m_listener.get().MakeWeak(m_isolate, this, &V8AbstractEventListener::weakEventListenerCallback);
     108    WeakHandleListener<V8AbstractEventListener>::makeWeak(m_isolate, m_listener.get(), this);
    110109}
    111110
  • trunk/Source/WebCore/bindings/v8/V8AbstractEventListener.h

    r141771 r147391  
    3434#include "EventListener.h"
    3535#include "ScopedPersistent.h"
     36#include "V8Utilities.h"
    3637#include "WorldContextHandle.h"
    3738#include <v8.h>
     
    5253    // but ALLOWs duplicated non-HTML event handlers.
    5354    class V8AbstractEventListener : public EventListener {
     55        friend class WeakHandleListener<V8AbstractEventListener>;
    5456    public:
    5557        virtual ~V8AbstractEventListener();
  • trunk/Source/WebCore/bindings/v8/V8MutationCallback.cpp

    r145379 r147391  
    3636namespace WebCore {
    3737
     38template<>
     39void WeakHandleListener<V8MutationCallback>::callback(v8::Isolate*, v8::Persistent<v8::Value>, V8MutationCallback* callback)
     40{
     41    callback->m_callback.clear();
     42}
     43
    3844V8MutationCallback::V8MutationCallback(v8::Handle<v8::Function> callback, ScriptExecutionContext* context, v8::Handle<v8::Object> owner, v8::Isolate* isolate)
    3945    : ActiveDOMCallback(context)
     
    4248{
    4349    owner->SetHiddenValue(V8HiddenPropertyName::callback(), callback);
    44     m_callback.get().MakeWeak(isolate, this, &V8MutationCallback::weakCallback);
     50    WeakHandleListener<V8MutationCallback>::makeWeak(isolate, m_callback.get(), this);
    4551}
    4652
  • trunk/Source/WebCore/bindings/v8/V8MutationCallback.h

    r145379 r147391  
    3131#include "MutationCallback.h"
    3232#include "ScopedPersistent.h"
     33#include "V8Utilities.h"
    3334#include <v8.h>
    3435#include <wtf/RefPtr.h>
     
    3940
    4041class V8MutationCallback : public MutationCallback, public ActiveDOMCallback {
     42    friend class WeakHandleListener<V8MutationCallback>;
    4143public:
    4244    static PassRefPtr<V8MutationCallback> create(v8::Handle<v8::Function> callback, ScriptExecutionContext* context, v8::Handle<v8::Object> owner, v8::Isolate* isolate)
     
    5254    V8MutationCallback(v8::Handle<v8::Function>, ScriptExecutionContext*, v8::Handle<v8::Object>, v8::Isolate*);
    5355
    54     static void weakCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> wrapper, void* parameter)
    55     {
    56         V8MutationCallback* object = static_cast<V8MutationCallback*>(parameter);
    57         object->m_callback.clear();
    58     }
    59 
    6056    ScopedPersistent<v8::Function> m_callback;
    6157    RefPtr<DOMWrapperWorld> m_world;
  • trunk/Source/WebCore/bindings/v8/V8NPObject.cpp

    r145906 r147391  
    161161}
    162162
    163 
    164 class V8NPTemplateMap
    165 {
     163class V8NPTemplateMap {
     164    friend class WeakHandleListener<V8NPTemplateMap, PrivateIdentifier>;
    166165public:
    167166    // NPIdentifier is PrivateIdentifier*.
     
    177176        ASSERT(!m_map.contains(key));
    178177        m_map.set(key, wrapper);
    179         wrapper.MakeWeak(m_isolate, key, weakCallback);
     178        WeakHandleListener<V8NPTemplateMap, PrivateIdentifier>::makeWeak(m_isolate, wrapper, key);
    180179    }
    181180
     
    191190        : m_isolate(isolate)
    192191    {
    193     }
    194 
    195     static void weakCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> object, void* context)
    196     {
    197         sharedInstance(isolate).dispose(static_cast<PrivateIdentifier*>(context));
    198192    }
    199193
     
    210204    v8::Isolate* m_isolate;
    211205};
     206
     207template<>
     208void WeakHandleListener<V8NPTemplateMap, PrivateIdentifier>::callback(v8::Isolate* isolate, v8::Persistent<v8::Value>, PrivateIdentifier* key)
     209{
     210    V8NPTemplateMap::sharedInstance(isolate).dispose(key);
     211}
    212212
    213213static v8::Handle<v8::Value> npObjectGetProperty(v8::Local<v8::Object> self, NPIdentifier identifier, v8::Local<v8::Value> key, v8::Isolate* isolate)
     
    385385}
    386386
    387 static void weakNPObjectCallback(v8::Isolate*, v8::Persistent<v8::Value>, void*);
    388 
    389387static DOMWrapperMap<NPObject>& staticNPObjectMap()
    390388{
    391     DEFINE_STATIC_LOCAL(DOMWrapperMap<NPObject>, npObjectMap, (v8::Isolate::GetCurrent(), &weakNPObjectCallback));
     389    DEFINE_STATIC_LOCAL(DOMWrapperMap<NPObject>, npObjectMap, (v8::Isolate::GetCurrent()));
    392390    return npObjectMap;
    393391}
    394392
    395 static void weakNPObjectCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> value, void*)
     393template<>
     394inline void WeakHandleListener<DOMWrapperMap<NPObject> >::callback(v8::Isolate* isolate, v8::Persistent<v8::Value> value, DOMWrapperMap<NPObject>*)
    396395{
    397396    ASSERT(value->IsObject());
  • trunk/Source/WebCore/bindings/v8/V8Utilities.h

    r141946 r147391  
    6464    bool getMessagePortArray(v8::Local<v8::Value>, MessagePortArray&, v8::Isolate*);
    6565
     66    // FIXME: We might move this to its own file.
     67    template<class OwnerType, class ArgumentType = OwnerType>
     68    class WeakHandleListener {
     69    public:
     70        template <class HandleType>
     71        static void makeWeak(v8::Isolate* isolate, HandleType handle, ArgumentType* parameter)
     72        {
     73            handle.MakeWeak(isolate, parameter, &invokeWeakCallback);
     74        }
     75
     76        static void callback(v8::Isolate*, v8::Persistent<v8::Value>, ArgumentType*);
     77
     78    private:
     79        static void invokeWeakCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> handle, void* parameter)
     80        {
     81            WeakHandleListener<OwnerType, ArgumentType>::callback(isolate, handle, static_cast<ArgumentType*>(parameter));
     82        }
     83    };
     84
    6685} // namespace WebCore
    6786
  • trunk/Source/WebCore/bindings/v8/V8ValueCache.cpp

    r145049 r147391  
    5858}
    5959
    60 static void cachedStringCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> wrapper, void* parameter)
     60template<>
     61void WeakHandleListener<StringCache, StringImpl>::callback(v8::Isolate* isolate, v8::Persistent<v8::Value> wrapper, StringImpl* stringImpl)
    6162{
    62     StringImpl* stringImpl = static_cast<StringImpl*>(parameter);
    6363    V8PerIsolateData::current()->stringCache()->remove(stringImpl);
    6464    wrapper.Dispose(isolate);
     
    100100    stringImpl->ref();
    101101    wrapper.MarkIndependent(isolate);
    102     wrapper.MakeWeak(isolate, stringImpl, cachedStringCallback);
     102    WeakHandleListener<StringCache, StringImpl>::makeWeak(isolate, wrapper, stringImpl);
    103103    m_stringCache.set(stringImpl, wrapper);
    104104
  • trunk/Source/WebCore/bindings/v8/custom/V8InjectedScriptManager.cpp

    r145353 r147391  
    4646namespace WebCore {
    4747
    48 static void WeakReferenceCallback(v8::Isolate* isolate, v8::Persistent<v8::Value> object, void* parameter)
     48template<>
     49void WeakHandleListener<InjectedScriptManager, InjectedScriptHost>::callback(v8::Isolate* isolate, v8::Persistent<v8::Value> object, InjectedScriptHost* host)
    4950{
    50     InjectedScriptHost* nativeObject = static_cast<InjectedScriptHost*>(parameter);
    51     nativeObject->deref();
     51    host->deref();
    5252    object.Dispose(isolate);
    5353    object.Clear();
     
    7171    host->ref();
    7272    v8::Persistent<v8::Object> weakHandle = v8::Persistent<v8::Object>::New(isolate, instance);
    73     weakHandle.MakeWeak(isolate, host, &WeakReferenceCallback);
     73    WeakHandleListener<InjectedScriptManager, InjectedScriptHost>::makeWeak(isolate, weakHandle, host);
    7474    return instance;
    7575}
Note: See TracChangeset for help on using the changeset viewer.