Bug 1288909, part 1 - Implement refcounting of XPCNativeSet. r=billm
authorAndrew McCreight <continuation@gmail.com>
Wed, 27 Jul 2016 16:38:30 -0700
changeset 314425 9769cc42f54995f1d087200259ad3464be881a2c
parent 314424 ff28b15107631c762ed5db714c8310260343bd27
child 314426 8dd979e76e1c4a73690a7f5636c769c51a30c0b4
push id20574
push usercbook@mozilla.com
push dateTue, 20 Sep 2016 10:05:16 +0000
treeherderfx-team@14705f779a46 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1288909, 1288870, 1290239
milestone52.0a1
Bug 1288909, part 1 - Implement refcounting of XPCNativeSet. r=billm This patch is similar to bug 1288870. Strong references: - XPCCallContext::mSet: Like XPCNativeInterface, this only roots it when |mState >= HAVE_NAME|, and again this only requires changing SystemIsBeingShutDown(). - XPCWrappedNativeProto::mSet and XPCWrappedNative::mSet. These become RefPtrs. - stack: AutoMarkingNativeSetPtr become RefPtr<XPCNativeSet>. This lets me eliminate some uses of AutoJSContext. This is the bulk of the patch. Weak references: - mNativeSetMap. This reference gets cleared in the dtor. This requires bug 1290239 to actually find the entry for removal. - mClassInfo2NativeSetMap. The reference is in the value for this hash table, and we don't have the key in the set dtor. Fortunately, the only code that adds to this table is XPCNativeSet::GetNewOrUsed(nsIClassInfo* classInfo), which in turn is only called by GetNewOrUsed(nsIClassInfo* classInfo). This code creates a new XPCWrappedNativeProto, which (with my patch) holds a strong reference to the set that has been added to the table. This set is never changed or released until the dtor for the proto, which calls ClearCacheEntryForClassInfo(), removing the entry from the hashtable. Thus, the lifetime of the set is always going to be longer than the lifetime of the entry. Other notes: - Like XPCNativeInterface, this class uses placement |new| that requires a special destruction function, which with my patch is hidden away in the refcounting code. - This patch delete a bunch of marking/sweeping code from XPCJSRuntime::FinalizeCallback(), because the lifetimes are managed by the refcounting now. Some of the marking code is left behind to be cleaned up in a later patch. - I didn't see any methods that had XPCNativeSet** outparams. - MOZ_COUNT_{CTOR,DTOR}(XPCNativeSet) is not needed because it is now refcounted. MozReview-Commit-ID: 7oTorCwda1n
js/xpconnect/src/XPCCallContext.cpp
js/xpconnect/src/XPCJSContext.cpp
js/xpconnect/src/XPCMaps.h
js/xpconnect/src/XPCWrappedNative.cpp
js/xpconnect/src/XPCWrappedNativeInfo.cpp
js/xpconnect/src/XPCWrappedNativeProto.cpp
js/xpconnect/src/xpcprivate.h
js/xpconnect/wrappers/WrapperFactory.cpp
--- a/js/xpconnect/src/XPCCallContext.cpp
+++ b/js/xpconnect/src/XPCCallContext.cpp
@@ -194,16 +194,17 @@ void
 XPCCallContext::SystemIsBeingShutDown()
 {
     // XXX This is pretty questionable since the per thread cleanup stuff
     // can be making this call on one thread for call contexts on another
     // thread.
     NS_WARNING("Shutting Down XPConnect even through there is a live XPCCallContext");
     mXPCJSContext = nullptr;
     mState = SYSTEM_SHUTDOWN;
+    mSet = nullptr;
     mInterface = nullptr;
 
     if (mPrevCallContext)
         mPrevCallContext->SystemIsBeingShutDown();
 }
 
 XPCCallContext::~XPCCallContext()
 {
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -815,47 +815,16 @@ XPCJSContext::FinalizeCallback(JSFreeOp*
                         XPCNativeSet* set = ccxp->GetSet();
                         if (set)
                             set->Mark();
                     }
                     ccxp = ccxp->GetPrevCallContext();
                 }
             }
 
-            // Do the sweeping. During a zone GC, only WrappedNativeProtos in
-            // collected zones will be marked. Therefore, some reachable
-            // NativeInterfaces will not be marked, so it is not safe to sweep
-            // them. We still need to unmark them, since the ones pointed to by
-            // WrappedNativeProtos in a zone being collected will be marked.
-            //
-            // Ideally, if NativeInterfaces from different zones were kept
-            // separate, we could sweep only the ones belonging to zones being
-            // collected. Currently, though, NativeInterfaces are shared between
-            // zones. This ought to be fixed.
-            bool doSweep = !isZoneGC;
-
-            if (doSweep) {
-                for (auto i = self->mClassInfo2NativeSetMap->Iter(); !i.Done(); i.Next()) {
-                    auto entry = static_cast<ClassInfo2NativeSetMap::Entry*>(i.Get());
-                    if (!entry->value->IsMarked())
-                        i.Remove();
-                }
-            }
-
-            for (auto i = self->mNativeSetMap->Iter(); !i.Done(); i.Next()) {
-                auto entry = static_cast<NativeSetMap::Entry*>(i.Get());
-                XPCNativeSet* set = entry->key_value;
-                if (set->IsMarked()) {
-                    set->Unmark();
-                } else if (doSweep) {
-                    XPCNativeSet::DestroyInstance(set);
-                    i.Remove();
-                }
-            }
-
 #ifdef DEBUG
             XPCWrappedNativeScope::ASSERT_NoInterfaceSetsAreMarked();
 #endif
 
             // Now we are going to recycle any unused WrappedNativeTearoffs.
             // We do this by iterating all the live callcontexts
             // and marking the tearoffs in use. And then we
             // iterate over all the WrappedNative wrappers and sweep their
--- a/js/xpconnect/src/XPCMaps.h
+++ b/js/xpconnect/src/XPCMaps.h
@@ -305,18 +305,16 @@ public:
     inline void Remove(nsIClassInfo* info)
     {
         NS_PRECONDITION(info,"bad param");
         mTable.Remove(info);
     }
 
     inline uint32_t Count() { return mTable.EntryCount(); }
 
-    PLDHashTable::Iterator Iter() { return mTable.Iter(); }
-
     // ClassInfo2NativeSetMap holds pointers to *some* XPCNativeSets.
     // So we don't want to count those XPCNativeSets, because they are better
     // counted elsewhere (i.e. in XPCJSContext::mNativeSetMap, which holds
     // pointers to *all* XPCNativeSets).  Hence the "Shallow".
     size_t ShallowSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
 private:
     ClassInfo2NativeSetMap();    // no implementation
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -426,25 +426,25 @@ XPCWrappedNative::GetNewOrUsed(xpcObject
             return NS_ERROR_FAILURE;
 
         wrapper = new XPCWrappedNative(helper.forgetCanonical(), proto);
     } else {
         RefPtr<XPCNativeInterface> iface = Interface;
         if (!iface)
             iface = XPCNativeInterface::GetISupports();
 
-        AutoMarkingNativeSetPtr set(cx);
         XPCNativeSetKey key(iface);
-        set = XPCNativeSet::GetNewOrUsed(&key);
+        RefPtr<XPCNativeSet> set =
+            XPCNativeSet::GetNewOrUsed(&key);
 
         if (!set)
             return NS_ERROR_FAILURE;
 
-        wrapper =
-            new XPCWrappedNative(helper.forgetCanonical(), Scope, set);
+        wrapper = new XPCWrappedNative(helper.forgetCanonical(), Scope,
+                                       set.forget());
     }
 
     MOZ_ASSERT(!xpc::WrapperFactory::IsXrayWrapper(parent),
                "Xray wrapper being used to parent XPCWrappedNative?");
 
     // We use an AutoMarkingPtr here because it is possible for JS gc to happen
     // after we have Init'd the wrapper but *before* we add it to the hashtable.
     // This would cause the mSet to get collected and we'd later crash. I've
@@ -561,29 +561,29 @@ XPCWrappedNative::XPCWrappedNative(alrea
 
     MOZ_ASSERT(mMaybeProto, "bad ctor param");
     MOZ_ASSERT(mSet, "bad ctor param");
 }
 
 // This ctor is used if this object will NOT have a proto.
 XPCWrappedNative::XPCWrappedNative(already_AddRefed<nsISupports>&& aIdentity,
                                    XPCWrappedNativeScope* aScope,
-                                   XPCNativeSet* aSet)
+                                   already_AddRefed<XPCNativeSet>&& aSet)
 
     : mMaybeScope(TagScope(aScope)),
       mSet(aSet),
       mScriptableInfo(nullptr)
 {
     MOZ_ASSERT(NS_IsMainThread());
 
     mIdentity = aIdentity;
     mFlatJSObject.setFlags(FLAT_JS_OBJECT_VALID);
 
     MOZ_ASSERT(aScope, "bad ctor param");
-    MOZ_ASSERT(aSet, "bad ctor param");
+    MOZ_ASSERT(mSet, "bad ctor param");
 }
 
 XPCWrappedNative::~XPCWrappedNative()
 {
     Destroy();
 }
 
 void
@@ -1009,26 +1009,24 @@ public:
 private:
     RootedObject mOldReflector;
     RootedObject mNewReflector;
 };
 
 bool
 XPCWrappedNative::ExtendSet(XPCNativeInterface* aInterface)
 {
-    AutoJSContext cx;
-
     if (!mSet->HasInterface(aInterface)) {
-        AutoMarkingNativeSetPtr newSet(cx);
         XPCNativeSetKey key(mSet, aInterface);
-        newSet = XPCNativeSet::GetNewOrUsed(&key);
+        RefPtr<XPCNativeSet> newSet =
+            XPCNativeSet::GetNewOrUsed(&key);
         if (!newSet)
             return false;
 
-        mSet = newSet;
+        mSet = newSet.forget();
     }
     return true;
 }
 
 XPCWrappedNativeTearOff*
 XPCWrappedNative::FindTearOff(XPCNativeInterface* aInterface,
                               bool needJSObject /* = false */,
                               nsresult* pError /* = nullptr */)
@@ -2159,17 +2157,17 @@ NS_IMETHODIMP XPCWrappedNative::DebugDum
             else
                 XPC_LOG_ALWAYS(("mMaybeProto @ %x", proto));
         } else
             XPC_LOG_ALWAYS(("Scope @ %x", GetScope()));
 
         if (depth && mSet)
             mSet->DebugDump(depth);
         else
-            XPC_LOG_ALWAYS(("mSet @ %x", mSet));
+            XPC_LOG_ALWAYS(("mSet @ %x", mSet.get()));
 
         XPC_LOG_ALWAYS(("mFlatJSObject of %x", mFlatJSObject.getPtr()));
         XPC_LOG_ALWAYS(("mIdentity of %x", mIdentity.get()));
         XPC_LOG_ALWAYS(("mScriptableInfo @ %x", mScriptableInfo));
 
         if (depth && mScriptableInfo) {
             XPC_LOG_INDENT();
             XPC_LOG_ALWAYS(("mScriptable @ %x", mScriptableInfo->GetCallback()));
--- a/js/xpconnect/src/XPCWrappedNativeInfo.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeInfo.cpp
@@ -457,69 +457,73 @@ XPCNativeSetKey::Hash() const
     }
 
     return h;
 }
 
 /***************************************************************************/
 // XPCNativeSet
 
+XPCNativeSet::~XPCNativeSet()
+{
+    // Remove |this| before we clear the interfaces to ensure that the
+    // hashtable look up is correct.
+    XPCJSContext::Get()->GetNativeSetMap()->Remove(this);
+
+    for (int i = 0; i < mInterfaceCount; i++) {
+        NS_RELEASE(mInterfaces[i]);
+    }
+}
+
 // static
-XPCNativeSet*
+already_AddRefed<XPCNativeSet>
 XPCNativeSet::GetNewOrUsed(const nsIID* iid)
 {
-    AutoJSContext cx;
-    AutoMarkingNativeSetPtr set(cx);
-
     RefPtr<XPCNativeInterface> iface =
         XPCNativeInterface::GetNewOrUsed(iid);
     if (!iface)
         return nullptr;
 
     XPCNativeSetKey key(iface);
 
     XPCJSContext* xpccx = XPCJSContext::Get();
     NativeSetMap* map = xpccx->GetNativeSetMap();
     if (!map)
         return nullptr;
 
-    set = map->Find(&key);
+    RefPtr<XPCNativeSet> set = map->Find(&key);
 
     if (set)
-        return set;
+        return set.forget();
 
     set = NewInstance({iface.forget()});
     if (!set)
         return nullptr;
 
     if (!map->AddNew(&key, set)) {
         NS_ERROR("failed to add our set!");
-        DestroyInstance(set);
         set = nullptr;
     }
 
-    return set;
+    return set.forget();
 }
 
 // static
-XPCNativeSet*
+already_AddRefed<XPCNativeSet>
 XPCNativeSet::GetNewOrUsed(nsIClassInfo* classInfo)
 {
-    AutoJSContext cx;
-    AutoMarkingNativeSetPtr set(cx);
     XPCJSContext* xpccx = XPCJSContext::Get();
-
     ClassInfo2NativeSetMap* map = xpccx->GetClassInfo2NativeSetMap();
     if (!map)
         return nullptr;
 
-    set = map->Find(classInfo);
+    RefPtr<XPCNativeSet> set = map->Find(classInfo);
 
     if (set)
-        return set;
+        return set.forget();
 
     nsIID** iidArray = nullptr;
     uint32_t iidCount = 0;
 
     if (NS_FAILED(classInfo->GetInterfaces(&iidCount, &iidArray))) {
         // Note: I'm making it OK for this call to fail so that one can add
         // nsIClassInfo to classes implemented in script without requiring this
         // method to be implemented.
@@ -562,24 +566,22 @@ XPCNativeSet::GetNewOrUsed(nsIClassInfo*
                 if (!map2)
                     goto out;
 
                 XPCNativeSetKey key(set);
 
                 XPCNativeSet* set2 = map2->Add(&key, set);
                 if (!set2) {
                     NS_ERROR("failed to add our set!");
-                    DestroyInstance(set);
                     set = nullptr;
                     goto out;
                 }
                 // It is okay to find an existing entry here because
                 // we did not look for one before we called Add().
                 if (set2 != set) {
-                    DestroyInstance(set);
                     set = set2;
                 }
             }
         } else
             set = GetNewOrUsed(&NS_GET_IID(nsISupports));
     } else
         set = GetNewOrUsed(&NS_GET_IID(nsISupports));
 
@@ -591,111 +593,107 @@ XPCNativeSet::GetNewOrUsed(nsIClassInfo*
         MOZ_ASSERT(set2, "failed to add our set!");
         MOZ_ASSERT(set2 == set, "hashtables inconsistent!");
     }
 
 out:
     if (iidArray)
         NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY(iidCount, iidArray);
 
-    return set;
+    return set.forget();
 }
 
 // static
 void
 XPCNativeSet::ClearCacheEntryForClassInfo(nsIClassInfo* classInfo)
 {
     XPCJSContext* xpccx = nsXPConnect::GetContextInstance();
     ClassInfo2NativeSetMap* map = xpccx->GetClassInfo2NativeSetMap();
     if (map)
         map->Remove(classInfo);
 }
 
 // static
-XPCNativeSet*
+already_AddRefed<XPCNativeSet>
 XPCNativeSet::GetNewOrUsed(XPCNativeSetKey* key)
 {
-    AutoJSContext cx;
-    AutoMarkingNativeSetPtr set(cx);
-    XPCJSContext* xpccx = XPCJSContext::Get();
-    NativeSetMap* map = xpccx->GetNativeSetMap();
+    NativeSetMap* map = XPCJSContext::Get()->GetNativeSetMap();
     if (!map)
         return nullptr;
 
-    set = map->Find(key);
+    RefPtr<XPCNativeSet> set = map->Find(key);
 
     if (set)
-        return set;
+        return set.forget();
 
     if (key->GetBaseSet())
         set = NewInstanceMutate(key);
     else
         set = NewInstance({key->GetAddition()});
 
     if (!set)
         return nullptr;
 
     if (!map->AddNew(key, set)) {
         NS_ERROR("failed to add our set!");
-        DestroyInstance(set);
         set = nullptr;
     }
 
-    return set;
+    return set.forget();
 }
 
 // static
-XPCNativeSet*
+already_AddRefed<XPCNativeSet>
 XPCNativeSet::GetNewOrUsed(XPCNativeSet* firstSet,
                            XPCNativeSet* secondSet,
                            bool preserveFirstSetOrder)
 {
     // Figure out how many interfaces we'll need in the new set.
     uint32_t uniqueCount = firstSet->mInterfaceCount;
     for (uint32_t i = 0; i < secondSet->mInterfaceCount; ++i) {
         if (!firstSet->HasInterface(secondSet->mInterfaces[i]))
             uniqueCount++;
     }
 
     // If everything in secondSet was a duplicate, we can just use the first
     // set.
     if (uniqueCount == firstSet->mInterfaceCount)
-        return firstSet;
+        return RefPtr<XPCNativeSet>(firstSet).forget();
 
     // If the secondSet is just a superset of the first, we can use it provided
     // that the caller doesn't care about ordering.
     if (!preserveFirstSetOrder && uniqueCount == secondSet->mInterfaceCount)
-        return secondSet;
+        return RefPtr<XPCNativeSet>(secondSet).forget();
 
     // Ok, darn. Now we have to make a new set.
     //
     // It would be faster to just create the new set all at once, but that
     // would involve wrangling with some pretty hairy code - especially since
     // a lot of stuff assumes that sets are created by adding one interface to an
     // existing set. So let's just do the slow and easy thing and hope that the
     // above optimizations handle the common cases.
-    XPCNativeSet* currentSet = firstSet;
+    RefPtr<XPCNativeSet> currentSet = firstSet;
     for (uint32_t i = 0; i < secondSet->mInterfaceCount; ++i) {
         XPCNativeInterface* iface = secondSet->mInterfaces[i];
         if (!currentSet->HasInterface(iface)) {
             // Create a new augmented set, inserting this interface at the end.
             XPCNativeSetKey key(currentSet, iface);
             currentSet = XPCNativeSet::GetNewOrUsed(&key);
             if (!currentSet)
                 return nullptr;
         }
     }
 
     // We've got the union set. Hand it back to the caller.
     MOZ_ASSERT(currentSet->mInterfaceCount == uniqueCount);
-    return currentSet;
+    return currentSet.forget();
 }
 
 // static
-XPCNativeSet*
+already_AddRefed<XPCNativeSet>
 XPCNativeSet::NewInstance(nsTArray<RefPtr<XPCNativeInterface>>&& array)
 {
     if (array.Length() == 0)
         return nullptr;
 
     // We impose the invariant:
     // "All sets have exactly one nsISupports interface and it comes first."
     // This is the place where we impose that rule - even if given inputs
@@ -710,17 +708,17 @@ XPCNativeSet::NewInstance(nsTArray<RefPt
     }
 
     // Use placement new to create an object with the right amount of space
     // to hold the members array
     int size = sizeof(XPCNativeSet);
     if (slots > 1)
         size += (slots - 1) * sizeof(XPCNativeInterface*);
     void* place = new char[size];
-    XPCNativeSet* obj = new(place) XPCNativeSet();
+    RefPtr<XPCNativeSet> obj = new(place) XPCNativeSet();
 
     // Stick the nsISupports in front and skip additional nsISupport(s)
     XPCNativeInterface** outp = (XPCNativeInterface**) &obj->mInterfaces;
     uint16_t memberCount = 1;   // for the one member in nsISupports
 
     NS_ADDREF(*(outp++) = isup);
 
     for (auto key = array.begin(); key != array.end(); key++) {
@@ -728,50 +726,50 @@ XPCNativeSet::NewInstance(nsTArray<RefPt
         if (isup == cur)
             continue;
         memberCount += cur->GetMemberCount();
         *(outp++) = cur.forget().take();
     }
     obj->mMemberCount = memberCount;
     obj->mInterfaceCount = slots;
 
-    return obj;
+    return obj.forget();
 }
 
 // static
-XPCNativeSet*
+already_AddRefed<XPCNativeSet>
 XPCNativeSet::NewInstanceMutate(XPCNativeSetKey* key)
 {
     XPCNativeSet* otherSet = key->GetBaseSet();
     XPCNativeInterface* newInterface = key->GetAddition();
 
     MOZ_ASSERT(otherSet);
 
     if (!newInterface)
         return nullptr;
 
     // Use placement new to create an object with the right amount of space
     // to hold the members array
     int size = sizeof(XPCNativeSet);
     size += otherSet->mInterfaceCount * sizeof(XPCNativeInterface*);
     void* place = new char[size];
-    XPCNativeSet* obj = new(place) XPCNativeSet();
+    RefPtr<XPCNativeSet> obj = new(place) XPCNativeSet();
 
     obj->mMemberCount = otherSet->GetMemberCount() +
         newInterface->GetMemberCount();
     obj->mInterfaceCount = otherSet->mInterfaceCount + 1;
 
     XPCNativeInterface** src = otherSet->mInterfaces;
     XPCNativeInterface** dest = obj->mInterfaces;
     for (uint16_t i = 0; i < otherSet->mInterfaceCount; i++) {
         NS_ADDREF(*dest++ = *src++);
     }
     NS_ADDREF(*dest++ = newInterface);
 
-    return obj;
+    return obj.forget();
 }
 
 // static
 void
 XPCNativeSet::DestroyInstance(XPCNativeSet* inst)
 {
     inst->~XPCNativeSet();
     delete [] (char*) inst;
--- a/js/xpconnect/src/XPCWrappedNativeProto.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeProto.cpp
@@ -12,17 +12,17 @@
 using namespace mozilla;
 
 #ifdef DEBUG
 int32_t XPCWrappedNativeProto::gDEBUG_LiveProtoCount = 0;
 #endif
 
 XPCWrappedNativeProto::XPCWrappedNativeProto(XPCWrappedNativeScope* Scope,
                                              nsIClassInfo* ClassInfo,
-                                             XPCNativeSet* Set)
+                                             already_AddRefed<XPCNativeSet>&& Set)
     : mScope(Scope),
       mJSProtoObject(nullptr),
       mClassInfo(ClassInfo),
       mSet(Set),
       mScriptableInfo(nullptr)
 {
     // This native object lives as long as its associated JSObject - killed
     // by finalization of the JSObject (or explicitly if Init fails).
@@ -161,22 +161,21 @@ XPCWrappedNativeProto::GetNewOrUsed(XPCW
     AutoMarkingWrappedNativeProtoPtr proto(cx);
     ClassInfo2WrappedNativeProtoMap* map = nullptr;
 
     map = scope->GetWrappedNativeProtoMap();
     proto = map->Find(classInfo);
     if (proto)
         return proto;
 
-    AutoMarkingNativeSetPtr set(cx);
-    set = XPCNativeSet::GetNewOrUsed(classInfo);
+    RefPtr<XPCNativeSet> set = XPCNativeSet::GetNewOrUsed(classInfo);
     if (!set)
         return nullptr;
 
-    proto = new XPCWrappedNativeProto(scope, classInfo, set);
+    proto = new XPCWrappedNativeProto(scope, classInfo, set.forget());
 
     if (!proto || !proto->Init(scriptableCreateInfo, callPostCreatePrototype)) {
         delete proto.get();
         return nullptr;
     }
 
     map->Add(classInfo, proto);
 
@@ -188,17 +187,17 @@ XPCWrappedNativeProto::DebugDump(int16_t
 {
 #ifdef DEBUG
     depth-- ;
     XPC_LOG_ALWAYS(("XPCWrappedNativeProto @ %x", this));
     XPC_LOG_INDENT();
         XPC_LOG_ALWAYS(("gDEBUG_LiveProtoCount is %d", gDEBUG_LiveProtoCount));
         XPC_LOG_ALWAYS(("mScope @ %x", mScope));
         XPC_LOG_ALWAYS(("mJSProtoObject @ %x", mJSProtoObject.get()));
-        XPC_LOG_ALWAYS(("mSet @ %x", mSet));
+        XPC_LOG_ALWAYS(("mSet @ %x", mSet.get()));
         XPC_LOG_ALWAYS(("mScriptableInfo @ %x", mScriptableInfo));
         if (depth && mScriptableInfo) {
             XPC_LOG_INDENT();
             XPC_LOG_ALWAYS(("mScriptable @ %x", mScriptableInfo->GetCallback()));
             XPC_LOG_ALWAYS(("mFlags of %x", (uint32_t)mScriptableInfo->GetFlags()));
             XPC_LOG_ALWAYS(("mJSClass @ %x", mScriptableInfo->GetJSClass()));
             XPC_LOG_OUTDENT();
         }
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -788,17 +788,17 @@ private:
 
     XPCCallContext*                 mPrevCallContext;
 
     XPCWrappedNative*               mWrapper;
     XPCWrappedNativeTearOff*        mTearOff;
 
     XPCNativeScriptableInfo*        mScriptableInfo;
 
-    XPCNativeSet*                   mSet;
+    RefPtr<XPCNativeSet>            mSet;
     RefPtr<XPCNativeInterface>      mInterface;
     XPCNativeMember*                mMember;
 
     JS::RootedId                    mName;
     bool                            mStaticMemberIsLocal;
 
     unsigned                        mArgc;
     JS::Value*                      mArgv;
@@ -1276,17 +1276,17 @@ private:
     XPCNativeMember            mMembers[1]; // always last - object sized for array
 };
 
 /***************************************************************************/
 // XPCNativeSetKey is used to key a XPCNativeSet in a NativeSetMap.
 // It represents a new XPCNativeSet we are considering constructing, without
 // requiring that the set actually be built.
 
-class XPCNativeSetKey final
+class MOZ_STACK_CLASS XPCNativeSetKey final
 {
 public:
     // This represents an existing set |baseSet|.
     explicit XPCNativeSetKey(XPCNativeSet* baseSet)
         : mBaseSet(baseSet), mAddition(nullptr)
     {
         MOZ_ASSERT(baseSet);
     }
@@ -1309,40 +1309,43 @@ public:
     XPCNativeSet* GetBaseSet() const {return mBaseSet;}
     XPCNativeInterface* GetAddition() const {return mAddition;}
 
     PLDHashNumber Hash() const;
 
     // Allow shallow copy
 
 private:
-    XPCNativeSet* mBaseSet;
-    XPCNativeInterface* mAddition;
+    RefPtr<XPCNativeSet> mBaseSet;
+    RefPtr<XPCNativeInterface> mAddition;
 };
 
 /***************************************************************************/
 // XPCNativeSet represents an ordered collection of XPCNativeInterface pointers.
 
 class XPCNativeSet final
 {
   public:
-    static XPCNativeSet* GetNewOrUsed(const nsIID* iid);
-    static XPCNativeSet* GetNewOrUsed(nsIClassInfo* classInfo);
-    static XPCNativeSet* GetNewOrUsed(XPCNativeSetKey* key);
+    NS_INLINE_DECL_REFCOUNTING_WITH_DESTROY(XPCNativeSet,
+                                            DestroyInstance(this))
+
+    static already_AddRefed<XPCNativeSet> GetNewOrUsed(const nsIID* iid);
+    static already_AddRefed<XPCNativeSet> GetNewOrUsed(nsIClassInfo* classInfo);
+    static already_AddRefed<XPCNativeSet> GetNewOrUsed(XPCNativeSetKey* key);
 
     // This generates a union set.
     //
     // If preserveFirstSetOrder is true, the elements from |firstSet| come first,
     // followed by any non-duplicate items from |secondSet|. If false, the same
     // algorithm is applied; but if we detect that |secondSet| is a superset of
     // |firstSet|, we return |secondSet| without worrying about whether the
     // ordering might differ from |firstSet|.
-    static XPCNativeSet* GetNewOrUsed(XPCNativeSet* firstSet,
-                                      XPCNativeSet* secondSet,
-                                      bool preserveFirstSetOrder);
+    static already_AddRefed<XPCNativeSet> GetNewOrUsed(XPCNativeSet* firstSet,
+                                                       XPCNativeSet* secondSet,
+                                                       bool preserveFirstSetOrder);
 
     static void ClearCacheEntryForClassInfo(nsIClassInfo* classInfo);
 
     inline bool FindMember(jsid name, XPCNativeMember** pMember,
                            uint16_t* pInterfaceIndex) const;
 
     inline bool FindMember(jsid name, XPCNativeMember** pMember,
                            RefPtr<XPCNativeInterface>* pInterface) const;
@@ -1383,45 +1386,39 @@ class XPCNativeSet final
     inline void TraceJS(JSTracer* trc) {}
     inline void AutoTrace(JSTracer* trc) {}
 
   public:
     void Unmark() {
         mMarked = 0;
     }
     bool IsMarked() const {
-        return !!mMarked;
+        return false;
     }
 
 #ifdef DEBUG
     inline void ASSERT_NotMarked();
 #endif
 
     void DebugDump(int16_t depth);
 
-    static void DestroyInstance(XPCNativeSet* inst);
-
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
   protected:
-    static XPCNativeSet* NewInstance(nsTArray<RefPtr<XPCNativeInterface>>&& array);
-    static XPCNativeSet* NewInstanceMutate(XPCNativeSetKey* key);
+    static already_AddRefed<XPCNativeSet> NewInstance(nsTArray<RefPtr<XPCNativeInterface>>&& array);
+    static already_AddRefed<XPCNativeSet> NewInstanceMutate(XPCNativeSetKey* key);
+
     XPCNativeSet()
       : mMemberCount(0), mInterfaceCount(0), mMarked(0)
-    {
-        MOZ_COUNT_CTOR(XPCNativeSet);
-    }
-    ~XPCNativeSet() {
-        for (int i = 0; i < mInterfaceCount; i++) {
-            NS_RELEASE(mInterfaces[i]);
-        }
-        MOZ_COUNT_DTOR(XPCNativeSet);
-    }
+    {}
+    ~XPCNativeSet();
     void* operator new(size_t, void* p) CPP_THROW_NEW {return p;}
 
+    static void DestroyInstance(XPCNativeSet* inst);
+
   private:
     uint16_t                mMemberCount;
     uint16_t                mInterfaceCount : 15;
     uint16_t                mMarked : 1;
     // Always last - object sized for array.
     // These are strong references.
     XPCNativeInterface*     mInterfaces[1];
 };
@@ -1685,31 +1682,31 @@ public:
 protected:
     // disable copy ctor and assignment
     XPCWrappedNativeProto(const XPCWrappedNativeProto& r) = delete;
     XPCWrappedNativeProto& operator= (const XPCWrappedNativeProto& r) = delete;
 
     // hide ctor
     XPCWrappedNativeProto(XPCWrappedNativeScope* Scope,
                           nsIClassInfo* ClassInfo,
-                          XPCNativeSet* Set);
+                          already_AddRefed<XPCNativeSet>&& Set);
 
     bool Init(const XPCNativeScriptableCreateInfo* scriptableCreateInfo,
               bool callPostCreatePrototype);
 
 private:
 #ifdef DEBUG
     static int32_t gDEBUG_LiveProtoCount;
 #endif
 
 private:
     XPCWrappedNativeScope*   mScope;
     JS::ObjectPtr            mJSProtoObject;
     nsCOMPtr<nsIClassInfo>   mClassInfo;
-    XPCNativeSet*            mSet;
+    RefPtr<XPCNativeSet>     mSet;
     XPCNativeScriptableInfo* mScriptableInfo;
 };
 
 /***********************************************/
 // XPCWrappedNativeTearOff represents the info needed to make calls to one
 // interface on the underlying native object of a XPCWrappedNative.
 
 class XPCWrappedNativeTearOff final
@@ -1851,17 +1848,17 @@ public:
      */
     JSObject*
     GetFlatJSObjectPreserveColor() const {return mFlatJSObject;}
 
     XPCNativeSet*
     GetSet() const {return mSet;}
 
     void
-    SetSet(XPCNativeSet* set) {mSet = set;}
+    SetSet(already_AddRefed<XPCNativeSet> set) {mSet = set;}
 
     static XPCWrappedNative* Get(JSObject* obj) {
         MOZ_ASSERT(IS_WN_REFLECTOR(obj));
         return (XPCWrappedNative*)js::GetObjectPrivate(obj);
     }
 
 private:
     inline void
@@ -1994,17 +1991,17 @@ protected:
 
     // This ctor is used if this object will have a proto.
     XPCWrappedNative(already_AddRefed<nsISupports>&& aIdentity,
                      XPCWrappedNativeProto* aProto);
 
     // This ctor is used if this object will NOT have a proto.
     XPCWrappedNative(already_AddRefed<nsISupports>&& aIdentity,
                      XPCWrappedNativeScope* aScope,
-                     XPCNativeSet* aSet);
+                     already_AddRefed<XPCNativeSet>&& aSet);
 
     virtual ~XPCWrappedNative();
     void Destroy();
 
     void UpdateScriptableInfo(XPCNativeScriptableInfo* si);
 
 private:
     enum {
@@ -2030,17 +2027,17 @@ public:
                                                                            XPCNativeScriptableCreateInfo& sciWrapper);
 
 private:
     union
     {
         XPCWrappedNativeScope* mMaybeScope;
         XPCWrappedNativeProto* mMaybeProto;
     };
-    XPCNativeSet* mSet;
+    RefPtr<XPCNativeSet> mSet;
     JS::TenuredHeap<JSObject*> mFlatJSObject;
     XPCNativeScriptableInfo* mScriptableInfo;
     XPCWrappedNativeTearOff mFirstTearOff;
 };
 
 /***************************************************************************
 ****************************************************************************
 *
@@ -2876,17 +2873,16 @@ class TypedAutoMarkingPtr : public AutoM
         if (mPtr)
             mPtr->Mark();
     }
 
   private:
     T* mPtr;
 };
 
-typedef TypedAutoMarkingPtr<XPCNativeSet> AutoMarkingNativeSetPtr;
 typedef TypedAutoMarkingPtr<XPCWrappedNative> AutoMarkingWrappedNativePtr;
 typedef TypedAutoMarkingPtr<XPCWrappedNativeTearOff> AutoMarkingWrappedNativeTearOffPtr;
 typedef TypedAutoMarkingPtr<XPCWrappedNativeProto> AutoMarkingWrappedNativeProtoPtr;
 
 /***************************************************************************/
 namespace xpc {
 // Allocates a string that grants all access ("AllAccess")
 char*
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -308,22 +308,22 @@ WrapperFactory::PrepareForWrapping(JSCon
     // This could be a problem for chrome code that passes XPCOM objects
     // across compartments, because the effects of QI would disappear across
     // compartments.
     //
     // So whenever we pull an XPCWN across compartments in this manner, we
     // give the destination object the union of the two native sets. We try
     // to do this cleverly in the common case to avoid too much overhead.
     XPCWrappedNative* newwn = XPCWrappedNative::Get(obj);
-    XPCNativeSet* unionSet = XPCNativeSet::GetNewOrUsed(newwn->GetSet(),
-                                                        wn->GetSet(), false);
+    RefPtr<XPCNativeSet> unionSet = XPCNativeSet::GetNewOrUsed(newwn->GetSet(),
+                                                               wn->GetSet(), false);
     if (!unionSet) {
         return;
     }
-    newwn->SetSet(unionSet);
+    newwn->SetSet(unionSet.forget());
 
     retObj.set(waive ? WaiveXray(cx, obj) : obj);
 }
 
 #ifdef DEBUG
 static void
 DEBUG_CheckUnwrapSafety(HandleObject obj, const js::Wrapper* handler,
                         JSCompartment* origin, JSCompartment* target)