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

toolkit/components/url-classifier/Classifier.cpp

@@ -792,7 +792,7 @@ Classifier::ApplyUpdatesBackground(TableUpdateArray& aUpdates,
   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;
@@ -853,7 +853,7 @@ Classifier::ApplyUpdatesForeground(nsresult aBackgroundRv,
 }
 
 nsresult
-Classifier::ApplyFullHashes(TableUpdateArray& aUpdates)
+Classifier::ApplyFullHashes(ConstTableUpdateArray& aUpdates)
 {
   MOZ_ASSERT(NS_GetCurrentThread() != mUpdateThread,
              "ApplyFullHashes() MUST NOT be called on update thread");
@@ -1144,7 +1144,7 @@ Classifier::CheckValidUpdate(TableUpdateArray& aUpdates,
   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;
     }
@@ -1304,7 +1304,7 @@ Classifier::UpdateTableV4(TableUpdateArray& aUpdates,
   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)) {
@@ -1369,7 +1369,7 @@ Classifier::UpdateTableV4(TableUpdateArray& aUpdates,
 }
 
 nsresult
-Classifier::UpdateCache(RefPtr<TableUpdate> aUpdate)
+Classifier::UpdateCache(RefPtr<const TableUpdate> aUpdate)
 {
   if (!aUpdate) {
     return NS_OK;
@@ -1385,7 +1385,7 @@ Classifier::UpdateCache(RefPtr<TableUpdate> aUpdate)
 
   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 {
@@ -1394,7 +1394,7 @@ Classifier::UpdateCache(RefPtr<TableUpdate> aUpdate)
       return NS_ERROR_FAILURE;
     }
 
-    RefPtr<TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
+    RefPtr<const TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
     lookupV4->AddFullHashResponseToCache(updateV4->FullHashResponse());
   }
 

toolkit/components/url-classifier/Classifier.h

@@ -80,7 +80,7 @@ public:
   /**
    * 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
@@ -154,7 +154,7 @@ private:
   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);

toolkit/components/url-classifier/HashStore.cpp

@@ -571,7 +571,7 @@ template<class T>
 static nsresult
 Merge(ChunkSet* aStoreChunks,
       FallibleTArray<T>* aStorePrefixes,
-      ChunkSet& aUpdateChunks,
+      const ChunkSet& aUpdateChunks,
       FallibleTArray<T>& aUpdatePrefixes,
       bool aAllowMerging = false)
 {

toolkit/components/url-classifier/HashStore.h

@@ -42,6 +42,10 @@ public:
   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() {}
@@ -53,6 +57,7 @@ private:
 };
 
 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
@@ -99,21 +104,22 @@ public:
                                        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;
@@ -162,8 +168,8 @@ public:
   }
 
   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; }

toolkit/components/url-classifier/LookupCache.cpp

@@ -646,8 +646,8 @@ LookupCacheV2::GetPrefixes(FallibleTArray<uint32_t>& 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;

toolkit/components/url-classifier/LookupCache.h

@@ -288,8 +288,8 @@ public:
 
   // 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

toolkit/components/url-classifier/LookupCacheV4.cpp

@@ -282,7 +282,7 @@ LookupCacheV4::ApplyUpdate(RefPtr<TableUpdateV4> aTableUpdate,
   // 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;
 
@@ -525,7 +525,7 @@ ReadValue(nsIInputStream* aInputStream, T& aValue)
 ////////////////////////////////////////////////////////////////////////
 
 nsresult
-LookupCacheV4::WriteMetadata(TableUpdateV4* aTableUpdate)
+LookupCacheV4::WriteMetadata(RefPtr<const TableUpdateV4> aTableUpdate)
 {
   NS_ENSURE_ARG_POINTER(aTableUpdate);
   if (nsUrlClassifierDBService::ShutdownHasStarted()) {

toolkit/components/url-classifier/LookupCacheV4.h

@@ -43,7 +43,7 @@ public:
 
   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;

toolkit/components/url-classifier/nsUrlClassifierDBService.cpp

@@ -873,7 +873,7 @@ nsUrlClassifierDBServiceWorker::CacheCompletions(CacheResultArray *results)
     LOG(("Active tables: %s", s.get()));
   }
 
-  TableUpdateArray updates;
+  ConstTableUpdateArray updates;
 
   for (uint32_t i = 0; i < resultsPtr->Length(); i++) {
     bool activeTable = false;

toolkit/components/url-classifier/tests/gtest/TestProtocolParser.cpp

@@ -89,8 +89,8 @@ TEST(UrlClassifierProtocolParser, SingleValueEncoding)
   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.