Skip to content

Commit add2b39

Browse files
geoffreygarengeoffreygaren
committed
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. Canonical link: https://commits.webkit.org/73790@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@84049 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 49e2cbd commit add2b39

6 files changed

Lines changed: 40 additions & 10 deletions

File tree

‎Source/WebCore/ChangeLog‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,34 @@
1+
2011-04-15 Geoffrey Garen <ggaren@apple.com>
2+
3+
Reviewed by Oliver Hunt.
4+
5+
DOM object handles are never removed from cache
6+
https://bugs.webkit.org/show_bug.cgi?id=58707
7+
8+
We were trying to remove hash table items by value instead of by key.
9+
10+
* bindings/js/DOMWrapperWorld.cpp:
11+
(WebCore::JSNodeHandleOwner::finalize): Changed to work more like
12+
DOMObjectHandleOwner::finalize because I'm going to merge them.
13+
14+
(WebCore::DOMObjectHandleOwner::finalize): Remove hash table items
15+
by key, not value. (Oops!) Use a helper function to make sure we get
16+
this right.
17+
18+
* bindings/js/JSDOMBinding.cpp:
19+
(WebCore::cacheDOMObjectWrapper): Store the hash table key as our weak
20+
handle context, so we can use it at destruction time.
21+
22+
* bindings/js/JSDOMBinding.h: Removed unnecessary include.
23+
24+
* bindings/js/JSNodeCustom.h:
25+
(WebCore::cacheDOMNodeWrapper): Store the hash table key as our weak
26+
handle context, so we can use it at destruction time.
27+
28+
* bindings/js/ScriptWrappable.h:
29+
(WebCore::ScriptWrappable::setWrapper): Forward context parameter, to
30+
support the above.
31+
132
2011-04-15 Kenneth Russell <kbr@google.com>
233

334
Unreviewed. Chromium Linux Release build fix due to unused variables.

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,21 +188,21 @@ bool JSNodeHandleOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown> han
188188
return isReachableFromDOM(jsNode, jsNode->impl(), m_world, markStack);
189189
}
190190

191-
void JSNodeHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void*)
191+
void JSNodeHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
192192
{
193193
JSNode* jsNode = static_cast<JSNode*>(handle.get().asCell());
194-
uncacheDOMNodeWrapper(m_world, jsNode->impl(), jsNode);
194+
uncacheDOMNodeWrapper(m_world, static_cast<Node*>(context), jsNode);
195195
}
196196

197197
bool DOMObjectHandleOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC::Unknown>, void*, JSC::MarkStack&)
198198
{
199199
return false;
200200
}
201201

202-
void DOMObjectHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void*)
202+
void DOMObjectHandleOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
203203
{
204204
DOMObject* domObject = static_cast<DOMObject*>(handle.get().asCell());
205-
m_world->m_wrappers.remove(domObject);
205+
uncacheDOMObjectWrapper(m_world, context, domObject);
206206
}
207207

208208
} // namespace WebCore

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,12 @@ DOMObject* getCachedDOMObjectWrapper(DOMWrapperWorld* world, void* objectHandle)
142142

143143
void cacheDOMObjectWrapper(DOMWrapperWorld* world, void* objectHandle, DOMObject* wrapper)
144144
{
145-
world->m_wrappers.set(objectHandle, Weak<DOMObject>(*world->globalData(), wrapper, world->domObjectHandleOwner()));
145+
world->m_wrappers.set(objectHandle, Weak<DOMObject>(*world->globalData(), wrapper, world->domObjectHandleOwner(), objectHandle));
146146
}
147147

148148
void uncacheDOMObjectWrapper(DOMWrapperWorld* world, void* objectHandle, DOMObject* wrapper)
149149
{
150-
ASSERT_UNUSED(wrapper, world->m_wrappers.get(objectHandle) == wrapper);
150+
ASSERT_UNUSED(wrapper, world->m_wrappers.find(objectHandle)->second.get() == wrapper);
151151
world->m_wrappers.remove(objectHandle);
152152
}
153153

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
#include "Document.h"
2929
#include <runtime/Completion.h>
3030
#include <runtime/Lookup.h>
31-
#include <runtime/WeakGCMap.h>
3231
#include <wtf/Forward.h>
3332
#include <wtf/Noncopyable.h>
3433

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ inline void cacheDOMNodeWrapper(DOMWrapperWorld* world, Node* node, JSNode* wrap
4242
{
4343
ASSERT(wrapper);
4444
if (world->isNormal()) {
45-
node->setWrapper(*world->globalData(), wrapper, world->jsNodeHandleOwner());
45+
node->setWrapper(*world->globalData(), wrapper, world->jsNodeHandleOwner(), node);
4646
return;
4747
}
4848
cacheDOMObjectWrapper(world, node, wrapper);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ class ScriptWrappable {
4343
return m_wrapper.get();
4444
}
4545

46-
void setWrapper(JSC::JSGlobalData& globalData, DOMObject* wrapper, JSC::WeakHandleOwner* wrapperOwner)
46+
void setWrapper(JSC::JSGlobalData& globalData, DOMObject* wrapper, JSC::WeakHandleOwner* wrapperOwner, void* context)
4747
{
48-
m_wrapper.set(globalData, wrapper, wrapperOwner);
48+
m_wrapper.set(globalData, wrapper, wrapperOwner, context);
4949
}
5050

5151
void clearWrapper()

0 commit comments

Comments
 (0)