Bug 1289137 - Make XPCNativeSet::NewInstance() take an nsTArray argument. r=billm draft
authorAndrew McCreight <continuation@gmail.com>
Wed, 27 Jul 2016 16:36:48 -0700
changeset 398609 3d8a310ac7c01a961c725ae44f45d26d80fc78b8
parent 398604 6cf0089510fad8deb866136f5b92bbced9498447
child 398610 985e9191630c3235c47eb2f1ee7932c82643f031
child 398833 2ed704eceb0e4ab2bed25976cf01d5902d72a3f8
push id25582
push userbmo:continuation@gmail.com
push dateTue, 09 Aug 2016 14:48:57 +0000
reviewersbillm
bugs1289137, 1288870
milestone51.0a1
Bug 1289137 - Make XPCNativeSet::NewInstance() take an nsTArray argument. r=billm I could clean up ArrayAutoMarkingPtr more, but it is going to be removed entirely in bug 1288870. MozReview-Commit-ID: Jyjc2ZfvF3i
js/xpconnect/src/XPCWrappedNativeInfo.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/xpconnect/src/XPCWrappedNativeInfo.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeInfo.cpp
@@ -482,19 +482,17 @@ XPCNativeSet::GetNewOrUsed(const nsIID* 
     if (!map)
         return nullptr;
 
     set = map->Find(&key);
 
     if (set)
         return set;
 
-    // hacky way to get a XPCNativeInterface** using the AutoPtr
-    XPCNativeInterface* temp[] = {iface};
-    set = NewInstance(temp, 1);
+    set = NewInstance({iface});
     if (!set)
         return nullptr;
 
     XPCNativeSet* set2 = map->Add(&key, set);
     if (!set2) {
         NS_ERROR("failed to add our set!");
         DestroyInstance(set);
         set = nullptr;
@@ -519,17 +517,16 @@ XPCNativeSet::GetNewOrUsed(nsIClassInfo*
         return nullptr;
 
     set = map->Find(classInfo);
 
     if (set)
         return set;
 
     nsIID** iidArray = nullptr;
-    AutoMarkingNativeInterfacePtrArrayPtr interfaceArray(cx);
     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.
 
         // Make sure these are set correctly...
@@ -537,46 +534,40 @@ XPCNativeSet::GetNewOrUsed(nsIClassInfo*
         iidCount = 0;
     }
 
     MOZ_ASSERT((iidCount && iidArray) || !(iidCount || iidArray), "GetInterfaces returned bad array");
 
     // !!! from here on we only exit through the 'out' label !!!
 
     if (iidCount) {
-        AutoMarkingNativeInterfacePtrArrayPtr
-            arr(cx, new XPCNativeInterface*[iidCount], iidCount, true);
-
-        interfaceArray = arr;
-
-        XPCNativeInterface** currentInterface = interfaceArray;
-        nsIID**              currentIID = iidArray;
-        uint16_t             interfaceCount = 0;
+        nsTArray<XPCNativeInterface*> interfaceArray(iidCount);
+        AutoMarkingNativeInterfacePtrArrayPtr arrayMarker(cx, interfaceArray);
+        nsIID** currentIID = iidArray;
 
         for (uint32_t i = 0; i < iidCount; i++) {
             nsIID* iid = *(currentIID++);
             if (!iid) {
                 NS_ERROR("Null found in classinfo interface list");
                 continue;
             }
 
             XPCNativeInterface* iface =
                 XPCNativeInterface::GetNewOrUsed(iid);
 
             if (!iface) {
                 // XXX warn here
                 continue;
             }
 
-            *(currentInterface++) = iface;
-            interfaceCount++;
+            interfaceArray.AppendElement(iface);
         }
 
-        if (interfaceCount) {
-            set = NewInstance(interfaceArray, interfaceCount);
+        if (interfaceArray.Length() > 0) {
+            set = NewInstance(Move(interfaceArray));
             if (set) {
                 NativeSetMap* map2 = rt->GetNativeSetMap();
                 if (!map2)
                     goto out;
 
                 XPCNativeSetKey key(set, nullptr, 0);
 
                 XPCNativeSet* set2 = map2->Add(&key, set);
@@ -603,18 +594,16 @@ XPCNativeSet::GetNewOrUsed(nsIClassInfo*
           map->Add(classInfo, set);
         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);
-    if (interfaceArray)
-        delete [] interfaceArray.get();
 
     return set;
 }
 
 // static
 void
 XPCNativeSet::ClearCacheEntryForClassInfo(nsIClassInfo* classInfo)
 {
@@ -642,17 +631,17 @@ XPCNativeSet::GetNewOrUsed(XPCNativeSet*
     set = map->Find(&key);
 
     if (set)
         return set;
 
     if (otherSet)
         set = NewInstanceMutate(otherSet, newInterface, position);
     else
-        set = NewInstance(&newInterface, 1);
+        set = NewInstance({newInterface});
 
     if (!set)
         return nullptr;
 
     XPCNativeSet* set2 = map->Add(&key, set);
     if (!set2) {
         NS_ERROR("failed to add our set!");
         DestroyInstance(set);
@@ -709,57 +698,51 @@ XPCNativeSet::GetNewOrUsed(XPCNativeSet*
 
     // We've got the union set. Hand it back to the caller.
     MOZ_ASSERT(currentSet->mInterfaceCount == uniqueCount);
     return currentSet;
 }
 
 // static
 XPCNativeSet*
-XPCNativeSet::NewInstance(XPCNativeInterface** array,
-                          uint16_t count)
+XPCNativeSet::NewInstance(nsTArray<XPCNativeInterface*>&& array)
 {
-    if (!array || !count)
+    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
     // that don't exactly follow the rule.
 
     XPCNativeInterface* isup = XPCNativeInterface::GetISupports();
-    uint16_t slots = count+1;
+    uint16_t slots = array.Length() + 1;
 
-    uint16_t i;
-    XPCNativeInterface** pcur;
-
-    for (i = 0, pcur = array; i < count; i++, pcur++) {
-        if (*pcur == isup)
+    for (auto key = array.begin(); key != array.end(); key++) {
+        if (*key == isup)
             slots--;
     }
 
     // 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();
 
     // Stick the nsISupports in front and skip additional nsISupport(s)
-    XPCNativeInterface** inp = array;
     XPCNativeInterface** outp = (XPCNativeInterface**) &obj->mInterfaces;
     uint16_t memberCount = 1;   // for the one member in nsISupports
 
     *(outp++) = isup;
 
-    for (i = 0; i < count; i++) {
-        XPCNativeInterface* cur;
-
-        if (isup == (cur = *(inp++)))
+    for (auto key = array.begin(); key != array.end(); key++) {
+        XPCNativeInterface* cur = *key;
+        if (isup == cur)
             continue;
         *(outp++) = cur;
         memberCount += cur->GetMemberCount();
     }
     obj->mMemberCount = memberCount;
     obj->mInterfaceCount = slots;
 
     return obj;
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -1397,18 +1397,17 @@ class XPCNativeSet final
 
     void DebugDump(int16_t depth);
 
     static void DestroyInstance(XPCNativeSet* inst);
 
     size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
 
   protected:
-    static XPCNativeSet* NewInstance(XPCNativeInterface** array,
-                                     uint16_t count);
+    static XPCNativeSet* NewInstance(nsTArray<XPCNativeInterface*>&& array);
     static XPCNativeSet* NewInstanceMutate(XPCNativeSet*       otherSet,
                                            XPCNativeInterface* newInterface,
                                            uint16_t            position);
     XPCNativeSet()
       : mMemberCount(0), mInterfaceCount(0), mMarked(0)
     {
         MOZ_COUNT_CTOR(XPCNativeSet);
     }
@@ -2907,58 +2906,41 @@ typedef TypedAutoMarkingPtr<XPCWrappedNa
 typedef TypedAutoMarkingPtr<XPCWrappedNativeTearOff> AutoMarkingWrappedNativeTearOffPtr;
 typedef TypedAutoMarkingPtr<XPCWrappedNativeProto> AutoMarkingWrappedNativeProtoPtr;
 typedef TypedAutoMarkingPtr<XPCNativeScriptableInfo> AutoMarkingNativeScriptableInfoPtr;
 
 template<class T>
 class ArrayAutoMarkingPtr : public AutoMarkingPtr
 {
   public:
-    explicit ArrayAutoMarkingPtr(JSContext* cx)
-      : AutoMarkingPtr(cx), mPtr(nullptr), mCount(0) {}
-    ArrayAutoMarkingPtr(JSContext* cx, T** ptr, uint32_t count, bool clear)
-      : AutoMarkingPtr(cx), mPtr(ptr), mCount(count)
-    {
-        if (!mPtr) mCount = 0;
-        else if (clear) memset(mPtr, 0, mCount*sizeof(T*));
-    }
-
-    T** get() const { return mPtr; }
-    operator T**() const { return mPtr; }
-    T** operator->() const { return mPtr; }
-
-    ArrayAutoMarkingPtr<T>& operator =(const ArrayAutoMarkingPtr<T>& other)
-    {
-        mPtr = other.mPtr;
-        mCount = other.mCount;
-        return *this;
-    }
+    ArrayAutoMarkingPtr(JSContext* cx, nsTArray<T*>& ptr)
+      : AutoMarkingPtr(cx), mPtr(ptr)
+    {}
 
   protected:
     virtual void TraceJS(JSTracer* trc)
     {
-        for (uint32_t i = 0; i < mCount; i++) {
+        for (uint32_t i = 0; i < mPtr.Length(); i++) {
             if (mPtr[i]) {
                 mPtr[i]->TraceJS(trc);
                 mPtr[i]->AutoTrace(trc);
             }
         }
     }
 
     virtual void MarkAfterJSFinalize()
     {
-        for (uint32_t i = 0; i < mCount; i++) {
+        for (uint32_t i = 0; i < mPtr.Length(); i++) {
             if (mPtr[i])
                 mPtr[i]->Mark();
         }
     }
 
   private:
-    T** mPtr;
-    uint32_t mCount;
+    nsTArray<T*>& mPtr;
 };
 
 typedef ArrayAutoMarkingPtr<XPCNativeInterface> AutoMarkingNativeInterfacePtrArrayPtr;
 
 /***************************************************************************/
 namespace xpc {
 // Allocates a string that grants all access ("AllAccess")
 char*