Bug 1333257 - Only cache V2 misses when doing Safe Browsing lookups. r?francois draft
authorDimi Lee <dlee@mozilla.com>
Thu, 26 Jan 2017 11:36:52 +0800
changeset 466510 b8627309c7466e86c1ef33c02e7163df46ced9a9
parent 466497 52a34f9a6cf112377299ab32132384e2dc1f543b
child 543456 f4de139503dceb57a0cf64cc8e60f2b8e073ea9d
push id42932
push userdlee@mozilla.com
push dateThu, 26 Jan 2017 03:49:44 +0000
reviewersfrancois
bugs1333257
milestone54.0a1
Bug 1333257 - Only cache V2 misses when doing Safe Browsing lookups. r?francois MozReview-Commit-ID: 6kvM6z5OnPw
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/LookupCache.h
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -493,18 +493,20 @@ Classifier::Check(const nsACString& aSpe
 
         result->hash.complete = lookupHash;
         result->mConfirmed = confirmed;
         result->mTableName.Assign(cache->TableName());
         result->mPartialHashLength = confirmed ? COMPLETE_SIZE : matchLength;
 
         if (LookupCache::Cast<LookupCacheV4>(cache)) {
           matchingStatistics |= PrefixMatch::eMatchV4Prefix;
+          result->mProtocolV2 = false;
         } else {
           matchingStatistics |= PrefixMatch::eMatchV2Prefix;
+          result->mProtocolV2 = true;
         }
       }
     }
 
     Telemetry::Accumulate(Telemetry::URLCLASSIFIER_PREFIX_MATCH,
                           static_cast<uint8_t>(matchingStatistics));
   }
 
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -21,17 +21,18 @@ namespace mozilla {
 namespace safebrowsing {
 
 #define MAX_HOST_COMPONENTS 5
 #define MAX_PATH_COMPONENTS 4
 
 class LookupResult {
 public:
   LookupResult() : mNoise(false), mProtocolConfirmed(false),
-                   mPartialHashLength(0), mConfirmed(false) {}
+                   mPartialHashLength(0), mConfirmed(false),
+                   mProtocolV2(true) {}
 
   // The fragment that matched in the LookupCache
   union {
     Prefix fixedLengthPrefix;
     Completion complete;
   } hash;
 
   const Completion &CompleteHash() {
@@ -67,16 +68,18 @@ public:
   bool mProtocolConfirmed;
 
   nsCString mTableName;
 
   uint32_t mPartialHashLength;
 
   // True as long as this lookup is complete and hasn't expired.
   bool mConfirmed;
+
+  bool mProtocolV2;
 };
 
 typedef nsTArray<LookupResult> LookupResultArray;
 
 struct CacheResult {
   AddComplete entry;
   nsCString table;
 
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -276,18 +276,22 @@ nsUrlClassifierDBServiceWorker::DoLookup
     PRIntervalTime clockEnd = PR_IntervalNow();
     LOG(("query took %dms\n",
          PR_IntervalToMilliseconds(clockEnd - clockStart)));
   }
 
   nsAutoPtr<LookupResultArray> completes(new LookupResultArray());
 
   for (uint32_t i = 0; i < results->Length(); i++) {
-    if (!mMissCache.Contains(results->ElementAt(i).hash.fixedLengthPrefix)) {
-      completes->AppendElement(results->ElementAt(i));
+    const LookupResult& lookupResult = results->ElementAt(i);
+
+    // mMissCache should only be used in V2.
+    if (!lookupResult.mProtocolV2 ||
+        !mMissCache.Contains(lookupResult.hash.fixedLengthPrefix)) {
+      completes->AppendElement(lookupResult);
     }
   }
 
   for (uint32_t i = 0; i < completes->Length(); i++) {
     if (!completes->ElementAt(i).Confirmed()) {
       // We're going to be doing a gethash request, add some extra entries.
       // Note that we cannot pass the first two by reference, because we
       // add to completes, whicah can cause completes to reallocate and move.
@@ -1136,25 +1140,26 @@ nsUrlClassifierLookupCallback::HandleRes
     LOG(("Confirmed result %X from table %s",
          result.PartialHashHex().get(), result.mTableName.get()));
 
     if (tables.IndexOf(result.mTableName) == nsTArray<nsCString>::NoIndex) {
       tables.AppendElement(result.mTableName);
     }
   }
 
+  // TODO: Bug 1333328, Refactor cache miss mechanism for v2.
   // Some parts of this gethash request generated no hits at all.
   // Prefixes must have been removed from the database since our last update.
   // Save the prefixes we checked to prevent repeated requests
   // until the next update.
   nsAutoPtr<PrefixArray> cacheMisses(new PrefixArray());
   if (cacheMisses) {
     for (uint32_t i = 0; i < mResults->Length(); i++) {
       LookupResult &result = mResults->ElementAt(i);
-      if (!result.Confirmed() && !result.mNoise) {
+      if (result.mProtocolV2 && !result.Confirmed() && !result.mNoise) {
         cacheMisses->AppendElement(result.hash.fixedLengthPrefix);
       }
     }
     // Hands ownership of the miss array back to the worker thread.
     mDBService->CacheMisses(cacheMisses.forget());
   }
 
   if (mCacheResults) {