Bug 1434206 - Make TableUpdate objects const as much as possible. r?gcp
I tried to make TableUpdateArray point to const TableUpdate objects
everywhere but there were two problems:
- HashStore::ApplyUpdate() triggers a few Merge() calls which include
sorting the underlying TableUpdate object first.
- LookupCacheV4::ApplyUpdate() calls TableUpdateV4::NewChecksum() when the
checksum is missing and that sets mChecksum.
MozReview-Commit-ID: LIhJcoxo7e7
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -787,17 +787,17 @@ Classifier::ApplyUpdatesBackground(Table
if (NS_FAILED(rv)) {
LOG(("Failed to copy in-use directory for update."));
return rv;
}
LOG(("Applying %zu table updates.", aUpdates.Length()));
for (uint32_t i = 0; i < aUpdates.Length(); i++) {
- RefPtr<TableUpdate> update = aUpdates[i];
+ RefPtr<const TableUpdate> update = aUpdates[i];
if (!update) {
// Previous UpdateHashStore() may have consumed this update..
continue;
}
// Run all updates for one table
nsAutoCString updateTable(update->TableName());
@@ -848,17 +848,17 @@ Classifier::ApplyUpdatesForeground(nsres
}
if (NS_ERROR_OUT_OF_MEMORY != aBackgroundRv) {
ResetTables(Clear_All, nsTArray<nsCString> { nsCString(aFailedTableName) });
}
return aBackgroundRv;
}
nsresult
-Classifier::ApplyFullHashes(TableUpdateArray& aUpdates)
+Classifier::ApplyFullHashes(ConstTableUpdateArray& aUpdates)
{
MOZ_ASSERT(NS_GetCurrentThread() != mUpdateThread,
"ApplyFullHashes() MUST NOT be called on update thread");
MOZ_ASSERT(!NS_IsMainThread(),
"ApplyFullHashes() must be called on the classifier worker thread.");
LOG(("Applying %zu table gethashes.", aUpdates.Length()));
@@ -1139,17 +1139,17 @@ bool
Classifier::CheckValidUpdate(TableUpdateArray& aUpdates,
const nsACString& aTable)
{
// take the quick exit if there is no valid update for us
// (common case)
uint32_t validupdates = 0;
for (uint32_t i = 0; i < aUpdates.Length(); i++) {
- RefPtr<TableUpdate> update = aUpdates[i];
+ RefPtr<const TableUpdate> update = aUpdates[i];
if (!update || !update->TableName().Equals(aTable)) {
continue;
}
if (update->Empty()) {
aUpdates[i] = nullptr;
continue;
}
validupdates++;
@@ -1299,17 +1299,17 @@ Classifier::UpdateTableV4(TableUpdateArr
nsresult rv = NS_OK;
// If there are multiple updates for the same table, prefixes1 & prefixes2
// will act as input and output in turn to reduce memory copy overhead.
PrefixStringMap prefixes1, prefixes2;
PrefixStringMap* input = &prefixes1;
PrefixStringMap* output = &prefixes2;
- TableUpdateV4* lastAppliedUpdate = nullptr;
+ RefPtr<const TableUpdateV4> lastAppliedUpdate = nullptr;
for (uint32_t i = 0; i < aUpdates.Length(); i++) {
RefPtr<TableUpdate> update = aUpdates[i];
if (!update || !update->TableName().Equals(aTable)) {
continue;
}
RefPtr<TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(update);
NS_ENSURE_TRUE(updateV4, NS_ERROR_UC_UPDATE_UNEXPECTED_VERSION);
@@ -1364,42 +1364,42 @@ Classifier::UpdateTableV4(TableUpdateArr
}
LOG(("Successfully updated %s\n", PromiseFlatCString(aTable).get()));
return NS_OK;
}
nsresult
-Classifier::UpdateCache(RefPtr<TableUpdate> aUpdate)
+Classifier::UpdateCache(RefPtr<const TableUpdate> aUpdate)
{
if (!aUpdate) {
return NS_OK;
}
nsAutoCString table(aUpdate->TableName());
LOG(("Classifier::UpdateCache(%s)", table.get()));
LookupCache *lookupCache = GetLookupCache(table);
if (!lookupCache) {
return NS_ERROR_FAILURE;
}
auto lookupV2 = LookupCache::Cast<LookupCacheV2>(lookupCache);
if (lookupV2) {
- RefPtr<TableUpdateV2> updateV2 = TableUpdate::Cast<TableUpdateV2>(aUpdate);
+ RefPtr<const TableUpdateV2> updateV2 = TableUpdate::Cast<TableUpdateV2>(aUpdate);
lookupV2->AddGethashResultToCache(updateV2->AddCompletes(),
updateV2->MissPrefixes());
} else {
auto lookupV4 = LookupCache::Cast<LookupCacheV4>(lookupCache);
if (!lookupV4) {
return NS_ERROR_FAILURE;
}
- RefPtr<TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
+ RefPtr<const TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
lookupV4->AddFullHashResponseToCache(updateV4->FullHashResponse());
}
#if defined(DEBUG)
lookupCache->DumpCache();
#endif
return NS_OK;
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -75,17 +75,17 @@ public:
* is fired. Once this function returns, AsyncApplyUpdates is
* no longer available.
*/
void FlushAndDisableAsyncUpdate();
/**
* Apply full hashes retrived from gethash to cache.
*/
- nsresult ApplyFullHashes(TableUpdateArray& aUpdates);
+ nsresult ApplyFullHashes(ConstTableUpdateArray& aUpdates);
/*
* Get a bunch of extra prefixes to query for completion
* and mask the real entry being requested
*/
nsresult ReadNoiseEntries(const Prefix& aPrefix,
const nsACString& aTableName,
uint32_t aCount,
@@ -149,17 +149,17 @@ private:
nsresult ScanStoreDir(nsIFile* aDirectory, nsTArray<nsCString>& aTables);
nsresult UpdateHashStore(TableUpdateArray& aUpdates,
const nsACString& aTable);
nsresult UpdateTableV4(TableUpdateArray& aUpdates,
const nsACString& aTable);
- nsresult UpdateCache(RefPtr<TableUpdate> aUpdates);
+ nsresult UpdateCache(RefPtr<const TableUpdate> aUpdates);
LookupCache *GetLookupCacheForUpdate(const nsACString& aTable) {
return GetLookupCache(aTable, true);
}
LookupCache *GetLookupCacheFrom(const nsACString& aTable,
nsTArray<LookupCache*>& aLookupCaches,
nsIFile* aRootStoreDirectory);
--- a/toolkit/components/url-classifier/HashStore.cpp
+++ b/toolkit/components/url-classifier/HashStore.cpp
@@ -566,17 +566,17 @@ HashStore::BeginUpdate()
return NS_OK;
}
template<class T>
static nsresult
Merge(ChunkSet* aStoreChunks,
FallibleTArray<T>* aStorePrefixes,
- ChunkSet& aUpdateChunks,
+ const ChunkSet& aUpdateChunks,
FallibleTArray<T>& aUpdatePrefixes,
bool aAllowMerging = false)
{
EntrySort(aUpdatePrefixes);
auto storeIter = aStorePrefixes->begin();
auto storeEnd = aStorePrefixes->end();
--- a/toolkit/components/url-classifier/HashStore.h
+++ b/toolkit/components/url-classifier/HashStore.h
@@ -37,27 +37,32 @@ public:
// Common interfaces.
const nsCString& TableName() const { return mTable; }
template<typename T>
static T* Cast(TableUpdate* aThat) {
return (T::TAG == aThat->Tag() ? reinterpret_cast<T*>(aThat) : nullptr);
}
+ template<typename T>
+ static const T* Cast(const TableUpdate* aThat) {
+ return (T::TAG == aThat->Tag() ? reinterpret_cast<const T*>(aThat) : nullptr);
+ }
protected:
virtual ~TableUpdate() {}
private:
virtual int Tag() const = 0;
const nsCString mTable;
};
typedef nsTArray<RefPtr<TableUpdate>> TableUpdateArray;
+typedef nsTArray<RefPtr<const TableUpdate>> ConstTableUpdateArray;
// A table update is built from a single update chunk from the server. As the
// protocol parser processes each chunk, it constructs a table update with the
// new hashes.
class TableUpdateV2 : public TableUpdate {
public:
explicit TableUpdateV2(const nsACString& aTable)
: TableUpdate(aTable) {}
@@ -94,31 +99,32 @@ public:
uint32_t aSubChunk);
MOZ_MUST_USE nsresult NewAddComplete(uint32_t aChunk,
const Completion& aCompletion);
MOZ_MUST_USE nsresult NewSubComplete(uint32_t aAddChunk,
const Completion& aCompletion,
uint32_t aSubChunk);
MOZ_MUST_USE nsresult NewMissPrefix(const Prefix& aPrefix);
- ChunkSet& AddChunks() { return mAddChunks; }
- ChunkSet& SubChunks() { return mSubChunks; }
+ const ChunkSet& AddChunks() const { return mAddChunks; }
+ const ChunkSet& SubChunks() const { return mSubChunks; }
// Expirations for chunks.
- ChunkSet& AddExpirations() { return mAddExpirations; }
- ChunkSet& SubExpirations() { return mSubExpirations; }
+ const ChunkSet& AddExpirations() const { return mAddExpirations; }
+ const ChunkSet& SubExpirations() const { return mSubExpirations; }
// Hashes associated with this chunk.
AddPrefixArray& AddPrefixes() { return mAddPrefixes; }
SubPrefixArray& SubPrefixes() { return mSubPrefixes; }
+ const AddCompleteArray& AddCompletes() const { return mAddCompletes; }
AddCompleteArray& AddCompletes() { return mAddCompletes; }
SubCompleteArray& SubCompletes() { return mSubCompletes; }
// Entries that cannot be completed.
- MissPrefixArray& MissPrefixes() { return mMissPrefixes; }
+ const MissPrefixArray& MissPrefixes() const { return mMissPrefixes; }
// For downcasting.
static const int TAG = 2;
private:
// The list of chunk numbers that we have for each of the type of chunks.
ChunkSet mAddChunks;
@@ -157,18 +163,18 @@ public:
bool Empty() const override
{
return mPrefixesMap.IsEmpty() &&
mRemovalIndiceArray.IsEmpty() &&
mFullHashResponseMap.IsEmpty();
}
bool IsFullUpdate() const { return mFullUpdate; }
- const PrefixStringMap& Prefixes() { return mPrefixesMap; }
- RemovalIndiceArray& RemovalIndices() { return mRemovalIndiceArray; }
+ const PrefixStringMap& Prefixes() const { return mPrefixesMap; }
+ const RemovalIndiceArray& RemovalIndices() const { return mRemovalIndiceArray; }
const nsACString& ClientState() const { return mClientState; }
const nsACString& Checksum() const { return mChecksum; }
const FullHashResponseMap& FullHashResponse() const { return mFullHashResponseMap; }
// For downcasting.
static const int TAG = 4;
void SetFullUpdate(bool aIsFullUpdate) { mFullUpdate = aIsFullUpdate; }
--- a/toolkit/components/url-classifier/LookupCache.cpp
+++ b/toolkit/components/url-classifier/LookupCache.cpp
@@ -641,18 +641,18 @@ LookupCacheV2::GetPrefixes(FallibleTArra
// This can happen if its a new table, so no error.
LOG(("GetPrefixes from empty LookupCache"));
return NS_OK;
}
return mPrefixSet->GetPrefixesNative(aAddPrefixes);
}
void
-LookupCacheV2::AddGethashResultToCache(AddCompleteArray& aAddCompletes,
- MissPrefixArray& aMissPrefixes,
+LookupCacheV2::AddGethashResultToCache(const AddCompleteArray& aAddCompletes,
+ const MissPrefixArray& aMissPrefixes,
int64_t aExpirySec)
{
int64_t defaultExpirySec = PR_Now() / PR_USEC_PER_SEC + V2_CACHE_DURATION_SEC;
if (aExpirySec != 0) {
defaultExpirySec = aExpirySec;
}
for (const AddComplete& add : aAddCompletes) {
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -283,18 +283,18 @@ public:
nsresult Build(AddPrefixArray& aAddPrefixes,
AddCompleteArray& aAddCompletes);
nsresult GetPrefixes(FallibleTArray<uint32_t>& aAddPrefixes);
// This will Clear() the passed arrays when done.
// 'aExpirySec' is used by testcase to config an expired time.
- void AddGethashResultToCache(AddCompleteArray& aAddCompletes,
- MissPrefixArray& aMissPrefixes,
+ void AddGethashResultToCache(const AddCompleteArray& aAddCompletes,
+ const MissPrefixArray& aMissPrefixes,
int64_t aExpirySec = 0);
#if DEBUG
void DumpCompletions();
#endif
static const int VER;
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -277,17 +277,17 @@ LookupCacheV4::ApplyUpdate(RefPtr<TableU
// addPSet contains prefixes stored in tableUpdate which should be merged with oldPSet.
VLPrefixSet oldPSet(aInputMap);
VLPrefixSet addPSet(aTableUpdate->Prefixes());
// RemovalIndiceArray is a sorted integer array indicating the index of prefix we should
// remove from the old prefix set(according to lexigraphic order).
// |removalIndex| is the current index of RemovalIndiceArray.
// |numOldPrefixPicked| is used to record how many prefixes we picked from the old map.
- TableUpdateV4::RemovalIndiceArray& removalArray = aTableUpdate->RemovalIndices();
+ const TableUpdateV4::RemovalIndiceArray& removalArray = aTableUpdate->RemovalIndices();
uint32_t removalIndex = 0;
int32_t numOldPrefixPicked = -1;
nsAutoCString smallestOldPrefix;
nsAutoCString smallestAddPrefix;
bool isOldMapEmpty = false, isAddMapEmpty = false;
@@ -520,17 +520,17 @@ ReadValue(nsIInputStream* aInputStream,
return rv;
}
} // end of unnamed namespace.
////////////////////////////////////////////////////////////////////////
nsresult
-LookupCacheV4::WriteMetadata(TableUpdateV4* aTableUpdate)
+LookupCacheV4::WriteMetadata(RefPtr<const TableUpdateV4> aTableUpdate)
{
NS_ENSURE_ARG_POINTER(aTableUpdate);
if (nsUrlClassifierDBService::ShutdownHasStarted()) {
return NS_ERROR_ABORT;
}
nsCOMPtr<nsIFile> metaFile;
nsresult rv = mStoreDirectory->Clone(getter_AddRefs(metaFile));
--- a/toolkit/components/url-classifier/LookupCacheV4.h
+++ b/toolkit/components/url-classifier/LookupCacheV4.h
@@ -38,17 +38,17 @@ public:
// ApplyUpdate will merge data stored in aTableUpdate with prefixes in aInputMap.
nsresult ApplyUpdate(RefPtr<TableUpdateV4> aTableUpdate,
PrefixStringMap& aInputMap,
PrefixStringMap& aOutputMap);
nsresult AddFullHashResponseToCache(const FullHashResponseMap& aResponseMap);
- nsresult WriteMetadata(TableUpdateV4* aTableUpdate);
+ nsresult WriteMetadata(RefPtr<const TableUpdateV4> aTableUpdate);
nsresult LoadMetadata(nsACString& aState, nsACString& aChecksum);
static const int VER;
static const uint32_t MAX_METADATA_VALUE_LENGTH;
protected:
virtual nsresult ClearPrefixes() override;
virtual nsresult StoreToFile(nsIFile* aFile) override;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -868,17 +868,17 @@ nsUrlClassifierDBServiceWorker::CacheCom
if (!s.IsEmpty()) {
s += ",";
}
s += tables[i];
}
LOG(("Active tables: %s", s.get()));
}
- TableUpdateArray updates;
+ ConstTableUpdateArray updates;
for (uint32_t i = 0; i < resultsPtr->Length(); i++) {
bool activeTable = false;
CacheResult* result = resultsPtr->ElementAt(i).get();
for (uint32_t table = 0; table < tables.Length(); table++) {
if (tables[table].Equals(result->table)) {
activeTable = true;
--- a/toolkit/components/url-classifier/tests/gtest/TestProtocolParser.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestProtocolParser.cpp
@@ -84,18 +84,18 @@ TEST(UrlClassifierProtocolParser, Single
response.SerializeToString(&s);
// Feed data to the protocol parser.
ProtocolParser* p = new ProtocolParserProtobuf();
p->SetRequestedTables({ nsCString("googpub-phish-proto") });
p->AppendStream(nsCString(s.c_str(), s.length()));
p->End();
- auto& tus = p->GetTableUpdates();
- auto tuv4 = TableUpdate::Cast<TableUpdateV4>(tus[0]);
+ const TableUpdateArray& tus = p->GetTableUpdates();
+ RefPtr<const TableUpdateV4> tuv4 = TableUpdate::Cast<TableUpdateV4>(tus[0]);
auto& prefixMap = tuv4->Prefixes();
for (auto iter = prefixMap.ConstIter(); !iter.Done(); iter.Next()) {
// This prefix map should contain only a single 4-byte prefixe.
ASSERT_EQ(iter.Key(), 4u);
// The fixed-length prefix string from ProtocolParser should
// exactly match the expected prefix string.
nsCString* prefix = iter.Data();