From b627ef067229a63b51ae6abb992a1de50453eafe Mon Sep 17 00:00:00 2001
From: Bobby Holley <bobbyholley@gmail.com>
Date: Mon, 3 Mar 2014 08:53:42 -0800
Subject: [PATCH] Bug 975419 - Trace the Incumbent Global from a CallbackObject
 (but check it too, just to be safe). r=bz,mccr8

---
 dom/bindings/CallbackObject.cpp | 16 +++++++++++++---
 dom/bindings/CallbackObject.h   | 25 +++++++++++++++++++------
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/dom/bindings/CallbackObject.cpp b/dom/bindings/CallbackObject.cpp
index a5057f9860a..4884fe93615 100644
--- a/dom/bindings/CallbackObject.cpp
+++ b/dom/bindings/CallbackObject.cpp
@@ -35,7 +35,7 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(CallbackObject)
 NS_IMPL_CYCLE_COLLECTION_CLASS(CallbackObject)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(CallbackObject)
-  tmp->DropCallback();
+  tmp->DropJSObjects();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mIncumbentGlobal)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(CallbackObject)
@@ -44,6 +44,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(CallbackObject)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(CallbackObject)
   NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mCallback)
+  NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mIncumbentJSGlobal)
 NS_IMPL_CYCLE_COLLECTION_TRACE_END
 
 CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
@@ -122,8 +123,17 @@ CallbackObject::CallSetup::CallSetup(CallbackObject* aCallback,
 
     mAutoEntryScript.construct(globalObject, mIsMainThread, cx);
     mAutoEntryScript.ref().SetWebIDLCallerPrincipal(webIDLCallerPrincipal);
-    if (aCallback->IncumbentGlobalOrNull()) {
-      mAutoIncumbentScript.construct(aCallback->IncumbentGlobalOrNull());
+    nsIGlobalObject* incumbent = aCallback->IncumbentGlobalOrNull();
+    if (incumbent) {
+      // The callback object traces its incumbent JS global, so in general it
+      // should be alive here. However, it's possible that we could run afoul
+      // of the same IPC global weirdness described above, wherein the
+      // nsIGlobalObject has severed its reference to the JS global. Let's just
+      // be safe here, so that nobody has to waste a day debugging gaia-ui tests.
+      if (!incumbent->GetGlobalJSObject()) {
+        return;
+      }
+      mAutoIncumbentScript.construct(incumbent);
     }
 
     // Unmark the callable (by invoking Callback() and not the CallbackPreserveColor()
diff --git a/dom/bindings/CallbackObject.h b/dom/bindings/CallbackObject.h
index 9465094b453..2663cc65613 100644
--- a/dom/bindings/CallbackObject.h
+++ b/dom/bindings/CallbackObject.h
@@ -57,7 +57,7 @@ public:
 
   virtual ~CallbackObject()
   {
-    DropCallback();
+    DropJSObjects();
   }
 
   JS::Handle<JSObject*> Callback() const
@@ -121,28 +121,41 @@ private:
   inline void Init(JSObject* aCallback, nsIGlobalObject* aIncumbentGlobal)
   {
     MOZ_ASSERT(aCallback && !mCallback);
-    // Set mCallback before we hold, on the off chance that a GC could somehow
-    // happen in there... (which would be pretty odd, granted).
+    // Set script objects before we hold, on the off chance that a GC could
+    // somehow happen in there... (which would be pretty odd, granted).
     mCallback = aCallback;
+    if (aIncumbentGlobal) {
+      mIncumbentGlobal = aIncumbentGlobal;
+      mIncumbentJSGlobal = aIncumbentGlobal->GetGlobalJSObject();
+    }
     mozilla::HoldJSObjects(this);
-
-    mIncumbentGlobal = aIncumbentGlobal;
   }
 
   CallbackObject(const CallbackObject&) MOZ_DELETE;
   CallbackObject& operator =(const CallbackObject&) MOZ_DELETE;
 
 protected:
-  void DropCallback()
+  void DropJSObjects()
   {
+    MOZ_ASSERT_IF(mIncumbentJSGlobal, mCallback);
     if (mCallback) {
       mCallback = nullptr;
+      mIncumbentJSGlobal = nullptr;
       mozilla::DropJSObjects(this);
     }
   }
 
   JS::Heap<JSObject*> mCallback;
+  // Ideally, we'd just hold a reference to the nsIGlobalObject, since that's
+  // what we need to pass to AutoIncumbentScript. Unfortunately, that doesn't
+  // hold the actual JS global alive. So we maintain an additional pointer to
+  // the JS global itself so that we can trace it.
+  //
+  // At some point we should consider trying to make native globals hold their
+  // scripted global alive, at which point we can get rid of the duplication
+  // here.
   nsCOMPtr<nsIGlobalObject> mIncumbentGlobal;
+  JS::TenuredHeap<JSObject*> mIncumbentJSGlobal;
 
   class MOZ_STACK_CLASS CallSetup
   {