Changeset 83773 in webkit


Ignore:
Timestamp:
Apr 13, 2011 3:15:28 PM (13 years ago)
Author:
ggaren@apple.com
Message:

2011-04-13 Geoffrey Garen <ggaren@apple.com>

Reviewed by Oliver Hunt.

Switched DOM wrappers to use HashMap of Weak<T> instead of WeakGCMap<T>
https://bugs.webkit.org/show_bug.cgi?id=58482


This will allow wrappers to make individual decisions about their lifetimes.

  • heap/HandleHeap.h: (JSC::HandleHeap::copyWeak): New function for copying a weak handle. It's wasn't previously possible to perform this operation using HandleHeap API because the HandleHeap doesn't expose its underlying Node structure.
  • heap/Local.h: (JSC::::set):
  • heap/Strong.h: (JSC::Strong::set): Added ASSERTs to verify that dead objects are not resurrected by placement into handles.

(JSC::swap): Added a swap helper, so use of Strong<T> inside a hash table
is efficient.

  • heap/Weak.h: (JSC::Weak::Weak): Fixed a bug where copying a weak pointer would not copy its weak callback and context.

(JSC::Weak::operator=): Added an assignment operator, since the default
C++ assignment operator did the wrong thing.

(JSC::Weak::set): Added ASSERTs to verify that dead objects are not
resurrected by placement into handles.

(JSC::swap): Added a swap helper, so use of Strong<T> inside a hash table
is efficient, and can be done without copying, which is illegal during
the handle finalization phase.

2011-04-13 Geoffrey Garen <ggaren@apple.com>

Reviewed by Oliver Hunt.

Switched DOM wrappers to use HashMap of Weak<T> instead of WeakGCMap<T>
https://bugs.webkit.org/show_bug.cgi?id=58482

This will allow wrappers to make individual decisions about their lifetimes.

  • bindings/js/DOMWrapperWorld.cpp: (WebCore::DOMWrapperWorld::DOMWrapperWorld): (WebCore::JSNodeHandleOwner::isReachableFromOpaqueRoots): (WebCore::JSNodeHandleOwner::finalize): (WebCore::DOMObjectHandleOwner::isReachableFromOpaqueRoots): (WebCore::DOMObjectHandleOwner::finalize):
  • bindings/js/DOMWrapperWorld.h: (WebCore::JSNodeHandleOwner::JSNodeHandleOwner): (WebCore::DOMObjectHandleOwner::DOMObjectHandleOwner): (WebCore::DOMWrapperWorld::jsNodeHandleOwner): (WebCore::DOMWrapperWorld::domObjectHandleOwner): Added handle owners for JSNode and DOMObject, our two hash table values. For now, the owners just take care to remove their handles from their respective hash tables.


Changed the hash table type to be a standard HashMap of weak pointers,
instead of a WeakGCMap.

  • bindings/js/JSDOMBinding.cpp: (WebCore::getCachedDOMObjectWrapper): (WebCore::cacheDOMObjectWrapper): (WebCore::cacheDOMNodeWrapper): (WebCore::isObservableThroughDOM): (WebCore::markDOMNodesForDocument): (WebCore::takeWrappers): (WebCore::updateDOMNodeDocument): (WebCore::markDOMObjectWrapper): (WebCore::markDOMNodeWrapper): Updated wrapper hash table access to accomodate its new data type.
  • bindings/js/JSNodeCustom.h: (WebCore::getCachedDOMNodeWrapper): Ditto.
  • dom/Document.h: Updated declaration to match the above.
Location:
trunk/Source
Files:
11 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r83751 r83773  
     12011-04-13  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Oliver Hunt.
     4
     5        Switched DOM wrappers to use HashMap of Weak<T> instead of WeakGCMap<T>
     6        https://bugs.webkit.org/show_bug.cgi?id=58482
     7       
     8        This will allow wrappers to make individual decisions about their lifetimes.
     9
     10        * heap/HandleHeap.h:
     11        (JSC::HandleHeap::copyWeak): New function for copying a weak handle.
     12        It's wasn't previously possible to perform this operation using HandleHeap
     13        API because the HandleHeap doesn't expose its underlying Node structure.
     14
     15        * heap/Local.h:
     16        (JSC::::set):
     17        * heap/Strong.h:
     18        (JSC::Strong::set): Added ASSERTs to verify that dead objects are not
     19        resurrected by placement into handles.
     20
     21        (JSC::swap): Added a swap helper, so use of Strong<T> inside a hash table
     22        is efficient.
     23
     24        * heap/Weak.h:
     25        (JSC::Weak::Weak): Fixed a bug where copying a weak pointer would not
     26        copy its weak callback and context.
     27
     28        (JSC::Weak::operator=): Added an assignment operator, since the default
     29        C++ assignment operator did the wrong thing.
     30
     31        (JSC::Weak::set): Added ASSERTs to verify that dead objects are not
     32        resurrected by placement into handles.
     33
     34        (JSC::swap): Added a swap helper, so use of Strong<T> inside a hash table
     35        is efficient, and can be done without copying, which is illegal during
     36        the handle finalization phase.
     37
    1382011-04-13  Oliver Hunt  <oliver@apple.com>
    239
  • trunk/Source/JavaScriptCore/heap/HandleHeap.h

    r83388 r83773  
    5959
    6060    void makeWeak(HandleSlot, WeakHandleOwner* = 0, void* context = 0);
     61    HandleSlot copyWeak(HandleSlot);
    6162
    6263    void markStrongHandles(HeapRootMarker&);
     
    166167}
    167168
     169inline HandleSlot HandleHeap::copyWeak(HandleSlot other)
     170{
     171    Node* node = toNode(allocate());
     172    node->makeWeak(toNode(other)->weakOwner(), toNode(other)->weakOwnerContext());
     173    writeBarrier(node->slot(), *other);
     174    *node->slot() = *other;
     175    return toHandle(node);
     176}
     177
    168178inline void HandleHeap::makeWeak(HandleSlot handle, WeakHandleOwner* weakOwner, void* context)
    169179{
  • trunk/Source/JavaScriptCore/heap/Local.h

    r83637 r83773  
    9292}
    9393
    94 template <typename T> inline void Local<T>::set(ExternalType value)
     94template <typename T> inline void Local<T>::set(ExternalType externalType)
    9595{
    9696    ASSERT(slot());
    97     *slot() = value;
     97    ASSERT(!HandleTypes<T>::toJSValue(externalType) || !HandleTypes<T>::toJSValue(externalType).isCell() || Heap::isMarked(HandleTypes<T>::toJSValue(externalType).asCell()));
     98    *slot() = externalType;
    9899}
    99100
  • trunk/Source/JavaScriptCore/heap/Strong.h

    r83664 r83773  
    138138        ASSERT(slot());
    139139        JSValue value = HandleTypes<T>::toJSValue(externalType);
     140        ASSERT(!value || !value.isCell() || Heap::isMarked(value.asCell()));
    140141        HandleHeap::heapFor(slot())->writeBarrier(slot(), value);
    141142        *slot() = value;
    142143    }
    143144};
     145
     146template<class T> inline void swap(Strong<T>& a, Strong<T>& b)
     147{
     148    a.swap(b);
     149}
    144150
    145151} // namespace JSC
  • trunk/Source/JavaScriptCore/heap/Weak.h

    r83664 r83773  
    5959        if (!other.slot())
    6060            return;
    61         setSlot(HandleHeap::heapFor(other.slot())->allocate());
    62         set(other.get());
     61        setSlot(HandleHeap::heapFor(other.slot())->copyWeak(other.slot()));
    6362    }
    6463
     
    6867        if (!other.slot())
    6968            return;
    70         setSlot(HandleHeap::heapFor(other.slot())->allocate());
    71         set(other.get());
     69        setSlot(HandleHeap::heapFor(other.slot())->copyWeak(other.slot()));
    7270    }
    7371   
     
    109107    }
    110108
     109    template <typename U> Weak& operator=(const Weak<U>& other)
     110    {
     111        clear();
     112        if (other.slot())
     113            setSlot(HandleHeap::heapFor(other.slot())->copyWeak(other.slot()));
     114        return *this;
     115    }
     116
     117    Weak& operator=(const Weak& other)
     118    {
     119        clear();
     120        if (other.slot())
     121            setSlot(HandleHeap::heapFor(other.slot())->copyWeak(other.slot()));
     122        return *this;
     123    }
     124
    111125private:
    112126    static HandleSlot hashTableDeletedValue() { return reinterpret_cast<HandleSlot>(-1); }
     
    116130        ASSERT(slot());
    117131        JSValue value = HandleTypes<T>::toJSValue(externalType);
     132        ASSERT(!value || !value.isCell() || Heap::isMarked(value.asCell()));
    118133        HandleHeap::heapFor(slot())->writeBarrier(slot(), value);
    119134        *slot() = value;
    120135    }
    121136};
     137
     138template<class T> inline void swap(Weak<T>& a, Weak<T>& b)
     139{
     140    a.swap(b);
     141}
    122142
    123143} // namespace JSC
  • trunk/Source/WebCore/ChangeLog

    r83771 r83773  
     12011-04-13  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Oliver Hunt.
     4
     5        Switched DOM wrappers to use HashMap of Weak<T> instead of WeakGCMap<T>
     6        https://bugs.webkit.org/show_bug.cgi?id=58482
     7
     8        This will allow wrappers to make individual decisions about their lifetimes.
     9
     10        * bindings/js/DOMWrapperWorld.cpp:
     11        (WebCore::DOMWrapperWorld::DOMWrapperWorld):
     12        (WebCore::JSNodeHandleOwner::isReachableFromOpaqueRoots):
     13        (WebCore::JSNodeHandleOwner::finalize):
     14        (WebCore::DOMObjectHandleOwner::isReachableFromOpaqueRoots):
     15        (WebCore::DOMObjectHandleOwner::finalize):
     16        * bindings/js/DOMWrapperWorld.h:
     17        (WebCore::JSNodeHandleOwner::JSNodeHandleOwner):
     18        (WebCore::DOMObjectHandleOwner::DOMObjectHandleOwner):
     19        (WebCore::DOMWrapperWorld::jsNodeHandleOwner):
     20        (WebCore::DOMWrapperWorld::domObjectHandleOwner): Added handle owners
     21        for JSNode and DOMObject, our two hash table values. For now, the owners
     22        just take care to remove their handles from their respective hash tables.
     23       
     24        Changed the hash table type to be a standard HashMap of weak pointers,
     25        instead of a WeakGCMap.
     26
     27        * bindings/js/JSDOMBinding.cpp:
     28        (WebCore::getCachedDOMObjectWrapper):
     29        (WebCore::cacheDOMObjectWrapper):
     30        (WebCore::cacheDOMNodeWrapper):
     31        (WebCore::isObservableThroughDOM):
     32        (WebCore::markDOMNodesForDocument):
     33        (WebCore::takeWrappers):
     34        (WebCore::updateDOMNodeDocument):
     35        (WebCore::markDOMObjectWrapper):
     36        (WebCore::markDOMNodeWrapper): Updated wrapper hash table access to
     37        accomodate its new data type.
     38
     39        * bindings/js/JSNodeCustom.h:
     40        (WebCore::getCachedDOMNodeWrapper): Ditto.
     41
     42        * dom/Document.h: Updated declaration to match the above.
     43
    1442011-04-13  Sam Weinig  <sam@webkit.org>
    245
  • trunk/Source/WebCore/bindings/js/DOMWrapperWorld.cpp

    r75265 r83773  
    2323
    2424#include "JSDOMWindow.h"
     25#include "JSNode.h"
    2526#include "ScriptController.h"
    2627#include "WebCoreJSClientData.h"
     28#include <heap/Weak.h>
    2729
    2830using namespace JSC;
     
    3335    : m_globalData(globalData)
    3436    , m_isNormal(isNormal)
     37    , m_jsNodeHandleOwner(this)
     38    , m_domObjectHandleOwner(this)
    3539{
    3640    JSGlobalData::ClientData* clientData = m_globalData->clientData;
     
    8084}
    8185
     86bool JSNodeHandleOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void*, JSC::MarkStack&)
     87{
     88    return false;
     89}
     90
     91void JSNodeHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void*)
     92{
     93    JSNode* jsNode = static_cast<JSNode*>(handle.get().asCell());
     94    Node* node = jsNode->impl();
     95    ASSERT(node->document());
     96    ASSERT(node->document()->getWrapperCache(m_world)->find(node)->second == jsNode);
     97    node->document()->getWrapperCache(m_world)->remove(node);
     98}
     99
     100bool DOMObjectHandleOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void*, JSC::MarkStack&)
     101{
     102    return false;
     103}
     104
     105void DOMObjectHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void*)
     106{
     107    DOMObject* domObject = static_cast<DOMObject*>(handle.get().asCell());
     108    m_world->m_wrappers.remove(domObject);
     109}
     110
    82111} // namespace WebCore
  • trunk/Source/WebCore/bindings/js/DOMWrapperWorld.h

    r77151 r83773  
    3333class ScriptController;
    3434
    35 typedef JSC::WeakGCMap<void*, DOMObject> DOMObjectWrapperMap;
    36 typedef JSC::WeakGCMap<StringImpl*, JSC::JSString> JSStringCache;
     35typedef HashMap<void*, JSC::Weak<DOMObject> > DOMObjectWrapperMap;
     36typedef JSC::WeakGCMap<StringImpl*, JSC::JSString> JSStringCache;
     37
     38class JSNodeHandleOwner : public JSC::WeakHandleOwner {
     39public:
     40    JSNodeHandleOwner(DOMWrapperWorld*);
     41    virtual bool isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void*, JSC::MarkStack&);
     42    virtual void finalize(JSC::Handle<JSC::Unknown>, void*);
     43
     44private:
     45    DOMWrapperWorld* m_world;
     46};
     47
     48inline JSNodeHandleOwner::JSNodeHandleOwner(DOMWrapperWorld* world)
     49    : m_world(world)
     50{
     51}
     52
     53class DOMObjectHandleOwner : public JSC::WeakHandleOwner {
     54public:
     55    DOMObjectHandleOwner(DOMWrapperWorld*);
     56    virtual bool isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void*, JSC::MarkStack&);
     57    virtual void finalize(JSC::Handle<JSC::Unknown>, void*);
     58
     59private:
     60    DOMWrapperWorld* m_world;
     61};
     62
     63inline DOMObjectHandleOwner::DOMObjectHandleOwner(DOMWrapperWorld* world)
     64    : m_world(world)
     65{
     66}
    3767
    3868class DOMWrapperWorld : public RefCounted<DOMWrapperWorld> {
     
    6090
    6191    JSC::JSGlobalData* globalData() const { return m_globalData; }
     92    JSNodeHandleOwner* jsNodeHandleOwner() { return &m_jsNodeHandleOwner; }
     93    DOMObjectHandleOwner* domObjectHandleOwner() { return &m_domObjectHandleOwner; }
    6294
    6395protected:
     
    69101    HashSet<ScriptController*> m_scriptControllersWithWindowShells;
    70102    bool m_isNormal;
     103    JSNodeHandleOwner m_jsNodeHandleOwner;
     104    DOMObjectHandleOwner m_domObjectHandleOwner;
    71105};
    72106
  • trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp

    r82173 r83773  
    159159DOMObject* getCachedDOMObjectWrapper(JSC::ExecState* exec, void* objectHandle)
    160160{
    161     return domObjectWrapperMapFor(exec).get(objectHandle);
     161    return domObjectWrapperMapFor(exec).get(objectHandle).get();
    162162}
    163163
    164164void cacheDOMObjectWrapper(JSC::ExecState* exec, void* objectHandle, DOMObject* wrapper)
    165165{
    166     domObjectWrapperMapFor(exec).set(exec->globalData(), objectHandle, wrapper);
     166    DOMWrapperWorld* world = currentWorld(exec);
     167    world->m_wrappers.set(objectHandle, Weak<DOMObject>(*world->globalData(), wrapper, world->domObjectHandleOwner()));
    167168}
    168169
     
    182183void cacheDOMNodeWrapper(JSC::ExecState* exec, Document* document, Node* node, JSNode* wrapper)
    183184{
     185    ASSERT(wrapper);
    184186    if (!document)
    185         domObjectWrapperMapFor(exec).set(exec->globalData(), node, wrapper);
     187        cacheDOMObjectWrapper(exec, node, wrapper);
    186188    else
    187         document->getWrapperCache(currentWorld(exec))->set(exec->globalData(), node, wrapper);
    188 
    189     if (currentWorld(exec)->isNormal())
     189        document->getWrapperCache(currentWorld(exec))->set(node, Weak<JSNode>(exec->globalData(), wrapper, currentWorld(exec)->jsNodeHandleOwner()));
     190
     191    if (currentWorld(exec)->isNormal()) {
    190192        node->setWrapper(exec->globalData(), wrapper);
     193        ASSERT(node->wrapper() == (document ? document->getWrapperCache(currentWorld(exec))->get(node).get() : domObjectWrapperMapFor(exec).get(node).get()));
     194    }
    191195}
    192196
     
    220224        if (node->isElementNode()) {
    221225            if (NamedNodeMap* attributes = static_cast<Element*>(node)->attributeMap()) {
    222                 if (DOMObject* wrapper = world->m_wrappers.get(attributes)) {
     226                if (DOMObject* wrapper = world->m_wrappers.get(attributes).get()) {
    223227                    // FIXME: This check seems insufficient, because NamedNodeMap items can have custom properties themselves.
    224228                    // Maybe it would be OK to just keep the wrapper alive, as it is done for CSSOM objects below.
     
    235239            if (static_cast<Element*>(node)->hasTagName(canvasTag)) {
    236240                if (CanvasRenderingContext* context = static_cast<HTMLCanvasElement*>(node)->renderingContext()) {
    237                     if (DOMObject* wrapper = world->m_wrappers.get(context)) {
     241                    if (DOMObject* wrapper = world->m_wrappers.get(context).get()) {
    238242                        if (wrapper->hasCustomProperties())
    239243                            return true;
     
    290294        JSWrapperCache::iterator nodeEnd = nodeDict->end();
    291295        for (JSWrapperCache::iterator nodeIt = nodeDict->begin(); nodeIt != nodeEnd; ++nodeIt) {
    292             if (isObservableThroughDOM(nodeIt.get().second, world))
    293                 markStack.deprecatedAppend(nodeIt.getSlot().second);
     296            JSNode* node = nodeIt->second.get();
     297            if (!isObservableThroughDOM(node, world))
     298                continue;
     299            markStack.deprecatedAppend(reinterpret_cast<JSCell**>(&node));
    294300        }
    295301    }
     
    328334        JSWrapperCacheMap& wrapperCacheMap = document->wrapperCacheMap();
    329335        for (JSWrapperCacheMap::iterator iter = wrapperCacheMap.begin(); iter != wrapperCacheMap.end(); ++iter) {
    330             if (JSNode* wrapper = iter->second->take(node))
    331                 wrapperSet.append(WrapperAndWorld(wrapper, iter->first));
     336            JSNode* wrapper = iter->second->take(node).get();
     337            if (!wrapper)
     338                continue;
     339            wrapperSet.append(WrapperAndWorld(wrapper, iter->first));
    332340        }
    333341    } else {
    334342        for (JSGlobalDataWorldIterator worldIter(JSDOMWindow::commonJSGlobalData()); worldIter; ++worldIter) {
    335343            DOMWrapperWorld* world = *worldIter;
    336             if (JSNode* wrapper = static_cast<JSNode*>(world->m_wrappers.take(node)))
     344            if (JSNode* wrapper = static_cast<JSNode*>(world->m_wrappers.take(node).get()))
    337345                wrapperSet.append(WrapperAndWorld(wrapper, world));
    338346        }
     
    350358        JSNode* wrapper = wrapperSet[i].first;
    351359        if (newDocument)
    352             newDocument->getWrapperCache(wrapperSet[i].second)->set(*wrapperSet[i].second->globalData(), node, wrapper);
     360            newDocument->getWrapperCache(wrapperSet[i].second)->set(node, Weak<JSNode>(*wrapperSet[i].second->globalData(), wrapper, wrapperSet[i].second->jsNodeHandleOwner()));
    353361        else
    354             wrapperSet[i].second->m_wrappers.set(*wrapperSet[i].second->globalData(), node, wrapper);
     362            wrapperSet[i].second->m_wrappers.set(node, Weak<DOMObject>(*wrapperSet[i].second->globalData(), wrapper, wrapperSet[i].second->domObjectHandleOwner()));
    355363    }
    356364}
     
    365373
    366374    for (JSGlobalDataWorldIterator worldIter(&globalData); worldIter; ++worldIter) {
    367         if (HandleSlot wrapperSlot = worldIter->m_wrappers.getSlot(object))
    368             markStack.deprecatedAppend(wrapperSlot);
     375        if (DOMObject* wrapper = worldIter->m_wrappers.get(object).get())
     376            markStack.deprecatedAppend(reinterpret_cast<JSCell**>(&wrapper));
    369377    }
    370378}
     
    375383        JSWrapperCacheMap& wrapperCacheMap = document->wrapperCacheMap();
    376384        for (JSWrapperCacheMap::iterator iter = wrapperCacheMap.begin(); iter != wrapperCacheMap.end(); ++iter) {
    377             if (HandleSlot wrapperSlot = iter->second->getSlot(node))
    378                 markStack.deprecatedAppend(wrapperSlot);
     385            JSNode* wrapper = iter->second->get(node).get();
     386            if (!wrapper)
     387                continue;
     388            markStack.deprecatedAppend(reinterpret_cast<JSCell**>(&wrapper));
    379389        }
    380390        return;
     
    382392
    383393    for (JSGlobalDataWorldIterator worldIter(JSDOMWindow::commonJSGlobalData()); worldIter; ++worldIter) {
    384         if (HandleSlot wrapperSlot = worldIter->m_wrappers.getSlot(node))
    385             markStack.deprecatedAppend(wrapperSlot);
     394        if (DOMObject* wrapper = worldIter->m_wrappers.get(node).get())
     395            markStack.deprecatedAppend(reinterpret_cast<JSCell**>(&wrapper));
    386396    }
    387397}
  • trunk/Source/WebCore/bindings/js/JSNodeCustom.h

    r55215 r83773  
    3535{
    3636    if (currentWorld(exec)->isNormal()) {
    37         ASSERT(node->wrapper() == (document ? document->getWrapperCache(currentWorld(exec))->get(node) : domObjectWrapperMapFor(exec).get(node)));
     37        ASSERT(node->wrapper() == (document ? document->getWrapperCache(currentWorld(exec))->get(node).get() : domObjectWrapperMapFor(exec).get(node).get()));
    3838        return static_cast<JSNode*>(node->wrapper());
    3939    }
    4040
    4141    if (document)
    42         return document->getWrapperCache(currentWorld(exec))->get(node);
    43     return static_cast<JSNode*>(domObjectWrapperMapFor(exec).get(node));
     42        return document->getWrapperCache(currentWorld(exec))->get(node).get();
     43    return static_cast<JSNode*>(domObjectWrapperMapFor(exec).get(node).get());
    4444}
    4545
  • trunk/Source/WebCore/dom/Document.h

    r83349 r83773  
    4646
    4747#if USE(JSC)
    48 #include <runtime/WeakGCMap.h>
     48#include <heap/Weak.h>
    4949#endif
    5050
     
    949949
    950950#if USE(JSC)
    951     typedef JSC::WeakGCMap<WebCore::Node*, JSNode> JSWrapperCache;
     951    typedef HashMap<WebCore::Node*, JSC::Weak<JSNode> > JSWrapperCache;
    952952    typedef HashMap<DOMWrapperWorld*, JSWrapperCache*> JSWrapperCacheMap;
    953953    JSWrapperCacheMap& wrapperCacheMap() { return m_wrapperCacheMap; }
Note: See TracChangeset for help on using the changeset viewer.