Bug 1319286 - Cache nsIUrlClassifierDBService.getTables result until next update. r=francois. draft
authorHenry Chang <hchang@mozilla.com>
Tue, 22 Nov 2016 10:39:58 +0800
changeset 442293 b652adb6fe696d6c25c423c8c7456247983d8050
parent 441589 57a8cde3f08ca9d60bcd8bdd698ceec687f0aed2
child 537767 6780c2641043740f1c6571b9270bb9401b1679f3
push id36665
push userhchang@mozilla.com
push dateTue, 22 Nov 2016 08:21:03 +0000
reviewersfrancois
bugs1319286
milestone53.0a1
Bug 1319286 - Cache nsIUrlClassifierDBService.getTables result until next update. r=francois. MozReview-Commit-ID: ItjTQNzCVED
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/Classifier.h
toolkit/components/url-classifier/tests/unit/test_listmanager.js
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -17,16 +17,17 @@
 #include "nsThreadUtils.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/Logging.h"
 #include "mozilla/SyncRunnable.h"
 #include "mozilla/Base64.h"
 #include "mozilla/Unused.h"
 #include "mozilla/TypedEnumBits.h"
 #include "nsIUrlClassifierUtils.h"
+#include "nsUrlClassifierDBService.h"
 
 // MOZ_LOG=UrlClassifierDbService:5
 extern mozilla::LazyLogModule gUrlClassifierDbServiceLog;
 #define LOG(args) MOZ_LOG(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug, args)
 #define LOG_ENABLED() MOZ_LOG_TEST(gUrlClassifierDbServiceLog, mozilla::LogLevel::Debug)
 
 #define STORE_DIRECTORY      NS_LITERAL_CSTRING("safebrowsing")
 #define TO_DELETE_DIR_SUFFIX NS_LITERAL_CSTRING("-to_delete")
@@ -139,16 +140,17 @@ Classifier::GetPrivateStoreDirectory(nsI
   }
 
   providerDirectory.forget(aPrivateStoreDirectory);
 
   return NS_OK;
 }
 
 Classifier::Classifier()
+  : mIsTableRequestResultOutdated(true)
 {
 }
 
 Classifier::~Classifier()
 {
   Close();
 }
 
@@ -343,16 +345,26 @@ Classifier::AbortUpdateAndReset(const ns
   // from an update.
   Unused << RemoveBackupTables();
   Unused << CleanToDelete();
 }
 
 void
 Classifier::TableRequest(nsACString& aResult)
 {
+  MOZ_ASSERT(NS_GetCurrentThread() == nsUrlClassifierDBService::BackgroundThread(),
+             "TableRequest must be called on the classifier worker thread.");
+
+  // This function and all disk I/O are guaranteed to occur
+  // on the same thread so we don't need to add a lock around.
+  if (!mIsTableRequestResultOutdated) {
+    aResult = mTableRequestResult;
+    return;
+  }
+
   // Generating v2 table info.
   nsTArray<nsCString> tables;
   ActiveTables(tables);
   for (uint32_t i = 0; i < tables.Length(); i++) {
     HashStore store(tables[i], GetProvider(tables[i]), mRootStoreDirectory);
 
     nsresult rv = store.Open();
     if (NS_FAILED(rv))
@@ -382,18 +394,23 @@ Classifier::TableRequest(nsACString& aRe
 
     aResult.Append('\n');
   }
 
   // Load meta data from *.metadata files in the root directory.
   // Specifically for v4 tables.
   nsCString metadata;
   nsresult rv = LoadMetadata(mRootStoreDirectory, metadata);
-  NS_ENSURE_SUCCESS_VOID(rv);
-  aResult.Append(metadata);
+  if (NS_SUCCEEDED(rv)) {
+    aResult.Append(metadata);
+  }
+
+  // Update the TableRequest result in-memory cache.
+  mTableRequestResult = aResult;
+  mIsTableRequestResultOutdated = false;
 }
 
 // This is used to record the matching statistics for v2 and v4.
 enum class PrefixMatch : uint8_t {
   eNoMatch = 0x00,
   eMatchV2Prefix = 0x01,
   eMatchV4Prefix = 0x02,
   eMatchBoth = eMatchV2Prefix | eMatchV4Prefix
@@ -529,16 +546,20 @@ Classifier::ApplyUpdates(nsTArray<TableU
         nsCString updateTable(aUpdates->ElementAt(i)->TableName());
 
         if (TableUpdate::Cast<TableUpdateV2>((*aUpdates)[i])) {
           rv = UpdateHashStore(aUpdates, updateTable);
         } else {
           rv = UpdateTableV4(aUpdates, updateTable);
         }
 
+        // We mark the table associated info outdated no matter the
+        // update is successful to avoid any possibile non-atomic update.
+        mIsTableRequestResultOutdated = true;
+
         if (NS_FAILED(rv)) {
           if (rv != NS_ERROR_OUT_OF_MEMORY) {
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
             DumpFailedUpdate();
 #endif
             AbortUpdateAndReset(updateTable);
           }
           return rv;
@@ -1004,16 +1025,19 @@ Classifier::UpdateHashStore(nsTArray<Tab
 
   return NS_OK;
 }
 
 nsresult
 Classifier::UpdateTableV4(nsTArray<TableUpdate*>* aUpdates,
                           const nsACString& aTable)
 {
+  MOZ_ASSERT(NS_GetCurrentThread() == nsUrlClassifierDBService::BackgroundThread(),
+             "UpdateTableV4 must be called on the classifier worker thread.");
+
   LOG(("Classifier::UpdateTableV4(%s)", PromiseFlatCString(aTable).get()));
 
   if (!CheckValidUpdate(aUpdates, aTable)) {
     return NS_OK;
   }
 
   LookupCacheV4* lookupCache =
     LookupCache::Cast<LookupCacheV4>(GetLookupCache(aTable));
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -154,14 +154,22 @@ private:
   nsCOMPtr<nsIFile> mBackupDirectory;
   nsCOMPtr<nsIFile> mToDeleteDirectory;
   nsCOMPtr<nsICryptoHash> mCryptoHash;
   nsTArray<LookupCache*> mLookupCaches;
   nsTArray<nsCString> mActiveTablesCache;
   uint32_t mHashKey;
   // Stores the last time a given table was updated (seconds).
   nsDataHashtable<nsCStringHashKey, int64_t> mTableFreshness;
+
+  // 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;
 };
 
 } // namespace safebrowsing
 } // namespace mozilla
 
 #endif
--- a/toolkit/components/url-classifier/tests/unit/test_listmanager.js
+++ b/toolkit/components/url-classifier/tests/unit/test_listmanager.js
@@ -355,17 +355,27 @@ function waitUntilMetaDataSaved(expected
 
       if (tableName !== 'test-phish-proto') {
         return false; // continue.
       }
 
       if (stateBase64 === btoa(expectedState) &&
           checksumBase64 === btoa(expectedChecksum)) {
         do_print('State has been saved to disk!');
-        callback();
+
+        // We slightly defer the callback to see if the in-memory
+        // |getTables| caching works correctly.
+        dbService.getTables(cachedMetadata => {
+          equal(cachedMetadata, metaData);
+          callback();
+        });
+
+        // Even though we haven't done callback at this moment
+        // but we still claim "we have" in order to stop repeating
+        // a new timer.
         didCallback = true;
       }
 
       return true; // break no matter whether the state is matching.
     });
 
     if (!didCallback) {
       do_timeout(1000, waitUntilMetaDataSaved.bind(null, expectedState,