Merge m-c to inbound. a=merge

toolkit/components/url-classifier/Classifier.cpp

@@ -627,19 +627,6 @@ 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()
 {
@@ -1478,13 +1465,6 @@ 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,8 +140,6 @@ 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,16 +356,6 @@ 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 = mFullHashCache.Get(prefix);
+  CachedFullHashResponse* fullHashResponse = mCache.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) {
-          mFullHashCache.Remove(prefix);
+          mCache.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) {
-      mFullHashCache.Remove(prefix);
+      mCache.Remove(prefix);
     }
   }
 
@@ -209,7 +209,7 @@ LookupCache::InvalidateExpiredCacheEntries()
 {
   int64_t nowSec = PR_Now() / PR_USEC_PER_SEC;
 
-  for (auto iter = mFullHashCache.Iter(); !iter.Done(); iter.Next()) {
+  for (auto iter = mCache.Iter(); !iter.Done(); iter.Next()) {
     CachedFullHashResponse* response = iter.Data();
     if (response->negativeCacheExpirySec < nowSec) {
       iter.Remove();
@@ -217,19 +217,10 @@ LookupCache::InvalidateExpiredCacheEntries()
   }
 }
 
-void
-LookupCache::CopyFullHashCache(const LookupCache* aSource)
-{
-  MOZ_ASSERT(aSource);
-
-  CopyClassHashTable<FullHashResponseMap>(aSource->mFullHashCache,
-                                          mFullHashCache);
-}
-
 void
 LookupCache::ClearCache()
 {
-  mFullHashCache.Clear();
+  mCache.Clear();
 }
 
 void
@@ -248,7 +239,7 @@ LookupCache::GetCacheInfo(nsIUrlClassifierCacheInfo** aCache)
   RefPtr<nsUrlClassifierCacheInfo> info = new nsUrlClassifierCacheInfo;
   info->table = mTableName;
 
-  for (auto iter = mFullHashCache.ConstIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = mCache.ConstIter(); !iter.Done(); iter.Next()) {
     RefPtr<nsUrlClassifierCacheEntry> entry = new nsUrlClassifierCacheEntry;
 
     // Set prefix of the cache entry.
@@ -514,7 +505,7 @@ LookupCache::DumpCache()
     return;
   }
 
-  for (auto iter = mFullHashCache.ConstIter(); !iter.Done(); iter.Next()) {
+  for (auto iter = mCache.ConstIter(); !iter.Done(); iter.Next()) {
     CachedFullHashResponse* response = iter.Data();
 
     nsAutoCString prefix;
@@ -658,8 +649,7 @@ LookupCacheV2::AddGethashResultToCache(AddCompleteArray& aAddCompletes,
     nsDependentCSubstring fullhash(
       reinterpret_cast<const char*>(add.CompleteHash().buf), COMPLETE_SIZE);
 
-    CachedFullHashResponse* response =
-      mFullHashCache.LookupOrAdd(add.ToUint32());
+    CachedFullHashResponse* response = mCache.LookupOrAdd(add.ToUint32());
     response->negativeCacheExpirySec = defaultExpirySec;
 
     FullHashExpiryCache& fullHashes = response->fullHashes;
@@ -667,9 +657,7 @@ LookupCacheV2::AddGethashResultToCache(AddCompleteArray& aAddCompletes,
   }
 
   for (const Prefix& prefix : aMissPrefixes) {
-    CachedFullHashResponse* response =
-      mFullHashCache.LookupOrAdd(prefix.ToUint32());
-
+    CachedFullHashResponse* response = mCache.LookupOrAdd(prefix.ToUint32());
     response->negativeCacheExpirySec = defaultExpirySec;
   }
 }

toolkit/components/url-classifier/LookupCache.h

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

toolkit/components/url-classifier/LookupCacheV4.cpp

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

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

@@ -558,6 +558,46 @@ 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"];
@@ -695,6 +735,7 @@ function run_test()
       testCachedResults,
       testCachedResultsWithSub,
       testCachedResultsWithExpire,
+      testCachedResultsUpdate,
       testCachedResultsFailure,
       testErrorList,
       testErrorListIndependent