From 376551de730fe96f4db9b897288e23f699e67f72 Mon Sep 17 00:00:00 2001 From: Peter Van der Beken Date: Thu, 19 Mar 2009 14:05:26 +0100 Subject: [PATCH] Backing out f385e435c082, fix for bug 481677 (Avoid hash lookups in XPCWrappedNative::GetNewOrUsed), to try to fix orange. --- js/src/xpconnect/src/xpccomponents.cpp | 2 +- js/src/xpconnect/src/xpcconvert.cpp | 2 +- js/src/xpconnect/src/xpcprivate.h | 9 +-- js/src/xpconnect/src/xpcwrappednative.cpp | 73 +++++++---------------- xpcom/glue/nsCOMPtr.h | 10 ---- 5 files changed, 27 insertions(+), 69 deletions(-) diff --git a/js/src/xpconnect/src/xpccomponents.cpp b/js/src/xpconnect/src/xpccomponents.cpp index 7c9d41e24d80..8d96c1bb0cf5 100644 --- a/js/src/xpconnect/src/xpccomponents.cpp +++ b/js/src/xpconnect/src/xpccomponents.cpp @@ -4050,7 +4050,7 @@ nsXPCComponents::AttachNewComponentsObject(XPCCallContext& ccx, return JS_FALSE; nsCOMPtr wrapper; - XPCWrappedNative::GetNewOrUsed(ccx, cholder, aScope, iface, nsnull, + XPCWrappedNative::GetNewOrUsed(ccx, cholder, aScope, iface, OBJ_IS_NOT_GLOBAL, getter_AddRefs(wrapper)); if(!wrapper) return JS_FALSE; diff --git a/js/src/xpconnect/src/xpcconvert.cpp b/js/src/xpconnect/src/xpcconvert.cpp index 6c847602832d..4a8aca083396 100644 --- a/js/src/xpconnect/src/xpcconvert.cpp +++ b/js/src/xpconnect/src/xpcconvert.cpp @@ -1142,7 +1142,7 @@ XPCConvert::NativeInterface2JSObject(XPCCallContext& ccx, else { rv = XPCWrappedNative::GetNewOrUsed(ccx, src, xpcscope, iface, - cache, isGlobal, + isGlobal, getter_AddRefs(strongWrapper)); wrapper = strongWrapper; diff --git a/js/src/xpconnect/src/xpcprivate.h b/js/src/xpconnect/src/xpcprivate.h index 232aa55e00a4..11517545839b 100644 --- a/js/src/xpconnect/src/xpcprivate.h +++ b/js/src/xpconnect/src/xpcprivate.h @@ -2248,16 +2248,11 @@ public: GetRuntime() const {XPCWrappedNativeScope* scope = GetScope(); return scope ? scope->GetRuntime() : nsnull;} - /** - * If Object has a nsWrapperCache it should be passed in. If a cache is - * passed in then cache->GetWrapper() must be null. - */ static nsresult GetNewOrUsed(XPCCallContext& ccx, nsISupports* Object, XPCWrappedNativeScope* Scope, XPCNativeInterface* Interface, - nsWrapperCache* cache, JSBool isGlobal, XPCWrappedNative** wrapper); @@ -2408,11 +2403,11 @@ protected: XPCWrappedNative(); // not implemented // This ctor is used if this object will have a proto. - XPCWrappedNative(already_AddRefed aIdentity, + XPCWrappedNative(nsISupports* aIdentity, XPCWrappedNativeProto* aProto); // This ctor is used if this object will NOT have a proto. - XPCWrappedNative(already_AddRefed aIdentity, + XPCWrappedNative(nsISupports* aIdentity, XPCWrappedNativeScope* aScope, XPCNativeSet* aSet); diff --git a/js/src/xpconnect/src/xpcwrappednative.cpp b/js/src/xpconnect/src/xpcwrappednative.cpp index 71eb4615787d..2d65096fc52a 100644 --- a/js/src/xpconnect/src/xpcwrappednative.cpp +++ b/js/src/xpconnect/src/xpcwrappednative.cpp @@ -287,14 +287,9 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx, nsISupports* Object, XPCWrappedNativeScope* Scope, XPCNativeInterface* Interface, - nsWrapperCache *cache, JSBool isGlobal, XPCWrappedNative** resultWrapper) { - NS_ASSERTION(!cache || !cache->GetWrapper(), - "We assume the caller already checked if it could get the " - "wrapper from the cache."); - nsresult rv; NS_ASSERTION(!Scope->GetRuntime()->GetThreadRunningGC(), @@ -330,37 +325,25 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx, AutoMarkingWrappedNativePtr wrapper(ccx); Native2WrappedNativeMap* map = Scope->GetWrappedNativeMap(); - if(!cache) - { - { // scoped lock - XPCAutoLock lock(mapLock); - wrapper = map->Find(identity); - if(wrapper) - wrapper->AddRef(); - } - - if(wrapper) - { - if(Interface && - !wrapper->FindTearOff(ccx, Interface, JS_FALSE, &rv)) - { - NS_RELEASE(wrapper); - NS_ASSERTION(NS_FAILED(rv), "returning NS_OK on failure"); - return rv; - } - DEBUG_CheckWrapperThreadSafety(wrapper); - *resultWrapper = wrapper; - return NS_OK; - } - } -#ifdef DEBUG - else if(!cache->GetWrapper()) { // scoped lock XPCAutoLock lock(mapLock); - NS_ASSERTION(!map->Find(identity), - "There's a wrapper in the hashtable but it wasn't cached?"); + wrapper = map->Find(identity); + if(wrapper) + wrapper->AddRef(); + } + + if(wrapper) + { + if(Interface && !wrapper->FindTearOff(ccx, Interface, JS_FALSE, &rv)) + { + NS_RELEASE(wrapper); + NS_ASSERTION(NS_FAILED(rv), "returning NS_OK on failure"); + return rv; + } + DEBUG_CheckWrapperThreadSafety(wrapper); + *resultWrapper = wrapper; + return NS_OK; } -#endif // There is a chance that the object wants to have the self-same JSObject // reflection regardless of the scope into which we are reflecting it. @@ -430,7 +413,7 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx, XPCWrappedNativeScope::FindInJSObjectScope(ccx, parent); if(betterScope != Scope) return GetNewOrUsed(ccx, identity, betterScope, Interface, - cache, isGlobal, resultWrapper); + isGlobal, resultWrapper); newParentVal = OBJECT_TO_JSVAL(parent); } @@ -439,11 +422,6 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx, // the preCreate call caused the wrapper to get created through some // interesting path (the DOM code tends to make this happen sometimes). - if(cache) - { - wrapper = static_cast(cache->GetWrapper()); - } - else { // scoped lock XPCAutoLock lock(mapLock); wrapper = map->Find(identity); @@ -482,7 +460,7 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx, proto->CacheOffsets(identity); - wrapper = new XPCWrappedNative(identity.get(), proto); + wrapper = new XPCWrappedNative(identity, proto); if(!wrapper) return NS_ERROR_FAILURE; } @@ -494,18 +472,13 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx, if(!set) return NS_ERROR_FAILURE; - wrapper = new XPCWrappedNative(identity.get(), Scope, set); + wrapper = new XPCWrappedNative(identity, Scope, set); if(!wrapper) return NS_ERROR_FAILURE; DEBUG_ReportShadowedMembers(set, wrapper, nsnull); } - // The strong reference was taken over by the wrapper, so make the nsCOMPtr - // forget about it. - // Note that identity is null from here on! - identity.forget(); - NS_ADDREF(wrapper); NS_ASSERTION(!XPCNativeWrapper::IsNativeWrapper(parent), @@ -677,7 +650,7 @@ XPCWrappedNative::GetUsedOnly(XPCCallContext& ccx, } // This ctor is used if this object will have a proto. -XPCWrappedNative::XPCWrappedNative(already_AddRefed aIdentity, +XPCWrappedNative::XPCWrappedNative(nsISupports* aIdentity, XPCWrappedNativeProto* aProto) : mMaybeProto(aProto), mSet(aProto->GetSet()), @@ -685,7 +658,7 @@ XPCWrappedNative::XPCWrappedNative(already_AddRefed aIdentity, mScriptableInfo(nsnull), mWrapper(nsnull) { - mIdentity = aIdentity.get(); + NS_ADDREF(mIdentity = aIdentity); NS_ASSERTION(mMaybeProto, "bad ctor param"); NS_ASSERTION(mSet, "bad ctor param"); @@ -694,7 +667,7 @@ XPCWrappedNative::XPCWrappedNative(already_AddRefed aIdentity, } // This ctor is used if this object will NOT have a proto. -XPCWrappedNative::XPCWrappedNative(already_AddRefed aIdentity, +XPCWrappedNative::XPCWrappedNative(nsISupports* aIdentity, XPCWrappedNativeScope* aScope, XPCNativeSet* aSet) @@ -704,7 +677,7 @@ XPCWrappedNative::XPCWrappedNative(already_AddRefed aIdentity, mScriptableInfo(nsnull), mWrapper(nsnull) { - mIdentity = aIdentity.get(); + NS_ADDREF(mIdentity = aIdentity); NS_ASSERTION(aScope, "bad ctor param"); NS_ASSERTION(aSet, "bad ctor param"); diff --git a/xpcom/glue/nsCOMPtr.h b/xpcom/glue/nsCOMPtr.h index dc3503a33052..1f5f93e3d1d9 100644 --- a/xpcom/glue/nsCOMPtr.h +++ b/xpcom/glue/nsCOMPtr.h @@ -1065,16 +1065,6 @@ class nsCOMPtr mRawPtr = temp; } - already_AddRefed - forget() - // return the value of mRawPtr and null out mRawPtr. Useful for - // already_AddRefed return values. - { - nsISupports* temp = 0; - swap(temp); - return temp; - } - void forget( nsISupports** rhs NS_OUTPARAM ) // Set the target of rhs to the value of mRawPtr and null out mRawPtr.