Bug 1575479 - Encapsulate mBaseHashtableET::mData. r=froydnj

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

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

toolkit/components/telemetry/core/Telemetry.cpp

@@ -561,13 +561,13 @@ bool TelemetryImpl::ReflectSQL(const SlowSQLEntryType* entry, const Stat* stat,
 
 bool TelemetryImpl::ReflectMainThreadSQL(SlowSQLEntryType* entry, JSContext* cx,
                                          JS::Handle<JSObject*> obj) {
-  return ReflectSQL(entry, &entry->mData.mainThread, cx, obj);
+  return ReflectSQL(entry, &entry->GetModifiableData()->mainThread, cx, obj);
 }
 
 bool TelemetryImpl::ReflectOtherThreadsSQL(SlowSQLEntryType* entry,
                                            JSContext* cx,
                                            JS::Handle<JSObject*> obj) {
-  return ReflectSQL(entry, &entry->mData.otherThreads, cx, obj);
+  return ReflectSQL(entry, &entry->GetModifiableData()->otherThreads, cx, obj);
 }
 
 bool TelemetryImpl::AddSQLInfo(JSContext* cx, JS::Handle<JSObject*> rootObj,
@@ -1262,18 +1262,18 @@ void TelemetryImpl::StoreSlowSQL(const nsACString& sql, uint32_t delay,
   if (!entry) {
     entry = slowSQLMap->PutEntry(sql);
     if (MOZ_UNLIKELY(!entry)) return;
-    entry->mData.mainThread.hitCount = 0;
-    entry->mData.mainThread.totalTime = 0;
-    entry->mData.otherThreads.hitCount = 0;
-    entry->mData.otherThreads.totalTime = 0;
+    entry->GetModifiableData()->mainThread.hitCount = 0;
+    entry->GetModifiableData()->mainThread.totalTime = 0;
+    entry->GetModifiableData()->otherThreads.hitCount = 0;
+    entry->GetModifiableData()->otherThreads.totalTime = 0;
   }
 
   if (NS_IsMainThread()) {
-    entry->mData.mainThread.hitCount++;
-    entry->mData.mainThread.totalTime += delay;
+    entry->GetModifiableData()->mainThread.hitCount++;
+    entry->GetModifiableData()->mainThread.totalTime += delay;
   } else {
-    entry->mData.otherThreads.hitCount++;
-    entry->mData.otherThreads.totalTime += delay;
+    entry->GetModifiableData()->otherThreads.hitCount++;
+    entry->GetModifiableData()->otherThreads.totalTime += delay;
   }
 }
 

toolkit/components/telemetry/core/TelemetryScalar.cpp

@@ -1409,7 +1409,7 @@ nsresult internal_GetEnumByScalarName(const StaticMutexAutoLock& lock,
   if (!entry) {
     return NS_ERROR_INVALID_ARG;
   }
-  *aId = entry->mData;
+  *aId = entry->GetData();
   return NS_OK;
 }
 
@@ -1921,7 +1921,7 @@ void internal_RegisterScalars(const StaticMutexAutoLock& lock,
       // Change the scalar to expired if needed.
       if (scalarInfo.mDynamicExpiration && !scalarInfo.builtin) {
         DynamicScalarInfo& scalarData =
-            (*gDynamicScalarInfo)[existingKey->mData.id];
+            (*gDynamicScalarInfo)[existingKey->GetData().id];
         scalarData.mDynamicExpiration = true;
       }
       continue;
@@ -1930,7 +1930,7 @@ void internal_RegisterScalars(const StaticMutexAutoLock& lock,
     gDynamicScalarInfo->AppendElement(scalarInfo);
     uint32_t scalarId = gDynamicScalarInfo->Length() - 1;
     CharPtrEntryType* entry = gScalarNameIDMap.PutEntry(scalarInfo.name());
-    entry->mData = ScalarKey{scalarId, true};
+    entry->SetData(ScalarKey{scalarId, true});
   }
 }
 
@@ -2415,7 +2415,7 @@ void TelemetryScalar::InitializeGlobalState(bool aCanRecordBase,
       static_cast<uint32_t>(mozilla::Telemetry::ScalarID::ScalarCount);
   for (uint32_t i = 0; i < scalarCount; i++) {
     CharPtrEntryType* entry = gScalarNameIDMap.PutEntry(gScalars[i].name());
-    entry->mData = ScalarKey{i, false};
+    entry->SetData(ScalarKey{i, false});
   }
 
   // To summarize dynamic events we need a dynamic scalar.

toolkit/components/telemetry/other/TelemetryIOInterposeObserver.cpp

@@ -75,7 +75,7 @@ void TelemetryIOInterposeObserver::Observe(Observation& aOb) {
   // Create a new entry or retrieve the existing one
   FileIOEntryType* entry = mFileStats.PutEntry(processedName);
   if (entry) {
-    FileStats& stats = entry->mData.mStats[mCurStage];
+    FileStats& stats = entry->GetModifiableData()->mStats[mCurStage];
     // Update the statistics
     stats.totalTime += (double)aOb.Duration().ToMilliseconds();
     switch (aOb.ObservedOperation()) {
@@ -105,7 +105,7 @@ bool TelemetryIOInterposeObserver::ReflectFileStats(FileIOEntryType* entry,
                                                     JS::Handle<JSObject*> obj) {
   JS::AutoValueArray<NUM_STAGES> stages(cx);
 
-  FileStatsByStage& statsByStage = entry->mData;
+  FileStatsByStage& statsByStage = *entry->GetModifiableData();
   for (int s = STAGE_STARTUP; s < NUM_STAGES; ++s) {
     FileStats& fileStats = statsByStage.mStats[s];
 

toolkit/components/telemetry/other/WebrtcTelemetry.cpp

@@ -20,9 +20,9 @@ void WebrtcTelemetry::RecordIceCandidateMask(const uint32_t iceCandidateBitmask,
   }
 
   if (success) {
-    entry->mData.webrtc.successCount++;
+    entry->GetModifiableData()->webrtc.successCount++;
   } else {
-    entry->mData.webrtc.failureCount++;
+    entry->GetModifiableData()->webrtc.failureCount++;
   }
 }
 
@@ -55,7 +55,7 @@ bool ReflectIceEntry(const WebrtcTelemetry::WebrtcIceCandidateType* entry,
 
 bool ReflectIceWebrtc(WebrtcTelemetry::WebrtcIceCandidateType* entry,
                       JSContext* cx, JS::Handle<JSObject*> obj) {
-  return ReflectIceEntry(entry, &entry->mData.webrtc, cx, obj);
+  return ReflectIceEntry(entry, &entry->GetData().webrtc, cx, obj);
 }
 
 bool WebrtcTelemetry::AddIceInfo(JSContext* cx, JS::Handle<JSObject*> iceObj) {

xpcom/ds/nsBaseHashtable.h

@@ -23,10 +23,19 @@ class nsBaseHashtable;  // forward declaration
 template <class KeyClass, class DataType>
 class nsBaseHashtableET : public KeyClass {
  public:
-  DataType mData;
-  friend class nsTHashtable<nsBaseHashtableET<KeyClass, DataType>>;
+  const DataType& GetData() const { return mData; }
+  DataType* GetModifiableData() { return &mData; }
+  template <typename U>
+  void SetData(U&& aData) {
+    mData = std::forward<U>(aData);
+  }
 
  private:
+  DataType mData;
+  friend class nsTHashtable<nsBaseHashtableET<KeyClass, DataType>>;
+  template <typename KeyClassX, typename DataTypeX, typename UserDataTypeX>
+  friend class nsBaseHashtable;
+
   typedef typename KeyClass::KeyType KeyType;
   typedef typename KeyClass::KeyTypePointer KeyTypePointer;
 

xpcom/ds/nsClassHashtable.h

@@ -80,9 +80,9 @@ T* nsClassHashtable<KeyClass, T>::LookupOrAdd(KeyType aKey,
   auto count = this->Count();
   typename base_type::EntryType* ent = this->PutEntry(aKey);
   if (count != this->Count()) {
-    ent->mData = new T(std::forward<Args>(aConstructionArgs)...);
+    ent->SetData(nsAutoPtr<T>(new T(std::forward<Args>(aConstructionArgs)...)));
   }
-  return ent->mData;
+  return ent->GetData();
 }
 
 template <class KeyClass, class T>
@@ -91,7 +91,7 @@ bool nsClassHashtable<KeyClass, T>::Get(KeyType aKey, T** aRetVal) const {
 
   if (ent) {
     if (aRetVal) {
-      *aRetVal = ent->mData;
+      *aRetVal = ent->GetData();
     }
 
     return true;
@@ -111,7 +111,7 @@ T* nsClassHashtable<KeyClass, T>::Get(KeyType aKey) const {
     return nullptr;
   }
 
-  return ent->mData;
+  return ent->GetData();
 }
 
 #endif  // nsClassHashtable_h__

xpcom/ds/nsDataHashtable.h

@@ -40,7 +40,7 @@ class nsDataHashtable : public nsBaseHashtable<KeyClass, DataType, DataType> {
    */
   DataType* GetValue(KeyType aKey) {
     if (EntryType* ent = this->GetEntry(aKey)) {
-      return &ent->mData;
+      return ent->GetModifiableData();
     }
     return nullptr;
   }
@@ -56,7 +56,7 @@ class nsDataHashtable : public nsBaseHashtable<KeyClass, DataType, DataType> {
   mozilla::Maybe<DataType> GetAndRemove(KeyType aKey) {
     mozilla::Maybe<DataType> value;
     if (EntryType* ent = this->GetEntry(aKey)) {
-      value.emplace(std::move(ent->mData));
+      value.emplace(std::move(*ent->GetModifiableData()));
       this->RemoveEntry(ent);
     }
     return value;

xpcom/ds/nsInterfaceHashtable.h

@@ -104,7 +104,7 @@ bool nsInterfaceHashtable<KeyClass, Interface>::Get(
 
   if (ent) {
     if (aInterface) {
-      *aInterface = ent->mData;
+      *aInterface = ent->GetData();
 
       NS_IF_ADDREF(*aInterface);
     }
@@ -129,7 +129,7 @@ already_AddRefed<Interface> nsInterfaceHashtable<KeyClass, Interface>::Get(
     return nullptr;
   }
 
-  nsCOMPtr<Interface> copy = ent->mData;
+  nsCOMPtr<Interface> copy = ent->GetData();
   return copy.forget();
 }
 
@@ -143,7 +143,7 @@ Interface* nsInterfaceHashtable<KeyClass, Interface>::GetWeak(
       *aFound = true;
     }
 
-    return ent->mData;
+    return ent->GetData();
   }
 
   // Key does not exist, return nullptr and set aFound to false
@@ -162,7 +162,7 @@ bool nsInterfaceHashtable<KeyClass, Interface>::Put(
     return false;
   }
 
-  ent->mData = aValue;
+  ent->SetData(std::move(aValue));
   return true;
 }
 
@@ -173,7 +173,7 @@ bool nsInterfaceHashtable<KeyClass, Interface>::Remove(KeyType aKey,
 
   if (ent) {
     if (aData) {
-      ent->mData.forget(aData);
+      ent->GetModifiableData()->forget(aData);
     }
     this->RemoveEntry(ent);
     return true;

xpcom/ds/nsRefPtrHashtable.h

@@ -96,7 +96,7 @@ bool nsRefPtrHashtable<KeyClass, PtrType>::Get(KeyType aKey,
 
   if (ent) {
     if (aRefPtr) {
-      *aRefPtr = ent->mData;
+      *aRefPtr = ent->GetData();
 
       NS_IF_ADDREF(*aRefPtr);
     }
@@ -121,7 +121,7 @@ already_AddRefed<PtrType> nsRefPtrHashtable<KeyClass, PtrType>::Get(
     return nullptr;
   }
 
-  RefPtr<PtrType> copy = ent->mData;
+  RefPtr<PtrType> copy = ent->GetData();
   return copy.forget();
 }
 
@@ -135,7 +135,7 @@ PtrType* nsRefPtrHashtable<KeyClass, PtrType>::GetWeak(KeyType aKey,
       *aFound = true;
     }
 
-    return ent->mData;
+    return ent->GetData();
   }
 
   // Key does not exist, return nullptr and set aFound to false
@@ -164,7 +164,7 @@ bool nsRefPtrHashtable<KeyClass, PtrType>::Put(KeyType aKey,
     return false;
   }
 
-  ent->mData = aData;
+  ent->SetData(aData);
 
   return true;
 }
@@ -176,7 +176,7 @@ bool nsRefPtrHashtable<KeyClass, PtrType>::Remove(KeyType aKey,
 
   if (ent) {
     if (aRefPtr) {
-      ent->mData.forget(aRefPtr);
+      ent->GetModifiableData()->forget(aRefPtr);
     }
     this->RemoveEntry(ent);
     return true;

xpcom/threads/nsEnvironment.cpp

@@ -144,9 +144,9 @@ nsEnvironment::Set(const nsAString& aName, const nsAString& aValue) {
   }
 
   PR_SetEnv(newData.get());
-  if (entry->mData) {
-    free(entry->mData);
+  if (entry->GetData()) {
+    free(entry->GetData());
   }
-  entry->mData = newData.release();
+  entry->SetData(newData.release());
   return NS_OK;
 }