Bug 1480121 - Remove the global stored in nsXPCWrappedJS. r=bzbarsky

Reasons for doing this:

* nsXPCWrappedJS has complicated GC behavior and we're seeing some oranges in this area.

* Due to the GC/CC complexity, the global stored in nsXPCWrappedJS *must be* the
  object's global in the root-wrapper (implies non-CCW) case. If we do that, the
  global is redundant because we can just get it from the object when we need it.

* For the CCW case, it probably doesn't matter too much which chrome global we
  use so we can use the compartment's first global - we now have an API for that.
  This may also save some memory because it avoids keeping globals alive unnecessarily
  and matches what we do for WrappedNatives and CCWs now. Furthermore, bug 1478359
  comment 12 suggests CCWs can only show up here for in-content XBL and that's in the
  process of being removed.

Differential Revision: https://phabricator.services.mozilla.com/D15096

--HG--
extra : moz-landing-system : lando

js/xpconnect/idl/nsIXPConnect.idl

@@ -71,7 +71,11 @@ interface nsIXPConnectWrappedJS : nsIXPConnectJSObjectHolder
     readonly attribute InterfaceInfoPtr InterfaceInfo;
     readonly attribute nsIIDPtr         InterfaceIID;
 
-    // Match the GetJSObject() signature.
+    // Returns the global object for our JS object. If this object is a
+    // cross-compartment wrapper, returns the compartment's first global.
+    // The global we return is guaranteed to be same-compartment with the
+    // object.
+    // Note: this matches the GetJSObject() signature.
     [notxpcom, nostdcall] JSObjectPtr GetJSObjectGlobal();
 
     void debugDump(in short depth);

js/xpconnect/src/XPCWrappedJS.cpp

@@ -68,10 +68,6 @@ bool nsXPCWrappedJS::CanSkip() {
   if (obj && JS::ObjectIsMarkedGray(obj)) {
     return false;
   }
-  JSObject* global = GetJSObjectGlobalPreserveColor();
-  if (global && JS::ObjectIsMarkedGray(global)) {
-    return false;
-  }
 
   // For non-root wrappers, check if the root wrapper will be
   // added to the CC graph.
@@ -133,8 +129,6 @@ NS_CYCLE_COLLECTION_CLASSNAME(nsXPCWrappedJS)::TraverseNative(
     MOZ_ASSERT(refcnt > 1);
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mJSObj");
     cb.NoteJSChild(JS::GCCellPtr(tmp->GetJSObjectPreserveColor()));
-    NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "mJSObjGlobal");
-    cb.NoteJSChild(JS::GCCellPtr(tmp->GetJSObjectGlobalPreserveColor()));
   }
 
   if (tmp->IsRootWrapper()) {
@@ -307,7 +301,6 @@ nsXPCWrappedJS::DeleteCycleCollectable(void) { delete this; }
 void nsXPCWrappedJS::TraceJS(JSTracer* trc) {
   MOZ_ASSERT(mRefCnt >= 2 && IsValid(), "must be strongly referenced");
   JS::TraceEdge(trc, &mJSObj, "nsXPCWrappedJS::mJSObj");
-  JS::TraceEdge(trc, &mJSObjGlobal, "nsXPCWrappedJS::mJSObjGlobal");
 }
 
 NS_IMETHODIMP
@@ -321,7 +314,14 @@ nsXPCWrappedJS::GetWeakReference(nsIWeakReference** aInstancePtr) {
 
 JSObject* nsXPCWrappedJS::GetJSObject() { return mJSObj; }
 
-JSObject* nsXPCWrappedJS::GetJSObjectGlobal() { return mJSObjGlobal; }
+JSObject* nsXPCWrappedJS::GetJSObjectGlobal() {
+  JSObject* obj = mJSObj;
+  if (js::IsCrossCompartmentWrapper(obj)) {
+    JS::Compartment* comp = js::GetObjectCompartment(obj);
+    return js::GetFirstGlobalInCompartment(comp);
+  }
+  return JS::GetNonCCWObjectGlobal(obj);
+}
 
 // static
 nsresult nsXPCWrappedJS::GetNewOrUsed(JSContext* cx, JS::HandleObject jsObj,
@@ -376,18 +376,14 @@ nsresult nsXPCWrappedJS::GetNewOrUsed(JSContext* cx, JS::HandleObject jsObj,
       return NS_ERROR_FAILURE;
     }
 
-    // Note: rootJSObj is never a CCW because GetRootJSObject unwraps. We
-    // also rely on this in nsXPCWrappedJS::UpdateObjectPointerAfterGC.
-    RootedObject global(cx, JS::GetNonCCWObjectGlobal(rootJSObj));
-    root = new nsXPCWrappedJS(cx, rootJSObj, global, rootClasp, nullptr, &rv);
+    root = new nsXPCWrappedJS(cx, rootJSObj, rootClasp, nullptr, &rv);
     if (NS_FAILED(rv)) {
       return rv;
     }
   }
 
-  RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
   RefPtr<nsXPCWrappedJS> wrapper =
-      new nsXPCWrappedJS(cx, jsObj, global, clasp, root, &rv);
+      new nsXPCWrappedJS(cx, jsObj, clasp, root, &rv);
   if (NS_FAILED(rv)) {
     return rv;
   }
@@ -396,18 +392,12 @@ nsresult nsXPCWrappedJS::GetNewOrUsed(JSContext* cx, JS::HandleObject jsObj,
 }
 
 nsXPCWrappedJS::nsXPCWrappedJS(JSContext* cx, JSObject* aJSObj,
-                               JSObject* aJSObjGlobal,
                                nsXPCWrappedJSClass* aClass,
                                nsXPCWrappedJS* root, nsresult* rv)
     : mJSObj(aJSObj),
-      mJSObjGlobal(aJSObjGlobal),
       mClass(aClass),
       mRoot(root ? root : this),
       mNext(nullptr) {
-  MOZ_ASSERT(JS_IsGlobalObject(aJSObjGlobal));
-  MOZ_RELEASE_ASSERT(js::GetObjectCompartment(aJSObj) ==
-                     js::GetObjectCompartment(aJSObjGlobal));
-
   *rv = InitStub(GetClass()->GetIID());
   // Continue even in the failure case, so that our refcounting/Destroy
   // behavior works correctly.
@@ -518,7 +508,6 @@ void nsXPCWrappedJS::Unlink() {
     }
 
     mJSObj = nullptr;
-    mJSObjGlobal = nullptr;
   }
 
   if (IsRootWrapper()) {
@@ -647,7 +636,6 @@ void nsXPCWrappedJS::SystemIsBeingShutDown() {
   // if we are not currently running an incremental GC.
   MOZ_ASSERT(!IsIncrementalGCInProgress(xpc_GetSafeJSContext()));
   *mJSObj.unsafeGet() = nullptr;
-  *mJSObjGlobal.unsafeGet() = nullptr;
 
   // Notify other wrappers in the chain.
   if (mNext) {

js/xpconnect/src/xpcprivate.h

@@ -1751,10 +1751,6 @@ class nsXPCWrappedJS final : protected nsAutoXPTCStub,
    */
   JSObject* GetJSObjectPreserveColor() const { return mJSObj.unbarrieredGet(); }
 
-  JSObject* GetJSObjectGlobalPreserveColor() const {
-    return mJSObjGlobal.unbarrieredGet();
-  }
-
   // Returns true if the wrapper chain contains references to multiple
   // compartments. If the wrapper chain contains references to multiple
   // compartments, then it must be registered on the XPCJSContext. Otherwise,
@@ -1790,11 +1786,6 @@ class nsXPCWrappedJS final : protected nsAutoXPTCStub,
   void UpdateObjectPointerAfterGC() {
     MOZ_ASSERT(IsRootWrapper());
     JS_UpdateWeakPointerAfterGC(&mJSObj);
-    JS_UpdateWeakPointerAfterGC(&mJSObjGlobal);
-    // Note: this is a root wrapper, so mJSObj is never a CCW. Therefore,
-    // if mJSObj is still alive, mJSObjGlobal must also still be alive,
-    // because marking a JSObject will also mark its global.
-    MOZ_ASSERT_IF(mJSObj, mJSObjGlobal);
   }
 
   bool IsAggregatedToNative() const { return mRoot->mOuter != nullptr; }
@@ -1817,9 +1808,8 @@ class nsXPCWrappedJS final : protected nsAutoXPTCStub,
 
  protected:
   nsXPCWrappedJS() = delete;
-  nsXPCWrappedJS(JSContext* cx, JSObject* aJSObj, JSObject* aJSObjGlobal,
-                 nsXPCWrappedJSClass* aClass, nsXPCWrappedJS* root,
-                 nsresult* rv);
+  nsXPCWrappedJS(JSContext* cx, JSObject* aJSObj, nsXPCWrappedJSClass* aClass,
+                 nsXPCWrappedJS* root, nsresult* rv);
 
   bool CanSkip();
   void Destroy();
@@ -1831,13 +1821,6 @@ class nsXPCWrappedJS final : protected nsAutoXPTCStub,
   }
 
   JS::Heap<JSObject*> mJSObj;
-  // A global object that must be same-compartment with mJSObj. This is the
-  // global/realm we enter when making calls into JS. Note that we cannot
-  // simply use mJSObj's global here because mJSObj might be a
-  // cross-compartment wrapper and CCWs are not associated with a single
-  // global. After removing in-content XBL, we no longer need this field
-  // because we can then assert against CCWs. See bug 1480121.
-  JS::Heap<JSObject*> mJSObjGlobal;
   RefPtr<nsXPCWrappedJSClass> mClass;
   nsXPCWrappedJS* mRoot;  // If mRoot != this, it is an owning pointer.
   nsXPCWrappedJS* mNext;