From 13d799f93ddc4cba66f8022557431467d65c570b Mon Sep 17 00:00:00 2001 From: Kyle Huey Date: Fri, 6 Jul 2012 10:28:51 -0700 Subject: [PATCH] Bug 765936. r=bent,peterv --- content/base/public/nsContentUtils.h | 2 ++ content/base/src/nsContentUtils.cpp | 11 +++++++++ dom/indexedDB/IDBFactory.cpp | 3 +++ dom/indexedDB/IDBRequest.cpp | 36 +--------------------------- dom/indexedDB/IDBRequest.h | 23 ------------------ dom/indexedDB/IDBWrapperCache.cpp | 17 +++++++------ dom/indexedDB/IDBWrapperCache.h | 8 +++++++ js/xpconnect/idl/nsIXPConnect.idl | 7 ++++++ js/xpconnect/src/XPCJSRuntime.cpp | 11 +++++++++ js/xpconnect/src/nsXPConnect.cpp | 11 +++++++++ js/xpconnect/src/xpcprivate.h | 1 + 11 files changed, 65 insertions(+), 65 deletions(-) diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h index 456c9ba0ffa4..df818d70dbd3 100644 --- a/content/base/public/nsContentUtils.h +++ b/content/base/public/nsContentUtils.h @@ -1282,6 +1282,8 @@ public: static nsresult DropJSObjects(void* aScriptObjectHolder); #ifdef DEBUG + static bool AreJSObjectsHeld(void* aScriptObjectHolder); + static void CheckCCWrapperTraversal(nsISupports* aScriptObjectHolder, nsWrapperCache* aCache); #endif diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp index 87d097a000fb..3bd27f9f2224 100644 --- a/content/base/src/nsContentUtils.cpp +++ b/content/base/src/nsContentUtils.cpp @@ -4453,6 +4453,17 @@ nsContentUtils::DropJSObjects(void* aScriptObjectHolder) return rv; } +#ifdef DEBUG +/* static */ +bool +nsContentUtils::AreJSObjectsHeld(void* aScriptHolder) +{ + bool isHeld = false; + sXPConnect->TestJSHolder(aScriptHolder, &isHeld); + return isHeld; +} +#endif + /* static */ void nsContentUtils::NotifyInstalledMenuKeyboardListener(bool aInstalling) diff --git a/dom/indexedDB/IDBFactory.cpp b/dom/indexedDB/IDBFactory.cpp index 628ac902f028..a0ecd8b5519f 100644 --- a/dom/indexedDB/IDBFactory.cpp +++ b/dom/indexedDB/IDBFactory.cpp @@ -18,6 +18,7 @@ #include "nsCharSeparatedTokenizer.h" #include "nsContentUtils.h" #include "nsDOMClassInfoID.h" +#include "nsGlobalWindow.h" #include "nsHashKeys.h" #include "nsPIDOMWindow.h" #include "nsServiceManagerUtils.h" @@ -413,6 +414,8 @@ IDBFactory::OpenCommon(const nsAString& aName, if (mWindow) { window = mWindow; + scriptOwner = + static_cast(window.get())->FastGetGlobalJSObject(); } else { scriptOwner = mOwningObject; diff --git a/dom/indexedDB/IDBRequest.cpp b/dom/indexedDB/IDBRequest.cpp index 8f562d90e59c..4980298479e6 100644 --- a/dom/indexedDB/IDBRequest.cpp +++ b/dom/indexedDB/IDBRequest.cpp @@ -32,7 +32,6 @@ IDBRequest::IDBRequest() mActorParent(nsnull), mErrorCode(NS_OK), mHaveResultOrErrorCode(false), - mRooted(false), mLineNo(0) { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); @@ -41,8 +40,6 @@ IDBRequest::IDBRequest() IDBRequest::~IDBRequest() { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); - - UnrootResultVal(); } // static @@ -74,7 +71,6 @@ IDBRequest::Reset() mResultVal = JSVAL_VOID; mHaveResultOrErrorCode = false; mError = nsnull; - UnrootResultVal(); } nsresult @@ -82,7 +78,6 @@ IDBRequest::NotifyHelperCompleted(HelperBase* aHelper) { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); NS_ASSERTION(!mHaveResultOrErrorCode, "Already called!"); - NS_ASSERTION(!PreservingWrapper(), "Already rooted?!"); NS_ASSERTION(JSVAL_IS_VOID(mResultVal), "Should be undefined!"); // See if our window is still valid. If not then we're going to pretend that @@ -116,7 +111,7 @@ IDBRequest::NotifyHelperCompleted(HelperBase* aHelper) JSAutoRequest ar(cx); JSAutoEnterCompartment ac; if (ac.enter(cx, global)) { - RootResultVal(); + AssertIsRooted(); rv = aHelper->GetSuccessResult(cx, &mResultVal); if (NS_FAILED(rv)) { @@ -144,7 +139,6 @@ IDBRequest::NotifyHelperSentResultsToChildProcess(nsresult aRv) { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); NS_ASSERTION(!mHaveResultOrErrorCode, "Already called!"); - NS_ASSERTION(!PreservingWrapper(), "Already rooted?!"); NS_ASSERTION(JSVAL_IS_VOID(mResultVal), "Should be undefined!"); // See if our window is still valid. If not then we're going to pretend that @@ -171,7 +165,6 @@ IDBRequest::SetError(nsresult aRv) mErrorCode = aRv; mResultVal = JSVAL_VOID; - UnrootResultVal(); } #ifdef DEBUG @@ -239,18 +232,6 @@ IDBRequest::FillScriptErrorEvent(nsScriptErrorEvent* aEvent) const aEvent->fileName = mFilename.get(); } -void -IDBRequest::RootResultValInternal() -{ - NS_HOLD_JS_OBJECTS(this, IDBRequest); -} - -void -IDBRequest::UnrootResultValInternal() -{ - NS_DROP_JS_OBJECTS(this, IDBRequest); -} - NS_IMETHODIMP IDBRequest::GetReadyState(nsAString& aReadyState) { @@ -327,7 +308,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(IDBRequest, IDBWrapperCache) tmp->mResultVal = JSVAL_VOID; - tmp->UnrootResultVal(); NS_CYCLE_COLLECTION_UNLINK_EVENT_HANDLER(success) NS_CYCLE_COLLECTION_UNLINK_EVENT_HANDLER(error) NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mSource) @@ -366,8 +346,6 @@ IDBRequest::PreHandleEvent(nsEventChainPreVisitor& aVisitor) IDBOpenDBRequest::~IDBOpenDBRequest() { NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); - - UnrootResultVal(); } // static @@ -400,18 +378,6 @@ IDBOpenDBRequest::SetTransaction(IDBTransaction* aTransaction) mTransaction = aTransaction; } -void -IDBOpenDBRequest::RootResultValInternal() -{ - NS_HOLD_JS_OBJECTS(this, IDBOpenDBRequest); -} - -void -IDBOpenDBRequest::UnrootResultValInternal() -{ - NS_DROP_JS_OBJECTS(this, IDBOpenDBRequest); -} - NS_IMPL_CYCLE_COLLECTION_CLASS(IDBOpenDBRequest) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(IDBOpenDBRequest, diff --git a/dom/indexedDB/IDBRequest.h b/dom/indexedDB/IDBRequest.h index 174a1cca01e7..78bb38d0d47e 100644 --- a/dom/indexedDB/IDBRequest.h +++ b/dom/indexedDB/IDBRequest.h @@ -87,25 +87,6 @@ protected: IDBRequest(); ~IDBRequest(); - virtual void RootResultValInternal(); - virtual void UnrootResultValInternal(); - - void RootResultVal() - { - if (!mRooted) { - RootResultValInternal(); - mRooted = true; - } - } - - void UnrootResultVal() - { - if (mRooted) { - UnrootResultValInternal(); - mRooted = false; - } - } - nsCOMPtr mSource; nsRefPtr mTransaction; @@ -120,7 +101,6 @@ protected: nsresult mErrorCode; bool mHaveResultOrErrorCode; - bool mRooted; nsString mFilename; PRUint32 mLineNo; @@ -149,9 +129,6 @@ public: protected: ~IDBOpenDBRequest(); - virtual void RootResultValInternal(); - virtual void UnrootResultValInternal(); - // Only touched on the main thread. NS_DECL_EVENT_HANDLER(blocked) NS_DECL_EVENT_HANDLER(upgradeneeded) diff --git a/dom/indexedDB/IDBWrapperCache.cpp b/dom/indexedDB/IDBWrapperCache.cpp index d6c023ca00fa..b00b0c191550 100644 --- a/dom/indexedDB/IDBWrapperCache.cpp +++ b/dom/indexedDB/IDBWrapperCache.cpp @@ -48,13 +48,7 @@ IDBWrapperCache::~IDBWrapperCache() bool IDBWrapperCache::SetScriptOwner(JSObject* aScriptOwner) { - if (!aScriptOwner) { - NS_ASSERTION(!mScriptOwner, - "Don't null out existing owner, we need to call " - "DropJSObjects!"); - - return true; - } + NS_ASSERTION(aScriptOwner, "This should never be null!"); mScriptOwner = aScriptOwner; @@ -70,3 +64,12 @@ IDBWrapperCache::SetScriptOwner(JSObject* aScriptOwner) return true; } + +#ifdef DEBUG +void +IDBWrapperCache::AssertIsRooted() const +{ + NS_ASSERTION(nsContentUtils::AreJSObjectsHeld(const_cast(this)), + "Why aren't we rooted?!"); +} +#endif diff --git a/dom/indexedDB/IDBWrapperCache.h b/dom/indexedDB/IDBWrapperCache.h index 84643daaa1e8..9e9dee7f90a7 100644 --- a/dom/indexedDB/IDBWrapperCache.h +++ b/dom/indexedDB/IDBWrapperCache.h @@ -46,6 +46,14 @@ public: nsDOMEventTargetHelper::FromSupports(aSupports)); } +#ifdef DEBUG + void AssertIsRooted() const; +#else + inline void AssertIsRooted() const + { + } +#endif + protected: IDBWrapperCache() : mScriptOwner(nsnull) diff --git a/js/xpconnect/idl/nsIXPConnect.idl b/js/xpconnect/idl/nsIXPConnect.idl index fdbe76d22876..23b0c9f43276 100644 --- a/js/xpconnect/idl/nsIXPConnect.idl +++ b/js/xpconnect/idl/nsIXPConnect.idl @@ -673,6 +673,13 @@ interface nsIXPConnect : nsISupports */ [noscript] void removeJSHolder(in voidPtr aHolder); + /** + * Test to see if a JS holder is in our hashtable. + * Only available in debug builds. + * @param aHolder The object to test for. + */ + [noscript] bool testJSHolder(in voidPtr aHolder); + /** * Note aJSContext as a child to the cycle collector. * @param aJSContext The JSContext to note. diff --git a/js/xpconnect/src/XPCJSRuntime.cpp b/js/xpconnect/src/XPCJSRuntime.cpp index 862a8e572528..a1ff351a0b28 100644 --- a/js/xpconnect/src/XPCJSRuntime.cpp +++ b/js/xpconnect/src/XPCJSRuntime.cpp @@ -296,6 +296,17 @@ XPCJSRuntime::RemoveJSHolder(void* aHolder) return NS_OK; } +nsresult +XPCJSRuntime::TestJSHolder(void* aHolder, bool* aRetval) +{ + if (!mJSHolders.ops) + return NS_ERROR_OUT_OF_MEMORY; + + *aRetval = !!JS_DHashTableOperate(&mJSHolders, aHolder, JS_DHASH_LOOKUP); + + return NS_OK; +} + // static void XPCJSRuntime::TraceBlackJS(JSTracer* trc, void* data) { diff --git a/js/xpconnect/src/nsXPConnect.cpp b/js/xpconnect/src/nsXPConnect.cpp index e530ce2000f8..22005d23f670 100644 --- a/js/xpconnect/src/nsXPConnect.cpp +++ b/js/xpconnect/src/nsXPConnect.cpp @@ -2260,6 +2260,17 @@ nsXPConnect::RemoveJSHolder(void* aHolder) return mRuntime->RemoveJSHolder(aHolder); } +NS_IMETHODIMP +nsXPConnect::TestJSHolder(void* aHolder, bool* aRetval) +{ +#ifdef DEBUG + return mRuntime->TestJSHolder(aHolder, aRetval); +#else + MOZ_ASSERT(false); + return NS_ERROR_FAILURE; +#endif +} + NS_IMETHODIMP nsXPConnect::SetReportAllJSExceptions(bool newval) { diff --git a/js/xpconnect/src/xpcprivate.h b/js/xpconnect/src/xpcprivate.h index 1c8e75659987..f107783a20ae 100644 --- a/js/xpconnect/src/xpcprivate.h +++ b/js/xpconnect/src/xpcprivate.h @@ -763,6 +763,7 @@ public: nsresult AddJSHolder(void* aHolder, nsScriptObjectTracer* aTracer); nsresult RemoveJSHolder(void* aHolder); + nsresult TestJSHolder(void* aHolder, bool* aRetval); static void SuspectWrappedNative(XPCWrappedNative *wrapper, nsCycleCollectionTraversalCallback &cb);