Bug 1359299 - Copy fullhash cache when update. r=hchang

After adopting the new thread model for safebrowsing, we will create a new
lookup cache for update so we can still check lookup cache at the same time.

Prefix set, completions will be generated when we open the new lookup cache
but it won't include cache, so we will loss cache after that.

This patch will copy cache data from old lookup cache to new lookup
cache while update.

MozReview-Commit-ID: L0WpiHOGIGm

--HG--
extra : rebase_source : ebaf249535561b87a983a3f24dacb8fbab7c096e

toolkit/components/url-classifier/Classifier.cpp

@@ -627,6 +627,19 @@ Classifier::RemoveUpdateIntermediaries()
   }
 }
 
+void
+Classifier::CopyFullHashCacheToNewLookupCache(LookupCache* aNewLookupCache)
+{
+  MOZ_ASSERT(aNewLookupCache);
+
+  for (auto c: mLookupCaches) {
+    if (c->TableName() == aNewLookupCache->TableName()) {
+      aNewLookupCache->CopyFullHashCache(c);
+      return;
+    }
+  }
+}
+
 void
 Classifier::MergeNewLookupCaches()
 {
@@ -1465,6 +1478,13 @@ Classifier::GetLookupCache(const nsACString& aTable, bool aForUpdate)
   }
   rv = cache->Open();
   if (NS_SUCCEEDED(rv)) {
+    if (aForUpdate) {
+      // Since update algorithm will invalidate expired cache entries, we need
+      // to copy fullhash cache in "old LookupCache" to "update Lookupcache" before
+      // applying an update.
+      CopyFullHashCacheToNewLookupCache(cache.get());
+    }
+
     lookupCaches.AppendElement(cache.get());
     return cache.release();
   }

toolkit/components/url-classifier/Classifier.h

@@ -140,6 +140,8 @@ private:
 
   void MergeNewLookupCaches(); // Merge mNewLookupCaches into mLookupCaches.
 
+  void CopyFullHashCacheToNewLookupCache(LookupCache* aNewLookupCache);
+
   // Remove any intermediary for update, including in-memory
   // and on-disk data.
   void RemoveUpdateIntermediaries();

toolkit/components/url-classifier/Entries.h

@@ -356,6 +356,16 @@ struct CachedFullHashResponse {
 
 typedef nsClassHashtable<nsUint32HashKey, CachedFullHashResponse> FullHashResponseMap;
 
+template<class T>
+void
+CopyClassHashTable(const T& aSource, T& aDestination)
+{
+  for (auto iter = aSource.ConstIter(); !iter.Done(); iter.Next()) {
+    auto value = aDestination.LookupOrAdd(iter.Key());
+    *value = *(iter.Data());
+  }
+}
+
 } // namespace safebrowsing
 } // namespace mozilla
 

toolkit/components/url-classifier/LookupCache.cpp

@@ -148,7 +148,7 @@ LookupCache::CheckCache(const Completion& aCompletion,
 
   uint32_t prefix = aCompletion.ToUint32();
 
-  CachedFullHashResponse* fullHashResponse = mCache.Get(prefix);
+  CachedFullHashResponse* fullHashResponse = mFullHashCache.Get(prefix);
   if (!fullHashResponse) {
     return NS_OK;
   }
@@ -178,7 +178,7 @@ LookupCache::CheckCache(const Completion& aCompletion,
         fullHashes.Remove(completion);
         if (fullHashes.Count() == 0 &&
             fullHashResponse->negativeCacheExpirySec < nowSec) {
-          mCache.Remove(prefix);
+          mFullHashCache.Remove(prefix);
         }
       }
     }
@@ -193,7 +193,7 @@ LookupCache::CheckCache(const Completion& aCompletion,
   } else {
     LOG(("Found an expired prefix in the negative cache"));
     if (fullHashes.Count() == 0) {
-      mCache.Remove(prefix);
+      mFullHashCache.Remove(prefix);
     }
   }
 
@@ -209,7 +209,7 @@ LookupCache::InvalidateExpiredCacheEntries()
 {
   int64_t nowSec = PR_Now() / PR_USEC_PER_SEC;
 
-  for (auto iter = mCache.Iter(); !iter.Done(); iter.Next()) {
+  for (auto iter = mFullHashCache.Iter(); !iter.Done(); iter.Next()) {
     CachedFullHashResponse* response = iter.Data();
     if (response->negativeCacheExpirySec < nowSec) {
       iter.Remove();
@@ -217,10 +217,19 @@ LookupCache::InvalidateExpiredCacheEntries()
   }
 }
 
+void
+LookupCache::CopyFullHashCache(const LookupCache* aSource)
+{
+  MOZ_ASSERT(aSource);
+
+  CopyClassHashTable<FullHashResponseMap>(aSource->mFullHashCache,
+                                          mFullHashCache);
+}
+
 void
 LookupCache::ClearCache()
 {
-  mCache.Clear();
+  mFullHashCache.Clear();
 }
 
 void
@@ -239,7 +248,7 @@ LookupCache::GetCacheInfo(nsIUrlClassifierCacheInfo** aCache)
   RefPtr<nsUrlClassifierCacheInfo> info = new nsUrlClassifierCacheInfo;
   info->table = mTableName;
 
-  for (auto iter = mCache.ConstIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = mFullHashCache.ConstIter(); !iter.Done(); iter.Next()) {
     RefPtr<nsUrlClassifierCacheEntry> entry = new nsUrlClassifierCacheEntry;
 
     // Set prefix of the cache entry.
@@ -505,7 +514,7 @@ LookupCache::DumpCache()
     return;
   }
 
-  for (auto iter = mCache.ConstIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = mFullHashCache.ConstIter(); !iter.Done(); iter.Next()) {
     CachedFullHashResponse* response = iter.Data();
 
     nsAutoCString prefix;
@@ -640,7 +649,7 @@ LookupCacheV2::AddGethashResultToCache(AddCompleteArray& aAddCompletes,
                                        MissPrefixArray& aMissPrefixes,
                                        int64_t aExpirySec)
 {
-  int64_t defaultExpirySec  = PR_Now() / PR_USEC_PER_SEC + V2_CACHE_DURATION_SEC;
+  int64_t defaultExpirySec = PR_Now() / PR_USEC_PER_SEC + V2_CACHE_DURATION_SEC;
   if (aExpirySec != 0) {
     defaultExpirySec = aExpirySec;
   }
@@ -649,7 +658,8 @@ LookupCacheV2::AddGethashResultToCache(AddCompleteArray& aAddCompletes,
     nsDependentCSubstring fullhash(
       reinterpret_cast<const char*>(add.CompleteHash().buf), COMPLETE_SIZE);
 
-    CachedFullHashResponse* response = mCache.LookupOrAdd(add.ToUint32());
+    CachedFullHashResponse* response =
+      mFullHashCache.LookupOrAdd(add.ToUint32());
     response->negativeCacheExpirySec = defaultExpirySec;
 
     FullHashExpiryCache& fullHashes = response->fullHashes;
@@ -657,7 +667,9 @@ LookupCacheV2::AddGethashResultToCache(AddCompleteArray& aAddCompletes,
   }
 
   for (const Prefix& prefix : aMissPrefixes) {
-    CachedFullHashResponse* response = mCache.LookupOrAdd(prefix.ToUint32());
+    CachedFullHashResponse* response =
+      mFullHashCache.LookupOrAdd(prefix.ToUint32());
+
     response->negativeCacheExpirySec = defaultExpirySec;
   }
 }

toolkit/components/url-classifier/LookupCache.h

@@ -199,12 +199,15 @@ public:
   // Called when update to clear expired entries.
   void InvalidateExpiredCacheEntries();
 
-  // Clear completions retrieved from gethash request.
+  // Copy fullhash cache from another LookupCache.
+  void CopyFullHashCache(const LookupCache* aSource);
+
+  // Clear fullhash cache from fullhash/gethash response.
   void ClearCache();
 
   // Check if completions can be found in cache.
   // Currently this is only used by testcase.
-  bool IsInCache(uint32_t key) { return mCache.Get(key); };
+  bool IsInCache(uint32_t key) { return mFullHashCache.Get(key); };
 
 #if DEBUG
   void DumpCache();
@@ -254,8 +257,8 @@ protected:
   // For gtest to inspect private members.
   friend class PerProviderDirectoryTestUtils;
 
-  // Cache gethash result.
-  FullHashResponseMap mCache;
+  // Cache stores fullhash response(V4)/gethash response(V2)
+  FullHashResponseMap mFullHashCache;
 };
 
 class LookupCacheV2 final : public LookupCache

toolkit/components/url-classifier/LookupCacheV4.cpp

@@ -330,10 +330,7 @@ LookupCacheV4::ApplyUpdate(TableUpdateV4* aTableUpdate,
 nsresult
 LookupCacheV4::AddFullHashResponseToCache(const FullHashResponseMap& aResponseMap)
 {
-  for (auto iter = aResponseMap.ConstIter(); !iter.Done(); iter.Next()) {
-    CachedFullHashResponse* response = mCache.LookupOrAdd(iter.Key());
-    *response = *(iter.Data());
-  }
+  CopyClassHashTable<FullHashResponseMap>(aResponseMap, mFullHashCache);
 
   return NS_OK;
 }

toolkit/components/url-classifier/tests/unit/test_partial.js

@@ -558,46 +558,6 @@ function testCachedResultsWithExpire() {
     });
 }
 
-function testCachedResultsUpdate()
-{
-  var existUrls = ["foo.com/a"];
-  setupCachedResults(existUrls, function() {
-    // This is called after setupCachedResults().  Verify that
-    // checking the url again does not cause a completer request.
-
-    // install a new completer, this one should never be queried.
-    var newCompleter = installCompleter('test-phish-simple', [[1, []]], []);
-
-    var assertions = {
-      "urlsExist" : existUrls,
-      "completerQueried" : [newCompleter, []]
-    };
-
-    var addUrls = ["foobar.org/a"];
-
-    var update2 = buildPhishingUpdate(
-        [
-          { "chunkNum" : 2,
-            "urls" : addUrls
-          }],
-        4);
-
-    checkAssertions(assertions, function () {
-      // Apply the update. The cached completes should be gone.
-      doStreamUpdate(update2, function() {
-        // Now the completer gets queried again.
-        var newCompleter2 = installCompleter('test-phish-simple', [[1, existUrls]], []);
-        var assertions2 = {
-          "tableData" : "test-phish-simple;a:1-2",
-          "urlsExist" : existUrls,
-          "completerQueried" : [newCompleter2, existUrls]
-        };
-        checkAssertions(assertions2, runNextTest);
-      }, updateError);
-    });
-  });
-}
-
 function testCachedResultsFailure()
 {
   var existUrls = ["foo.com/a"];
@@ -735,7 +695,6 @@ function run_test()
       testCachedResults,
       testCachedResultsWithSub,
       testCachedResultsWithExpire,
-      testCachedResultsUpdate,
       testCachedResultsFailure,
       testErrorList,
       testErrorListIndependent