diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index 7ee9eb4f7d5a..1401f7ee2539 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -229,14 +229,6 @@ ValueObserver::Observe(nsISupports *aSubject, return NS_OK; } -// We want to have an atomic pointer to the preference data that should be -// written. Make a struct so that we can capture the array and the length. -struct PrefWriteData -{ - UniquePtr mData; - uint32_t mCount; -}; - // Write the preference data to a file. // class PreferencesWriter final @@ -246,10 +238,8 @@ public: { } - // Note that this method will free the contents of aPrefs, but not - // the aPrefs itself. static - nsresult Write(nsIFile* aFile, char* aPrefs[], uint32_t aPrefCount) + nsresult Write(nsIFile* aFile, PrefSaveData& aPrefs) { nsCOMPtr outStreamSink; nsCOMPtr outStream; @@ -270,20 +260,31 @@ public: return rv; } + struct CharComparator + { + bool LessThan(const mozilla::UniqueFreePtr& a, + const mozilla::UniqueFreePtr& b) const + { + return strcmp(a.get(), b.get()) < 0; + } + bool Equals(const mozilla::UniqueFreePtr& a, + const mozilla::UniqueFreePtr& b) const + { + return strcmp(a.get(), b.get()) == 0; + } + }; + /* Sort the preferences to make a readable file on disk */ - NS_QuickSort(aPrefs, aPrefCount, sizeof(char *), - pref_CompareStrings, nullptr); + aPrefs.Sort(CharComparator()); // write out the file header outStream->Write(kPrefFileHeader, sizeof(kPrefFileHeader) - 1, &writeAmount); - for (uint32_t valueIdx = 0; valueIdx < aPrefCount; valueIdx++) { - char*& pref = aPrefs[valueIdx]; + for (auto& prefptr : aPrefs) { + char* pref = prefptr.get(); MOZ_ASSERT(pref); outStream->Write(pref, strlen(pref), &writeAmount); outStream->Write(NS_LINEBREAK, NS_LINEBREAK_LEN, &writeAmount); - free(pref); - pref = nullptr; } // tell the safe output stream to overwrite the real prefs file @@ -322,9 +323,9 @@ public: // This is the data that all of the runnables (see below) will attempt // to write. It will always have the most up to date version, or be // null, if the up to date information has already been written out. - static Atomic sPendingWriteData; + static Atomic sPendingWriteData; }; -Atomic PreferencesWriter::sPendingWriteData((PrefWriteData*)0); +Atomic PreferencesWriter::sPendingWriteData(nullptr); class PWRunnable : public Runnable { @@ -333,16 +334,13 @@ public: NS_IMETHOD Run() override { - PrefWriteData* prefs = PreferencesWriter::sPendingWriteData.exchange(nullptr); + mozilla::UniquePtr prefs(PreferencesWriter::sPendingWriteData.exchange(nullptr)); // If we get a nullptr on the exchange, it means that somebody // else has already processed the request, and we can just return. nsresult rv = NS_OK; if (prefs) { - // The ::Write call will zero out the prefs->mData array, but not free it. - // The UniquePtr going out of scope will do that when we free prefs. - rv = PreferencesWriter::Write(mFile, prefs->mData.get(), prefs->mCount); - delete prefs; + rv = PreferencesWriter::Write(mFile, *prefs); // Make a copy of these so we can have them in runnable lambda. // nsIFile is only there so that we would never release the @@ -1216,16 +1214,15 @@ Preferences::WritePrefFile(nsIFile* aFile, SaveMethod aSaveMethod) if (AllowOffMainThreadSave()) { nsresult rv = NS_OK; - PrefWriteData* prefs = new PrefWriteData; - prefs->mData = pref_savePrefs(gHashTable, &prefs->mCount); + mozilla::UniquePtr prefs = + MakeUnique(pref_savePrefs(gHashTable)); // Put the newly constructed preference data into sPendingWriteData // for the next request to pick up - prefs = PreferencesWriter::sPendingWriteData.exchange(prefs); + prefs.reset(PreferencesWriter::sPendingWriteData.exchange(prefs.release())); if (prefs) { // There was a previous request that hasn't been processed, - // and this is the data it had. Delete the old data and return. - delete prefs; + // and this is the data it had. return rv; } else { // There were no previous requests, dispatch one since @@ -1249,12 +1246,8 @@ Preferences::WritePrefFile(nsIFile* aFile, SaveMethod aSaveMethod) // This will do a main thread write. It is safe to do it this way // as AllowOffMainThreadSave() returns a consistent value for the // lifetime of the parent process. - uint32_t prefCount = 0; - UniquePtr prefsData = pref_savePrefs(gHashTable, &prefCount); - - // The ::Write call will zero out the prefsData array, but not free it. - // The UniquePtr going out of scope will do that. - return PreferencesWriter::Write(aFile, prefsData.get(), prefCount); + PrefSaveData prefsData = pref_savePrefs(gHashTable); + return PreferencesWriter::Write(aFile, prefsData); } static nsresult openPrefFile(nsIFile* aFile) diff --git a/modules/libpref/prefapi.cpp b/modules/libpref/prefapi.cpp index ad46574772cb..d6dd816298cb 100644 --- a/modules/libpref/prefapi.cpp +++ b/modules/libpref/prefapi.cpp @@ -312,20 +312,11 @@ pref_SetPref(const dom::PrefSetting& aPref) return rv; } -UniquePtr -pref_savePrefs(PLDHashTable* aTable, uint32_t* aPrefCount) +PrefSaveData +pref_savePrefs(PLDHashTable* aTable) { - // This function allocates the entries in the savedPrefs array it returns. - // It is the callers responsibility to go through the array and free - // all of them. The aPrefCount entries will be non-null. Any end padding - // is an implementation detail and may change. - MOZ_ASSERT(aPrefCount); - auto savedPrefs = MakeUnique(aTable->EntryCount()); + PrefSaveData savedPrefs(aTable->EntryCount()); - // This is not necessary, but leaving it in for now - memset(savedPrefs.get(), 0, aTable->EntryCount() * sizeof(char*)); - - int32_t j = 0; for (auto iter = aTable->Iter(); !iter.Done(); iter.Next()) { auto pref = static_cast(iter.Get()); @@ -364,14 +355,13 @@ pref_savePrefs(PLDHashTable* aTable, uint32_t* aPrefCount) nsAutoCString prefName; str_escape(pref->key, prefName); - savedPrefs[j++] = ToNewCString(prefPrefix + - prefName + - NS_LITERAL_CSTRING("\", ") + - prefValue + - NS_LITERAL_CSTRING(");")); + savedPrefs.AppendElement()-> + reset(ToNewCString(prefPrefix + + prefName + + NS_LITERAL_CSTRING("\", ") + + prefValue + + NS_LITERAL_CSTRING(");"))); } - *aPrefCount = j; - return savedPrefs; } @@ -452,24 +442,6 @@ pref_GetPrefFromEntry(PrefHashEntry *aHashEntry, dom::PrefSetting* aPref) aPref->userValue().get_PrefValue().type())); } - -int -pref_CompareStrings(const void *v1, const void *v2, void *unused) -{ - char *s1 = *(char**) v1; - char *s2 = *(char**) v2; - - if (!s1) - { - if (!s2) - return 0; - return -1; - } - if (!s2) - return 1; - return strcmp(s1, s2); -} - bool PREF_HasUserPref(const char *pref_name) { if (!gHashTable) diff --git a/modules/libpref/prefapi_private_data.h b/modules/libpref/prefapi_private_data.h index 170b79399523..fc3cd9b11eeb 100644 --- a/modules/libpref/prefapi_private_data.h +++ b/modules/libpref/prefapi_private_data.h @@ -20,8 +20,11 @@ class PrefSetting; } // namespace dom } // namespace mozilla -mozilla::UniquePtr -pref_savePrefs(PLDHashTable* aTable, uint32_t* aPrefCount); + +typedef nsTArray > PrefSaveData; + +PrefSaveData +pref_savePrefs(PLDHashTable* aTable); nsresult pref_SetPref(const mozilla::dom::PrefSetting& aPref); @@ -37,7 +40,6 @@ void pref_SetWatchingPref(bool watching); #endif -int pref_CompareStrings(const void *v1, const void *v2, void* unused); PrefHashEntry* pref_HashTableLookup(const char *key); bool