Changeset 84049 in webkit

Timestamp:

Apr 15, 2011 4:38:04 PM (13 years ago)

Author:

ggaren@apple.com

Message:

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

Reviewed by Oliver Hunt.

DOM object handles are never removed from cache
​https://bugs.webkit.org/show_bug.cgi?id=58707

We were trying to remove hash table items by value instead of by key.

• bindings/js/DOMWrapperWorld.cpp: (WebCore::JSNodeHandleOwner::finalize): Changed to work more like DOMObjectHandleOwner::finalize because I'm going to merge them.

(WebCore::DOMObjectHandleOwner::finalize): Remove hash table items
by key, not value. (Oops!) Use a helper function to make sure we get
this right.

• bindings/js/JSDOMBinding.cpp: (WebCore::cacheDOMObjectWrapper): Store the hash table key as our weak handle context, so we can use it at destruction time.

• bindings/js/JSDOMBinding.h: Removed unnecessary include.

• bindings/js/JSNodeCustom.h: (WebCore::cacheDOMNodeWrapper): Store the hash table key as our weak handle context, so we can use it at destruction time.

• bindings/js/ScriptWrappable.h: (WebCore::ScriptWrappable::setWrapper): Forward context parameter, to support the above.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/DOMWrapperWorld.cpp (modified) (2 diffs)
• bindings/js/JSDOMBinding.cpp (modified) (1 diff)
• bindings/js/JSDOMBinding.h (modified) (1 diff)
• bindings/js/JSNodeCustom.h (modified) (1 diff)
• bindings/js/ScriptWrappable.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-04-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        DOM object handles are never removed from cache
+        https://bugs.webkit.org/show_bug.cgi?id=58707
+
+        We were trying to remove hash table items by value instead of by key.
+
+        * bindings/js/DOMWrapperWorld.cpp:
+        (WebCore::JSNodeHandleOwner::finalize): Changed to work more like
+        DOMObjectHandleOwner::finalize because I'm going to merge them.
+
+        (WebCore::DOMObjectHandleOwner::finalize): Remove hash table items
+        by key, not value. (Oops!) Use a helper function to make sure we get
+        this right.
+
+        * bindings/js/JSDOMBinding.cpp:
+        (WebCore::cacheDOMObjectWrapper): Store the hash table key as our weak
+        handle context, so we can use it at destruction time.
+
+        * bindings/js/JSDOMBinding.h: Removed unnecessary include.
+
+        * bindings/js/JSNodeCustom.h:
+        (WebCore::cacheDOMNodeWrapper): Store the hash table key as our weak
+        handle context, so we can use it at destruction time.
+
+        * bindings/js/ScriptWrappable.h:
+        (WebCore::ScriptWrappable::setWrapper): Forward context parameter, to
+        support the above.
+
 2011-04-15  Kenneth Russell  <kbr@google.com>
 

trunk/Source/WebCore/bindings/js/DOMWrapperWorld.cpp

@@ -189 +189 @@
 }
 
-void JSNodeHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void*)
+void JSNodeHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
 {
     JSNode* jsNode = static_cast<JSNode*>(handle.get().asCell());
-    uncacheDOMNodeWrapper(m_world, jsNode->impl(), jsNode);
+    uncacheDOMNodeWrapper(m_world, static_cast<Node*>(context), jsNode);
 }
 
@@ -200 +200 @@
 }
 
-void DOMObjectHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void*)
+void DOMObjectHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
 {
     DOMObject* domObject = static_cast<DOMObject*>(handle.get().asCell());
-    m_world->m_wrappers.remove(domObject);
+    uncacheDOMObjectWrapper(m_world, context, domObject);
 }
 

trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp

@@ -143 +143 @@
 void cacheDOMObjectWrapper(DOMWrapperWorld* world, void* objectHandle, DOMObject* wrapper) 
 {
-    world->m_wrappers.set(objectHandle, Weak<DOMObject>(*world->globalData(), wrapper, world->domObjectHandleOwner()));
+    world->m_wrappers.set(objectHandle, Weak<DOMObject>(*world->globalData(), wrapper, world->domObjectHandleOwner(), objectHandle));
 }
 
 void uncacheDOMObjectWrapper(DOMWrapperWorld* world, void* objectHandle, DOMObject* wrapper)
 {
-    ASSERT_UNUSED(wrapper, world->m_wrappers.get(objectHandle) == wrapper);
+    ASSERT_UNUSED(wrapper, world->m_wrappers.find(objectHandle)->second.get() == wrapper);
     world->m_wrappers.remove(objectHandle);
 }

trunk/Source/WebCore/bindings/js/JSDOMBinding.h

@@ -29 +29 @@
 #include <runtime/Completion.h>
 #include <runtime/Lookup.h>
-#include <runtime/WeakGCMap.h>
 #include <wtf/Forward.h>
 #include <wtf/Noncopyable.h>

trunk/Source/WebCore/bindings/js/JSNodeCustom.h

@@ -43 +43 @@
     ASSERT(wrapper);
     if (world->isNormal()) {
-        node->setWrapper(*world->globalData(), wrapper, world->jsNodeHandleOwner());
+        node->setWrapper(*world->globalData(), wrapper, world->jsNodeHandleOwner(), node);
         return;
     }

trunk/Source/WebCore/bindings/js/ScriptWrappable.h

@@ -44 +44 @@
     }
 
-    void setWrapper(JSC::JSGlobalData& globalData, DOMObject* wrapper, JSC::WeakHandleOwner* wrapperOwner)
+    void setWrapper(JSC::JSGlobalData& globalData, DOMObject* wrapper, JSC::WeakHandleOwner* wrapperOwner, void* context)
     {
-        m_wrapper.set(globalData, wrapper, wrapperOwner);
+        m_wrapper.set(globalData, wrapper, wrapperOwner, context);
     }