Bug 1311933 - P1. Use integer as the key of safebrowsing cache. r=francois

In Bug 1323953, we always send 4-bytes prefix for completion and the prefix is also
used as the key to store cache result from gethash request.
Since it is always 4-bytes, we could convert it to integer for simplicity.

MozReview-Commit-ID: Lkvrg0wvX5Z

toolkit/components/url-classifier/Entries.h

@@ -318,7 +318,6 @@ typedef nsClassHashtable<nsUint32HashKey, nsCString> PrefixStringMap;
 
 typedef nsDataHashtable<nsCStringHashKey, int64_t> TableFreshnessMap;
 
-typedef nsCStringHashKey VLHashPrefixString;
 typedef nsCStringHashKey FullHashString;
 
 typedef nsDataHashtable<FullHashString, int64_t> FullHashExpiryCache;
@@ -354,7 +353,7 @@ struct CachedFullHashResponse {
   }
 };
 
-typedef nsClassHashtable<VLHashPrefixString, CachedFullHashResponse> FullHashResponseMap;
+typedef nsClassHashtable<nsUint32HashKey, CachedFullHashResponse> FullHashResponseMap;
 
 } // namespace safebrowsing
 } // namespace mozilla

toolkit/components/url-classifier/HashStore.cpp

@@ -194,11 +194,11 @@ TableUpdateV4::NewChecksum(const std::string& aChecksum)
 }
 
 nsresult
-TableUpdateV4::NewFullHashResponse(const nsACString& aPrefix,
+TableUpdateV4::NewFullHashResponse(const Prefix& aPrefix,
                                    CachedFullHashResponse& aResponse)
 {
   CachedFullHashResponse* response =
-    mFullHashResponseMap.LookupOrAdd(aPrefix);
+    mFullHashResponseMap.LookupOrAdd(aPrefix.ToUint32());
   if (!response) {
     return NS_ERROR_OUT_OF_MEMORY;
   }

toolkit/components/url-classifier/HashStore.h

@@ -179,7 +179,7 @@ public:
   void NewRemovalIndices(const uint32_t* aIndices, size_t aNumOfIndices);
   void SetNewClientState(const nsACString& aState) { mClientState = aState; }
   void NewChecksum(const std::string& aChecksum);
-  nsresult NewFullHashResponse(const nsACString& aPrefix,
+  nsresult NewFullHashResponse(const Prefix& aPrefix,
                                CachedFullHashResponse& aResponse);
 
 private:

toolkit/components/url-classifier/LookupCache.h

@@ -119,6 +119,7 @@ public:
   }
 
   nsCString table;
+  Prefix prefix;
 };
 
 class CacheResultV2 final : public CacheResult
@@ -131,6 +132,7 @@ public:
 
   bool operator==(const CacheResultV2& aOther) const {
     return table == aOther.table &&
+           prefix == aOther.prefix &&
            completion == aOther.completion &&
            addChunk == aOther.addChunk;
   }
@@ -147,11 +149,11 @@ class CacheResultV4 final : public CacheResult
 public:
   static const int VER;
 
-  nsCString prefix;
   CachedFullHashResponse response;
 
   bool operator==(const CacheResultV4& aOther) const {
-    return prefix == aOther.prefix &&
+    return table == aOther.table &&
+           prefix == aOther.prefix &&
            response == aOther.response;
   }
 

toolkit/components/url-classifier/LookupCacheV4.cpp

@@ -112,9 +112,8 @@ LookupCacheV4::Has(const Completion& aCompletion,
   }
 
   // We always send 4-bytes for completion(Bug 1323953) so the prefix used to
-  // lookup for cache should be 4-bytes too.
+  // lookup for cache should be 4-bytes(uint32_t) too.
-  nsDependentCSubstring prefix(reinterpret_cast<const char*>(aCompletion.buf),
+  uint32_t prefix = aCompletion.ToUint32();
-                               PREFIX_SIZE);
 
   // Check if prefix can be found in cache.
   CachedFullHashResponse* fullHashResponse = mCache.Get(prefix);
@@ -663,12 +662,9 @@ LookupCacheV4::DumpCache()
   }
 
   for (auto iter = mCache.ConstIter(); !iter.Done(); iter.Next()) {
-    nsAutoCString strPrefix;
-    CStringToHexString(iter.Key(), strPrefix);
-
     CachedFullHashResponse* response = iter.Data();
-    LOG(("Caches prefix: %s, Expire time: %s",
+    LOG(("Caches prefix: %X, Expire time: %s",
-         strPrefix.get(),
+         iter.Key(),
          GetFormattedTimeString(response->negativeCacheExpirySec).get()));
 
     FullHashExpiryCache& fullHashes = response->fullHashes;

toolkit/components/url-classifier/nsUrlClassifierDBService.cpp

@@ -1196,6 +1196,7 @@ nsUrlClassifierLookupCallback::CompletionV2(const nsACString& aCompleteHash,
   auto result = new CacheResultV2;
 
   result->table = aTableName;
+  result->prefix.Assign(aCompleteHash);
   result->completion.Assign(aCompleteHash);
   result->addChunk = aChunkId;
 
@@ -1228,7 +1229,7 @@ nsUrlClassifierLookupCallback::CompletionV4(const nsACString& aPartialHash,
   int64_t nowSec = PR_Now() / PR_USEC_PER_SEC;
 
   result->table = aTableName;
-  result->prefix = aPartialHash;
+  result->prefix.Assign(aPartialHash);
   result->response.negativeCacheExpirySec = nowSec + aNegativeCacheDuration;
 
   // Fill in positive cache entries.

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

@@ -14,8 +14,12 @@ SetupCacheEntry(LookupCacheV4* aLookupCache,
                 bool aPosExpired = false)
 {
   FullHashResponseMap map;
-  CachedFullHashResponse* response = map.LookupOrAdd(
+
-    GeneratePrefix(aCompletion, PREFIX_SIZE));
+  Prefix prefix;
+  nsCOMPtr<nsICryptoHash> cryptoHash = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID);
+  prefix.FromPlaintext(aCompletion, cryptoHash);
+
+  CachedFullHashResponse* response = map.LookupOrAdd(prefix.ToUint32());
 
   response->negativeCacheExpirySec = aNegExpired ? EXPIRED_TIME_SEC : NOTEXPIRED_TIME_SEC;
   response->fullHashes.Put(GeneratePrefix(aCompletion, COMPLETE_SIZE),
@@ -198,8 +202,10 @@ TEST(CachingV4, NegativeCacheExpire)
   UniquePtr<LookupCacheV4> cache = SetupLookupCacheV4(array);
 
   FullHashResponseMap map;
-  CachedFullHashResponse* response = map.LookupOrAdd(
+  Prefix prefix;
-    GeneratePrefix(NEG_CACHE_EXPIRED_URL, PREFIX_SIZE));
+  nsCOMPtr<nsICryptoHash> cryptoHash = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID);
+  prefix.FromPlaintext(NEG_CACHE_EXPIRED_URL, cryptoHash);
+  CachedFullHashResponse* response = map.LookupOrAdd(prefix.ToUint32());
 
   response->negativeCacheExpirySec = EXPIRED_TIME_SEC;
 
@@ -212,21 +218,3 @@ TEST(CachingV4, NegativeCacheExpire)
   // The second time it should not be found in the cache again
   TestCache(NEG_CACHE_EXPIRED_URL, true, false, false, cache.get());
 }
-
-// This testcase check we only lookup cache with 4-bytes prefix
-TEST(CachingV4, Ensure4BytesLookup)
-{
-  _PrefixArray array = { GeneratePrefix(CACHED_URL, 8) };
-  UniquePtr<LookupCacheV4> cache = SetupLookupCacheV4(array);
-
-  FullHashResponseMap map;
-  CachedFullHashResponse* response = map.LookupOrAdd(
-    GeneratePrefix(CACHED_URL, 5));
-
-  response->negativeCacheExpirySec = NOTEXPIRED_TIME_SEC;
-  response->fullHashes.Put(GeneratePrefix(CACHED_URL, COMPLETE_SIZE),
-                                          NOTEXPIRED_TIME_SEC);
-  cache->AddFullHashResponseToCache(map);
-
-  TestCache(CACHED_URL, true, false, false, cache.get());
-}