Skip to content

Commit 9f52f44

Browse files
geoffreygarengeoffreygaren
committed
2011-04-15 Geoffrey Garen <ggaren@apple.com>
 Reviewed by Oliver Hunt. Some mechanical DOM wrapper cleanup https://bugs.webkit.org/show_bug.cgi?id=58689 * WebCore.exp.in: Export! * bindings/js/DOMWrapperWorld.cpp: (WebCore::isReachableFromDOM): Inverted the inDocument test to make the relationship of the special cases to the normal case clearer. * bindings/js/JSArrayBufferViewHelper.h: (WebCore::toJSArrayBufferView): * bindings/js/JSCSSRuleCustom.cpp: (WebCore::toJS): * bindings/js/JSCSSValueCustom.cpp: (WebCore::toJS): * bindings/js/JSDOMBinding.cpp: (WebCore::getCachedDOMObjectWrapper): (WebCore::cacheDOMObjectWrapper): * bindings/js/JSDOMBinding.h: (WebCore::createDOMObjectWrapper): (WebCore::getDOMObjectWrapper): (WebCore::createDOMNodeWrapper): (WebCore::getDOMNodeWrapper): Changed DOM wrapper functions to operate in terms of DOMWrapperWorlds instead of ExecStates. This is clearer, and ever-so-slightly faster. Removed hasCachedXXX functions, now that they're unused. * bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSDOMWindow::history): (WebCore::JSDOMWindow::location): * bindings/js/JSDocumentCustom.cpp: (WebCore::JSDocument::location): (WebCore::toJS): * bindings/js/JSElementCustom.cpp: (WebCore::toJSNewlyCreated): * bindings/js/JSEventCustom.cpp: (WebCore::toJS): * bindings/js/JSHTMLCollectionCustom.cpp: (WebCore::toJS): * bindings/js/JSImageDataCustom.cpp: (WebCore::toJS): * bindings/js/JSNodeCustom.cpp: (WebCore::createWrapperInline): * bindings/js/JSNodeCustom.h: (WebCore::getCachedDOMNodeWrapper): (WebCore::cacheDOMNodeWrapper): (WebCore::toJS): * bindings/js/JSSVGPathSegCustom.cpp: (WebCore::toJS): * bindings/js/JSStyleSheetCustom.cpp: (WebCore::toJS): Updated for changes above. * xml/XMLHttpRequest.cpp: (WebCore::XMLHttpRequest::dropProtection): Removed use of hasCachedDOMObjectWrapper because XHR is almost always created and used by JavaScript, so it's simpler to just always report extra cost. Canonical link: https://commits.webkit.org/73771@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@84029 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 88c4768 commit 9f52f44

19 files changed

Lines changed: 121 additions & 85 deletions

‎Source/WebCore/ChangeLog‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,65 @@
1+
2011-04-15 Geoffrey Garen <ggaren@apple.com>
2+
3+
Reviewed by Oliver Hunt.
4+
5+
Some mechanical DOM wrapper cleanup
6+
https://bugs.webkit.org/show_bug.cgi?id=58689
7+
8+
* WebCore.exp.in: Export!
9+
10+
* bindings/js/DOMWrapperWorld.cpp:
11+
(WebCore::isReachableFromDOM): Inverted the inDocument test to make the
12+
relationship of the special cases to the normal case clearer.
13+
14+
* bindings/js/JSArrayBufferViewHelper.h:
15+
(WebCore::toJSArrayBufferView):
16+
* bindings/js/JSCSSRuleCustom.cpp:
17+
(WebCore::toJS):
18+
* bindings/js/JSCSSValueCustom.cpp:
19+
(WebCore::toJS):
20+
* bindings/js/JSDOMBinding.cpp:
21+
(WebCore::getCachedDOMObjectWrapper):
22+
(WebCore::cacheDOMObjectWrapper):
23+
* bindings/js/JSDOMBinding.h:
24+
(WebCore::createDOMObjectWrapper):
25+
(WebCore::getDOMObjectWrapper):
26+
(WebCore::createDOMNodeWrapper):
27+
(WebCore::getDOMNodeWrapper): Changed DOM wrapper functions to operate
28+
in terms of DOMWrapperWorlds instead of ExecStates. This is clearer,
29+
and ever-so-slightly faster.
30+
31+
Removed hasCachedXXX functions, now that they're unused.
32+
33+
* bindings/js/JSDOMWindowCustom.cpp:
34+
(WebCore::JSDOMWindow::history):
35+
(WebCore::JSDOMWindow::location):
36+
* bindings/js/JSDocumentCustom.cpp:
37+
(WebCore::JSDocument::location):
38+
(WebCore::toJS):
39+
* bindings/js/JSElementCustom.cpp:
40+
(WebCore::toJSNewlyCreated):
41+
* bindings/js/JSEventCustom.cpp:
42+
(WebCore::toJS):
43+
* bindings/js/JSHTMLCollectionCustom.cpp:
44+
(WebCore::toJS):
45+
* bindings/js/JSImageDataCustom.cpp:
46+
(WebCore::toJS):
47+
* bindings/js/JSNodeCustom.cpp:
48+
(WebCore::createWrapperInline):
49+
* bindings/js/JSNodeCustom.h:
50+
(WebCore::getCachedDOMNodeWrapper):
51+
(WebCore::cacheDOMNodeWrapper):
52+
(WebCore::toJS):
53+
* bindings/js/JSSVGPathSegCustom.cpp:
54+
(WebCore::toJS):
55+
* bindings/js/JSStyleSheetCustom.cpp:
56+
(WebCore::toJS): Updated for changes above.
57+
58+
* xml/XMLHttpRequest.cpp:
59+
(WebCore::XMLHttpRequest::dropProtection): Removed use of hasCachedDOMObjectWrapper
60+
because XHR is almost always created and used by JavaScript, so it's
61+
simpler to just always report extra cost.
62+
163
2011-04-15 Andreas Kling <kling@webkit.org>
264

365
Rolling out accidental part of r84010.

‎Source/WebCore/WebCore.exp.in‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,6 @@ __ZN7WebCore14DocumentLoaderC2ERKNS_15ResourceRequestERKNS_14SubstituteDataE
350350
__ZN7WebCore14DocumentLoaderD2Ev
351351
__ZN7WebCore14DocumentWriter11setEncodingERKN3WTF6StringEb
352352
__ZN7WebCore14ResourceHandle12releaseProxyEv
353-
__ZN7WebCore25getCachedDOMObjectWrapperEPN3JSC9ExecStateEPv
354353
__ZN7WebCore14ResourceHandle20forceContentSniffingEv
355354
__ZN7WebCore14ResourceLoader14cancelledErrorEv
356355
__ZN7WebCore14ResourceLoader19setShouldBufferDataEb
@@ -595,6 +594,7 @@ __ZN7WebCore25PluginMainThreadScheduler16unregisterPluginEP4_NPP
595594
__ZN7WebCore25PluginMainThreadScheduler9schedulerEv
596595
__ZN7WebCore25addLanguageChangeObserverEPvPFvS0_E
597596
__ZN7WebCore25contextMenuItemTagOutlineEv
597+
__ZN7WebCore25getCachedDOMObjectWrapperEPNS_15DOMWrapperWorldEPv
598598
__ZN7WebCore26CSSMutableStyleDeclarationC1Ev
599599
__ZN7WebCore26UserTypingGestureIndicator27processingUserTypingGestureEv
600600
__ZN7WebCore26UserTypingGestureIndicator28focusedElementAtGestureStartEv

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

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -120,27 +120,26 @@ static bool isObservable(JSNode* jsNode, Node* node, DOMWrapperWorld* world)
120120

121121
static inline bool isReachableFromDOM(JSNode* jsNode, Node* node, DOMWrapperWorld* world, MarkStack& markStack)
122122
{
123-
if (node->inDocument())
124-
return isObservable(jsNode, node, world) && markStack.containsOpaqueRoot(root(node));
125-
126-
// If a wrapper is the last reference to an image or script element
127-
// that is loading but not in the document, the wrapper is observable
128-
// because it is the only thing keeping the image element alive, and if
129-
// the image element is destroyed, its load event will not fire.
130-
// FIXME: The DOM should manage this issue without the help of JavaScript wrappers.
131-
if (node->hasTagName(imgTag) && !static_cast<HTMLImageElement*>(node)->haveFiredLoadEvent())
132-
return true;
133-
if (node->hasTagName(scriptTag) && !static_cast<HTMLScriptElement*>(node)->haveFiredLoadEvent())
134-
return true;
135-
#if ENABLE(VIDEO)
136-
if (node->hasTagName(audioTag) && !static_cast<HTMLAudioElement*>(node)->paused())
137-
return true;
138-
#endif
139-
140-
// If a node is firing event listeners, its wrapper is observable because
141-
// its wrapper is responsible for marking those event listeners.
142-
if (node->isFiringEventListeners())
143-
return true;
123+
if (!node->inDocument()) {
124+
// If a wrapper is the last reference to an image or script element
125+
// that is loading but not in the document, the wrapper is observable
126+
// because it is the only thing keeping the image element alive, and if
127+
// the image element is destroyed, its load event will not fire.
128+
// FIXME: The DOM should manage this issue without the help of JavaScript wrappers.
129+
if (node->hasTagName(imgTag) && !static_cast<HTMLImageElement*>(node)->haveFiredLoadEvent())
130+
return true;
131+
if (node->hasTagName(scriptTag) && !static_cast<HTMLScriptElement*>(node)->haveFiredLoadEvent())
132+
return true;
133+
#if ENABLE(VIDEO)
134+
if (node->hasTagName(audioTag) && !static_cast<HTMLAudioElement*>(node)->paused())
135+
return true;
136+
#endif
137+
138+
// If a node is firing event listeners, its wrapper is observable because
139+
// its wrapper is responsible for marking those event listeners.
140+
if (node->isFiringEventListeners())
141+
return true;
142+
}
144143

145144
return isObservable(jsNode, node, world) && markStack.containsOpaqueRoot(root(node));
146145
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ static JSC::JSValue toJSArrayBufferView(JSC::ExecState* exec, JSDOMGlobalObject*
163163
if (!object)
164164
return JSC::jsNull();
165165

166-
if (DOMObject* wrapper = getCachedDOMObjectWrapper(exec, object))
166+
if (DOMObject* wrapper = getCachedDOMObjectWrapper(currentWorld(exec), object))
167167
return wrapper;
168168

169169
exec->heap()->reportExtraMemoryCost(object->byteLength());

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, CSSRule* rule)
6363
if (!rule)
6464
return jsNull();
6565

66-
DOMObject* wrapper = getCachedDOMObjectWrapper(exec, rule);
66+
DOMObject* wrapper = getCachedDOMObjectWrapper(currentWorld(exec), rule);
6767
if (wrapper)
6868
return wrapper;
6969

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, CSSValue* value)
4949
if (!value)
5050
return jsNull();
5151

52-
DOMObject* wrapper = getCachedDOMObjectWrapper(exec, value);
52+
DOMObject* wrapper = getCachedDOMObjectWrapper(currentWorld(exec), value);
5353

5454
if (wrapper)
5555
return wrapper;

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

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -135,32 +135,13 @@ const JSC::HashTable* getHashTableForGlobalData(JSGlobalData& globalData, const
135135
return DOMObjectHashTableMap::mapFor(globalData).get(staticTable);
136136
}
137137

138-
bool hasCachedDOMObjectWrapperUnchecked(JSGlobalData* globalData, void* objectHandle)
138+
DOMObject* getCachedDOMObjectWrapper(DOMWrapperWorld* world, void* objectHandle)
139139
{
140-
for (JSGlobalDataWorldIterator worldIter(globalData); worldIter; ++worldIter) {
141-
if (worldIter->m_wrappers.get(objectHandle))
142-
return true;
143-
}
144-
return false;
145-
}
146-
147-
bool hasCachedDOMObjectWrapper(JSGlobalData* globalData, void* objectHandle)
148-
{
149-
for (JSGlobalDataWorldIterator worldIter(globalData); worldIter; ++worldIter) {
150-
if (worldIter->m_wrappers.get(objectHandle))
151-
return true;
152-
}
153-
return false;
154-
}
155-
156-
DOMObject* getCachedDOMObjectWrapper(JSC::ExecState* exec, void* objectHandle)
157-
{
158-
return domObjectWrapperMapFor(exec).get(objectHandle).get();
140+
return world->m_wrappers.get(objectHandle).get();
159141
}
160142

161-
void cacheDOMObjectWrapper(JSC::ExecState* exec, void* objectHandle, DOMObject* wrapper)
143+
void cacheDOMObjectWrapper(DOMWrapperWorld* world, void* objectHandle, DOMObject* wrapper)
162144
{
163-
DOMWrapperWorld* world = currentWorld(exec);
164145
world->m_wrappers.set(objectHandle, Weak<DOMObject>(*world->globalData(), wrapper, world->domObjectHandleOwner()));
165146
}
166147

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

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,18 +113,16 @@ namespace WebCore {
113113
}
114114
};
115115

116-
DOMObject* getCachedDOMObjectWrapper(JSC::ExecState*, void* objectHandle);
117-
bool hasCachedDOMObjectWrapper(JSC::JSGlobalData*, void* objectHandle);
118-
void cacheDOMObjectWrapper(JSC::ExecState*, void* objectHandle, DOMObject* wrapper);
116+
DOMObject* getCachedDOMObjectWrapper(DOMWrapperWorld*, void* objectHandle);
117+
void cacheDOMObjectWrapper(DOMWrapperWorld*, void* objectHandle, DOMObject* wrapper);
119118
void uncacheDOMObjectWrapper(DOMWrapperWorld*, void* objectHandle, DOMObject* wrapper);
120119

121-
JSNode* getCachedDOMNodeWrapper(JSC::ExecState*, Node*);
122-
void cacheDOMNodeWrapper(JSC::ExecState*, Node*, JSNode* wrapper);
120+
JSNode* getCachedDOMNodeWrapper(DOMWrapperWorld*, Node*);
121+
void cacheDOMNodeWrapper(DOMWrapperWorld*, Node*, JSNode* wrapper);
123122
void uncacheDOMNodeWrapper(DOMWrapperWorld*, Node*, JSNode* wrapper);
124123

125124
void markActiveObjectsForContext(JSC::MarkStack&, JSC::JSGlobalData&, ScriptExecutionContext*);
126125
void markDOMObjectWrapper(JSC::MarkStack&, JSC::JSGlobalData& globalData, void* object);
127-
bool hasCachedDOMObjectWrapperUnchecked(JSC::JSGlobalData*, void* objectHandle);
128126

129127
JSC::Structure* getCachedDOMStructure(JSDOMGlobalObject*, const JSC::ClassInfo*);
130128
JSC::Structure* cacheDOMStructure(JSDOMGlobalObject*, NonNullPassRefPtr<JSC::Structure>, const JSC::ClassInfo*);
@@ -156,17 +154,17 @@ namespace WebCore {
156154
template<class WrapperClass, class DOMClass> inline DOMObject* createDOMObjectWrapper(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, DOMClass* object)
157155
{
158156
ASSERT(object);
159-
ASSERT(!getCachedDOMObjectWrapper(exec, object));
157+
ASSERT(!getCachedDOMObjectWrapper(currentWorld(exec), object));
160158
// FIXME: new (exec) could use a different globalData than the globalData this wrapper is cached on.
161159
WrapperClass* wrapper = new (exec) WrapperClass(getDOMStructure<WrapperClass>(exec, globalObject), globalObject, object);
162-
cacheDOMObjectWrapper(exec, object, wrapper);
160+
cacheDOMObjectWrapper(currentWorld(exec), object, wrapper);
163161
return wrapper;
164162
}
165163
template<class WrapperClass, class DOMClass> inline JSC::JSValue getDOMObjectWrapper(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, DOMClass* object)
166164
{
167165
if (!object)
168166
return JSC::jsNull();
169-
if (DOMObject* wrapper = getCachedDOMObjectWrapper(exec, object))
167+
if (DOMObject* wrapper = getCachedDOMObjectWrapper(currentWorld(exec), object))
170168
return wrapper;
171169
return createDOMObjectWrapper<WrapperClass>(exec, globalObject, object);
172170
}
@@ -175,18 +173,18 @@ namespace WebCore {
175173
template<class WrapperClass, class DOMClass> inline JSNode* createDOMNodeWrapper(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, DOMClass* node)
176174
{
177175
ASSERT(node);
178-
ASSERT(!getCachedDOMNodeWrapper(exec, node));
176+
ASSERT(!getCachedDOMNodeWrapper(currentWorld(exec), node));
179177
WrapperClass* wrapper = new (exec) WrapperClass(getDOMStructure<WrapperClass>(exec, globalObject), globalObject, node);
180178
// FIXME: The entire function can be removed, once we fix caching.
181179
// This function is a one-off hack to make Nodes cache in the right global object.
182-
cacheDOMNodeWrapper(exec, node, wrapper);
180+
cacheDOMNodeWrapper(currentWorld(exec), node, wrapper);
183181
return wrapper;
184182
}
185183
template<class WrapperClass, class DOMClass> inline JSC::JSValue getDOMNodeWrapper(JSC::ExecState* exec, JSDOMGlobalObject* globalObject, DOMClass* node)
186184
{
187185
if (!node)
188186
return JSC::jsNull();
189-
if (JSC::JSCell* wrapper = getCachedDOMNodeWrapper(exec, node))
187+
if (JSC::JSCell* wrapper = getCachedDOMNodeWrapper(currentWorld(exec), node))
190188
return wrapper;
191189
return createDOMNodeWrapper<WrapperClass>(exec, globalObject, node);
192190
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -441,24 +441,24 @@ JSValue JSDOMWindow::lookupSetter(ExecState* exec, const Identifier& propertyNam
441441
JSValue JSDOMWindow::history(ExecState* exec) const
442442
{
443443
History* history = impl()->history();
444-
if (DOMObject* wrapper = getCachedDOMObjectWrapper(exec, history))
444+
if (DOMObject* wrapper = getCachedDOMObjectWrapper(currentWorld(exec), history))
445445
return wrapper;
446446

447447
JSDOMWindow* window = const_cast<JSDOMWindow*>(this);
448448
JSHistory* jsHistory = new (exec) JSHistory(getDOMStructure<JSHistory>(exec, window), window, history);
449-
cacheDOMObjectWrapper(exec, history, jsHistory);
449+
cacheDOMObjectWrapper(currentWorld(exec), history, jsHistory);
450450
return jsHistory;
451451
}
452452

453453
JSValue JSDOMWindow::location(ExecState* exec) const
454454
{
455455
Location* location = impl()->location();
456-
if (DOMObject* wrapper = getCachedDOMObjectWrapper(exec, location))
456+
if (DOMObject* wrapper = getCachedDOMObjectWrapper(currentWorld(exec), location))
457457
return wrapper;
458458

459459
JSDOMWindow* window = const_cast<JSDOMWindow*>(this);
460460
JSLocation* jsLocation = new (exec) JSLocation(getDOMStructure<JSLocation>(exec, window), window, location);
461-
cacheDOMObjectWrapper(exec, location, jsLocation);
461+
cacheDOMObjectWrapper(currentWorld(exec), location, jsLocation);
462462
return jsLocation;
463463
}
464464

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,11 @@ JSValue JSDocument::location(ExecState* exec) const
6868
return jsNull();
6969

7070
Location* location = frame->domWindow()->location();
71-
if (DOMObject* wrapper = getCachedDOMObjectWrapper(exec, location))
71+
if (DOMObject* wrapper = getCachedDOMObjectWrapper(currentWorld(exec), location))
7272
return wrapper;
7373

7474
JSLocation* jsLocation = new (exec) JSLocation(getDOMStructure<JSLocation>(exec, globalObject()), globalObject(), location);
75-
cacheDOMObjectWrapper(exec, location, jsLocation);
75+
cacheDOMObjectWrapper(currentWorld(exec), location, jsLocation);
7676
return jsLocation;
7777
}
7878

@@ -100,7 +100,7 @@ JSValue toJS(ExecState* exec, JSDOMGlobalObject* globalObject, Document* documen
100100
if (!document)
101101
return jsNull();
102102

103-
DOMObject* wrapper = getCachedDOMNodeWrapper(exec, document);
103+
DOMObject* wrapper = getCachedDOMNodeWrapper(currentWorld(exec), document);
104104
if (wrapper)
105105
return wrapper;
106106

0 commit comments

Comments
 (0)