diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 946fd13f4ac229fdc1c0c8ae99fc529bc69d2dcd..72ed58b12b1531298861e7b934c2af2412a4c6b8 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,60 @@ +2009-09-18 Darin Adler + + Reviewed by Sam Weinig. + + Each wrapped Objective-C object should use a single RuntimeObjectImp + https://bugs.webkit.org/show_bug.cgi?id=29351 + rdar://problem/7142294 + + * WebCore.base.exp: Added a newly-needed exported symbol. + + * bindings/objc/DOMInternal.h: Eliminated unused + createWrapperCacheWithIntegerKeys; it has not been needed since the + RGBColor wrappers were reworked. + * bindings/objc/DOMInternal.mm: Ditto. + + * bridge/objc/objc_instance.h: Made the create function non-inline. + * bridge/objc/objc_instance.mm: + (createInstanceWrapperCache): Added. Creates an appropriate map table. + (ObjcInstance::create): Moved here from header. Uses NSMapGet and + NSMapInsert to cache the instance in a map table. + (ObjcInstance::~ObjcInstance): Added a call to NSMapRemove to remove + the instance from the map table. + + * bridge/qt/qt_instance.cpp: + (JSC::Bindings::QtInstance::~QtInstance): Remove unneeded code to remove + the instance from cachedObjects, which no longer exists. + (JSC::Bindings::QtInstance::newRuntimeObject): Renamed to overload new + bottleneck. Caching is now handled by the base class. + + * bridge/runtime.cpp: + (JSC::Bindings::Instance::Instance): Initialize m_runtimeObject to 0. + (JSC::Bindings::Instance::~Instance): Assert m_runtimeObject is 0. + (JSC::Bindings::Instance::createRuntimeObject): Use m_runtimeObject + if it's already set. Set m_runtimeObject and call addRuntimeObject + if it's not. + (JSC::Bindings::Instance::newRuntimeObject): Added. Virtual function, + used only by createRuntimeObject. + (JSC::Bindings::Instance::willDestroyRuntimeObject): Added. + Calls removeRuntimeObject and then clears m_runtimeObject. + (JSC::Bindings::Instance::willInvalidateRuntimeObject): Added. + Clears m_runtimeObject. + + * bridge/runtime.h: Made createRuntimeObject non-virtual. Added + willDestroyRuntimeObject, willInvalidateRuntimeObject, + newRuntimeObject, and m_runtimeObject. + + * bridge/runtime_object.cpp: + (JSC::RuntimeObjectImp::RuntimeObjectImp): Removed addRuntimeObject + call, now handled by caller. + (JSC::RuntimeObjectImp::~RuntimeObjectImp): Replaced removeRuntimeObject + call with willDestroyRuntimeObject call; the latter nows calls + removeRuntimeObject. + (JSC::RuntimeObjectImp::invalidate): Added willInvalidateRuntimeObject + call. + + * bridge/runtime_object.h: Made invalidate non-virtual. + 2009-09-17 Kenneth Rohde Christiansen Reviewed by Simon Hausmann. diff --git a/WebCore/WebCore.base.exp b/WebCore/WebCore.base.exp index 2f36de8197bdee9a46ecd77027cea297dbff8901..008e799f02d60d793d7a8d782047afd49de38ef3 100644 --- a/WebCore/WebCore.base.exp +++ b/WebCore/WebCore.base.exp @@ -133,6 +133,7 @@ __Z4coreP19DOMDocumentFragment __Z4coreP22DOMCSSStyleDeclaration __Z4coreP7DOMNode __Z4coreP8DOMRange +__ZN3JSC8Bindings8Instance16newRuntimeObjectEPNS_9ExecStateE __ZN7WebCore10MouseEventC1ERKNS_12AtomicStringEbbN3WTF10PassRefPtrINS_9DOMWindowEEEiiiiibbbbtNS5_INS_11EventTargetEEENS5_INS_9ClipboardEEEb __ZN7WebCore10ScrollView17setScrollbarModesENS_13ScrollbarModeES1_ __ZN7WebCore10ScrollView20setCanHaveScrollbarsEb diff --git a/WebCore/bindings/objc/DOMInternal.h b/WebCore/bindings/objc/DOMInternal.h index 48f5d2f8673090098593602d05feea59a1f2174f..72f63d2ca57d5df86ea5f8f4e9e97c42bf58b2de 100644 --- a/WebCore/bindings/objc/DOMInternal.h +++ b/WebCore/bindings/objc/DOMInternal.h @@ -61,7 +61,6 @@ namespace WebCore { // Create an NSMapTable mapping from pointers to ObjC objects held with zeroing weak references. NSMapTable* createWrapperCache(); -NSMapTable* createWrapperCacheWithIntegerKeys(); // Same, but from integers to ObjC objects. id createDOMWrapper(JSC::JSObject*, PassRefPtr origin, PassRefPtr current); diff --git a/WebCore/bindings/objc/DOMInternal.mm b/WebCore/bindings/objc/DOMInternal.mm index 9b26e5973c5edb943e12c6c19906a843748598c8..993a3ade5938e8bb4290cdf3d53cb343c1a012c1 100644 --- a/WebCore/bindings/objc/DOMInternal.mm +++ b/WebCore/bindings/objc/DOMInternal.mm @@ -49,18 +49,6 @@ NSMapTable* createWrapperCache() #endif } -NSMapTable* createWrapperCacheWithIntegerKeys() -{ -#ifdef BUILDING_ON_TIGER - return NSCreateMapTable(NSIntMapKeyCallBacks, NSNonRetainedObjectMapValueCallBacks, 0); -#else - // NSMapTable with zeroing weak pointers is the recommended way to build caches like this under garbage collection. - NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsIntegerPersonality; - NSPointerFunctionsOptions valueOptions = NSPointerFunctionsZeroingWeakMemory | NSPointerFunctionsObjectPersonality; - return [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0]; -#endif -} - NSObject* getDOMWrapper(DOMObjectInternal* impl) { if (!DOMWrapperCache) diff --git a/WebCore/bridge/objc/objc_instance.h b/WebCore/bridge/objc/objc_instance.h index 6f1eb0d3391ed96e48a480c2029e284d6047761a..64cd4919b0157261115e0a267db5227ce9cdd467 100644 --- a/WebCore/bridge/objc/objc_instance.h +++ b/WebCore/bridge/objc/objc_instance.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2003 Apple Computer, Inc. All rights reserved. + * Copyright (C) 2003, 2009 Apple Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -37,16 +37,12 @@ class ObjcClass; class ObjcInstance : public Instance { public: - static PassRefPtr create(ObjectStructPtr instance, PassRefPtr rootObject) - { - return adoptRef(new ObjcInstance(instance, rootObject)); - } - + static PassRefPtr create(ObjectStructPtr, PassRefPtr); + virtual ~ObjcInstance(); + static void setGlobalException(NSString*, JSGlobalObject* exceptionEnvironment = 0); // A null exceptionEnvironment means the exception should propogate to any execution environment. - ~ObjcInstance(); - - virtual Class *getClass() const; + virtual Class* getClass() const; virtual JSValue valueOf(ExecState*) const; virtual JSValue defaultValue(ExecState*, PreferredPrimitiveType) const; @@ -71,7 +67,7 @@ protected: private: static void moveGlobalExceptionToExecState(ExecState*); - ObjcInstance(ObjectStructPtr instance, PassRefPtr); + ObjcInstance(ObjectStructPtr, PassRefPtr); RetainPtr _instance; mutable ObjcClass *_class; diff --git a/WebCore/bridge/objc/objc_instance.mm b/WebCore/bridge/objc/objc_instance.mm index b84f7bb14c4aecdf6c0ce3a60dc4eef259c28a14..f7550e4b23aba78d5c9cf648e40ed5eff212cbdc 100644 --- a/WebCore/bridge/objc/objc_instance.mm +++ b/WebCore/bridge/objc/objc_instance.mm @@ -1,5 +1,5 @@ /* - * Copyright (C) 2004, 2008 Apple Computer, Inc. All rights reserved. + * Copyright (C) 2004, 2008, 2009 Apple Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -44,8 +44,21 @@ using namespace JSC::Bindings; using namespace JSC; -static NSString* s_exception; +static NSString *s_exception; static JSGlobalObject* s_exceptionEnvironment; // No need to protect this value, since we just use it for a pointer comparison. +static NSMapTable *s_instanceWrapperCache; + +static NSMapTable *createInstanceWrapperCache() +{ +#ifdef BUILDING_ON_TIGER + return NSCreateMapTable(NSNonRetainedObjectMapKeyCallBacks, NSNonOwnedPointerMapValueCallBacks, 0); +#else + // NSMapTable with zeroing weak pointers is the recommended way to build caches like this under garbage collection. + NSPointerFunctionsOptions keyOptions = NSPointerFunctionsZeroingWeakMemory | NSPointerFunctionsObjectPersonality; + NSPointerFunctionsOptions valueOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality; + return [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0]; +#endif +} void ObjcInstance::setGlobalException(NSString* exception, JSGlobalObject* exceptionEnvironment) { @@ -74,7 +87,7 @@ void ObjcInstance::moveGlobalExceptionToExecState(ExecState* exec) s_exceptionEnvironment = 0; } -ObjcInstance::ObjcInstance(ObjectStructPtr instance, PassRefPtr rootObject) +ObjcInstance::ObjcInstance(id instance, PassRefPtr rootObject) : Instance(rootObject) , _instance(instance) , _class(0) @@ -83,13 +96,30 @@ ObjcInstance::ObjcInstance(ObjectStructPtr instance, PassRefPtr root { } +PassRefPtr ObjcInstance::create(id instance, PassRefPtr rootObject) +{ + if (!s_instanceWrapperCache) + s_instanceWrapperCache = createInstanceWrapperCache(); + if (void* existingWrapper = NSMapGet(s_instanceWrapperCache, instance)) + return static_cast(existingWrapper); + RefPtr wrapper = adoptRef(new ObjcInstance(instance, rootObject)); + NSMapInsert(s_instanceWrapperCache, instance, wrapper.get()); + return wrapper.release(); +} + ObjcInstance::~ObjcInstance() { - // -finalizeForWebScript and -dealloc/-finalize may require autorelease pools. + // Both -finalizeForWebScript and -dealloc/-finalize of _instance may require autorelease pools. NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init]; + + ASSERT(s_instanceWrapperCache); + ASSERT(_instance); + NSMapRemove(s_instanceWrapperCache, _instance.get()); + if ([_instance.get() respondsToSelector:@selector(finalizeForWebScript)]) [_instance.get() performSelector:@selector(finalizeForWebScript)]; _instance = 0; + [pool drain]; } diff --git a/WebCore/bridge/qt/qt_instance.cpp b/WebCore/bridge/qt/qt_instance.cpp index 58280e3deaa03823042fdcede13804519595805a..05460140593e1f333a3d6d331bf6b20153e00049 100644 --- a/WebCore/bridge/qt/qt_instance.cpp +++ b/WebCore/bridge/qt/qt_instance.cpp @@ -43,16 +43,10 @@ namespace Bindings { typedef QMultiHash QObjectInstanceMap; static QObjectInstanceMap cachedInstances; -// Cache JSObjects -typedef QHash InstanceJSObjectMap; -static InstanceJSObjectMap cachedObjects; - // Derived RuntimeObject class QtRuntimeObjectImp : public RuntimeObjectImp { public: QtRuntimeObjectImp(ExecState*, PassRefPtr); - ~QtRuntimeObjectImp(); - virtual void invalidate(); static const ClassInfo s_info; @@ -64,9 +58,6 @@ public: instance->markAggregate(markStack); } -protected: - void removeFromCache(); - private: virtual const ClassInfo* classInfo() const { return &s_info; } }; @@ -78,25 +69,6 @@ QtRuntimeObjectImp::QtRuntimeObjectImp(ExecState* exec, PassRefPtr ins { } -QtRuntimeObjectImp::~QtRuntimeObjectImp() -{ - removeFromCache(); -} - -void QtRuntimeObjectImp::invalidate() -{ - removeFromCache(); - RuntimeObjectImp::invalidate(); -} - -void QtRuntimeObjectImp::removeFromCache() -{ - JSLock lock(SilenceAssertionsOnly); - QtInstance* key = cachedObjects.key(this); - if (key) - cachedObjects.remove(key); -} - // QtInstance QtInstance::QtInstance(QObject* o, PassRefPtr rootObject, QScriptEngine::ValueOwnership ownership) : Instance(rootObject) @@ -112,7 +84,6 @@ QtInstance::~QtInstance() { JSLock lock(SilenceAssertionsOnly); - cachedObjects.remove(this); cachedInstances.remove(m_hashkey); // clean up (unprotect from gc) the JSValues we've created @@ -190,16 +161,10 @@ Class* QtInstance::getClass() const return m_class; } -RuntimeObjectImp* QtInstance::createRuntimeObject(ExecState* exec) +RuntimeObjectImp* QtInstance::newRuntimeObject(ExecState* exec) { JSLock lock(SilenceAssertionsOnly); - RuntimeObjectImp* ret = static_cast(cachedObjects.value(this)); - if (!ret) { - ret = new (exec) QtRuntimeObjectImp(exec, this); - cachedObjects.insert(this, ret); - ret = static_cast(cachedObjects.value(this)); - } - return ret; + return new (exec) QtRuntimeObjectImp(exec, this); } void QtInstance::markAggregate(MarkStack& markStack) diff --git a/WebCore/bridge/runtime.cpp b/WebCore/bridge/runtime.cpp index 693440664234fc933bfbad965e83e5c20afe2ccd..eac8586a36e1dc881dac90895bb9cb5ceb3008a9 100644 --- a/WebCore/bridge/runtime.cpp +++ b/WebCore/bridge/runtime.cpp @@ -48,12 +48,14 @@ Array::~Array() Instance::Instance(PassRefPtr rootObject) : _rootObject(rootObject) + , m_runtimeObject(0) { ASSERT(_rootObject); } Instance::~Instance() { + ASSERT(!m_runtimeObject); } static KJSDidExecuteFunctionPtr s_didExecuteFunction; @@ -79,12 +81,38 @@ void Instance::end() } RuntimeObjectImp* Instance::createRuntimeObject(ExecState* exec) +{ + ASSERT(_rootObject); + ASSERT(_rootObject->isValid()); + if (m_runtimeObject) + return m_runtimeObject; + JSLock lock(SilenceAssertionsOnly); + m_runtimeObject = newRuntimeObject(exec); + _rootObject->addRuntimeObject(m_runtimeObject); + return m_runtimeObject; +} + +RuntimeObjectImp* Instance::newRuntimeObject(ExecState* exec) { JSLock lock(SilenceAssertionsOnly); - return new (exec) RuntimeObjectImp(exec, this); } +void Instance::willDestroyRuntimeObject() +{ + ASSERT(_rootObject); + ASSERT(_rootObject->isValid()); + ASSERT(m_runtimeObject); + _rootObject->removeRuntimeObject(m_runtimeObject); + m_runtimeObject = 0; +} + +void Instance::willInvalidateRuntimeObject() +{ + ASSERT(m_runtimeObject); + m_runtimeObject = 0; +} + RootObject* Instance::rootObject() const { return _rootObject && _rootObject->isValid() ? _rootObject.get() : 0; diff --git a/WebCore/bridge/runtime.h b/WebCore/bridge/runtime.h index e0280209160a9f68fefbaab58ee81ae359320354..72ee3877d7961007296326c08b2421f45bf07b74 100644 --- a/WebCore/bridge/runtime.h +++ b/WebCore/bridge/runtime.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2003, 2008 Apple Inc. All rights reserved. + * Copyright (C) 2003, 2008, 2009 Apple Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -86,8 +86,10 @@ public: void begin(); void end(); - virtual Class *getClass() const = 0; - virtual RuntimeObjectImp* createRuntimeObject(ExecState*); + virtual Class* getClass() const = 0; + RuntimeObjectImp* createRuntimeObject(ExecState*); + void willInvalidateRuntimeObject(); + void willDestroyRuntimeObject(); // Returns false if the value was not set successfully. virtual bool setValueOfUndefinedField(ExecState*, const Identifier&, JSValue) { return false; } @@ -119,6 +121,11 @@ protected: virtual void virtualEnd() { } RefPtr _rootObject; + +private: + virtual RuntimeObjectImp* newRuntimeObject(ExecState*); + + RuntimeObjectImp* m_runtimeObject; }; class Array : public Noncopyable { diff --git a/WebCore/bridge/runtime_object.cpp b/WebCore/bridge/runtime_object.cpp index f731af117286ff765c25730673908c7e1e415c01..3fd8024bc688c3285e37d2f403ced87ead9bf207 100644 --- a/WebCore/bridge/runtime_object.cpp +++ b/WebCore/bridge/runtime_object.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2003, 2008 Apple Inc. All rights reserved. + * Copyright (C) 2003, 2008, 2009 Apple Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -28,7 +28,6 @@ #include "JSDOMBinding.h" #include "runtime_method.h" -#include "runtime_root.h" #include #include @@ -46,25 +45,25 @@ RuntimeObjectImp::RuntimeObjectImp(ExecState* exec, PassRefPtr instanc : JSObject(deprecatedGetDOMStructure(exec)) , m_instance(instance) { - m_instance->rootObject()->addRuntimeObject(this); } RuntimeObjectImp::RuntimeObjectImp(ExecState*, PassRefPtr structure, PassRefPtr instance) : JSObject(structure) , m_instance(instance) { - m_instance->rootObject()->addRuntimeObject(this); } RuntimeObjectImp::~RuntimeObjectImp() { if (m_instance) - m_instance->rootObject()->removeRuntimeObject(this); + m_instance->willDestroyRuntimeObject(); } void RuntimeObjectImp::invalidate() { ASSERT(m_instance); + if (m_instance) + m_instance->willInvalidateRuntimeObject(); m_instance = 0; } diff --git a/WebCore/bridge/runtime_object.h b/WebCore/bridge/runtime_object.h index 2b29e2e3d264eaef5074c85cb95bb0fdf079e196..5aa02eae8cfd7065e6ed23eecea09d5f0e2b2cdd 100644 --- a/WebCore/bridge/runtime_object.h +++ b/WebCore/bridge/runtime_object.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2003, 2008 Apple Inc. All rights reserved. + * Copyright (C) 2003, 2008, 2009 Apple Inc. All rights reserved. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions @@ -34,13 +34,12 @@ namespace JSC { class RuntimeObjectImp : public JSObject { public: RuntimeObjectImp(ExecState*, PassRefPtr); - virtual ~RuntimeObjectImp(); virtual bool getOwnPropertySlot(ExecState*, const Identifier& propertyName, PropertySlot&); virtual bool getOwnPropertyDescriptor(ExecState*, const Identifier& propertyName, PropertyDescriptor&); virtual void put(ExecState*, const Identifier& propertyName, JSValue, PutPropertySlot&); - virtual bool deleteProperty(ExecState* , const Identifier& propertyName); + virtual bool deleteProperty(ExecState*, const Identifier& propertyName); virtual JSValue defaultValue(ExecState*, PreferredPrimitiveType) const; virtual CallType getCallData(CallData&); virtual ConstructType getConstructData(ConstructData&); @@ -48,7 +47,8 @@ public: virtual void getPropertyNames(ExecState*, PropertyNameArray&); virtual void getOwnPropertyNames(ExecState*, PropertyNameArray&); - virtual void invalidate(); + void invalidate(); + Bindings::Instance* getInternalInstance() const { return m_instance.get(); } static JSObject* throwInvalidAccessError(ExecState*);