Bug 1480121 - Remove the global stored in nsXPCWrappedJS. r=bzbarsky, a=RyanVM
authorJan de Mooij <jdemooij@mozilla.com>
Thu, 20 Dec 2018 19:13:43 +0000
changeset 509266 ef2cd013fe73a216735629f5fab931c6ccbdd29d
parent 509265 3b399bd35f3738467209b182df03c560bf3a6b02
child 509267 c511b04377a198c6da03b32ea27e7d042e75fda1
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky, RyanVM
bugs1480121, 1478359
milestone65.0
Bug 1480121 - Remove the global stored in nsXPCWrappedJS. r=bzbarsky, a=RyanVM 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
js/src/jsfriendapi.cpp
js/src/jsfriendapi.h
js/src/vm/Compartment.h
js/xpconnect/idl/nsIXPConnect.idl
js/xpconnect/src/XPCWrappedJS.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/src/jsfriendapi.cpp
+++ b/js/src/jsfriendapi.cpp
@@ -1158,16 +1158,22 @@ JS_FRIEND_API JS::Realm* js::GetAnyRealm
     return nullptr;
   }
 
   RealmsInZoneIter realm(zone);
   MOZ_ASSERT(!realm.done());
   return realm.get();
 }
 
+JS_FRIEND_API JSObject* js::GetFirstGlobalInCompartment(JS::Compartment* comp) {
+  JSObject* global = comp->firstRealm()->maybeGlobal();
+  MOZ_ASSERT(global);
+  return global;
+}
+
 void JS::ObjectPtr::finalize(JSRuntime* rt) {
   if (IsIncrementalBarrierNeeded(rt->mainContextFromOwnThread())) {
     IncrementalPreWriteBarrier(value);
   }
   value = nullptr;
 }
 
 void JS::ObjectPtr::finalize(JSContext* cx) { finalize(cx->runtime()); }
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -527,16 +527,22 @@ extern JS_FRIEND_API bool CheckGrayMarki
 #ifdef JS_HAS_CTYPES
 extern JS_FRIEND_API size_t
 SizeOfDataIfCDataObject(mozilla::MallocSizeOf mallocSizeOf, JSObject* obj);
 #endif
 
 // Note: this returns nullptr iff |zone| is the atoms zone.
 extern JS_FRIEND_API JS::Realm* GetAnyRealmInZone(JS::Zone* zone);
 
+// Returns the first realm's global in a compartment. Note: this is not
+// guaranteed to always be the same realm because individual realms can be
+// collected by the GC.
+extern JS_FRIEND_API JSObject* GetFirstGlobalInCompartment(
+    JS::Compartment* comp);
+
 /*
  * Shadow declarations of JS internal structures, for access by inline access
  * functions below. Do not use these structures in any other way. When adding
  * new fields for access by inline methods, make sure to add static asserts to
  * the original header file to ensure that offsets are consistent.
  */
 namespace shadow {
 
--- a/js/src/vm/Compartment.h
+++ b/js/src/vm/Compartment.h
@@ -455,16 +455,18 @@ class JS::Compartment {
   }
 
   // Note: Unrestricted access to the zone's runtime from an arbitrary
   // thread can easily lead to races. Use this method very carefully.
   JSRuntime* runtimeFromAnyThread() const { return runtime_; }
 
   RealmVector& realms() { return realms_; }
 
+  Realm* firstRealm() const { return realms_[0]; }
+
   void assertNoCrossCompartmentWrappers() {
     MOZ_ASSERT(crossCompartmentWrappers.empty());
   }
 
   void addSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf,
                               size_t* compartmentObjects,
                               size_t* crossCompartmentWrappersTables,
                               size_t* compartmentsPrivateData);
--- a/js/xpconnect/idl/nsIXPConnect.idl
+++ b/js/xpconnect/idl/nsIXPConnect.idl
@@ -66,17 +66,21 @@ public:
 
 [builtinclass, uuid(3a01b0d6-074b-49ed-bac3-08c76366cae4)]
 interface nsIXPConnectWrappedJS : nsIXPConnectJSObjectHolder
 {
     /* attribute 'JSObject' inherited from 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);
 
     void aggregatedQueryInterface(in nsIIDRef uuid,
                                   [iid_is(uuid),retval] out nsQIResult result);
 
 };
--- a/js/xpconnect/src/XPCWrappedJS.cpp
+++ b/js/xpconnect/src/XPCWrappedJS.cpp
@@ -63,20 +63,16 @@ bool nsXPCWrappedJS::CanSkip() {
     return true;
   }
 
   // If this wrapper holds a gray object, need to trace it.
   JSObject* obj = GetJSObjectPreserveColor();
   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.
   if (!IsRootWrapper()) {
     // mRoot points to null after unlinking.
     NS_ENSURE_TRUE(mRoot, false);
     return mRoot->CanSkip();
   }
@@ -128,18 +124,16 @@ NS_CYCLE_COLLECTION_CLASSNAME(nsXPCWrapp
   // that is not subject to finalization alive.
   NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "self");
   cb.NoteXPCOMChild(s);
 
   if (tmp->IsValid()) {
     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()) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "aggregated native");
     cb.NoteXPCOMChild(tmp->GetAggregatedNativeObject());
   } else {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(cb, "root");
     cb.NoteXPCOMChild(ToSupports(tmp->GetRootWrapper()));
@@ -302,31 +296,37 @@ MozExternalRefCountType nsXPCWrappedJS::
 }
 
 NS_IMETHODIMP_(void)
 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
 nsXPCWrappedJS::GetWeakReference(nsIWeakReference** aInstancePtr) {
   if (!IsRootWrapper()) {
     return mRoot->GetWeakReference(aInstancePtr);
   }
 
   return nsSupportsWeakReference::GetWeakReference(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,
                                       REFNSIID aIID,
                                       nsXPCWrappedJS** wrapperResult) {
   // Do a release-mode assert against accessing nsXPCWrappedJS off-main-thread.
   MOZ_RELEASE_ASSERT(NS_IsMainThread(),
                      "nsXPCWrappedJS::GetNewOrUsed called off main thread");
@@ -371,48 +371,38 @@ nsresult nsXPCWrappedJS::GetNewOrUsed(JS
     // root wrapper, and the wrapper we are trying to make isn't
     // a root.
     RefPtr<nsXPCWrappedJSClass> rootClasp =
         nsXPCWrappedJSClass::GetNewOrUsed(cx, NS_GET_IID(nsISupports));
     if (!rootClasp) {
       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;
   }
   wrapper.forget(wrapperResult);
   return NS_OK;
 }
 
 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.
 
   // There is an extra AddRef to support weak references to wrappers
   // that are subject to finalization. See the top of the file for more
   // details.
   NS_ADDREF_THIS();
@@ -513,17 +503,16 @@ void nsXPCWrappedJS::Unlink() {
       }
 
       if (mRefCnt > 1) {
         RemoveFromRootSet();
       }
     }
 
     mJSObj = nullptr;
-    mJSObjGlobal = nullptr;
   }
 
   if (IsRootWrapper()) {
     ClearWeakReferences();
   } else if (mRoot) {
     // unlink this wrapper
     nsXPCWrappedJS* cur = mRoot;
     while (1) {
@@ -642,17 +631,16 @@ void nsXPCWrappedJS::SystemIsBeingShutDo
   // work (and avoid crashing some platforms).
 
   // Clear the contents of the pointer using unsafeGet() to avoid
   // triggering post barriers in shutdown, as this will access the chunk
   // containing mJSObj, which may have been freed at this point. This is safe
   // 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) {
     mNext->SystemIsBeingShutDown();
   }
 }
 
 size_t nsXPCWrappedJS::SizeOfIncludingThis(
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1748,20 +1748,16 @@ class nsXPCWrappedJS final : protected n
    * object returned is not guaranteed to be kept alive past the next CC.
    *
    * This should only be called if you are certain that the return value won't
    * be passed into a JS API function and that it won't be stored without
    * being rooted (or otherwise signaling the stored value to the CC).
    */
   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,
   // it should be registered in the CompartmentPrivate for the compartment of
   // the root's JS object. This will only return correct results when called
   // on the root wrapper and will assert if not called on a root wrapper.
   bool IsMultiCompartment() const;
 
@@ -1787,21 +1783,16 @@ class nsXPCWrappedJS final : protected n
   // These two methods are used by JSObject2WrappedJSMap::FindDyingJSObjects
   // to find non-rooting wrappers for dying JS objects. See the top of
   // XPCWrappedJS.cpp for more details.
   bool IsSubjectToFinalization() const { return IsValid() && mRefCnt == 1; }
 
   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; }
   nsISupports* GetAggregatedNativeObject() const { return mRoot->mOuter; }
   void SetAggregatedNativeObject(nsISupports* aNative) {
     MOZ_ASSERT(aNative);
     if (mRoot->mOuter) {
       MOZ_ASSERT(mRoot->mOuter == aNative,
@@ -1814,37 +1805,29 @@ class nsXPCWrappedJS final : protected n
   void TraceJS(JSTracer* trc);
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
   virtual ~nsXPCWrappedJS();
 
  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();
   void Unlink();
 
  private:
   JS::Compartment* Compartment() const {
     return js::GetObjectCompartment(mJSObj.unbarrieredGet());
   }
 
   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;
   nsCOMPtr<nsISupports> mOuter;  // only set in root
 };
 
 /***************************************************************************
 ****************************************************************************