Diff
Modified: trunk/Source/_javascript_Core/API/JSClassRef.cpp (118482 => 118483)
--- trunk/Source/_javascript_Core/API/JSClassRef.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/API/JSClassRef.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -219,6 +219,6 @@
prototype->setPrototype(exec->globalData(), parentPrototype);
}
- jsClassData.cachedPrototype = PassWeak<JSObject>(prototype, 0);
+ jsClassData.cachedPrototype = PassWeak<JSObject>(prototype);
return prototype;
}
Modified: trunk/Source/_javascript_Core/API/JSWeakObjectMapRefPrivate.cpp (118482 => 118483)
--- trunk/Source/_javascript_Core/API/JSWeakObjectMapRefPrivate.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/API/JSWeakObjectMapRefPrivate.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -72,7 +72,7 @@
{
ExecState* exec = toJS(ctx);
APIEntryShim entryShim(exec);
- map->map().take(key);
+ map->map().remove(key);
}
// We need to keep this function in the build to keep the nightlies running.
Modified: trunk/Source/_javascript_Core/ChangeLog (118482 => 118483)
--- trunk/Source/_javascript_Core/ChangeLog 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-05-25 06:52:00 UTC (rev 118483)
@@ -1,3 +1,70 @@
+2012-05-24 Geoffrey Garen <[email protected]>
+
+ WebKit should be lazy-finalization-safe (esp. the DOM)
+ https://bugs.webkit.org/show_bug.cgi?id=87456
+
+ Reviewed by Filip Pizlo.
+
+ Lazy finalization adds one twist to weak pointer use:
+
+ A HashMap of weak pointers may contain logically null entries.
+ (Weak pointers behave as-if null once their payloads die.)
+ Insertion must not assume that a pre-existing entry is
+ necessarily valid, and iteration must not assume that all
+ entries can be dereferenced.
+
+ (Previously, I thought that it also added a second twist:
+
+ A demand-allocated weak pointer may replace a dead payload
+ before the payload's finalizer runs. In that case, when the
+ payload's finalizer runs, the payload has already been
+ overwritten, and the finalizer should not clear the payload,
+ which now points to something new.
+
+ But that's not the case here, since we cancel the old payload's
+ finalizer when we over-write it. I've added ASSERTs to verify this
+ assumption, in case it ever changes.)
+
+ * API/JSClassRef.cpp:
+ (OpaqueJSClass::prototype): No need to specify null; that's the default.
+
+ * API/JSWeakObjectMapRefPrivate.cpp: Use remove, since take() is gone.
+
+ * heap/PassWeak.h:
+ (WeakImplAccessor::was): This is no longer a debug-only function, since
+ it's required to reason about lazily finalized pointers.
+
+ * heap/Weak.h:
+ (JSC::weakAdd):
+ (JSC::weakRemove):
+ (JSC::weakClear): Added these helper functions for the common idioms of
+ what clients want to do in their weak pointer finalizers.
+
+ * jit/JITStubs.cpp:
+ (JSC::JITThunks::hostFunctionStub): Use the new idioms. Otherwise, we
+ would return NULL for a "zombie" executable weak pointer that was waiting
+ for finalization (item (2)), and finalizing a dead executable weak pointer
+ would potentially destroy a new, live one (item (1)).
+
+ * runtime/RegExpCache.cpp:
+ (JSC::RegExpCache::lookupOrCreate):
+ (JSC::RegExpCache::finalize): Ditto.
+
+ (JSC::RegExpCache::invalidateCode): Check for null while iterating. (See
+ item (2).)
+
+ * runtime/Structure.cpp:
+ (JSC::StructureTransitionTable::contains):
+ (JSC::StructureTransitionTable::add): Use get and set instead of add and
+ contains, since add and contains are not compatible with lazy finalization.
+
+ * runtime/WeakGCMap.h:
+ (WeakGCMap):
+ (JSC::WeakGCMap::clear):
+ (JSC::WeakGCMap::remove): Removed a bunch of code that was incompatible with
+ lazy finalization because I didn't feel like making it compatible, and I had
+ no way to test it.
+
2012-05-24 Filip Pizlo <[email protected]>
REGRESSION (r118013-r118031): Loops/Reloads under www.yahoo.com, quits after three tries with error
Modified: trunk/Source/_javascript_Core/heap/PassWeak.h (118482 => 118483)
--- trunk/Source/_javascript_Core/heap/PassWeak.h 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/heap/PassWeak.h 2012-05-25 06:52:00 UTC (rev 118483)
@@ -46,9 +46,7 @@
T& operator*() const;
GetType get() const;
-#if !ASSERT_DISABLED
bool was(GetType) const;
-#endif
};
template<typename T> class PassWeak : public WeakImplAccessor<PassWeak<T>, T> {
@@ -102,12 +100,10 @@
return jsCast<T*>(static_cast<const Base*>(this)->m_impl->jsValue().asCell());
}
-#if !ASSERT_DISABLED
template<typename Base, typename T> inline bool WeakImplAccessor<Base, T>::was(typename WeakImplAccessor<Base, T>::GetType other) const
{
return jsCast<T*>(static_cast<const Base*>(this)->m_impl->jsValue().asCell()) == other;
}
-#endif
template<typename T> inline PassWeak<T>::PassWeak()
: m_impl(0)
Modified: trunk/Source/_javascript_Core/heap/Weak.h (118482 => 118483)
--- trunk/Source/_javascript_Core/heap/Weak.h 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/heap/Weak.h 2012-05-25 06:52:00 UTC (rev 118483)
@@ -26,9 +26,10 @@
#ifndef Weak_h
#define Weak_h
-#include <wtf/Assertions.h>
#include "PassWeak.h"
#include "WeakSetInlines.h"
+#include <wtf/Assertions.h>
+#include <wtf/HashMap.h>
namespace JSC {
@@ -150,6 +151,32 @@
return reinterpret_cast<WeakImpl*>(-1);
}
+// This function helps avoid modifying a weak table while holding an iterator into it. (Object allocation
+// can run a finalizer that modifies the table. We avoid that by requiring a pre-constructed object as our value.)
+template<typename T, typename U> inline void weakAdd(HashMap<T, Weak<U> >& map, const T& key, PassWeak<U> value)
+{
+ ASSERT(!map.get(key));
+ map.set(key, value); // The table may still have a zombie for value.
+}
+
+template<typename T, typename U> inline void weakRemove(HashMap<T, Weak<U> >& map, const T& key, typename Weak<U>::GetType value)
+{
+ typename HashMap<T, Weak<U> >::iterator it = map.find(key);
+ ASSERT_UNUSED(value, value);
+ ASSERT(it != map.end());
+ ASSERT(it->second.was(value));
+ ASSERT(!it->second);
+ map.remove(it);
+}
+
+template<typename T> inline void weakClear(Weak<T>& weak, typename Weak<T>::GetType value)
+{
+ ASSERT_UNUSED(value, value);
+ ASSERT(weak.was(value));
+ ASSERT(!weak);
+ weak.clear();
+}
+
} // namespace JSC
namespace WTF {
Modified: trunk/Source/_javascript_Core/jit/JITStubs.cpp (118482 => 118483)
--- trunk/Source/_javascript_Core/jit/JITStubs.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/jit/JITStubs.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -3458,27 +3458,31 @@
NativeExecutable* JITThunks::hostFunctionStub(JSGlobalData* globalData, NativeFunction function, NativeFunction constructor)
{
- HostFunctionStubMap::AddResult result = m_hostFunctionStubMap->add(function, PassWeak<NativeExecutable>());
- if (!result.iterator->second)
- result.iterator->second = PassWeak<NativeExecutable>(NativeExecutable::create(*globalData, JIT::compileCTINativeCall(globalData, function), function, MacroAssemblerCodeRef::createSelfManagedCodeRef(ctiNativeConstruct()), constructor, NoIntrinsic));
- return result.iterator->second.get();
+ if (NativeExecutable* nativeExecutable = m_hostFunctionStubMap->get(function))
+ return nativeExecutable;
+
+ NativeExecutable* nativeExecutable = NativeExecutable::create(*globalData, JIT::compileCTINativeCall(globalData, function), function, MacroAssemblerCodeRef::createSelfManagedCodeRef(ctiNativeConstruct()), constructor, NoIntrinsic);
+ weakAdd(*m_hostFunctionStubMap, function, PassWeak<NativeExecutable>(nativeExecutable));
+ return nativeExecutable;
}
NativeExecutable* JITThunks::hostFunctionStub(JSGlobalData* globalData, NativeFunction function, ThunkGenerator generator, Intrinsic intrinsic)
{
- HostFunctionStubMap::AddResult entry = m_hostFunctionStubMap->add(function, PassWeak<NativeExecutable>());
- if (!entry.iterator->second) {
- MacroAssemblerCodeRef code;
- if (generator) {
- if (globalData->canUseJIT())
- code = generator(globalData);
- else
- code = MacroAssemblerCodeRef();
- } else
- code = JIT::compileCTINativeCall(globalData, function);
- entry.iterator->second = PassWeak<NativeExecutable>(NativeExecutable::create(*globalData, code, function, MacroAssemblerCodeRef::createSelfManagedCodeRef(ctiNativeConstruct()), callHostFunctionAsConstructor, intrinsic));
- }
- return entry.iterator->second.get();
+ if (NativeExecutable* nativeExecutable = m_hostFunctionStubMap->get(function))
+ return nativeExecutable;
+
+ MacroAssemblerCodeRef code;
+ if (generator) {
+ if (globalData->canUseJIT())
+ code = generator(globalData);
+ else
+ code = MacroAssemblerCodeRef();
+ } else
+ code = JIT::compileCTINativeCall(globalData, function);
+
+ NativeExecutable* nativeExecutable = NativeExecutable::create(*globalData, code, function, MacroAssemblerCodeRef::createSelfManagedCodeRef(ctiNativeConstruct()), callHostFunctionAsConstructor, intrinsic);
+ weakAdd(*m_hostFunctionStubMap, function, PassWeak<NativeExecutable>(nativeExecutable));
+ return nativeExecutable;
}
void JITThunks::clearHostFunctionStubs()
Modified: trunk/Source/_javascript_Core/runtime/RegExpCache.cpp (118482 => 118483)
--- trunk/Source/_javascript_Core/runtime/RegExpCache.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/runtime/RegExpCache.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -36,17 +36,15 @@
RegExp* RegExpCache::lookupOrCreate(const UString& patternString, RegExpFlags flags)
{
RegExpKey key(flags, patternString);
- RegExpCacheMap::iterator result = m_weakCache.find(key);
- if (result != m_weakCache.end())
- return result->second.get();
+ if (RegExp* regExp = m_weakCache.get(key))
+ return regExp;
+
RegExp* regExp = RegExp::createWithoutCaching(*m_globalData, patternString, flags);
#if ENABLE(REGEXP_TRACING)
m_globalData->addRegExpToTrace(regExp);
#endif
- // We need to do a second lookup to add the RegExp as
- // allocating it may have caused a gc cycle, which in
- // turn may have removed items from the cache.
- m_weakCache.add(key, PassWeak<RegExp>(regExp, this));
+
+ weakAdd(m_weakCache, key, PassWeak<RegExp>(regExp, this));
return regExp;
}
@@ -59,7 +57,7 @@
void RegExpCache::finalize(Handle<Unknown> handle, void*)
{
RegExp* regExp = static_cast<RegExp*>(handle.get().asCell());
- m_weakCache.remove(regExp->key());
+ weakRemove(m_weakCache, regExp->key(), regExp);
regExp->invalidateCode();
}
@@ -79,9 +77,14 @@
for (int i = 0; i < maxStrongCacheableEntries; i++)
m_strongCache[i].clear();
m_nextEntryInStrongCache = 0;
+
RegExpCacheMap::iterator end = m_weakCache.end();
- for (RegExpCacheMap::iterator ptr = m_weakCache.begin(); ptr != end; ++ptr)
- ptr->second->invalidateCode();
+ for (RegExpCacheMap::iterator it = m_weakCache.begin(); it != end; ++it) {
+ RegExp* regExp = it->second.get();
+ if (!regExp) // Skip zombies.
+ continue;
+ regExp->invalidateCode();
+ }
}
}
Modified: trunk/Source/_javascript_Core/runtime/Structure.cpp (118482 => 118483)
--- trunk/Source/_javascript_Core/runtime/Structure.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/runtime/Structure.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -67,7 +67,7 @@
Structure* transition = singleTransition();
return transition && transition->m_nameInPrevious == rep && transition->m_attributesInPrevious == attributes;
}
- return map()->contains(make_pair(rep, attributes));
+ return map()->get(make_pair(rep, attributes));
}
inline Structure* StructureTransitionTable::get(StringImpl* rep, unsigned attributes) const
@@ -101,13 +101,7 @@
// Newer versions of the STL have an std::make_pair function that takes rvalue references.
// When either of the parameters are bitfields, the C++ compiler will try to bind them as lvalues, which is invalid. To work around this, use unary "+" to make the parameter an rvalue.
// See https://bugs.webkit.org/show_bug.cgi?id=59261 for more details
- TransitionMap::AddResult result = map()->add(globalData, make_pair(structure->m_nameInPrevious, +structure->m_attributesInPrevious), structure);
- if (!result.isNewEntry) {
- // There already is an entry! - we should only hit this when despecifying.
- ASSERT(result.iterator.get().second->m_specificValueInPrevious);
- ASSERT(!structure->m_specificValueInPrevious);
- map()->set(globalData, result.iterator.get().first, structure);
- }
+ map()->set(globalData, make_pair(structure->m_nameInPrevious, +structure->m_attributesInPrevious), structure);
}
void Structure::dumpStatistics()
Modified: trunk/Source/_javascript_Core/runtime/WeakGCMap.h (118482 => 118483)
--- trunk/Source/_javascript_Core/runtime/WeakGCMap.h 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/_javascript_Core/runtime/WeakGCMap.h 2012-05-25 06:52:00 UTC (rev 118483)
@@ -53,80 +53,25 @@
typedef HashMap<KeyType, WeakImpl*, HashArg, KeyTraitsArg> MapType;
typedef typename HandleTypes<MappedType>::ExternalType ExternalType;
- typedef typename MapType::iterator map_iterator;
public:
-
- struct iterator {
- friend class WeakGCMap;
- iterator(map_iterator iter)
- : m_iterator(iter)
- {
- }
-
- std::pair<KeyType, ExternalType> get() const { return std::make_pair(m_iterator->first, HandleTypes<MappedType>::getFromSlot(const_cast<JSValue*>(&m_iterator->second->jsValue()))); }
-
- iterator& operator++() { ++m_iterator; return *this; }
-
- // postfix ++ intentionally omitted
-
- // Comparison.
- bool operator==(const iterator& other) const { return m_iterator == other.m_iterator; }
- bool operator!=(const iterator& other) const { return m_iterator != other.m_iterator; }
-
- private:
- map_iterator m_iterator;
- };
-
- typedef WTF::HashTableAddResult<iterator> AddResult;
-
WeakGCMap()
{
}
- bool isEmpty() { return m_map.isEmpty(); }
void clear()
{
- map_iterator end = m_map.end();
- for (map_iterator ptr = m_map.begin(); ptr != end; ++ptr)
+ typename MapType::iterator end = m_map.end();
+ for (typename MapType::iterator ptr = m_map.begin(); ptr != end; ++ptr)
WeakSet::deallocate(ptr->second);
m_map.clear();
}
- bool contains(const KeyType& key) const
- {
- return m_map.contains(key);
- }
-
- iterator find(const KeyType& key)
- {
- return m_map.find(key);
- }
-
- void remove(iterator iter)
- {
- ASSERT(iter.m_iterator != m_map.end());
- WeakImpl* impl = iter.m_iterator->second;
- ASSERT(impl);
- WeakSet::deallocate(impl);
- m_map.remove(iter.m_iterator);
- }
-
ExternalType get(const KeyType& key) const
{
return HandleTypes<MappedType>::getFromSlot(const_cast<JSValue*>(&m_map.get(key)->jsValue()));
}
- AddResult add(JSGlobalData&, const KeyType& key, ExternalType value)
- {
- typename MapType::AddResult result = m_map.add(key, 0);
- if (result.isNewEntry)
- result.iterator->second = WeakSet::allocate(value, this, FinalizerCallback::finalizerContextFor(key));
-
- // WeakGCMap exposes a different iterator, so we need to wrap it and create our own AddResult.
- return AddResult(iterator(result.iterator), result.isNewEntry);
- }
-
void set(JSGlobalData&, const KeyType& key, ExternalType value)
{
typename MapType::AddResult result = m_map.add(key, 0);
@@ -135,21 +80,14 @@
result.iterator->second = WeakSet::allocate(value, this, FinalizerCallback::finalizerContextFor(key));
}
- ExternalType take(const KeyType& key)
+ void remove(const KeyType& key)
{
WeakImpl* impl = m_map.take(key);
if (!impl)
- return HashTraits<ExternalType>::emptyValue();
- ExternalType result = HandleTypes<MappedType>::getFromSlot(const_cast<JSValue*>(&impl->jsValue()));
+ return;
WeakSet::deallocate(impl);
- return result;
}
- size_t size() { return m_map.size(); }
-
- iterator begin() { return iterator(m_map.begin()); }
- iterator end() { return iterator(m_map.end()); }
-
~WeakGCMap()
{
clear();
Modified: trunk/Source/WebCore/ChangeLog (118482 => 118483)
--- trunk/Source/WebCore/ChangeLog 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebCore/ChangeLog 2012-05-25 06:52:00 UTC (rev 118483)
@@ -1,3 +1,37 @@
+2012-05-24 Geoffrey Garen <[email protected]>
+
+ WebKit should be lazy-finalization-safe (esp. the DOM)
+ https://bugs.webkit.org/show_bug.cgi?id=87456
+
+ Reviewed by Filip Pizlo.
+
+ * bindings/js/DOMWrapperWorld.cpp:
+ (WebCore::JSStringOwner::finalize):
+ * bindings/js/JSDOMBinding.cpp:
+ (WebCore::jsStringSlowCase):
+ * bindings/js/JSDOMBinding.h:
+ (WebCore::cacheWrapper):
+ (WebCore::uncacheWrapper): Use the new idioms.
+
+ (WebCore::jsString): Use get instead of find because get is simpler in
+ the case of entries that are logically null.
+
+ (WebCore::domObjectWrapperMapFor): Removed, since it was unused.
+
+ * bindings/js/ScriptWrappable.h:
+ (WebCore::ScriptWrappable::clearWrapper): Use the new idioms.
+
+ * bridge/runtime_root.cpp:
+ (JSC::Bindings::RootObject::invalidate): Check for null while iterating,
+ since that's possible now.
+
+ (JSC::Bindings::RootObject::addRuntimeObject):
+ (JSC::Bindings::RootObject::removeRuntimeObject):
+ (JSC::Bindings::RootObject::finalize): Use the new idioms.
+
+ * bridge/runtime_root.h:
+ (RootObject): Clarified the word "need".
+
2012-05-24 Kent Tamura <[email protected]>
PAGE_POPUP: window.setValueAndClosePopup should be moved to a
Modified: trunk/Source/WebCore/bindings/js/DOMWrapperWorld.cpp (118482 => 118483)
--- trunk/Source/WebCore/bindings/js/DOMWrapperWorld.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebCore/bindings/js/DOMWrapperWorld.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -34,8 +34,7 @@
{
JSString* jsString = jsCast<JSString*>(handle.get().asCell());
StringImpl* stringImpl = static_cast<StringImpl*>(context);
- ASSERT_UNUSED(jsString, m_world->m_stringCache.find(stringImpl)->second.was(jsString));
- m_world->m_stringCache.remove(stringImpl);
+ weakRemove(m_world->m_stringCache, stringImpl, jsString);
}
DOMWrapperWorld::DOMWrapperWorld(JSC::JSGlobalData* globalData, bool isNormal)
Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp (118482 => 118483)
--- trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -49,7 +49,7 @@
JSValue jsStringSlowCase(ExecState* exec, JSStringCache& stringCache, StringImpl* stringImpl)
{
JSString* wrapper = jsString(exec, UString(stringImpl));
- stringCache.add(stringImpl, PassWeak<JSString>(wrapper, currentWorld(exec)->stringWrapperOwner(), stringImpl));
+ weakAdd(stringCache, stringImpl, PassWeak<JSString>(wrapper, currentWorld(exec)->stringWrapperOwner(), stringImpl));
return wrapper;
}
Modified: trunk/Source/WebCore/bindings/js/JSDOMBinding.h (118482 => 118483)
--- trunk/Source/WebCore/bindings/js/JSDOMBinding.h 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebCore/bindings/js/JSDOMBinding.h 2012-05-25 06:52:00 UTC (rev 118483)
@@ -142,16 +142,14 @@
if (setInlineCachedWrapper(world, domObject, wrapper))
return;
JSC::PassWeak<JSDOMWrapper> passWeak(wrapper, wrapperOwner(world, domObject), wrapperContext(world, domObject));
- DOMObjectWrapperMap::AddResult result = world->m_wrappers.add(domObject, passWeak);
- ASSERT_UNUSED(result, result.isNewEntry);
+ weakAdd(world->m_wrappers, (void*)domObject, passWeak);
}
template <typename DOMClass> inline void uncacheWrapper(DOMWrapperWorld* world, DOMClass* domObject, JSDOMWrapper* wrapper)
{
if (clearInlineCachedWrapper(world, domObject, wrapper))
return;
- ASSERT(world->m_wrappers.find(domObject)->second.was(wrapper));
- world->m_wrappers.remove(domObject);
+ weakRemove(world->m_wrappers, (void*)domObject, wrapper);
}
#define CREATE_DOM_WRAPPER(exec, globalObject, className, object) createWrapper<JS##className>(exec, globalObject, static_cast<className*>(object))
@@ -349,18 +347,12 @@
return jsString(exec, stringToUString(s));
JSStringCache& stringCache = currentWorld(exec)->m_stringCache;
- JSStringCache::iterator it = stringCache.find(stringImpl);
- if (it != stringCache.end())
- return it->second.get();
+ if (JSC::JSString* string = stringCache.get(stringImpl))
+ return string;
return jsStringSlowCase(exec, stringCache, stringImpl);
}
- inline DOMObjectWrapperMap& domObjectWrapperMapFor(JSC::ExecState* exec)
- {
- return currentWorld(exec)->m_wrappers;
- }
-
inline String ustringToString(const JSC::UString& u)
{
return u.impl();
Modified: trunk/Source/WebCore/bindings/js/ScriptWrappable.h (118482 => 118483)
--- trunk/Source/WebCore/bindings/js/ScriptWrappable.h 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebCore/bindings/js/ScriptWrappable.h 2012-05-25 06:52:00 UTC (rev 118483)
@@ -51,8 +51,7 @@
void clearWrapper(JSDOMWrapper* wrapper)
{
- ASSERT_UNUSED(wrapper, m_wrapper.was(wrapper));
- m_wrapper.clear();
+ weakClear(m_wrapper, wrapper);
}
private:
Modified: trunk/Source/WebCore/bridge/runtime_root.cpp (118482 => 118483)
--- trunk/Source/WebCore/bridge/runtime_root.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebCore/bridge/runtime_root.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -105,7 +105,10 @@
{
HashMap<RuntimeObject*, JSC::Weak<RuntimeObject> >::iterator end = m_runtimeObjects.end();
for (HashMap<RuntimeObject*, JSC::Weak<RuntimeObject> >::iterator it = m_runtimeObjects.begin(); it != end; ++it) {
- it->second.get()->invalidate();
+ RuntimeObject* runtimeObject = it->second.get();
+ if (!runtimeObject) // Skip zombies.
+ continue;
+ runtimeObject->invalidate();
}
m_runtimeObjects.clear();
@@ -179,29 +182,23 @@
void RootObject::addRuntimeObject(JSGlobalData&, RuntimeObject* object)
{
ASSERT(m_isValid);
- ASSERT(!m_runtimeObjects.get(object));
-
- m_runtimeObjects.set(object, JSC::PassWeak<RuntimeObject>(object, this));
+ weakAdd(m_runtimeObjects, object, JSC::PassWeak<RuntimeObject>(object, this));
}
void RootObject::removeRuntimeObject(RuntimeObject* object)
{
if (!m_isValid)
return;
-
- ASSERT(m_runtimeObjects.get(object));
-
- m_runtimeObjects.take(object);
+ weakRemove(m_runtimeObjects, object, object);
}
void RootObject::finalize(JSC::Handle<JSC::Unknown> handle, void*)
{
RuntimeObject* object = static_cast<RuntimeObject*>(asObject(handle.get()));
- ASSERT(m_runtimeObjects.contains(object));
RefPtr<RootObject> protect(this);
object->invalidate();
- m_runtimeObjects.remove(object);
+ weakRemove(m_runtimeObjects, object, object);
}
} } // namespace JSC::Bindings
Modified: trunk/Source/WebCore/bridge/runtime_root.h (118482 => 118483)
--- trunk/Source/WebCore/bridge/runtime_root.h 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebCore/bridge/runtime_root.h 2012-05-25 06:52:00 UTC (rev 118483)
@@ -93,7 +93,7 @@
Strong<JSGlobalObject> m_globalObject;
ProtectCountSet m_protectCountSet;
- HashMap<RuntimeObject*, JSC::Weak<RuntimeObject> > m_runtimeObjects; // Really need a WeakGCSet, but this will do.
+ HashMap<RuntimeObject*, JSC::Weak<RuntimeObject> > m_runtimeObjects; // We use a map to implement a set.
HashSet<InvalidationCallback*> m_invalidationCallbacks;
};
Modified: trunk/Source/WebKit2/ChangeLog (118482 => 118483)
--- trunk/Source/WebKit2/ChangeLog 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebKit2/ChangeLog 2012-05-25 06:52:00 UTC (rev 118483)
@@ -1,3 +1,18 @@
+2012-05-24 Geoffrey Garen <[email protected]>
+
+ WebKit should be lazy-finalization-safe (esp. the DOM)
+ https://bugs.webkit.org/show_bug.cgi?id=87456
+
+ Reviewed by Filip Pizlo.
+
+ * WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp:
+ (WebKit::NPRuntimeObjectMap::getOrCreateJSObject): Use the new idioms.
+
+ (WebKit::NPRuntimeObjectMap::invalidate): Check for null while iterating,
+ since that's possible now.
+
+ (WebKit::NPRuntimeObjectMap::finalize): Use the new idioms.
+
2012-05-24 Brady Eidson <[email protected]>
Fix the build after http://trac.webkit.org/changeset/118441
Modified: trunk/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp (118482 => 118483)
--- trunk/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp 2012-05-25 06:50:38 UTC (rev 118482)
+++ trunk/Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp 2012-05-25 06:52:00 UTC (rev 118483)
@@ -104,8 +104,7 @@
return jsNPObject;
JSNPObject* jsNPObject = JSNPObject::create(globalObject, this, npObject);
- m_jsNPObjects.set(npObject, JSC::PassWeak<JSNPObject>(jsNPObject, this, npObject));
-
+ weakAdd(m_jsNPObjects, npObject, JSC::PassWeak<JSNPObject>(jsNPObject, this, npObject));
return jsNPObject;
}
@@ -212,8 +211,12 @@
Vector<NPObject*> objects;
- for (HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator ptr = m_jsNPObjects.begin(), end = m_jsNPObjects.end(); ptr != end; ++ptr)
- objects.append(ptr->second->leakNPObject());
+ for (HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator ptr = m_jsNPObjects.begin(), end = m_jsNPObjects.end(); ptr != end; ++ptr) {
+ JSNPObject* jsNPObject = ptr->second.get();
+ if (!jsNPObject) // Skip zombies.
+ continue;
+ objects.append(jsNPObject->leakNPObject());
+ }
m_jsNPObjects.clear();
@@ -293,12 +296,7 @@
void NPRuntimeObjectMap::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
{
JSNPObject* object = jsCast<JSNPObject*>(asObject(handle.get()));
-
- HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator found = m_jsNPObjects.find(static_cast<NPObject*>(context));
- ASSERT(found != m_jsNPObjects.end());
- ASSERT(found->second.was(object));
- m_jsNPObjects.remove(found);
-
+ weakRemove(m_jsNPObjects, static_cast<NPObject*>(context), object);
addToInvalidationQueue(object->leakNPObject());
}
