Bug 1691894 - Fix EntryHandle to only work with DataType rather than wrapping UserDataType. r=xpcom-reviewers,necko-reviewers,nika,jonco,valentin

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

dom/base/DocumentOrShadowRoot.cpp

@@ -756,7 +756,8 @@ nsRadioGroupStruct* DocumentOrShadowRoot::GetRadioGroup(
 nsRadioGroupStruct* DocumentOrShadowRoot::GetOrCreateRadioGroup(
     const nsAString& aName) {
   return mRadioGroups.WithEntryHandle(aName, [](auto&& entry) {
-    return entry.OrInsertWith([] { return new nsRadioGroupStruct(); }).get();
+    return entry.OrInsertWith([] { return MakeUnique<nsRadioGroupStruct>(); })
+        .get();
   });
 }
 

dom/base/nsDOMMutationObserver.cpp

@@ -325,7 +325,7 @@ void nsMutationReceiver::ContentRemoved(nsIContent* aChild,
                 return entry
                     .OrInsertWith([&isNewEntry] {
                       isNewEntry = true;
-                      return new nsCOMArray<nsMutationReceiver>();
+                      return MakeUnique<nsCOMArray<nsMutationReceiver>>();
                     })
                     .get();
               });
@@ -1041,8 +1041,9 @@ void nsAutoMutationBatch::Done() {
         auto* const transientReceivers =
             ob->mTransientReceivers.WithEntryHandle(removed, [](auto&& entry) {
               return entry
-                  .OrInsertWith(
-                      [] { return new nsCOMArray<nsMutationReceiver>(); })
+                  .OrInsertWith([] {
+                    return MakeUnique<nsCOMArray<nsMutationReceiver>>();
+                  })
                   .get();
             });
         for (uint32_t k = 0; k < allObservers.Length(); ++k) {

dom/base/nsFrameMessageManager.cpp

@@ -243,7 +243,8 @@ void nsFrameMessageManager::AddMessageListener(const nsAString& aMessageName,
       mListeners.WithEntryHandle(aMessageName, [](auto&& entry) {
         return entry
             .OrInsertWith([] {
-              return new nsAutoTObserverArray<nsMessageListenerInfo, 1>();
+              return MakeUnique<
+                  nsAutoTObserverArray<nsMessageListenerInfo, 1>>();
             })
             .get();
       });
@@ -317,7 +318,8 @@ void nsFrameMessageManager::AddWeakMessageListener(
       mListeners.WithEntryHandle(aMessageName, [](auto&& entry) {
         return entry
             .OrInsertWith([] {
-              return new nsAutoTObserverArray<nsMessageListenerInfo, 1>();
+              return MakeUnique<
+                  nsAutoTObserverArray<nsMessageListenerInfo, 1>>();
             })
             .get();
       });

dom/broadcastchannel/BroadcastChannelService.cpp

@@ -85,7 +85,7 @@ void BroadcastChannelService::RegisterActor(
       mAgents.WithEntryHandle(aOriginChannelKey, [](auto&& entry) {
         return entry
             .OrInsertWith(
-                [] { return new nsTArray<BroadcastChannelParent*>(); })
+                [] { return MakeUnique<nsTArray<BroadcastChannelParent*>>(); })
             .get();
       });
 

dom/commandhandler/nsCommandManager.cpp

@@ -82,7 +82,9 @@ nsCommandManager::AddCommandObserver(nsIObserver* aCommandObserver,
   // for each command in the table, we make a list of observers for that command
   auto* const commandObservers =
       mObserversTable.WithEntryHandle(aCommandToObserve, [](auto&& entry) {
-        return entry.OrInsertWith([] { return new ObserverList; }).get();
+        return entry
+            .OrInsertWith([] { return mozilla::MakeUnique<ObserverList>(); })
+            .get();
       });
 
   // need to check that this command observer hasn't already been registered

dom/media/ogg/OggCodecState.cpp

@@ -1712,8 +1712,8 @@ bool SkeletonState::DecodeFisbone(ogg_packet* aPacket) {
               entry.OrInsertWith([i, msgHead, msgProbe]() {
                 uint32_t nameLen =
                     strlen(kFieldTypeMaps[i].mPatternToRecognize);
-                return new nsCString(msgHead + nameLen,
-                                     msgProbe - msgHead - nameLen);
+                return MakeUnique<nsCString>(msgHead + nameLen,
+                                             msgProbe - msgHead - nameLen);
               });
             });
             isContentTypeParsed = i == 0 ? true : isContentTypeParsed;
@@ -1735,7 +1735,7 @@ bool SkeletonState::DecodeFisbone(ogg_packet* aPacket) {
       // mMsgFieldStore has an entry for serialno already.
       return false;
     }
-    entry.Insert(field.release());
+    entry.Insert(std::move(field));
     return true;
   });
 }

dom/serviceworkers/ServiceWorkerManager.cpp

@@ -548,7 +548,8 @@ RefPtr<GenericErrorResultPromise> ServiceWorkerManager::StartControllingClient(
 
         aRegistrationInfo->StartControllingClient();
 
-        entry.Insert(new ControlledClientData(clientHandle, aRegistrationInfo));
+        entry.Insert(
+            MakeUnique<ControlledClientData>(clientHandle, aRegistrationInfo));
 
         clientHandle->OnDetach()->Then(
             GetMainThreadSerialEventTarget(), __func__,
@@ -1911,7 +1912,8 @@ void ServiceWorkerManager::AddScopeAndRegistration(
   auto* const data =
       swm->mRegistrationInfos.WithEntryHandle(scopeKey, [](auto&& entry) {
         return entry
-            .OrInsertWith([] { return new RegistrationDataPerPrincipal(); })
+            .OrInsertWith(
+                [] { return MakeUnique<RegistrationDataPerPrincipal>(); })
             .get();
       });
 

dom/workers/RuntimeService.cpp

@@ -1156,7 +1156,7 @@ bool RuntimeService::RegisterWorker(WorkerPrivate& aWorkerPrivate) {
                 NS_ASSERTION(!parent, "Shouldn't have a parent here!");
                 Unused << parent;  // silence clang -Wunused-lambda-capture in
                                    // opt builds
-                WorkerDomainInfo* wdi = new WorkerDomainInfo();
+                auto wdi = MakeUnique<WorkerDomainInfo>();
                 wdi->mDomain = domain;
                 return wdi;
               })
@@ -1227,7 +1227,7 @@ bool RuntimeService::RegisterWorker(WorkerPrivate& aWorkerPrivate) {
               [](auto&& entry) {
                 return entry
                     .OrInsertWith(
-                        [] { return new nsTArray<WorkerPrivate*>(1); })
+                        [] { return MakeUnique<nsTArray<WorkerPrivate*>>(1); })
                     .get();
               });
           !windowArray->Contains(&aWorkerPrivate)) {

gfx/thebes/gfxFcPlatformFontList.cpp

@@ -1593,7 +1593,7 @@ void gfxFcPlatformFontList::InitSharedFontListForPlatform() {
       // Add new family record if one doesn't already exist.
       faceListPtr = faces.WithEntryHandle(keyName, [&](auto&& faceList) {
         if (!faceList) {
-          faceList.Insert(new FaceInitArray);
+          faceList.Insert(MakeUnique<FaceInitArray>());
           FontVisibility visibility =
               aAppFont ? FontVisibility::Base : GetVisibilityForFamily(keyName);
           families.AppendElement(fontlist::Family::InitData(
@@ -1652,7 +1652,7 @@ void gfxFcPlatformFontList::InitSharedFontListForPlatform() {
 
       faces.WithEntryHandle(keyName, [&](auto&& faceList) {
         if (!faceList) {
-          faceList.Insert(new FaceInitArray);
+          faceList.Insert(MakeUnique<FaceInitArray>());
           FontVisibility visibility =
               aAppFont ? FontVisibility::Base : GetVisibilityForFamily(keyName);
           families.AppendElement(fontlist::Family::InitData(

js/public/RootingAPI.h

@@ -312,12 +312,13 @@ class MOZ_NON_MEMMOVABLE Heap : public js::HeapBase<T, Heap<T>> {
   explicit Heap(const T& p) { init(p); }
 
   /*
-   * For Heap, move semantics are equivalent to copy semantics. In C++, a
-   * copy constructor taking const-ref is the way to get a single function
-   * that will be used for both lvalue and rvalue copies, so we can simply
-   * omit the rvalue variant.
+   * For Heap, move semantics are equivalent to copy semantics. However, we want
+   * the copy constructor to be explicit, and an explicit move constructor
+   * breaks common usage of move semantics, so we need to define both, even
+   * though they are equivalent.
    */
   explicit Heap(const Heap<T>& other) { init(other.ptr); }
+  Heap(Heap<T>&& other) { init(other.ptr); }
 
   Heap& operator=(Heap<T>&& other) {
     set(other.unbarrieredGet());

layout/base/PresShell.cpp

@@ -10549,7 +10549,8 @@ void ReflowCountMgr::Add(const char* aName, nsIFrame* aFrame) {
 
   if (mDumpFrameCounts) {
     auto* const counter = mCounts.WithEntryHandle(aName, [this](auto&& entry) {
-      return entry.OrInsertWith([this] { return new ReflowCounter(this); })
+      return entry
+          .OrInsertWith([this] { return MakeUnique<ReflowCounter>(this); })
           .get();
     });
     counter->Add();
@@ -10563,7 +10564,7 @@ void ReflowCountMgr::Add(const char* aName, nsIFrame* aFrame) {
         mIndiFrameCounts.WithEntryHandle(key, [&](auto&& entry) {
           return entry
               .OrInsertWith([&aName, &aFrame, this]() {
-                auto counter = new IndiReflowCounter(this);
+                auto counter = MakeUnique<IndiReflowCounter>(this);
                 counter->mFrame = aFrame;
                 counter->mName.AssignASCII(aName);
                 return counter;
@@ -10655,7 +10656,7 @@ void ReflowCountMgr::PaintCount(const char* aName,
 void ReflowCountMgr::DoGrandTotals() {
   mCounts.WithEntryHandle(kGrandTotalsStr, [this](auto&& entry) {
     if (!entry) {
-      entry.Insert(new ReflowCounter(this));
+      entry.Insert(MakeUnique<ReflowCounter>(this));
     } else {
       entry.Data()->ClearTotals();
     }
@@ -10727,7 +10728,7 @@ void ReflowCountMgr::DoIndiTotalsTree() {
 void ReflowCountMgr::DoGrandHTMLTotals() {
   mCounts.WithEntryHandle(kGrandTotalsStr, [this](auto&& entry) {
     if (!entry) {
-      entry.Insert(new ReflowCounter(this));
+      entry.Insert(MakeUnique<ReflowCounter>(this));
     } else {
       entry.Data()->ClearTotals();
     }
@@ -10800,7 +10801,7 @@ void ReflowCountMgr::ClearTotals() {
 void ReflowCountMgr::ClearGrandTotals() {
   mCounts.WithEntryHandle(kGrandTotalsStr, [&](auto&& entry) {
     if (!entry) {
-      entry.Insert(new ReflowCounter(this));
+      entry.Insert(MakeUnique<ReflowCounter>(this));
     } else {
       entry.Data()->ClearTotals();
       entry.Data()->SetTotalsCache();

layout/base/nsCounterManager.cpp

@@ -309,7 +309,7 @@ bool nsCounterManager::AddCounterChanges(nsIFrame* aFrame) {
 nsCounterList* nsCounterManager::CounterListFor(nsAtom* aCounterName) {
   MOZ_ASSERT(aCounterName);
   return mNames.WithEntryHandle(aCounterName, [](auto&& entry) {
-    return entry.OrInsertWith([] { return new nsCounterList(); }).get();
+    return entry.OrInsertWith([] { return MakeUnique<nsCounterList>(); }).get();
   });
 }
 

layout/base/nsRefreshDriver.cpp

@@ -1355,7 +1355,8 @@ bool nsRefreshDriver::AddImageRequest(imgIRequest* aRequest) {
     mRequests.PutEntry(aRequest);
   } else {
     auto* const start = mStartTable.WithEntryHandle(delay, [](auto&& entry) {
-      return entry.OrInsertWith([] { return new ImageStartData(); }).get();
+      return entry.OrInsertWith([] { return MakeUnique<ImageStartData>(); })
+          .get();
     });
     start->mEntries.PutEntry(aRequest);
   }

layout/style/ImageLoader.cpp

@@ -187,7 +187,7 @@ void ImageLoader::AssociateRequestToFrame(imgIRequest* aRequest,
                 nsLayoutUtils::RegisterImageRequestIfAnimated(
                     presContext, aRequest, nullptr);
               }
-              return new FrameSet();
+              return MakeUnique<FrameSet>();
             })
             .get();
       });
@@ -197,7 +197,7 @@ void ImageLoader::AssociateRequestToFrame(imgIRequest* aRequest,
         return entry
             .OrInsertWith([=]() {
               aFrame->SetHasImageRequest(true);
-              return new RequestSet();
+              return MakeUnique<RequestSet>();
             })
             .get();
       });
@@ -442,7 +442,7 @@ already_AddRefed<imgRequestProxy> ImageLoader::LoadImage(
     return nullptr;
   }
   sImages->WithEntryHandle(request, [](auto&& entry) {
-    entry.OrInsertWith([] { return new ImageTableEntry(); });
+    entry.OrInsertWith([] { return MakeUnique<ImageTableEntry>(); });
   });
   return request.forget();
 }

layout/style/ShadowParts.cpp

@@ -107,7 +107,7 @@ ShadowParts ShadowParts::Parse(const nsAString& aString) {
     }
     nsAtom* second = mapping.second.get();
     parts.mMappings.WithEntryHandle(mapping.first, [&](auto&& entry) {
-      entry.OrInsertWith([] { return new PartList(); })
+      entry.OrInsertWith([] { return MakeUnique<PartList>(); })
           ->AppendElement(std::move(mapping.second));
     });
     parts.mReverseMappings.GetOrInsert(second) = std::move(mapping.first);

modules/libpref/Preferences.cpp

@@ -2612,7 +2612,7 @@ nsPrefBranch::AddObserverImpl(const nsACString& aDomain, nsIObserver* aObserver,
       NS_WARNING("Ignoring duplicate observer.");
       delete pCallback;
     } else {
-      p.Insert(pCallback);
+      p.Insert(UniquePtr<PrefCallback>{pCallback});
 
       // We must pass a fully qualified preference name to the callback
       // aDomain == nullptr is the only possible failure, and we trapped it with

netwerk/dns/ChildDNSService.cpp

@@ -138,7 +138,8 @@ nsresult ChildDNSService::AsyncResolveInternal(
                         aOriginAttributes, flags, originalListenerAddr, key);
     mPendingRequests.WithEntryHandle(key, [&](auto&& entry) {
       entry
-          .OrInsertWith([] { return new nsTArray<RefPtr<DNSRequestSender>>(); })
+          .OrInsertWith(
+              [] { return MakeUnique<nsTArray<RefPtr<DNSRequestSender>>>(); })
           ->AppendElement(sender);
     });
   }

xpcom/base/nsTraceRefcnt.cpp

@@ -463,7 +463,7 @@ static intptr_t GetSerialNumber(void* aPtr, bool aCreate) {
           "it.");
     }
 
-    auto& record = entry.Insert(new SerialNumberRecord());
+    auto& record = entry.Insert(MakeUnique<SerialNumberRecord>());
     WalkTheStackSavingLocations(record->allocationStack);
     if (gLogJSStacks) {
       record->SaveJSStack();

xpcom/ds/nsBaseHashtable.h

@@ -74,6 +74,7 @@ class nsBaseHashtableET : public KeyClass {
   typedef typename KeyClass::KeyTypePointer KeyTypePointer;
 
   explicit nsBaseHashtableET(KeyTypePointer aKey);
+  nsBaseHashtableET(KeyTypePointer aKey, const DataType& aData);
   nsBaseHashtableET(KeyTypePointer aKey, DataType&& aData);
   nsBaseHashtableET(nsBaseHashtableET<KeyClass, DataType>&& aToMove);
   ~nsBaseHashtableET() = default;
@@ -210,7 +211,7 @@ class nsBaseHashtable
    */
   void Put(KeyType aKey, const UserDataType& aData) {
     WithEntryHandle(aKey, [&aData](auto entryHandle) {
-      entryHandle.InsertOrUpdate(aData);
+      entryHandle.InsertOrUpdate(Converter::Wrap(aData));
     });
   }
 
@@ -220,7 +221,7 @@ class nsBaseHashtable
       if (!maybeEntryHandle) {
         return false;
       }
-      maybeEntryHandle->InsertOrUpdate(aData);
+      maybeEntryHandle->InsertOrUpdate(Converter::Wrap(aData));
       return true;
     });
   }
@@ -232,7 +233,7 @@ class nsBaseHashtable
    */
   void Put(KeyType aKey, UserDataType&& aData) {
     WithEntryHandle(aKey, [&aData](auto entryHandle) {
-      entryHandle.InsertOrUpdate(std::move(aData));
+      entryHandle.InsertOrUpdate(Converter::Wrap(std::move(aData)));
     });
   }
 
@@ -242,7 +243,7 @@ class nsBaseHashtable
       if (!maybeEntryHandle) {
         return false;
       }
-      maybeEntryHandle->InsertOrUpdate(std::move(aData));
+      maybeEntryHandle->InsertOrUpdate(Converter::Wrap(std::move(aData)));
       return true;
     });
   }
@@ -400,6 +401,9 @@ class nsBaseHashtable
    * place, which should be used if the provision of the value is not trivial
    * (e.g. allocates a heap object). Finally, there's InsertOrUpdate that
    * handles both existing and non-existing entries.
+   *
+   * Note that all functions of EntryHandle only deal with DataType, not with
+   * UserDataType.
    */
   class EntryHandle : protected nsTHashtable<EntryType>::EntryHandle {
    public:
@@ -424,12 +428,13 @@ class nsBaseHashtable
      * Inserts a new entry with the handle's key and the value passed to this
      * function.
      *
+     * \tparam U DataType must be constructible from U
      * \pre !HasEntry()
      * \post HasEntry()
      */
     template <typename U>
     DataType& Insert(U&& aData) {
-      Base::InsertInternal(Converter::Wrap(std::forward<U>(aData)));
+      Base::InsertInternal(std::forward<U>(aData));
       return Data();
     }
 
@@ -438,6 +443,7 @@ class nsBaseHashtable
      * the value passed to this function. The value is not consumed if no insert
      * takes place.
      *
+     * \tparam U DataType must be constructible from U
      * \post HasEntry()
      */
     template <typename U>
@@ -453,6 +459,7 @@ class nsBaseHashtable
      * the result of the functor passed to this function. The functor is not
      * called if no insert takes place.
      *
+     * \tparam F must return a value that DataType is constructible from
      * \post HasEntry()
      */
     template <typename F>
@@ -467,12 +474,13 @@ class nsBaseHashtable
      * Updates the entry with the handle's key by the value passed to this
      * function.
      *
+     * \tparam U DataType must be assignable from U
      * \pre HasEntry()
      */
     template <typename U>
     DataType& Update(U&& aData) {
       MOZ_RELEASE_ASSERT(HasEntry());
-      Data() = Converter::Wrap(std::forward<U>(aData));
+      Data() = std::forward<U>(aData);
       return Data();
     }
 
@@ -480,6 +488,8 @@ class nsBaseHashtable
      * If an entry with the handle's key already exists, updates its value by
      * the value passed to this function. The value is not consumed if no update
      * takes place.
+     *
+     * \tparam U DataType must be assignable from U
      */
     template <typename U>
     void OrUpdate(U&& aData) {
@@ -492,6 +502,8 @@ class nsBaseHashtable
      * If an entry with the handle's key already exists, updates its value by
      * the the result of the functor passed to this function. The functor is not
      * called if no update takes place.
+     *
+     * \tparam F must return a value that DataType is assignable from
      */
     template <typename F>
     void OrUpdateWith(F&& aFunc) {
@@ -505,6 +517,7 @@ class nsBaseHashtable
      * value passed to this function. Otherwise, it updates the entry by the
      * value passed to this function.
      *
+     * \tparam U DataType must be constructible and assignable from U
      * \post HasEntry()
      */
     template <typename U>
@@ -667,6 +680,11 @@ template <class KeyClass, class DataType>
 nsBaseHashtableET<KeyClass, DataType>::nsBaseHashtableET(KeyTypePointer aKey)
     : KeyClass(aKey), mData() {}
 
+template <class KeyClass, class DataType>
+nsBaseHashtableET<KeyClass, DataType>::nsBaseHashtableET(KeyTypePointer aKey,
+                                                         const DataType& aData)
+    : KeyClass(aKey), mData(aData) {}
+
 template <class KeyClass, class DataType>
 nsBaseHashtableET<KeyClass, DataType>::nsBaseHashtableET(KeyTypePointer aKey,
                                                          DataType&& aData)