diff --git a/toolkit/components/telemetry/core/Telemetry.cpp b/toolkit/components/telemetry/core/Telemetry.cpp index c3fbfab18851..0599e46d2165 100644 --- a/toolkit/components/telemetry/core/Telemetry.cpp +++ b/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 obj) { - return ReflectSQL(entry, &entry->GetModifiableData()->mainThread, cx, obj); + return ReflectSQL(entry, &entry->mData.mainThread, cx, obj); } bool TelemetryImpl::ReflectOtherThreadsSQL(SlowSQLEntryType* entry, JSContext* cx, JS::Handle obj) { - return ReflectSQL(entry, &entry->GetModifiableData()->otherThreads, cx, obj); + return ReflectSQL(entry, &entry->mData.otherThreads, cx, obj); } bool TelemetryImpl::AddSQLInfo(JSContext* cx, JS::Handle 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->GetModifiableData()->mainThread.hitCount = 0; - entry->GetModifiableData()->mainThread.totalTime = 0; - entry->GetModifiableData()->otherThreads.hitCount = 0; - entry->GetModifiableData()->otherThreads.totalTime = 0; + entry->mData.mainThread.hitCount = 0; + entry->mData.mainThread.totalTime = 0; + entry->mData.otherThreads.hitCount = 0; + entry->mData.otherThreads.totalTime = 0; } if (NS_IsMainThread()) { - entry->GetModifiableData()->mainThread.hitCount++; - entry->GetModifiableData()->mainThread.totalTime += delay; + entry->mData.mainThread.hitCount++; + entry->mData.mainThread.totalTime += delay; } else { - entry->GetModifiableData()->otherThreads.hitCount++; - entry->GetModifiableData()->otherThreads.totalTime += delay; + entry->mData.otherThreads.hitCount++; + entry->mData.otherThreads.totalTime += delay; } } diff --git a/toolkit/components/telemetry/core/TelemetryScalar.cpp b/toolkit/components/telemetry/core/TelemetryScalar.cpp index 827ca03a3d60..195c09ac0429 100644 --- a/toolkit/components/telemetry/core/TelemetryScalar.cpp +++ b/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->GetData(); + *aId = entry->mData; 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->GetData().id]; + (*gDynamicScalarInfo)[existingKey->mData.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->SetData(ScalarKey{scalarId, true}); + entry->mData = ScalarKey{scalarId, true}; } } @@ -2415,7 +2415,7 @@ void TelemetryScalar::InitializeGlobalState(bool aCanRecordBase, static_cast(mozilla::Telemetry::ScalarID::ScalarCount); for (uint32_t i = 0; i < scalarCount; i++) { CharPtrEntryType* entry = gScalarNameIDMap.PutEntry(gScalars[i].name()); - entry->SetData(ScalarKey{i, false}); + entry->mData = ScalarKey{i, false}; } // To summarize dynamic events we need a dynamic scalar. diff --git a/toolkit/components/telemetry/other/TelemetryIOInterposeObserver.cpp b/toolkit/components/telemetry/other/TelemetryIOInterposeObserver.cpp index d41fd253d8f9..4b3f2afcafcd 100644 --- a/toolkit/components/telemetry/other/TelemetryIOInterposeObserver.cpp +++ b/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->GetModifiableData()->mStats[mCurStage]; + FileStats& stats = entry->mData.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 obj) { JS::AutoValueArray stages(cx); - FileStatsByStage& statsByStage = *entry->GetModifiableData(); + FileStatsByStage& statsByStage = entry->mData; for (int s = STAGE_STARTUP; s < NUM_STAGES; ++s) { FileStats& fileStats = statsByStage.mStats[s]; diff --git a/toolkit/components/telemetry/other/WebrtcTelemetry.cpp b/toolkit/components/telemetry/other/WebrtcTelemetry.cpp index 651fc4a1f4a2..a9fff1f3be10 100644 --- a/toolkit/components/telemetry/other/WebrtcTelemetry.cpp +++ b/toolkit/components/telemetry/other/WebrtcTelemetry.cpp @@ -20,9 +20,9 @@ void WebrtcTelemetry::RecordIceCandidateMask(const uint32_t iceCandidateBitmask, } if (success) { - entry->GetModifiableData()->webrtc.successCount++; + entry->mData.webrtc.successCount++; } else { - entry->GetModifiableData()->webrtc.failureCount++; + entry->mData.webrtc.failureCount++; } } @@ -55,7 +55,7 @@ bool ReflectIceEntry(const WebrtcTelemetry::WebrtcIceCandidateType* entry, bool ReflectIceWebrtc(WebrtcTelemetry::WebrtcIceCandidateType* entry, JSContext* cx, JS::Handle obj) { - return ReflectIceEntry(entry, &entry->GetData().webrtc, cx, obj); + return ReflectIceEntry(entry, &entry->mData.webrtc, cx, obj); } bool WebrtcTelemetry::AddIceInfo(JSContext* cx, JS::Handle iceObj) { diff --git a/xpcom/ds/PLDHashTable.cpp b/xpcom/ds/PLDHashTable.cpp index 40c845aff1cd..002699a9fe48 100644 --- a/xpcom/ds/PLDHashTable.cpp +++ b/xpcom/ds/PLDHashTable.cpp @@ -729,36 +729,6 @@ PLDHashTable::Iterator::Iterator(PLDHashTable* aTable) } } -PLDHashTable::Iterator::Iterator(PLDHashTable* aTable, EndIteratorTag aTag) - : mTable(aTable), - mCurrent(mTable->mEntryStore.SlotForIndex(0, mTable->mEntrySize, - mTable->Capacity())), - mNexts(mTable->EntryCount()), - mNextsLimit(mTable->EntryCount()), - mHaveRemoved(false), - mEntrySize(aTable->mEntrySize) { -#ifdef DEBUG - mTable->mChecker.StartReadOp(); -#endif - - MOZ_ASSERT(Done()); -} - -PLDHashTable::Iterator::Iterator(const Iterator& aOther) - : mTable(aOther.mTable), - mCurrent(aOther.mCurrent), - mNexts(aOther.mNexts), - mNextsLimit(aOther.mNextsLimit), - mHaveRemoved(aOther.mHaveRemoved), - mEntrySize(aOther.mEntrySize) { - // TODO: Is this necessary? - MOZ_ASSERT(!mHaveRemoved); - -#ifdef DEBUG - mTable->mChecker.StartReadOp(); -#endif -} - PLDHashTable::Iterator::~Iterator() { if (mTable) { if (mHaveRemoved) { diff --git a/xpcom/ds/PLDHashTable.h b/xpcom/ds/PLDHashTable.h index 1a65ae694f4e..8346aa113f80 100644 --- a/xpcom/ds/PLDHashTable.h +++ b/xpcom/ds/PLDHashTable.h @@ -571,8 +571,6 @@ class PLDHashTable { class Iterator { public: explicit Iterator(PLDHashTable* aTable); - struct EndIteratorTag {}; - Iterator(PLDHashTable* aTable, EndIteratorTag aTag); Iterator(Iterator&& aOther); ~Iterator(); @@ -593,13 +591,6 @@ class PLDHashTable { // must not be called on that entry afterwards. void Remove(); - bool operator==(const Iterator& aOther) const { - MOZ_ASSERT(mTable == aOther.mTable); - return mNexts == aOther.mNexts; - } - - Iterator Clone() const { return {*this}; } - protected: PLDHashTable* mTable; // Main table pointer. @@ -616,7 +607,7 @@ class PLDHashTable { void MoveToNextLiveEntry(); Iterator() = delete; - Iterator(const Iterator&); + Iterator(const Iterator&) = delete; Iterator& operator=(const Iterator&) = delete; Iterator& operator=(const Iterator&&) = delete; }; diff --git a/xpcom/ds/nsBaseHashtable.h b/xpcom/ds/nsBaseHashtable.h index 90af9d855342..166c0d3f3222 100644 --- a/xpcom/ds/nsBaseHashtable.h +++ b/xpcom/ds/nsBaseHashtable.h @@ -23,19 +23,10 @@ class nsBaseHashtable; // forward declaration template class nsBaseHashtableET : public KeyClass { public: - const DataType& GetData() const { return mData; } - DataType* GetModifiableData() { return &mData; } - template - void SetData(U&& aData) { - mData = std::forward(aData); - } - - private: DataType mData; friend class nsTHashtable>; - template - friend class nsBaseHashtable; + private: typedef typename KeyClass::KeyType KeyType; typedef typename KeyClass::KeyTypePointer KeyTypePointer; @@ -396,91 +387,6 @@ class nsBaseHashtable return Iterator(const_cast(this)); } - // STL-style iterators to allow the use in range-based for loops, e.g. - template - class base_iterator - : public std::iterator { - public: - using typename std::iterator::value_type; - using typename std::iterator::difference_type; - - using iterator_type = base_iterator; - using const_iterator_type = base_iterator; - - using EndIteratorTag = PLDHashTable::Iterator::EndIteratorTag; - - base_iterator(base_iterator&& aOther) = default; - - base_iterator& operator=(base_iterator&& aOther) { - // User-defined because the move assignment operator is deleted in - // PLDHashtable::Iterator. - return operator=(static_cast(aOther)); - } - - base_iterator(const base_iterator& aOther) - : mIterator{aOther.mIterator.Clone()} {} - base_iterator& operator=(const base_iterator& aOther) { - // Since PLDHashTable::Iterator has no assignment operator, we destroy and - // recreate mIterator. - mIterator.~Iterator(); - new (&mIterator) PLDHashTable::Iterator(aOther.mIterator.Clone()); - return *this; - } - - explicit base_iterator(PLDHashTable::Iterator aFrom) - : mIterator{std::move(aFrom)} {} - - explicit base_iterator(const nsBaseHashtable* aTable) - : mIterator{&const_cast(aTable)->mTable} {} - - base_iterator(const nsBaseHashtable* aTable, EndIteratorTag aTag) - : mIterator{&const_cast(aTable)->mTable, aTag} {} - - bool operator==(const iterator_type& aRhs) const { - return mIterator == aRhs.mIterator; - } - bool operator!=(const iterator_type& aRhs) const { - return !(*this == aRhs); - } - - value_type* operator->() const { - return static_cast(mIterator.Get()); - } - value_type& operator*() const { - return *static_cast(mIterator.Get()); - } - - iterator_type& operator++() { - mIterator.Next(); - return *this; - } - iterator_type operator++(int) { - iterator_type it = *this; - ++*this; - return it; - } - - operator const_iterator_type() const { - return const_iterator_type{mIterator.Clone()}; - } - - private: - PLDHashTable::Iterator mIterator; - }; - using const_iterator = base_iterator; - using iterator = base_iterator; - - iterator begin() { return iterator{this}; } - const_iterator begin() const { return const_iterator{this}; } - const_iterator cbegin() const { return begin(); } - iterator end() { return iterator{this, typename iterator::EndIteratorTag{}}; } - const_iterator end() const { - return const_iterator{this, typename const_iterator::EndIteratorTag{}}; - } - const_iterator cend() const { return end(); } - /** * reset the hashtable, removing all entries */ diff --git a/xpcom/ds/nsClassHashtable.h b/xpcom/ds/nsClassHashtable.h index 4ea28144b27e..2a98f3a79874 100644 --- a/xpcom/ds/nsClassHashtable.h +++ b/xpcom/ds/nsClassHashtable.h @@ -80,9 +80,9 @@ T* nsClassHashtable::LookupOrAdd(KeyType aKey, auto count = this->Count(); typename base_type::EntryType* ent = this->PutEntry(aKey); if (count != this->Count()) { - ent->SetData(nsAutoPtr(new T(std::forward(aConstructionArgs)...))); + ent->mData = new T(std::forward(aConstructionArgs)...); } - return ent->GetData(); + return ent->mData; } template @@ -91,7 +91,7 @@ bool nsClassHashtable::Get(KeyType aKey, T** aRetVal) const { if (ent) { if (aRetVal) { - *aRetVal = ent->GetData(); + *aRetVal = ent->mData; } return true; @@ -111,7 +111,7 @@ T* nsClassHashtable::Get(KeyType aKey) const { return nullptr; } - return ent->GetData(); + return ent->mData; } #endif // nsClassHashtable_h__ diff --git a/xpcom/ds/nsDataHashtable.h b/xpcom/ds/nsDataHashtable.h index 0dbe7a076fa5..033b4d1d61d3 100644 --- a/xpcom/ds/nsDataHashtable.h +++ b/xpcom/ds/nsDataHashtable.h @@ -40,7 +40,7 @@ class nsDataHashtable : public nsBaseHashtable { */ DataType* GetValue(KeyType aKey) { if (EntryType* ent = this->GetEntry(aKey)) { - return ent->GetModifiableData(); + return &ent->mData; } return nullptr; } @@ -56,7 +56,7 @@ class nsDataHashtable : public nsBaseHashtable { mozilla::Maybe GetAndRemove(KeyType aKey) { mozilla::Maybe value; if (EntryType* ent = this->GetEntry(aKey)) { - value.emplace(std::move(*ent->GetModifiableData())); + value.emplace(std::move(ent->mData)); this->RemoveEntry(ent); } return value; diff --git a/xpcom/ds/nsInterfaceHashtable.h b/xpcom/ds/nsInterfaceHashtable.h index ad3adc1ee164..9135a4cf4c3e 100644 --- a/xpcom/ds/nsInterfaceHashtable.h +++ b/xpcom/ds/nsInterfaceHashtable.h @@ -104,7 +104,7 @@ bool nsInterfaceHashtable::Get( if (ent) { if (aInterface) { - *aInterface = ent->GetData(); + *aInterface = ent->mData; NS_IF_ADDREF(*aInterface); } @@ -129,7 +129,7 @@ already_AddRefed nsInterfaceHashtable::Get( return nullptr; } - nsCOMPtr copy = ent->GetData(); + nsCOMPtr copy = ent->mData; return copy.forget(); } @@ -143,7 +143,7 @@ Interface* nsInterfaceHashtable::GetWeak( *aFound = true; } - return ent->GetData(); + return ent->mData; } // Key does not exist, return nullptr and set aFound to false @@ -162,7 +162,7 @@ bool nsInterfaceHashtable::Put( return false; } - ent->SetData(std::move(aValue)); + ent->mData = aValue; return true; } @@ -173,7 +173,7 @@ bool nsInterfaceHashtable::Remove(KeyType aKey, if (ent) { if (aData) { - ent->GetModifiableData()->forget(aData); + ent->mData.forget(aData); } this->RemoveEntry(ent); return true; diff --git a/xpcom/ds/nsRefPtrHashtable.h b/xpcom/ds/nsRefPtrHashtable.h index fd3e86defea7..f3e5980ac7ad 100644 --- a/xpcom/ds/nsRefPtrHashtable.h +++ b/xpcom/ds/nsRefPtrHashtable.h @@ -96,7 +96,7 @@ bool nsRefPtrHashtable::Get(KeyType aKey, if (ent) { if (aRefPtr) { - *aRefPtr = ent->GetData(); + *aRefPtr = ent->mData; NS_IF_ADDREF(*aRefPtr); } @@ -121,7 +121,7 @@ already_AddRefed nsRefPtrHashtable::Get( return nullptr; } - RefPtr copy = ent->GetData(); + RefPtr copy = ent->mData; return copy.forget(); } @@ -135,7 +135,7 @@ PtrType* nsRefPtrHashtable::GetWeak(KeyType aKey, *aFound = true; } - return ent->GetData(); + return ent->mData; } // Key does not exist, return nullptr and set aFound to false @@ -164,7 +164,7 @@ bool nsRefPtrHashtable::Put(KeyType aKey, return false; } - ent->SetData(aData); + ent->mData = aData; return true; } @@ -176,7 +176,7 @@ bool nsRefPtrHashtable::Remove(KeyType aKey, if (ent) { if (aRefPtr) { - ent->GetModifiableData()->forget(aRefPtr); + ent->mData.forget(aRefPtr); } this->RemoveEntry(ent); return true; diff --git a/xpcom/tests/gtest/TestHashtables.cpp b/xpcom/tests/gtest/TestHashtables.cpp index dfb94118ba49..26779f0fbd4d 100644 --- a/xpcom/tests/gtest/TestHashtables.cpp +++ b/xpcom/tests/gtest/TestHashtables.cpp @@ -17,8 +17,6 @@ #include "gtest/gtest.h" -#include - namespace TestHashtables { class TestUniChar // for nsClassHashtable @@ -37,11 +35,6 @@ class TestUniChar // for nsClassHashtable struct EntityNode { const char* mStr; // never owns buffer uint32_t mUnicode; - - bool operator<(const EntityNode& aOther) const { - return mUnicode < aOther.mUnicode || - (mUnicode == aOther.mUnicode && strcmp(mStr, aOther.mStr) < 0); - } }; static const EntityNode gEntities[] = { @@ -293,68 +286,6 @@ TEST(Hashtables, DataHashtable) ASSERT_EQ(count, uint32_t(0)); } -TEST(Hashtables, DataHashtable_STLIterators) -{ - nsDataHashtable UniToEntity(ENTITY_COUNT); - - for (auto& entity : gEntities) { - UniToEntity.Put(entity.mUnicode, entity.mStr); - } - - // operators, including conversion from iterator to const_iterator - nsDataHashtable::const_iterator ci = - UniToEntity.begin(); - ++ci; - ASSERT_EQ(1, std::distance(UniToEntity.cbegin(), ci++)); - ASSERT_EQ(2, std::distance(UniToEntity.cbegin(), ci)); - ASSERT_TRUE(ci == ci); - auto otherCi = ci; - ++otherCi; - ++ci; - ASSERT_TRUE(&*ci == &*otherCi); - - // STL algorithms (just to check that the iterator sufficiently conforms with - // the actual syntactical requirements of those algorithms). - std::for_each(UniToEntity.cbegin(), UniToEntity.cend(), - [](const auto& entry) {}); - std::find_if(UniToEntity.cbegin(), UniToEntity.cend(), - [](const auto& entry) { return entry.GetKey() == 42; }); - std::accumulate( - UniToEntity.cbegin(), UniToEntity.cend(), 0u, - [](size_t sum, const auto& entry) { return sum + entry.GetKey(); }); - std::any_of(UniToEntity.cbegin(), UniToEntity.cend(), - [](const auto& entry) { return entry.GetKey() == 42; }); - std::max_element(UniToEntity.cbegin(), UniToEntity.cend(), - [](const auto& lhs, const auto& rhs) { - return lhs.GetKey() > rhs.GetKey(); - }); - - // const range-based for - { - std::set entities(gEntities, gEntities + ENTITY_COUNT); - for (const auto& entity : - const_cast&>( - UniToEntity)) { - ASSERT_EQ(1u, - entities.erase(EntityNode{entity.GetData(), entity.GetKey()})); - } - ASSERT_TRUE(entities.empty()); - } - - // non-const range-based for - { - std::set entities(gEntities, gEntities + ENTITY_COUNT); - for (auto& entity : UniToEntity) { - ASSERT_EQ(1u, - entities.erase(EntityNode{entity.GetData(), entity.GetKey()})); - - entity.SetData(nullptr); - ASSERT_EQ(nullptr, entity.GetData()); - } - ASSERT_TRUE(entities.empty()); - } -} - TEST(Hashtables, ClassHashtable) { // check a class-hashtable @@ -388,46 +319,6 @@ TEST(Hashtables, ClassHashtable) ASSERT_EQ(count, uint32_t(0)); } -TEST(Hashtables, ClassHashtable_RangeBasedFor) -{ - // check a class-hashtable - nsClassHashtable EntToUniClass(ENTITY_COUNT); - - for (auto& entity : gEntities) { - auto* temp = new TestUniChar(entity.mUnicode); - EntToUniClass.Put(nsDependentCString(entity.mStr), temp); - } - - // const range-based for - { - std::set entities(gEntities, gEntities + ENTITY_COUNT); - for (const auto& entity : - const_cast&>( - EntToUniClass)) { - const char* str; - entity.GetKey().GetData(&str); - ASSERT_EQ(1u, - entities.erase(EntityNode{str, entity.GetData()->GetChar()})); - } - ASSERT_TRUE(entities.empty()); - } - - // non-const range-based for - { - std::set entities(gEntities, gEntities + ENTITY_COUNT); - for (auto& entity : EntToUniClass) { - const char* str; - entity.GetKey().GetData(&str); - ASSERT_EQ(1u, - entities.erase(EntityNode{str, entity.GetData()->GetChar()})); - - entity.SetData(nsAutoPtr{}); - ASSERT_EQ(nullptr, entity.GetData()); - } - ASSERT_TRUE(entities.empty()); - } -} - TEST(Hashtables, DataHashtableWithInterfaceKey) { // check a data-hashtable with an interface key diff --git a/xpcom/threads/nsEnvironment.cpp b/xpcom/threads/nsEnvironment.cpp index ae9c6b4e8779..2502ffc6f395 100644 --- a/xpcom/threads/nsEnvironment.cpp +++ b/xpcom/threads/nsEnvironment.cpp @@ -144,9 +144,9 @@ nsEnvironment::Set(const nsAString& aName, const nsAString& aValue) { } PR_SetEnv(newData.get()); - if (entry->GetData()) { - free(entry->GetData()); + if (entry->mData) { + free(entry->mData); } - entry->SetData(newData.release()); + entry->mData = newData.release(); return NS_OK; }