Bug 1333328 - Remove mTableFreshness and gFreshnessGuarantee. r?francois draft
authorDimiL <dlee@mozilla.com>
Thu, 04 May 2017 11:13:21 +0800
changeset 577612 af08df05137f8d1bd51f7cf71612d9b29d081868
parent 577611 056ece5abb0ef2e8aa207c11d473da6adad0d0bf
child 577736 d8b701b13e3a20aed48e02eb99ae84c97ad84288
push id58735
push userbmo:dlee@mozilla.com
push dateMon, 15 May 2017 03:55:17 +0000
reviewersfrancois
bugs1333328
milestone55.0a1
Bug 1333328 - Remove mTableFreshness and gFreshnessGuarantee. r?francois Functions and Preference related to mTableFreshness and gFreshnessGuarantee could be removed since we will no longer require them. MozReview-Commit-ID: IAa0UHLSQ56
modules/libpref/init/all.js
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/Classifier.h
toolkit/components/url-classifier/content/listmanager.js
toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
toolkit/components/url-classifier/nsUrlClassifierProxies.h
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -5215,21 +5215,16 @@ pref("urlclassifier.gethashnoise", 4);
 
 // Gethash timeout for Safebrowsing.
 pref("urlclassifier.gethash.timeout_ms", 5000);
 // Update server response timeout for Safebrowsing.
 pref("urlclassifier.update.response_timeout_ms", 5000);
 // Download update timeout for Safebrowsing.
 pref("urlclassifier.update.timeout_ms", 60000);
 
-// If an urlclassifier table has not been updated in this number of seconds,
-// a gethash request will be forced to check that the result is still in
-// the database.
-pref("urlclassifier.max-complete-age", 2700);
-
 // Name of the about: page contributed by safebrowsing to handle display of error
 // pages on phishing/malware hits.  (bug 399233)
 pref("urlclassifier.alternate_error_page", "blocked");
 
 // Enable phishing protection
 pref("browser.safebrowsing.phishing.enabled", true);
 
 // Enable malware protection
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -287,17 +287,16 @@ Classifier::Reset()
 
     mRootStoreDirectory->Remove(true);
     mBackupDirectory->Remove(true);
     mUpdatingDirectory->Remove(true);
     mToDeleteDirectory->Remove(true);
 
     CreateStoreDirectory();
 
-    mTableFreshness.Clear();
     RegenActiveTables();
   };
 
   if (!mUpdateThread) {
     LOG(("Async update has been disabled. Just Reset() on worker thread."));
     resetFunc();
     return;
   }
@@ -306,18 +305,16 @@ Classifier::Reset()
   SyncRunnable::DispatchToThread(mUpdateThread, r);
 }
 
 void
 Classifier::ResetTables(ClearType aType, const nsTArray<nsCString>& aTables)
 {
   for (uint32_t i = 0; i < aTables.Length(); i++) {
     LOG(("Resetting table: %s", aTables[i].get()));
-    // Spoil this table by marking it as no known freshness
-    mTableFreshness.Remove(aTables[i]);
     LookupCache *cache = GetLookupCache(aTables[i]);
     if (cache) {
       // Remove any cached Completes for this table if clear type is Clear_Cache
       if (aType == Clear_Cache) {
         cache->ClearCache();
       } else {
         cache->ClearAll();
       }
@@ -614,18 +611,16 @@ void
 Classifier::RemoveUpdateIntermediaries()
 {
   // Remove old LookupCaches.
   for (auto c: mNewLookupCaches) {
     delete c;
   }
   mNewLookupCaches.Clear();
 
-  mNewTableFreshness.Clear();
-
   // Remove the "old" directory. (despite its looking-new name)
   if (NS_FAILED(mUpdatingDirectory->Remove(true))) {
     // If the directory is locked from removal for some reason,
     // we will fail here and it doesn't matter until the next
     // update. (the next udpate will fail due to the removable
     // "safebrowsing-udpating" directory.)
     LOG(("Failed to remove updating directory."));
   }
@@ -680,31 +675,26 @@ Classifier::SwapInNewTablesAndCleanup()
     return rv;
   }
 
   // Step 2. Merge mNewLookupCaches into mLookupCaches. The outdated
   // LookupCaches will be stored in mNewLookupCaches and be cleaned
   // up later.
   MergeNewLookupCaches();
 
-  // Step 3. Merge mTableFreshnessForUpdate.
-  for (auto itr = mNewTableFreshness.ConstIter(); !itr.Done(); itr.Next()) {
-    mTableFreshness.Put(itr.Key(), itr.Data());
-  }
-
-  // Step 4. Re-generate active tables based on on-disk tables.
+  // Step 3. Re-generate active tables based on on-disk tables.
   rv = RegenActiveTables();
   if (NS_FAILED(rv)) {
     LOG(("Failed to re-generate active tables!"));
   }
 
-  // Step 5. Clean up intermediaries for update.
+  // Step 4. Clean up intermediaries for update.
   RemoveUpdateIntermediaries();
 
-  // Step 6. Invalidate cached tableRequest request.
+  // Step 5. Invalidate cached tableRequest request.
   mIsTableRequestResultOutdated = true;
 
   LOG(("Done swap in updated tables."));
 
   return rv;
 }
 
 void Classifier::FlushAndDisableAsyncUpdate()
@@ -890,33 +880,16 @@ Classifier::ApplyFullHashes(nsTArray<Tab
     NS_ENSURE_SUCCESS(rv, rv);
 
     aUpdates->ElementAt(i) = nullptr;
   }
 
   return NS_OK;
 }
 
-int64_t
-Classifier::GetLastUpdateTime(const nsACString& aTableName)
-{
-  int64_t age;
-  bool found = mTableFreshness.Get(aTableName, &age);
-  return found ? (age * PR_MSEC_PER_SEC) : 0;
-}
-
-void
-Classifier::SetLastUpdateTime(const nsACString &aTable,
-                              uint64_t updateTime)
-{
-  LOG(("Marking table %s as last updated on %" PRIu64,
-       PromiseFlatCString(aTable).get(), updateTime));
-  mTableFreshness.Put(aTable, updateTime / PR_MSEC_PER_SEC);
-}
-
 void
 Classifier::DropStores()
 {
   for (uint32_t i = 0; i < mLookupCaches.Length(); i++) {
     delete mLookupCaches[i];
   }
   mLookupCaches.Clear();
 }
@@ -1297,19 +1270,17 @@ Classifier::UpdateHashStore(nsTArray<Tab
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_BUILD_PREFIX_FAILURE);
 
 #if defined(DEBUG)
   lookupCache->DumpCompletions();
 #endif
   rv = lookupCache->WriteFile();
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
 
-  int64_t now = (PR_Now() / PR_USEC_PER_SEC);
   LOG(("Successfully updated %s", store.TableName().get()));
-  mNewTableFreshness.Put(store.TableName(), now);
 
   return NS_OK;
 }
 
 nsresult
 Classifier::UpdateTableV4(nsTArray<TableUpdate*>* aUpdates,
                           const nsACString& aTable)
 {
@@ -1398,20 +1369,17 @@ Classifier::UpdateTableV4(nsTArray<Table
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
 
   if (lastAppliedUpdate) {
     LOG(("Write meta data of the last applied update."));
     rv = lookupCache->WriteMetadata(lastAppliedUpdate);
     NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
   }
 
-
-  int64_t now = (PR_Now() / PR_USEC_PER_SEC);
   LOG(("Successfully updated %s\n", PromiseFlatCString(aTable).get()));
-  mNewTableFreshness.Put(aTable, now);
 
   return NS_OK;
 }
 
 nsresult
 Classifier::UpdateCache(TableUpdate* aUpdate)
 {
   if (!aUpdate) {
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -80,18 +80,16 @@ public:
    */
   void FlushAndDisableAsyncUpdate();
 
   /**
    * Apply full hashes retrived from gethash to cache.
    */
   nsresult ApplyFullHashes(nsTArray<TableUpdate*>* aUpdates);
 
-  void SetLastUpdateTime(const nsACString& aTableName, uint64_t updateTime);
-  int64_t GetLastUpdateTime(const nsACString& aTableName);
   nsresult CacheCompletions(const CacheResultArray& aResults);
   uint32_t GetHashKey(void) { return mHashKey; }
   /*
    * Get a bunch of extra prefixes to query for completion
    * and mask the real entry being requested
    */
   nsresult ReadNoiseEntries(const Prefix& aPrefix,
                             const nsACString& aTableName,
@@ -203,19 +201,16 @@ private:
   // Used for atomically updating the other dirs.
   nsCOMPtr<nsIFile> mBackupDirectory;
   nsCOMPtr<nsIFile> mUpdatingDirectory; // For update only.
   nsCOMPtr<nsIFile> mToDeleteDirectory;
   nsCOMPtr<nsICryptoHash> mCryptoHash;
   nsTArray<LookupCache*> mLookupCaches; // For query only.
   nsTArray<nsCString> mActiveTablesCache;
   uint32_t mHashKey;
-  // Stores the last time a given table was updated (seconds).
-  TableFreshnessMap mTableFreshness;
-  TableFreshnessMap mNewTableFreshness;
 
   // In-memory cache for the result of TableRequest. See
   // nsIUrlClassifierDBService.getTables for the format.
   nsCString mTableRequestResult;
 
   // Whether mTableRequestResult is outdated and needs to
   // be reloaded from disk.
   bool mIsTableRequestResultOutdated;
--- a/toolkit/components/url-classifier/content/listmanager.js
+++ b/toolkit/components/url-classifier/content/listmanager.js
@@ -237,26 +237,16 @@ PROT_ListManager.prototype.kickoffUpdate
       let targetPref = "browser.safebrowsing.provider." + provider + ".nextupdatetime";
       let nextUpdate = this.prefs_.getPref(targetPref);
       if (nextUpdate) {
         updateDelay = Math.min(maxDelayMs, Math.max(0, nextUpdate - Date.now()));
         log("Next update at " + nextUpdate);
       }
       log("Next update " + updateDelay + "ms from now");
 
-      // Set the last update time to verify if data is still valid.
-      let freshnessPref = "browser.safebrowsing.provider." + provider + ".lastupdatetime";
-      let freshness = this.prefs_.getPref(freshnessPref);
-      if (freshness) {
-        Object.keys(this.tablesData).forEach(function(table) {
-        if (this.tablesData[table].provider === provider) {
-          this.dbService_.setLastUpdateTime(table, freshness);
-        }}, this);
-      }
-
       this.updateCheckers_[updateUrl] =
         new G_Alarm(BindToObject(this.checkForUpdates, this, updateUrl),
                     updateDelay, false /* repeating */);
     } else {
       log("No updates needed or already initialized for " + updateUrl);
     }
   }
 }
--- a/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
+++ b/toolkit/components/url-classifier/nsIUrlClassifierDBService.idl
@@ -106,23 +106,16 @@ interface nsIUrlClassifierDBService : ns
    * Set the nsIUrlClassifierCompleter object for a given table.  This
    * object will be used to request complete versions of partial
    * hashes.
    */
   void setHashCompleter(in ACString tableName,
                         in nsIUrlClassifierHashCompleter completer);
 
   /**
-   * 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:
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -95,19 +95,16 @@ using namespace mozilla::safebrowsing;
 // MOZ_LOG=UrlClassifierDbService:5
 LazyLogModule gUrlClassifierDbServiceLog("UrlClassifierDbService");
 #define LOG(args) MOZ_LOG(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug, args)
 #define LOG_ENABLED() MOZ_LOG_TEST(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug)
 
 #define GETHASH_NOISE_PREF      "urlclassifier.gethashnoise"
 #define GETHASH_NOISE_DEFAULT   4
 
-#define CONFIRM_AGE_PREF        "urlclassifier.max-complete-age"
-#define CONFIRM_AGE_DEFAULT_SEC (45 * 60)
-
 // 30 minutes as the maximum negative cache duration.
 #define MAXIMUM_NEGATIVE_CACHE_DURATION_SEC (30 * 60 * 1000)
 
 // TODO: The following two prefs are to be removed after we
 //       roll out full v4 hash completion. See Bug 1331534.
 #define TAKE_V4_COMPLETION_RESULT_PREF    "browser.safebrowsing.temporary.take_v4_completion_result"
 #define TAKE_V4_COMPLETION_RESULT_DEFAULT false
 
@@ -117,17 +114,16 @@ class nsUrlClassifierDBServiceWorker;
 static nsUrlClassifierDBService* sUrlClassifierDBService;
 
 nsIThread* nsUrlClassifierDBService::gDbBackgroundThread = nullptr;
 
 // Once we've committed to shutting down, don't do work in the background
 // thread.
 static bool gShuttingDownThread = false;
 
-static mozilla::Atomic<uint32_t, Relaxed> gFreshnessGuarantee(CONFIRM_AGE_DEFAULT_SEC);
 static uint32_t sGethashNoise = GETHASH_NOISE_DEFAULT;
 
 NS_IMPL_ISUPPORTS(nsUrlClassifierDBServiceWorker,
                   nsIUrlClassifierDBService)
 
 nsUrlClassifierDBServiceWorker::nsUrlClassifierDBServiceWorker()
   : mInStream(false)
   , mGethashNoise(0)
@@ -719,41 +715,27 @@ nsUrlClassifierDBServiceWorker::ResetDat
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBServiceWorker::ReloadDatabase()
 {
   nsTArray<nsCString> tables;
-  nsTArray<int64_t> lastUpdateTimes;
   nsresult rv = mClassifier->ActiveTables(tables);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // We need to make sure lastupdatetime is set after reload database
-  // Otherwise request will be skipped if it is not confirmed.
-  for (uint32_t table = 0; table < tables.Length(); table++) {
-    lastUpdateTimes.AppendElement(mClassifier->GetLastUpdateTime(tables[table]));
-  }
-
   // This will null out mClassifier
   rv = CloseDb();
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Create new mClassifier and load prefixset and completions from disk.
   rv = OpenDb();
   NS_ENSURE_SUCCESS(rv, rv);
 
-  for (uint32_t table = 0; table < tables.Length(); table++) {
-    int64_t time = lastUpdateTimes[table];
-    if (time) {
-      mClassifier->SetLastUpdateTime(tables[table], lastUpdateTimes[table]);
-    }
-  }
-
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsUrlClassifierDBServiceWorker::CancelUpdate()
 {
   LOG(("nsUrlClassifierDBServiceWorker::CancelUpdate"));
 
@@ -947,28 +929,16 @@ nsUrlClassifierDBServiceWorker::OpenDb()
   rv = classifier->Open(*mCacheDir);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mClassifier = classifier;
 
   return NS_OK;
 }
 
-nsresult
-nsUrlClassifierDBServiceWorker::SetLastUpdateTime(const nsACString &table,
-                                                  uint64_t updateTime)
-{
-  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");
   if (mLastResults) {
     mLastResults->Clear();
   }
   return NS_OK;
@@ -1671,18 +1641,16 @@ nsUrlClassifierDBService::Init()
     return NS_OK;
   default:
     // No other process type is supported!
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   sGethashNoise = Preferences::GetUint(GETHASH_NOISE_PREF,
     GETHASH_NOISE_DEFAULT);
-  gFreshnessGuarantee = Preferences::GetInt(CONFIRM_AGE_PREF,
-    CONFIRM_AGE_DEFAULT_SEC);
   ReadTablesFromPrefs();
   nsresult rv;
 
   {
     // Force PSM loading on main thread
     nsCOMPtr<nsICryptoHash> dummy = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -1739,18 +1707,16 @@ nsUrlClassifierDBService::Init()
   observerService->AddObserver(this, "profile-before-change", false);
 
   // XXX: Do we *really* need to be able to change all of these at runtime?
   // Note: These observers should only be added when everything else above has
   //       succeeded. Failing to do so can cause long shutdown times in certain
   //       situations. See Bug 1247798 and Bug 1244803.
   Preferences::AddUintVarCache(&sGethashNoise, GETHASH_NOISE_PREF,
     GETHASH_NOISE_DEFAULT);
-  Preferences::AddAtomicUintVarCache(&gFreshnessGuarantee, CONFIRM_AGE_PREF,
-    CONFIRM_AGE_DEFAULT_SEC);
 
   for (uint8_t i = 0; i < kObservedPrefs.Length(); i++) {
     Preferences::AddStrongObserver(this, kObservedPrefs[i].get());
   }
 
   return NS_OK;
 }
 
@@ -2103,25 +2069,16 @@ nsUrlClassifierDBService::SetHashComplet
   } 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
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -207,32 +207,16 @@ UrlClassifierDBServiceWorkerProxy::Cache
 NS_IMETHODIMP
 UrlClassifierDBServiceWorkerProxy::CacheCompletionsRunnable::Run()
 {
   mTarget->CacheCompletions(mEntries);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-UrlClassifierDBServiceWorkerProxy::SetLastUpdateTime(const nsACString& table,
-                                                     uint64_t lastUpdateTime)
-{
-  nsCOMPtr<nsIRunnable> r =
-    new SetLastUpdateTimeRunnable(mTarget, table, lastUpdateTime);
-  return DispatchToWorkerThread(r);
-}
-
-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()
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -150,34 +150,16 @@ public:
   private:
     RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
 
     nsCString mSpec;
     nsCString mTables;
     mozilla::safebrowsing::LookupResultArray* mResults;
   };
 
-  class SetLastUpdateTimeRunnable : public mozilla::Runnable
-  {
-  public:
-    SetLastUpdateTimeRunnable(nsUrlClassifierDBServiceWorker* aTarget,
-                              const nsACString& table,
-                              uint64_t updateTime)
-      : mTarget(aTarget),
-        mTable(table),
-        mUpdateTime(updateTime)
-    { }
-
-    NS_DECL_NSIRUNNABLE
-  private:
-    RefPtr<nsUrlClassifierDBServiceWorker> mTarget;
-    nsCString mTable;
-    uint64_t mUpdateTime;
-  };
-
   class ClearLastResultsRunnable : public mozilla::Runnable
   {
   public:
     explicit ClearLastResultsRunnable(nsUrlClassifierDBServiceWorker* aTarget)
       : mTarget(aTarget)
     { }
 
     NS_DECL_NSIRUNNABLE