Bug 1164518 - Avoid unnecessary DB updates when caching Safe Browsing results. r?gcp draft
authorFrancois Marier <francois@mozilla.com>
Fri, 04 Mar 2016 12:33:20 -0800
changeset 337035 6f8f8f0ed995c5a6f40d24fa4c67107ef1668be4
parent 336250 f510f63b93af675aa5fe16da201328bad2d97b4d
child 515568 816aaa0739cfc71e30f89cb4070cc65927b601be
push id12255
push userfmarier@mozilla.com
push dateFri, 04 Mar 2016 20:38:43 +0000
reviewersgcp
bugs1164518
milestone47.0a1
Bug 1164518 - Avoid unnecessary DB updates when caching Safe Browsing results. r?gcp MozReview-Commit-ID: HYNaTdCRohL
toolkit/components/url-classifier/Entries.h
toolkit/components/url-classifier/LookupCache.h
toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
toolkit/components/url-classifier/nsUrlClassifierProxies.h
--- a/toolkit/components/url-classifier/Entries.h
+++ b/toolkit/components/url-classifier/Entries.h
@@ -176,16 +176,23 @@ struct AddComplete {
   template<class T>
   int Compare(const T& other) const {
     int cmp = complete.Compare(other.CompleteHash());
     if (cmp != 0) {
       return cmp;
     }
     return addChunk - other.addChunk;
   }
+
+  bool operator!=(const AddComplete& aOther) const {
+    if (addChunk != aOther.addChunk) {
+      return true;
+    }
+    return complete != aOther.complete;
+  }
 };
 
 struct SubPrefix {
   // The hash to subtract.
   Prefix prefix;
   // The chunk number of the add chunk to which the hash belonged.
   uint32_t addChunk;
   // The chunk number of this sub chunk.
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -62,16 +62,23 @@ public:
   nsCString mTableName;
 };
 
 typedef nsTArray<LookupResult> LookupResultArray;
 
 struct CacheResult {
   AddComplete entry;
   nsCString table;
+
+  bool operator==(const CacheResult& aOther) const {
+    if (entry != aOther.entry) {
+      return false;
+    }
+    return table == aOther.table;
+  }
 };
 typedef nsTArray<CacheResult> CacheResultArray;
 
 class LookupCache {
 public:
   // Check for a canonicalized IP address.
   static bool IsCanonicalizedIP(const nsACString& aHost);
 
--- a/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
@@ -102,16 +102,21 @@ interface nsIUrlClassifierDBService : ns
 
   /**
    * Set the last update time for the given table. We use this to
    * remember freshness past restarts. Time is in milliseconds since epoch.
    */
   void setLastUpdateTime(in ACString tableName,
                          in unsigned long long lastUpdateTime);
 
+  /**
+   * Forget the results that were used in the last DB update.
+   */
+  void clearLastResults();
+
   ////////////////////////////////////////////////////////////////////////////
   // Incremental update methods.
   //
   // An update to the database has the following steps:
   //
   // 1) The update process is started with beginUpdate().  The client
   //    passes an nsIUrlClassifierUpdateObserver object which will be
   //    notified as the update is processed by the dbservice.
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -655,16 +655,21 @@ nsUrlClassifierDBServiceWorker::CacheCom
 {
   LOG(("nsUrlClassifierDBServiceWorker::CacheCompletions [%p]", this));
   if (!mClassifier)
     return NS_OK;
 
   // Ownership is transferred in to us
   nsAutoPtr<CacheResultArray> resultsPtr(results);
 
+  if (mLastResults == *resultsPtr) {
+    LOG(("Skipping completions that have just been cached already."));
+    return NS_OK;
+  }
+
   nsAutoPtr<ProtocolParser> pParse(new ProtocolParser());
   nsTArray<TableUpdate*> updates;
 
   // Only cache results for tables that we have, don't take
   // in tables we might accidentally have hit during a completion.
   // This happens due to goog vs googpub lists existing.
   nsTArray<nsCString> tables;
   nsresult rv = mClassifier->ActiveTables(tables);
@@ -697,16 +702,17 @@ nsUrlClassifierDBServiceWorker::CacheCom
       updates.AppendElement(tu);
       pParse->ForgetTableUpdates();
     } else {
       LOG(("Completion received, but table is not active, so not caching."));
     }
    }
 
   mClassifier->ApplyUpdates(&updates);
+  mLastResults = *resultsPtr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBServiceWorker::CacheMisses(PrefixArray *results)
 {
   LOG(("nsUrlClassifierDBServiceWorker::CacheMisses [%p] %d",
        this, results->Length()));
@@ -753,16 +759,25 @@ nsUrlClassifierDBServiceWorker::SetLastU
   MOZ_ASSERT(!NS_IsMainThread(), "Must be on the background thread");
   MOZ_ASSERT(mClassifier, "Classifier connection must be opened");
 
   mClassifier->SetLastUpdateTime(table, updateTime);
 
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsUrlClassifierDBServiceWorker::ClearLastResults()
+{
+  MOZ_ASSERT(!NS_IsMainThread(), "Must be on the background thread");
+  mLastResults.Clear();
+  return NS_OK;
+}
+
+
 // -------------------------------------------------------------------------
 // nsUrlClassifierLookupCallback
 //
 // This class takes the results of a lookup found on the worker thread
 // and handles any necessary partial hash expansions before calling
 // the client callback.
 
 class nsUrlClassifierLookupCallback final : public nsIUrlClassifierLookupCallback
@@ -1453,30 +1468,38 @@ NS_IMETHODIMP
 nsUrlClassifierDBService::SetHashCompleter(const nsACString &tableName,
                                            nsIUrlClassifierHashCompleter *completer)
 {
   if (completer) {
     mCompleters.Put(tableName, completer);
   } else {
     mCompleters.Remove(tableName);
   }
-
+  ClearLastResults();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBService::SetLastUpdateTime(const nsACString &tableName,
                                             uint64_t lastUpdateTime)
 {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
   return mWorkerProxy->SetLastUpdateTime(tableName, lastUpdateTime);
 }
 
 NS_IMETHODIMP
+nsUrlClassifierDBService::ClearLastResults()
+{
+  NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
+
+  return mWorkerProxy->ClearLastResults();
+}
+
+NS_IMETHODIMP
 nsUrlClassifierDBService::BeginUpdate(nsIUrlClassifierUpdateObserver *observer,
                                       const nsACString &updateTables)
 {
   NS_ENSURE_TRUE(gDbBackgroundThread, NS_ERROR_NOT_INITIALIZED);
 
   if (mInUpdate) {
     LOG(("Already updating, not available"));
     return NS_ERROR_NOT_AVAILABLE;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -207,16 +207,19 @@ private:
   nsTArray<mozilla::safebrowsing::TableUpdate*> mTableUpdates;
 
   int32_t mUpdateWait;
 
   // Entries that cannot be completed. We expect them to die at
   // the next update
   PrefixArray mMissCache;
 
+  // Stores the last results that triggered a table update.
+  CacheResultArray mLastResults;
+
   nsresult mUpdateStatus;
   nsTArray<nsCString> mUpdateTables;
 
   nsCOMPtr<nsIUrlClassifierUpdateObserver> mUpdateObserver;
   bool mInStream;
 
   // The number of noise entries to add to the set of lookup results.
   uint32_t mGethashNoise;
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -227,16 +227,29 @@ UrlClassifierDBServiceWorkerProxy::SetLa
 
 NS_IMETHODIMP
 UrlClassifierDBServiceWorkerProxy::SetLastUpdateTimeRunnable::Run()
 {
   mTarget->SetLastUpdateTime(mTable, mUpdateTime);
   return NS_OK;
 }
 
+NS_IMETHODIMP
+UrlClassifierDBServiceWorkerProxy::ClearLastResults()
+{
+  nsCOMPtr<nsIRunnable> r = new ClearLastResultsRunnable(mTarget);
+  return DispatchToWorkerThread(r);
+}
+
+NS_IMETHODIMP
+UrlClassifierDBServiceWorkerProxy::ClearLastResultsRunnable::Run()
+{
+  return mTarget->ClearLastResults();
+}
+
 NS_IMPL_ISUPPORTS(UrlClassifierLookupCallbackProxy,
                   nsIUrlClassifierLookupCallback)
 
 NS_IMETHODIMP
 UrlClassifierLookupCallbackProxy::LookupComplete
   (LookupResultArray * aResults)
 {
   nsCOMPtr<nsIRunnable> r = new LookupCompleteRunnable(mTarget, aResults);
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -186,16 +186,28 @@ public:
 
     NS_DECL_NSIRUNNABLE
   private:
     RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
     nsCString mTable;
     uint64_t mUpdateTime;
   };
 
+  class ClearLastResultsRunnable : public nsRunnable
+  {
+  public:
+    explicit ClearLastResultsRunnable(nsUrlClassifierDBServiceWorker* aTarget)
+      : mTarget(aTarget)
+    { }
+
+    NS_DECL_NSIRUNNABLE
+  private:
+    RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
+  };
+
 public:
   nsresult DoLocalLookup(const nsACString& spec,
                          const nsACString& tables,
                          mozilla::safebrowsing::LookupResultArray* results);
 
 private:
   ~UrlClassifierDBServiceWorkerProxy() {}