Backed out changeset b3574d03261b (bug 1319286) for GTest Assertion failures
authorIris Hsiao <ihsiao@mozilla.com>
Thu, 24 Nov 2016 13:26:39 +0800
changeset 324044 a35328bd4d0742434319ebd2e9cd7de3b075f660
parent 324043 b3574d03261b8512a6824e1d62ac0c8c9a5b676e
child 324045 3a60967cd9ed58bf2ec9573b0185e3537a42518c
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
bugs1319286
milestone53.0a1
backs outb3574d03261b8512a6824e1d62ac0c8c9a5b676e
Backed out changeset b3574d03261b (bug 1319286) for GTest Assertion failures
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,17 +17,16 @@
 #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")
@@ -140,17 +139,16 @@ Classifier::GetPrivateStoreDirectory(nsI
   }
 
   providerDirectory.forget(aPrivateStoreDirectory);
 
   return NS_OK;
 }
 
 Classifier::Classifier()
-  : mIsTableRequestResultOutdated(true)
 {
 }
 
 Classifier::~Classifier()
 {
   Close();
 }
 
@@ -345,26 +343,16 @@ 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))
@@ -394,23 +382,18 @@ 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);
-  if (NS_SUCCEEDED(rv)) {
-    aResult.Append(metadata);
-  }
-
-  // Update the TableRequest result in-memory cache.
-  mTableRequestResult = aResult;
-  mIsTableRequestResultOutdated = false;
+  NS_ENSURE_SUCCESS_VOID(rv);
+  aResult.Append(metadata);
 }
 
 // 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
@@ -546,20 +529,16 @@ 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;
@@ -1025,19 +1004,16 @@ 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,22 +154,14 @@ 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,27 +355,17 @@ 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!');
-
-        // 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.
+        callback();
         didCallback = true;
       }
 
       return true; // break no matter whether the state is matching.
     });
 
     if (!didCallback) {
       do_timeout(1000, waitUntilMetaDataSaved.bind(null, expectedState,