Bug 811206, Fix JSHolder drop handling, part 3 (fixes), r=mccr8,khuey

content/base/public/nsContentUtils.h

@@ -2239,8 +2239,6 @@ class nsContentUtils
   static nsILineBreaker* sLineBreaker;
   static nsIWordBreaker* sWordBreaker;
 
-  static uint32_t sJSGCThingRootCount;
-
 #ifdef IBMBIDI
   static nsIBidiKeyboard* sBidiKeyboard;
 #endif

content/base/src/nsContentUtils.cpp

@@ -208,7 +208,6 @@ nsIContentPolicy *nsContentUtils::sContentPolicyService;
 bool nsContentUtils::sTriedToGetContentPolicy = false;
 nsILineBreaker *nsContentUtils::sLineBreaker;
 nsIWordBreaker *nsContentUtils::sWordBreaker;
-uint32_t nsContentUtils::sJSGCThingRootCount;
 #ifdef IBMBIDI
 nsIBidiKeyboard *nsContentUtils::sBidiKeyboard = nullptr;
 #endif
@@ -4526,6 +4525,10 @@ nsContentUtils::HoldJSObjects(void* aScriptObjectHolder,
 nsresult
 nsContentUtils::DropJSObjects(void* aScriptObjectHolder)
 {
+  if (!sXPConnect) {
+    return NS_OK;
+  }
+
   return sXPConnect->RemoveJSHolder(aScriptObjectHolder);
 }
 
@@ -6863,9 +6866,8 @@ nsContentUtils::ReleaseWrapper(void* aScriptObjectHolder,
     if (aCache->IsDOMBinding() && obj) {
       xpc::GetObjectScope(obj)->RemoveDOMExpandoObject(obj);
     }
-    DropJSObjects(aScriptObjectHolder);
-
     aCache->SetPreservingWrapper(false);
+    DropJSObjects(aScriptObjectHolder);
   }
 }
 

content/canvas/src/ImageData.cpp

@@ -46,8 +46,8 @@ void
 ImageData::DropData()
 {
   if (mData) {
-    NS_DROP_JS_OBJECTS(this, ImageData);
     mData = NULL;
+    NS_DROP_JS_OBJECTS(this, ImageData);
   }
 }
 

content/events/src/nsDOMMessageEvent.cpp

@@ -62,9 +62,9 @@ void
 nsDOMMessageEvent::UnrootData()
 {
   NS_ASSERTION(mDataRooted, "...");
-  NS_DROP_JS_OBJECTS(this, nsDOMMessageEvent);
   mDataRooted = false;
   mData = JSVAL_VOID;
+  NS_DROP_JS_OBJECTS(this, nsDOMMessageEvent);
 }
 
 NS_IMETHODIMP

content/events/src/nsDOMNotifyAudioAvailableEvent.cpp

@@ -38,8 +38,8 @@ NS_IMPL_RELEASE_INHERITED(nsDOMNotifyAudioAvailableEvent, nsDOMEvent)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsDOMNotifyAudioAvailableEvent, nsDOMEvent)
   if (tmp->mCachedArray) {
-    NS_DROP_JS_OBJECTS(tmp, nsDOMNotifyAudioAvailableEvent);
     tmp->mCachedArray = nullptr;
+    NS_DROP_JS_OBJECTS(tmp, nsDOMNotifyAudioAvailableEvent);
   }
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -60,8 +60,8 @@ nsDOMNotifyAudioAvailableEvent::~nsDOMNotifyAudioAvailableEvent()
 {
   MOZ_COUNT_DTOR(nsDOMNotifyAudioAvailableEvent);
   if (mCachedArray) {
-    NS_DROP_JS_OBJECTS(this, nsDOMNotifyAudioAvailableEvent);
     mCachedArray = nullptr;
+    NS_DROP_JS_OBJECTS(this, nsDOMNotifyAudioAvailableEvent);
   }
 }
 

content/media/webaudio/AudioBuffer.cpp

@@ -54,6 +54,7 @@ AudioBuffer::AudioBuffer(AudioContext* aContext, uint32_t aLength,
 
 AudioBuffer::~AudioBuffer()
 {
+  mChannels.Clear();
   NS_DROP_JS_OBJECTS(this, AudioBuffer);
 }
 

content/xbl/src/nsXBLDocumentInfo.cpp

@@ -522,8 +522,9 @@ nsXBLDocumentInfo::~nsXBLDocumentInfo()
     mGlobalObject->ClearGlobalObjectOwner(); // just in case
   }
   if (mBindingTable) {
-    NS_DROP_JS_OBJECTS(this, nsXBLDocumentInfo);
     delete mBindingTable;
+    mBindingTable = nullptr;
+    NS_DROP_JS_OBJECTS(this, nsXBLDocumentInfo);
   }
 }
 

content/xul/content/src/nsXULElement.cpp

@@ -2491,8 +2491,8 @@ void
 nsXULPrototypeScript::UnlinkJSObjects()
 {
     if (mScriptObject.mObject) {
-        nsContentUtils::DropJSObjects(this);
         mScriptObject.mObject = nullptr;
+        nsContentUtils::DropJSObjects(this);
     }
 }
 

dom/base/DOMRequest.cpp

@@ -54,7 +54,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(DOMRequest,
                                                 nsDOMEventTargetHelper)
   if (tmp->mRooted) {
-    tmp->mResult = JSVAL_VOID;
     tmp->UnrootResultVal();
   }
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mError)
@@ -187,6 +186,7 @@ void
 DOMRequest::UnrootResultVal()
 {
   NS_ASSERTION(mRooted, "Don't call me if not rooted!");
+  mResult = JSVAL_VOID;
   NS_DROP_JS_OBJECTS(this, DOMRequest);
   mRooted = false;
 }

dom/base/nsJSEnvironment.cpp

@@ -4144,12 +4144,13 @@ nsJSArgArray::~nsJSArgArray()
 void
 nsJSArgArray::ReleaseJSObjects()
 {
-  if (mArgc > 0)
-    NS_DROP_JS_OBJECTS(this, nsJSArgArray);
   if (mArgv) {
     PR_DELETE(mArgv);
   }
-  mArgc = 0;
+  if (mArgc > 0) {
+    mArgc = 0;
+    NS_DROP_JS_OBJECTS(this, nsJSArgArray);
+  }
 }
 
 // QueryInterface implementation for nsJSArgArray

dom/base/nsJSTimeoutHandler.cpp

@@ -141,17 +141,12 @@ nsJSScriptTimeoutHandler::~nsJSScriptTimeoutHandler()
 void
 nsJSScriptTimeoutHandler::ReleaseJSObjects()
 {
-  if (mExpr || mFunObj) {
-    if (mExpr) {
-      NS_DROP_JS_OBJECTS(this, nsJSScriptTimeoutHandler);
-      mExpr = nullptr;
-    } else if (mFunObj) {
-      NS_DROP_JS_OBJECTS(this, nsJSScriptTimeoutHandler);
-      mFunObj = nullptr;
-    } else {
-      NS_WARNING("No func and no expr - roots may not have been removed");
-    }
+  if (mExpr) {
+    mExpr = nullptr;
+  } else {
+    mFunObj = nullptr;
   }
+  NS_DROP_JS_OBJECTS(this, nsJSScriptTimeoutHandler);
 }
 
 nsresult

dom/bindings/CallbackFunction.h

@@ -91,8 +91,8 @@ class CallbackFunction : public nsISupports
   void DropCallback()
   {
     if (mCallable) {
-      NS_DROP_JS_OBJECTS(this, CallbackFunction);
       mCallable = nullptr;
+      NS_DROP_JS_OBJECTS(this, CallbackFunction);
       nsLayoutStatics::Release();
     }
   }

dom/bluetooth/BluetoothAdapter.cpp

@@ -163,6 +163,8 @@ BluetoothAdapter::Unroot()
   if (!mIsRooted) {
     return;
   }
+  mJsUuids = nullptr;
+  mJsDeviceAddresses = nullptr;
   NS_DROP_JS_OBJECTS(this, BluetoothAdapter);
   mIsRooted = false;
 }

dom/bluetooth/BluetoothDevice.cpp

@@ -87,6 +87,8 @@ void
 BluetoothDevice::Unroot()
 {
   if (mIsRooted) {
+    mJsUuids = nullptr;
+    mJsServices = nullptr;
     NS_DROP_JS_OBJECTS(this, BluetoothDevice);
     mIsRooted = false;
   }

dom/devicestorage/nsDeviceStorage.cpp

@@ -1141,8 +1141,8 @@ nsDOMDeviceStorageCursor::Continue()
   if (mRooted) {
     // We call onsuccess multiple times. clear the last
     // rooted result.
-    NS_DROP_JS_OBJECTS(this, nsDOMDeviceStorageCursor);
     mResult = JSVAL_VOID;
+    NS_DROP_JS_OBJECTS(this, nsDOMDeviceStorageCursor);
     mDone = false;
     mRooted = false;
   }

dom/indexedDB/IDBCursor.cpp

@@ -369,12 +369,28 @@ IDBCursor::~IDBCursor()
     NS_ASSERTION(!mActorChild, "Should have cleared in Send__delete__!");
   }
 
-  if (mRooted) {
-    NS_DROP_JS_OBJECTS(this, IDBCursor);
-  }
+  DropJSObjects();
   IDBObjectStore::ClearCloneReadInfo(mCloneReadInfo);
 }
 
+void
+IDBCursor::DropJSObjects()
+{
+  if (!mRooted) {
+    return;
+  }
+  mScriptOwner = nullptr;
+  mCachedKey = JSVAL_VOID;
+  mCachedPrimaryKey = JSVAL_VOID;
+  mCachedValue = JSVAL_VOID;
+  mHaveCachedKey = false;
+  mHaveCachedPrimaryKey = false;
+  mHaveCachedValue = false;
+  mRooted = false;
+  mHaveValue = false;
+  NS_DROP_JS_OBJECTS(this, IDBCursor);
+}
+
 nsresult
 IDBCursor::ContinueInternal(const Key& aKey,
                             int32_t aCount)
@@ -455,18 +471,7 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(IDBCursor)
   // Don't unlink mObjectStore, mIndex, or mTransaction!
-  if (tmp->mRooted) {
-    NS_DROP_JS_OBJECTS(tmp, IDBCursor);
-    tmp->mScriptOwner = nullptr;
-    tmp->mCachedKey = JSVAL_VOID;
-    tmp->mCachedPrimaryKey = JSVAL_VOID;
-    tmp->mCachedValue = JSVAL_VOID;
-    tmp->mHaveCachedKey = false;
-    tmp->mHaveCachedPrimaryKey = false;
-    tmp->mHaveCachedValue = false;
-    tmp->mRooted = false;
-    tmp->mHaveValue = false;
-  }
+  tmp->DropJSObjects();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mRequest)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 

dom/indexedDB/IDBCursor.h

@@ -151,6 +151,8 @@ class IDBCursor MOZ_FINAL : public nsIIDBCursorWithValue
   IDBCursor();
   ~IDBCursor();
 
+  void DropJSObjects();
+
   static
   already_AddRefed<IDBCursor>
   CreateCommon(IDBRequest* aRequest,

dom/indexedDB/IDBDatabase.cpp

@@ -244,8 +244,6 @@ IDBDatabase::~IDBDatabase()
       mgr->UnregisterDatabase(this);
     }
   }
-
-  nsContentUtils::ReleaseWrapper(static_cast<nsIDOMEventTarget*>(this), this);
 }
 
 void

dom/indexedDB/IDBFactory.cpp

@@ -77,6 +77,7 @@ IDBFactory::~IDBFactory()
     NS_ASSERTION(!mActorChild, "Should have cleared in Send__delete__!");
   }
   if (mRootedOwningObject) {
+    mOwningObject = nullptr;
     NS_DROP_JS_OBJECTS(this, IDBFactory);
   }
 }

dom/indexedDB/IDBIndex.cpp

@@ -414,6 +414,7 @@ IDBIndex::~IDBIndex()
   NS_ASSERTION(!mActorParent, "Actor parent owns us, how can we be dying?!");
 
   if (mRooted) {
+    mCachedKeyPath = JSVAL_VOID;
     NS_DROP_JS_OBJECTS(this, IDBIndex);
   }
 

dom/indexedDB/IDBKeyRange.cpp

@@ -318,14 +318,7 @@ NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(IDBKeyRange)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(IDBKeyRange)
-  if (tmp->mRooted) {
-    NS_DROP_JS_OBJECTS(tmp, IDBKeyRange);
-    tmp->mCachedLowerVal = JSVAL_VOID;
-    tmp->mCachedUpperVal = JSVAL_VOID;
-    tmp->mHaveCachedLowerVal = false;
-    tmp->mHaveCachedUpperVal = false;
-    tmp->mRooted = false;
-  }
+  tmp->DropJSObjects();
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IDBKeyRange)
@@ -339,11 +332,23 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(IDBKeyRange)
 
 DOMCI_DATA(IDBKeyRange, IDBKeyRange)
 
-IDBKeyRange::~IDBKeyRange()
+void
+IDBKeyRange::DropJSObjects()
 {
-  if (mRooted) {
-    NS_DROP_JS_OBJECTS(this, IDBKeyRange);
+  if (!mRooted) {
+    return;
   }
+  mCachedLowerVal = JSVAL_VOID;
+  mCachedUpperVal = JSVAL_VOID;
+  mHaveCachedLowerVal = false;
+  mHaveCachedUpperVal = false;
+  mRooted = false;
+  NS_DROP_JS_OBJECTS(this, IDBKeyRange);
+}
+
+IDBKeyRange::~IDBKeyRange()
+{
+  DropJSObjects();
 }
 
 NS_IMETHODIMP

dom/indexedDB/IDBKeyRange.h

@@ -150,6 +150,8 @@ class IDBKeyRange MOZ_FINAL : public nsIIDBKeyRange
   template <class T>
   void ToSerializedKeyRange(T& aKeyRange);
 
+  void DropJSObjects();
+
 private:
   ~IDBKeyRange();
 

dom/indexedDB/IDBObjectStore.cpp

@@ -1543,6 +1543,7 @@ IDBObjectStore::~IDBObjectStore()
   }
 
   if (mRooted) {
+    mCachedKeyPath = JSVAL_VOID;
     NS_DROP_JS_OBJECTS(this, IDBObjectStore);
   }
 }

dom/indexedDB/IDBRequest.cpp

@@ -40,6 +40,7 @@ IDBRequest::IDBRequest()
 
 IDBRequest::~IDBRequest()
 {
+  mResultVal = JSVAL_VOID;
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 }
 

dom/indexedDB/IDBTransaction.cpp

@@ -189,8 +189,6 @@ IDBTransaction::~IDBTransaction()
     mActorChild->Send__delete__(mActorChild);
     NS_ASSERTION(!mActorChild, "Should have cleared in Send__delete__!");
   }
-
-  nsContentUtils::ReleaseWrapper(static_cast<nsIDOMEventTarget*>(this), this);
 }
 
 void

dom/indexedDB/IDBWrapperCache.cpp

@@ -20,8 +20,8 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(IDBWrapperCache,
                                                 nsDOMEventTargetHelper)
   if (tmp->mScriptOwner) {
-    NS_DROP_JS_OBJECTS(tmp, IDBWrapperCache);
     tmp->mScriptOwner = nullptr;
+    NS_DROP_JS_OBJECTS(tmp, IDBWrapperCache);
   }
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
@@ -40,9 +40,9 @@ NS_IMPL_RELEASE_INHERITED(IDBWrapperCache, nsDOMEventTargetHelper)
 
 IDBWrapperCache::~IDBWrapperCache()
 {
-  if (mScriptOwner) {
-    NS_DROP_JS_OBJECTS(this, IDBWrapperCache);
-  }
+  mScriptOwner = nullptr;
+  nsContentUtils::ReleaseWrapper(this, this);
+  NS_DROP_JS_OBJECTS(this, IDBWrapperCache);
 }
 
 bool