Bug 1434206 - Keep LookupCache objects in smart pointers. r?gcp draft
authorFrancois Marier <francois@mozilla.com>
Wed, 16 May 2018 19:13:48 -0700
changeset 467870 b1565f3aacdb
parent 467869 a1a382f2e773
child 467871 3fdd157bb698
push id180
push userfmarier@mozilla.com
push date2018-05-29 01:16 +0000
reviewersgcp
bugs1434206
milestone62.0a1
Bug 1434206 - Keep LookupCache objects in smart pointers. r?gcp The existing mix of UniquePtr and raw pointers is confusing when trying to figure out the exact lifetime of these objects. MozReview-Commit-ID: Br4S7BXEFKs
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/Classifier.h
toolkit/components/url-classifier/LookupCache.h
toolkit/components/url-classifier/LookupCacheV4.h
toolkit/components/url-classifier/tests/gtest/Common.cpp
toolkit/components/url-classifier/tests/gtest/Common.h
toolkit/components/url-classifier/tests/gtest/TestCaching.cpp
toolkit/components/url-classifier/tests/gtest/TestClassifier.cpp
toolkit/components/url-classifier/tests/gtest/TestLookupCacheV4.cpp
toolkit/components/url-classifier/tests/gtest/TestPerProviderDirectory.cpp
toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -279,17 +279,17 @@ 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()));
-    LookupCache *cache = GetLookupCache(aTables[i]);
+    RefPtr<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();
       }
     }
@@ -422,20 +422,20 @@ Classifier::Check(const nsACString& aSpe
   // components.
   nsTArray<nsCString> fragments;
   nsresult rv = LookupCache::GetLookupFragments(aSpec, &fragments);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsTArray<nsCString> activeTables;
   SplitTables(aTables, activeTables);
 
-  nsTArray<LookupCache*> cacheArray;
+  LookupCacheArray cacheArray;
   for (uint32_t i = 0; i < activeTables.Length(); i++) {
     LOG(("Checking table %s", activeTables[i].get()));
-    LookupCache *cache = GetLookupCache(activeTables[i]);
+    RefPtr<LookupCache> cache = GetLookupCache(activeTables[i]);
     if (cache) {
       cacheArray.AppendElement(cache);
     } else {
       return NS_ERROR_FAILURE;
     }
   }
 
   // Now check each lookup fragment against the entries in the DB.
@@ -446,17 +446,17 @@ Classifier::Check(const nsACString& aSpe
     if (LOG_ENABLED()) {
       nsAutoCString checking;
       lookupHash.ToHexString(checking);
       LOG(("Checking fragment %s, hash %s (%X)", fragments[i].get(),
            checking.get(), lookupHash.ToUint32()));
     }
 
     for (uint32_t i = 0; i < cacheArray.Length(); i++) {
-      LookupCache *cache = cacheArray[i];
+      RefPtr<LookupCache> cache = cacheArray[i];
       bool has, confirmed;
       uint32_t matchLength;
 
       rv = cache->Has(lookupHash, &has, &matchLength, &confirmed);
       NS_ENSURE_SUCCESS(rv, rv);
 
       if (has) {
         LookupResult *result = aResults.AppendElement(fallible);
@@ -548,19 +548,16 @@ SwapDirectoryContent(nsIFile* aDir1,
 
   return rv;
 }
 
 void
 Classifier::RemoveUpdateIntermediaries()
 {
   // Remove old LookupCaches.
-  for (auto c: mNewLookupCaches) {
-    delete c;
-  }
   mNewLookupCaches.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.)
@@ -871,45 +868,42 @@ Classifier::ApplyFullHashes(ConstTableUp
 
   return NS_OK;
 }
 
 void
 Classifier::GetCacheInfo(const nsACString& aTable,
                          nsIUrlClassifierCacheInfo** aCache)
 {
-  LookupCache* lookupCache = GetLookupCache(aTable);
+  RefPtr<LookupCache> lookupCache = GetLookupCache(aTable);
   if (!lookupCache) {
     return;
   }
 
   lookupCache->GetCacheInfo(aCache);
 }
 
 void
 Classifier::DropStores()
 {
-  for (uint32_t i = 0; i < mLookupCaches.Length(); i++) {
-    delete mLookupCaches[i];
-  }
   mLookupCaches.Clear();
 }
 
 nsresult
 Classifier::RegenActiveTables()
 {
   mActiveTablesCache.Clear();
 
   nsTArray<nsCString> foundTables;
   ScanStoreDir(mRootStoreDirectory, foundTables);
 
   for (uint32_t i = 0; i < foundTables.Length(); i++) {
     nsCString table(foundTables[i]);
 
-    LookupCache *lookupCache = GetLookupCache(table);
+    RefPtr<LookupCache> lookupCache = GetLookupCache(table);
     if (!lookupCache) {
       LOG(("Inactive table (no cache): %s", table.get()));
       continue;
     }
 
     if (!lookupCache->IsPrimed()) {
       LOG(("Inactive table (cache not primed): %s", table.get()));
       continue;
@@ -1195,24 +1189,29 @@ Classifier::UpdateHashStore(TableUpdateA
   }
 
   nsresult rv = store.Open();
   NS_ENSURE_SUCCESS(rv, rv);
   rv = store.BeginUpdate();
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Read the part of the store that is (only) in the cache
-  LookupCacheV2* lookupCache =
-    LookupCache::Cast<LookupCacheV2>(GetLookupCacheForUpdate(store.TableName()));
-  if (!lookupCache) {
+  RefPtr<LookupCacheV2> lookupCacheV2;
+  {
+    RefPtr<LookupCache> lookupCache = GetLookupCacheForUpdate(store.TableName());
+    if (lookupCache) {
+      lookupCacheV2 = LookupCache::Cast<LookupCacheV2>(lookupCache);
+    }
+  }
+  if (!lookupCacheV2) {
     return NS_ERROR_UC_UPDATE_TABLE_NOT_FOUND;
   }
 
   FallibleTArray<uint32_t> AddPrefixHashes;
-  rv = lookupCache->GetPrefixes(AddPrefixHashes);
+  rv = lookupCacheV2->GetPrefixes(AddPrefixHashes);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = store.AugmentAdds(AddPrefixHashes);
   NS_ENSURE_SUCCESS(rv, rv);
   AddPrefixHashes.Clear();
 
   uint32_t applied = 0;
 
   for (uint32_t i = 0; i < aUpdates.Length(); i++) {
@@ -1255,23 +1254,23 @@ Classifier::UpdateHashStore(TableUpdateA
   LOG(("  %zu sub prefixes", store.SubPrefixes().Length()));
   LOG(("  %zu sub completions", store.SubCompletes().Length()));
 
   rv = store.WriteFile();
   NS_ENSURE_SUCCESS(rv, rv);
 
   // At this point the store is updated and written out to disk, but
   // the data is still in memory.  Build our quick-lookup table here.
-  rv = lookupCache->Build(store.AddPrefixes(), store.AddCompletes());
+  rv = lookupCacheV2->Build(store.AddPrefixes(), store.AddCompletes());
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_BUILD_PREFIX_FAILURE);
 
 #if defined(DEBUG)
-  lookupCache->DumpCompletions();
+  lookupCacheV2->DumpCompletions();
 #endif
-  rv = lookupCache->WriteFile();
+  rv = lookupCacheV2->WriteFile();
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
 
   LOG(("Successfully updated %s", store.TableName().get()));
 
   return NS_OK;
 }
 
 nsresult
@@ -1285,19 +1284,24 @@ Classifier::UpdateTableV4(TableUpdateArr
   }
 
   LOG(("Classifier::UpdateTableV4(%s)", PromiseFlatCString(aTable).get()));
 
   if (!CheckValidUpdate(aUpdates, aTable)) {
     return NS_OK;
   }
 
-  LookupCacheV4* lookupCache =
-    LookupCache::Cast<LookupCacheV4>(GetLookupCacheForUpdate(aTable));
-  if (!lookupCache) {
+  RefPtr<LookupCacheV4> lookupCacheV4;
+  {
+    RefPtr<LookupCache> lookupCache = GetLookupCacheForUpdate(aTable);
+    if (lookupCache) {
+     lookupCacheV4 = LookupCache::Cast<LookupCacheV4>(lookupCache);
+    }
+  }
+  if (!lookupCacheV4) {
     return NS_ERROR_UC_UPDATE_TABLE_NOT_FOUND;
   }
 
   nsresult rv = NS_OK;
 
   // If there are multiple updates for the same table, prefixes1 & prefixes2
   // will act as input and output in turn to reduce memory copy overhead.
   PrefixStringMap prefixes1, prefixes2;
@@ -1312,59 +1316,59 @@ Classifier::UpdateTableV4(TableUpdateArr
     }
 
     RefPtr<TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(update);
     NS_ENSURE_TRUE(updateV4, NS_ERROR_UC_UPDATE_UNEXPECTED_VERSION);
 
     if (updateV4->IsFullUpdate()) {
       input->Clear();
       output->Clear();
-      rv = lookupCache->ApplyUpdate(updateV4, *input, *output);
+      rv = lookupCacheV4->ApplyUpdate(updateV4, *input, *output);
       if (NS_FAILED(rv)) {
         return rv;
       }
     } else {
       // If both prefix sets are empty, this means we are doing a partial update
       // without a prior full/partial update in the loop. In this case we should
       // get prefixes from the lookup cache first.
       if (prefixes1.IsEmpty() && prefixes2.IsEmpty()) {
-        lookupCache->GetPrefixes(prefixes1);
+        lookupCacheV4->GetPrefixes(prefixes1);
       } else {
         MOZ_ASSERT(prefixes1.IsEmpty() ^ prefixes2.IsEmpty());
 
         // When there are multiple partial updates, input should always point
         // to the non-empty prefix set(filled by previous full/partial update).
         // output should always point to the empty prefix set.
         input = prefixes1.IsEmpty() ? &prefixes2 : &prefixes1;
         output = prefixes1.IsEmpty() ? &prefixes1 : &prefixes2;
       }
 
-      rv = lookupCache->ApplyUpdate(updateV4, *input, *output);
+      rv = lookupCacheV4->ApplyUpdate(updateV4, *input, *output);
       if (NS_FAILED(rv)) {
         return rv;
       }
 
       input->Clear();
     }
 
     // Keep track of the last applied update.
     lastAppliedUpdate = updateV4;
 
     aUpdates[i] = nullptr;
   }
 
-  rv = lookupCache->Build(*output);
+  rv = lookupCacheV4->Build(*output);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_BUILD_PREFIX_FAILURE);
 
-  rv = lookupCache->WriteFile();
+  rv = lookupCacheV4->WriteFile();
   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);
+    rv = lookupCacheV4->WriteMetadata(lastAppliedUpdate);
     NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
   }
 
   LOG(("Successfully updated %s\n", PromiseFlatCString(aTable).get()));
 
   return NS_OK;
 }
 
@@ -1373,51 +1377,51 @@ Classifier::UpdateCache(RefPtr<const Tab
 {
   if (!aUpdate) {
     return NS_OK;
   }
 
   nsAutoCString table(aUpdate->TableName());
   LOG(("Classifier::UpdateCache(%s)", table.get()));
 
-  LookupCache *lookupCache = GetLookupCache(table);
+  RefPtr<LookupCache> lookupCache = GetLookupCache(table);
   if (!lookupCache) {
     return NS_ERROR_FAILURE;
   }
 
-  auto lookupV2 = LookupCache::Cast<LookupCacheV2>(lookupCache);
+  RefPtr<LookupCacheV2> lookupV2 = LookupCache::Cast<LookupCacheV2>(lookupCache);
   if (lookupV2) {
     RefPtr<const TableUpdateV2> updateV2 = TableUpdate::Cast<TableUpdateV2>(aUpdate);
     lookupV2->AddGethashResultToCache(updateV2->AddCompletes(),
                                       updateV2->MissPrefixes());
   } else {
-    auto lookupV4 = LookupCache::Cast<LookupCacheV4>(lookupCache);
+    RefPtr<LookupCacheV4> lookupV4 = LookupCache::Cast<LookupCacheV4>(lookupCache);
     if (!lookupV4) {
       return NS_ERROR_FAILURE;
     }
 
     RefPtr<const TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
     lookupV4->AddFullHashResponseToCache(updateV4->FullHashResponse());
   }
 
 #if defined(DEBUG)
   lookupCache->DumpCache();
 #endif
 
   return NS_OK;
 }
 
-LookupCache *
+RefPtr<LookupCache>
 Classifier::GetLookupCache(const nsACString& aTable, bool aForUpdate)
 {
   // GetLookupCache(aForUpdate==true) can only be called on update thread.
   MOZ_ASSERT_IF(aForUpdate, NS_GetCurrentThread() == mUpdateThread);
 
-  nsTArray<LookupCache*>& lookupCaches = aForUpdate ? mNewLookupCaches
-                                                    : mLookupCaches;
+  LookupCacheArray& lookupCaches = aForUpdate ? mNewLookupCaches
+                                              : mLookupCaches;
   auto& rootStoreDirectory = aForUpdate ? mUpdatingDirectory
                                         : mRootStoreDirectory;
 
   for (auto c: lookupCaches) {
     if (c->TableName().Equals(aTable)) {
       return c;
     }
   }
@@ -1425,32 +1429,32 @@ Classifier::GetLookupCache(const nsACStr
   // We don't want to create lookupcache when shutdown is already happening.
   if (nsUrlClassifierDBService::ShutdownHasStarted()) {
     return nullptr;
   }
 
   // TODO : Bug 1302600, It would be better if we have a more general non-main
   //        thread method to convert table name to protocol version. Currently
   //        we can only know this by checking if the table name ends with '-proto'.
-  UniquePtr<LookupCache> cache;
+  RefPtr<LookupCache> cache;
   nsCString provider = GetProvider(aTable);
   if (StringEndsWith(aTable, NS_LITERAL_CSTRING("-proto"))) {
-    cache = MakeUnique<LookupCacheV4>(aTable, provider, rootStoreDirectory);
+    cache = new LookupCacheV4(aTable, provider, rootStoreDirectory);
   } else {
-    cache = MakeUnique<LookupCacheV2>(aTable, provider, rootStoreDirectory);
+    cache = new LookupCacheV2(aTable, provider, rootStoreDirectory);
   }
 
   nsresult rv = cache->Init();
   if (NS_FAILED(rv)) {
     return nullptr;
   }
   rv = cache->Open();
   if (NS_SUCCEEDED(rv)) {
-    lookupCaches.AppendElement(cache.get());
-    return cache.release();
+    lookupCaches.AppendElement(cache);
+    return cache;
   }
 
   // At this point we failed to open LookupCache.
   //
   // GetLookupCache for update and for other usage will run on update thread
   // and worker thread respectively (Bug 1339760). Removing stuff only in
   // their own realms potentially increases the concurrency.
 
@@ -1471,22 +1475,22 @@ nsresult
 Classifier::ReadNoiseEntries(const Prefix& aPrefix,
                              const nsACString& aTableName,
                              uint32_t aCount,
                              PrefixArray& aNoiseEntries)
 {
   FallibleTArray<uint32_t> prefixes;
   nsresult rv;
 
-  LookupCache *cache = GetLookupCache(aTableName);
+  RefPtr<LookupCache> cache = GetLookupCache(aTableName);
   if (!cache) {
     return NS_ERROR_FAILURE;
   }
 
-  LookupCacheV2* cacheV2 = LookupCache::Cast<LookupCacheV2>(cache);
+  RefPtr<LookupCacheV2> cacheV2 = LookupCache::Cast<LookupCacheV2>(cache);
   if (cacheV2) {
     rv = cacheV2->GetPrefixes(prefixes);
   } else {
     rv = LookupCache::Cast<LookupCacheV4>(cache)->GetFixedLengthPrefixes(prefixes);
   }
 
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1557,25 +1561,30 @@ Classifier::LoadMetadata(nsIFile* aDirec
     NS_ENSURE_SUCCESS(rv, rv);
 
     int32_t dot = tableName.RFind(METADATA_SUFFIX);
     if (dot == -1) {
       continue;
     }
     tableName.Cut(dot, METADATA_SUFFIX.Length());
 
-    LookupCacheV4* lookupCache =
-      LookupCache::Cast<LookupCacheV4>(GetLookupCache(tableName));
-    if (!lookupCache) {
+    RefPtr<LookupCacheV4> lookupCacheV4;
+    {
+      RefPtr<LookupCache> lookupCache = GetLookupCache(tableName);
+      if (lookupCache) {
+        lookupCacheV4 = LookupCache::Cast<LookupCacheV4>(lookupCache);
+      }
+    }
+    if (!lookupCacheV4) {
       continue;
     }
 
     nsCString state;
     nsCString checksum;
-    rv = lookupCache->LoadMetadata(state, checksum);
+    rv = lookupCacheV4->LoadMetadata(state, checksum);
     if (NS_FAILED(rv)) {
       LOG(("Failed to get metadata for table %s", tableName.get()));
       continue;
     }
 
     // The state might include '\n' so that we have to encode.
     nsAutoCString stateBase64;
     rv = Base64Encode(state, stateBase64);
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -111,18 +111,18 @@ public:
                                            const nsACString& aTableName,
                                            const nsACString& aProvider,
                                            nsIFile** aPrivateStoreDirectory);
 
   // Swap in in-memory and on-disk database and remove all
   // update intermediaries.
   nsresult SwapInNewTablesAndCleanup();
 
-  LookupCache *GetLookupCache(const nsACString& aTable,
-                              bool aForUpdate = false);
+  RefPtr<LookupCache> GetLookupCache(const nsACString& aTable,
+                                     bool aForUpdate = false);
 
   void GetCacheInfo(const nsACString& aTable,
                     nsIUrlClassifierCacheInfo** aCache);
 
 private:
   void DropStores();
   void DeleteTables(nsIFile* aDirectory, const nsTArray<nsCString>& aTables);
 
@@ -151,23 +151,23 @@ private:
   nsresult UpdateHashStore(TableUpdateArray& aUpdates,
                            const nsACString& aTable);
 
   nsresult UpdateTableV4(TableUpdateArray& aUpdates,
                          const nsACString& aTable);
 
   nsresult UpdateCache(RefPtr<const TableUpdate> aUpdates);
 
-  LookupCache *GetLookupCacheForUpdate(const nsACString& aTable) {
+  RefPtr<LookupCache> GetLookupCacheForUpdate(const nsACString& aTable) {
     return GetLookupCache(aTable, true);
   }
 
-  LookupCache *GetLookupCacheFrom(const nsACString& aTable,
-                                  nsTArray<LookupCache*>& aLookupCaches,
-                                  nsIFile* aRootStoreDirectory);
+  RefPtr<LookupCache> GetLookupCacheFrom(const nsACString& aTable,
+                                         LookupCacheArray& aLookupCaches,
+                                         nsIFile* aRootStoreDirectory);
 
 
   bool CheckValidUpdate(TableUpdateArray& aUpdates,
                         const nsACString& aTable);
 
   nsresult LoadMetadata(nsIFile* aDirectory, nsACString& aResult);
 
   static nsCString GetProvider(const nsACString& aTableName);
@@ -197,30 +197,30 @@ private:
   // Root dir of the Local profile.
   nsCOMPtr<nsIFile> mCacheDirectory;
   // Main directory where to store the databases.
   nsCOMPtr<nsIFile> mRootStoreDirectory;
   // Used for atomically updating the other dirs.
   nsCOMPtr<nsIFile> mBackupDirectory;
   nsCOMPtr<nsIFile> mUpdatingDirectory; // For update only.
   nsCOMPtr<nsIFile> mToDeleteDirectory;
-  nsTArray<LookupCache*> mLookupCaches; // For query only.
+  LookupCacheArray mLookupCaches; // For query only.
   nsTArray<nsCString> mActiveTablesCache;
   uint32_t mHashKey;
 
   // 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;
 
   // The copy of mLookupCaches for update only.
-  nsTArray<LookupCache*> mNewLookupCaches;
+  LookupCacheArray mNewLookupCaches;
 
   bool mUpdateInterrupted;
 
   nsCOMPtr<nsIThread> mUpdateThread; // For async update.
 
   // Identical to mRootStoreDirectory but for update only because
   // nsIFile is not thread safe and mRootStoreDirectory needs to
   // be accessed in CopyInUseDirForUpdate().
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -179,17 +179,18 @@ public:
   //  mail.hostname.com/foo/bar -> [hostname.com, mail.hostname.com]
   //  www.mail.hostname.com/foo/bar -> [hostname.com, mail.hostname.com]
   static nsresult GetHostKeys(const nsACString& aSpec,
                               nsTArray<nsCString>* aHostKeys);
 
   LookupCache(const nsACString& aTableName,
               const nsACString& aProvider,
               nsIFile* aStoreFile);
-  virtual ~LookupCache() {}
+
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(LookupCache);
 
   const nsCString &TableName() const { return mTableName; }
 
   // The directory handle where we operate will
   // be moved away when a backup is made.
   nsresult UpdateRootDirHandle(nsIFile* aRootStoreDirectory);
 
   // Write data stored in lookup cache to disk.
@@ -238,16 +239,18 @@ private:
 
   virtual nsresult StoreToFile(nsIFile* aFile) = 0;
   virtual nsresult LoadFromFile(nsIFile* aFile) = 0;
   virtual size_t SizeOfPrefixSet() const = 0;
 
   virtual int Ver() const = 0;
 
 protected:
+  virtual ~LookupCache() {}
+
   // Check completions in positive cache and prefix in negative cache.
   // 'aHas' and 'aConfirmed' are output parameters.
   nsresult CheckCache(const Completion& aCompletion,
                       bool* aHas,
                       bool* aConfirmed);
 
   bool mPrimed; // true when the PrefixSet has been loaded (or constructed)
   const nsCString mTableName;
@@ -257,24 +260,25 @@ protected:
 
   // For gtest to inspect private members.
   friend class PerProviderDirectoryTestUtils;
 
   // Cache stores fullhash response(V4)/gethash response(V2)
   FullHashResponseMap mFullHashCache;
 };
 
+typedef nsTArray<RefPtr<LookupCache>> LookupCacheArray;
+
 class LookupCacheV2 final : public LookupCache
 {
 public:
   explicit LookupCacheV2(const nsACString& aTableName,
                          const nsACString& aProvider,
                          nsIFile* aStoreFile)
     : LookupCache(aTableName, aProvider, aStoreFile) {}
-  ~LookupCacheV2() {}
 
   virtual nsresult Init() override;
   virtual nsresult Open() override;
   virtual void ClearAll() override;
   virtual nsresult Has(const Completion& aCompletion,
                        bool* aHas,
                        uint32_t* aMatchLength,
                        bool* aConfirmed) override;
@@ -302,16 +306,18 @@ protected:
   nsresult ReadCompletions();
 
   virtual nsresult ClearPrefixes() override;
   virtual nsresult StoreToFile(nsIFile* aFile) override;
   virtual nsresult LoadFromFile(nsIFile* aFile) override;
   virtual size_t SizeOfPrefixSet() const override;
 
 private:
+  ~LookupCacheV2() {}
+
   virtual int Ver() const override { return VER; }
 
   // Construct a Prefix Set with known prefixes.
   // This will Clear() aAddPrefixes when done.
   nsresult ConstructPrefixSet(AddPrefixArray& aAddPrefixes);
 
   // Full length hashes obtained in update request
   CompletionArray mUpdateCompletions;
--- a/toolkit/components/url-classifier/LookupCacheV4.h
+++ b/toolkit/components/url-classifier/LookupCacheV4.h
@@ -16,17 +16,16 @@ class TableUpdateV4;
 
 class LookupCacheV4 final : public LookupCache
 {
 public:
   explicit LookupCacheV4(const nsACString& aTableName,
                          const nsACString& aProvider,
                          nsIFile* aStoreFile)
     : LookupCache(aTableName, aProvider, aStoreFile) {}
-  ~LookupCacheV4() {}
 
   virtual nsresult Init() override;
   virtual nsresult Has(const Completion& aCompletion,
                        bool* aHas,
                        uint32_t* aMatchLength,
                        bool* aConfirmed) override;
 
   virtual bool IsEmpty() const override;
@@ -51,16 +50,18 @@ public:
 
 protected:
   virtual nsresult ClearPrefixes() override;
   virtual nsresult StoreToFile(nsIFile* aFile) override;
   virtual nsresult LoadFromFile(nsIFile* aFile) override;
   virtual size_t SizeOfPrefixSet() const override;
 
 private:
+  ~LookupCacheV4() {}
+
   virtual int Ver() const override { return VER; }
 
   nsresult VerifyChecksum(const nsACString& aChecksum);
 
   RefPtr<VariableLengthPrefixSet> mVLPrefixSet;
 };
 
 } // namespace safebrowsing
--- a/toolkit/components/url-classifier/tests/gtest/Common.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/Common.cpp
@@ -177,25 +177,25 @@ static nsresult
 BuildCache(LookupCacheV4* cache, const _PrefixArray& prefixArray)
 {
   PrefixStringMap map;
   PrefixArrayToPrefixStringMap(prefixArray, map);
   return cache->Build(map);
 }
 
 template<typename T>
-UniquePtr<T>
+RefPtr<T>
 SetupLookupCache(const _PrefixArray& prefixArray)
 {
   nsCOMPtr<nsIFile> file;
   NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file));
 
   file->AppendNative(GTEST_SAFEBROWSING_DIR);
 
-  UniquePtr<T> cache = MakeUnique<T>(GTEST_TABLE, EmptyCString(), file);
+  RefPtr<T> cache = new T(GTEST_TABLE, EmptyCString(), file);
   nsresult rv = cache->Init();
   EXPECT_EQ(rv, NS_OK);
 
-  rv = BuildCache(cache.get(), prefixArray);
+  rv = BuildCache(cache, prefixArray);
   EXPECT_EQ(rv, NS_OK);
 
-  return Move(cache);
+  return cache;
 }
--- a/toolkit/components/url-classifier/tests/gtest/Common.h
+++ b/toolkit/components/url-classifier/tests/gtest/Common.h
@@ -41,9 +41,9 @@ void PrefixArrayToPrefixStringMap(const 
 nsresult PrefixArrayToAddPrefixArrayV2(const nsTArray<nsCString>& prefixArray,
                                        AddPrefixArray& out);
 
 // Generate a hash prefix from string
 nsCString GeneratePrefix(const nsCString& aFragment, uint8_t aLength);
 
 // Create a LookupCacheV4 object with sepecified prefix array.
 template<typename T>
-UniquePtr<T> SetupLookupCache(const _PrefixArray& prefixArray);
+RefPtr<T> SetupLookupCache(const _PrefixArray& prefixArray);
--- a/toolkit/components/url-classifier/tests/gtest/TestCaching.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestCaching.cpp
@@ -70,21 +70,21 @@ TestCache(const Completion aCompletion,
     inCache = aCache->IsInCache(aCompletion.ToUint32());
   } else {
     _PrefixArray array = { GeneratePrefix(_Fragment("cache.notexpired.com/"), 10),
                            GeneratePrefix(_Fragment("cache.expired.com/"), 8),
                            GeneratePrefix(_Fragment("gound.com/"), 5),
                            GeneratePrefix(_Fragment("small.com/"), 4)
                          };
 
-    UniquePtr<T> cache = SetupLookupCache<T>(array);
+    RefPtr<T> cache = SetupLookupCache<T>(array);
 
     // Create an expired entry and a non-expired entry
-    SetupCacheEntry(cache.get(), _Fragment("cache.notexpired.com/"));
-    SetupCacheEntry(cache.get(), _Fragment("cache.expired.com/"), true, true);
+    SetupCacheEntry(cache, _Fragment("cache.notexpired.com/"));
+    SetupCacheEntry(cache, _Fragment("cache.expired.com/"), true, true);
 
     cache->Has(aCompletion, &has, &matchLength, &confirmed);
     inCache = cache->IsInCache(aCompletion.ToUint32());
   }
 
   EXPECT_EQ(has, aExpectedHas);
   EXPECT_EQ(confirmed, aExpectedConfirmed);
   EXPECT_EQ(inCache, aExpectedInCache);
@@ -193,22 +193,22 @@ TEST(UrlClassifierCaching, InNegativeCac
 template<typename T>
 void TestInvalidateExpiredCacheEntry()
 {
   _PrefixArray array = { GeneratePrefix(CACHED_URL, 10),
                          GeneratePrefix(NEG_CACHE_EXPIRED_URL, 8),
                          GeneratePrefix(POS_CACHE_EXPIRED_URL, 5),
                          GeneratePrefix(BOTH_CACHE_EXPIRED_URL, 4)
                        };
-  UniquePtr<T> cache = SetupLookupCache<T>(array);
+  RefPtr<T> cache = SetupLookupCache<T>(array);
 
-  SetupCacheEntry(cache.get(), CACHED_URL, false, false);
-  SetupCacheEntry(cache.get(), NEG_CACHE_EXPIRED_URL, true, false);
-  SetupCacheEntry(cache.get(), POS_CACHE_EXPIRED_URL, false, true);
-  SetupCacheEntry(cache.get(), BOTH_CACHE_EXPIRED_URL, true, true);
+  SetupCacheEntry(cache, CACHED_URL, false, false);
+  SetupCacheEntry(cache, NEG_CACHE_EXPIRED_URL, true, false);
+  SetupCacheEntry(cache, POS_CACHE_EXPIRED_URL, false, true);
+  SetupCacheEntry(cache, BOTH_CACHE_EXPIRED_URL, true, true);
 
   // Before invalidate
   TestCache<T>(CACHED_URL, true, true, true, cache.get());
   TestCache<T>(NEG_CACHE_EXPIRED_URL, true, true, true, cache.get());
   TestCache<T>(POS_CACHE_EXPIRED_URL, true, false, true, cache.get());
   TestCache<T>(BOTH_CACHE_EXPIRED_URL, true, false, true, cache.get());
 
   // Call InvalidateExpiredCacheEntry to remove cache entries whose negative cache
@@ -236,17 +236,17 @@ TEST(UrlClassifierCaching, InvalidateExp
 }
 
 // This testcase check if an cache entry whose negative cache time is expired
 // and it doesn't have any postive cache entries in it, it should be removed
 // from cache after calling |Has|.
 TEST(UrlClassifierCaching, NegativeCacheExpireV2)
 {
   _PrefixArray array = { GeneratePrefix(NEG_CACHE_EXPIRED_URL, 8) };
-  UniquePtr<LookupCacheV2> cache = SetupLookupCache<LookupCacheV2>(array);
+  RefPtr<LookupCacheV2> cache = SetupLookupCache<LookupCacheV2>(array);
 
   nsCOMPtr<nsICryptoHash> cryptoHash = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID);
 
   MissPrefixArray misses;
   Prefix* prefix = misses.AppendElement(fallible);
   prefix->FromPlaintext(NEG_CACHE_EXPIRED_URL);
 
   AddCompleteArray dummy;
@@ -257,17 +257,17 @@ TEST(UrlClassifierCaching, NegativeCache
 
   // It should be removed after calling Has API.
   TestCache<LookupCacheV2>(NEG_CACHE_EXPIRED_URL, true, false, false, cache.get());
 }
 
 TEST(UrlClassifierCaching, NegativeCacheExpireV4)
 {
   _PrefixArray array = { GeneratePrefix(NEG_CACHE_EXPIRED_URL, 8) };
-  UniquePtr<LookupCacheV4> cache = SetupLookupCache<LookupCacheV4>(array);
+  RefPtr<LookupCacheV4> cache = SetupLookupCache<LookupCacheV4>(array);
 
   FullHashResponseMap map;
   Prefix prefix;
   nsCOMPtr<nsICryptoHash> cryptoHash = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID);
   prefix.FromPlaintext(NEG_CACHE_EXPIRED_URL);
   CachedFullHashResponse* response = map.LookupOrAdd(prefix.ToUint32());
 
   response->negativeCacheExpirySec = EXPIRED_TIME_SEC;
--- a/toolkit/components/url-classifier/tests/gtest/TestClassifier.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestClassifier.cpp
@@ -25,48 +25,55 @@ GetClassifier()
   return Move(classifier);
 }
 
 static nsresult
 SetupLookupCacheV4(Classifier* classifier,
                    const _PrefixArray& aPrefixArray,
                    const nsACString& aTable)
 {
-  LookupCacheV4* lookupCache =
-    LookupCache::Cast<LookupCacheV4>(classifier->GetLookupCache(aTable, false));
+  RefPtr<LookupCache> lookupCache = classifier->GetLookupCache(aTable, false);
   if (!lookupCache) {
     return NS_ERROR_FAILURE;
   }
+  RefPtr<LookupCacheV4> lookupCacheV4 = LookupCache::Cast<LookupCacheV4>(lookupCache);
+  if (!lookupCacheV4) {
+    return NS_ERROR_FAILURE;
+  }
 
   PrefixStringMap map;
   PrefixArrayToPrefixStringMap(aPrefixArray, map);
 
-  return lookupCache->Build(map);
+  return lookupCacheV4->Build(map);
 }
 
 static nsresult
 SetupLookupCacheV2(Classifier* classifier,
                    const _PrefixArray& aPrefixArray,
                    const nsACString& aTable)
 {
-  LookupCacheV2* lookupCache =
-    LookupCache::Cast<LookupCacheV2>(classifier->GetLookupCache(aTable, false));
+  RefPtr<LookupCache> lookupCache = classifier->GetLookupCache(aTable, false);
   if (!lookupCache) {
     return NS_ERROR_FAILURE;
   }
+  RefPtr<LookupCacheV2> lookupCacheV2 =
+    LookupCache::Cast<LookupCacheV2>(lookupCache);
+  if (!lookupCacheV2) {
+    return NS_ERROR_FAILURE;
+  }
 
   AddPrefixArray prefixes;
   AddCompleteArray completions;
   nsresult rv = PrefixArrayToAddPrefixArrayV2(aPrefixArray, prefixes);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   EntrySort(prefixes);
-  return lookupCache->Build(prefixes, completions);
+  return lookupCacheV2->Build(prefixes, completions);
 }
 
 static void
 TestReadNoiseEntries(Classifier* classifier,
                      const _PrefixArray& aPrefixArray,
                      const nsCString& aTable,
                      const nsCString& aFragment)
 {
--- a/toolkit/components/url-classifier/tests/gtest/TestLookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestLookupCacheV4.cpp
@@ -9,17 +9,17 @@ TestHasPrefix(const _Fragment& aFragment
 {
   _PrefixArray array = { GeneratePrefix(_Fragment("bravo.com/"), 32),
                          GeneratePrefix(_Fragment("browsing.com/"), 8),
                          GeneratePrefix(_Fragment("gound.com/"), 5),
                          GeneratePrefix(_Fragment("small.com/"), 4)
                        };
 
   RunTestInNewThread([&] () -> void {
-    UniquePtr<LookupCache> cache = SetupLookupCache<LookupCacheV4>(array);
+    RefPtr<LookupCache> cache = SetupLookupCache<LookupCacheV4>(array);
 
     Completion lookupHash;
     lookupHash.FromPlaintext(aFragment);
 
     bool has, confirmed;
     uint32_t matchLength;
     // Freshness is not used in V4 so we just put dummy values here.
     TableFreshnessMap dummy;
--- a/toolkit/components/url-classifier/tests/gtest/TestPerProviderDirectory.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestPerProviderDirectory.cpp
@@ -18,81 +18,109 @@ public:
 
 } // end of namespace safebrowsing
 } // end of namespace mozilla
 
 using namespace mozilla;
 using namespace mozilla::safebrowsing;
 
 template<typename T>
-void VerifyPrivateStorePath(const char* aTableName,
-                            const char* aProvider,
+void VerifyPrivateStorePath(T* target,
+                            const nsCString& aTableName,
+                            const nsCString& aProvider,
                             nsIFile* aRootDir,
                             bool aUsePerProviderStore)
 {
   nsString rootStorePath;
   nsresult rv = aRootDir->GetPath(rootStorePath);
   EXPECT_EQ(rv, NS_OK);
 
-  T target(nsCString(aTableName), nsCString(aProvider), aRootDir);
-
   nsIFile* privateStoreDirectory =
-    PerProviderDirectoryTestUtils::InspectStoreDirectory(target);
+    PerProviderDirectoryTestUtils::InspectStoreDirectory(*target);
 
   nsString privateStorePath;
   rv = privateStoreDirectory->GetPath(privateStorePath);
   ASSERT_EQ(rv, NS_OK);
 
   nsString expectedPrivateStorePath = rootStorePath;
 
   if (aUsePerProviderStore) {
     // Use API to append "provider" to the root directoy path
     nsCOMPtr<nsIFile> expectedPrivateStoreDir;
     rv = aRootDir->Clone(getter_AddRefs(expectedPrivateStoreDir));
     ASSERT_EQ(rv, NS_OK);
 
-    expectedPrivateStoreDir->AppendNative(nsCString(aProvider));
+    expectedPrivateStoreDir->AppendNative(aProvider);
     rv = expectedPrivateStoreDir->GetPath(expectedPrivateStorePath);
     ASSERT_EQ(rv, NS_OK);
   }
 
   printf("table: %s\nprovider: %s\nroot path: %s\nprivate path: %s\n\n",
-         aTableName,
-         aProvider,
+         aTableName.get(),
+         aProvider.get(),
          NS_ConvertUTF16toUTF8(rootStorePath).get(),
          NS_ConvertUTF16toUTF8(privateStorePath).get());
 
   ASSERT_TRUE(privateStorePath == expectedPrivateStorePath);
 }
 
 TEST(UrlClassifierPerProviderDirectory, LookupCache)
 {
   RunTestInNewThread([] () -> void {
     nsCOMPtr<nsIFile> rootDir;
     NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(rootDir));
 
     // For V2 tables (NOT ending with '-proto'), root directory should be
     // used as the private store.
-    VerifyPrivateStorePath<LookupCacheV2>("goog-phish-shavar", "google", rootDir, false);
+    {
+      nsAutoCString table("goog-phish-shavar");
+      nsAutoCString provider("google");
+      RefPtr<LookupCacheV2> lc = new LookupCacheV2(table, provider, rootDir);
+      VerifyPrivateStorePath<LookupCacheV2>(lc, table, provider, rootDir, false);
+    }
 
     // For V4 tables, if provider is found, use per-provider subdirectory;
     // If not found, use root directory.
-    VerifyPrivateStorePath<LookupCacheV4>("goog-noprovider-proto", "", rootDir, false);
-    VerifyPrivateStorePath<LookupCacheV4>("goog-phish-proto", "google4", rootDir, true);
+    {
+      nsAutoCString table("goog-noprovider-proto");
+      nsAutoCString provider("");
+      RefPtr<LookupCacheV4> lc = new LookupCacheV4(table, provider, rootDir);
+      VerifyPrivateStorePath<LookupCacheV4>(lc, table, provider, rootDir, false);
+    }
+    {
+      nsAutoCString table("goog-phish-proto");
+      nsAutoCString provider("google4");
+      RefPtr<LookupCacheV4> lc = new LookupCacheV4(table, provider, rootDir);
+      VerifyPrivateStorePath<LookupCacheV4>(lc, table, provider, rootDir, true);
+    }
   });
 }
 
 TEST(UrlClassifierPerProviderDirectory, HashStore)
 {
   RunTestInNewThread([] () -> void {
     nsCOMPtr<nsIFile> rootDir;
     NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(rootDir));
 
     // For V2 tables (NOT ending with '-proto'), root directory should be
     // used as the private store.
-    VerifyPrivateStorePath<HashStore>("goog-phish-shavar", "google", rootDir, false);
-
+    {
+      nsAutoCString table("goog-phish-shavar");
+      nsAutoCString provider("google");
+      HashStore hs(table, provider, rootDir);
+      VerifyPrivateStorePath(&hs, table, provider, rootDir, false);
+    }
     // For V4 tables, if provider is found, use per-provider subdirectory;
     // If not found, use root directory.
-    VerifyPrivateStorePath<HashStore>("goog-noprovider-proto", "", rootDir, false);
-    VerifyPrivateStorePath<HashStore>("goog-phish-proto", "google4", rootDir, true);
+    {
+      nsAutoCString table("goog-noprovider-proto");
+      nsAutoCString provider("");
+      HashStore hs(table, provider, rootDir);
+      VerifyPrivateStorePath(&hs, table, provider, rootDir, false);
+    }
+    {
+      nsAutoCString table("goog-phish-proto");
+      nsAutoCString provider("google4");
+      HashStore hs(table, provider, rootDir);
+      VerifyPrivateStorePath(&hs, table, provider, rootDir, true);
+    }
   });
 }
--- a/toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
@@ -250,21 +250,22 @@ testPartialUpdate(PrefixStringMap& add,
 static void
 testOpenLookupCache()
 {
   nsCOMPtr<nsIFile> file;
   NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file));
   file->AppendNative(GTEST_SAFEBROWSING_DIR);
 
   RunTestInNewThread([&] () -> void {
-    LookupCacheV4 cache(nsCString(GTEST_TABLE), EmptyCString(), file);
-    nsresult rv = cache.Init();
+    RefPtr<LookupCacheV4> cache = new LookupCacheV4(nsCString(GTEST_TABLE),
+                                                    EmptyCString(), file);
+    nsresult rv = cache->Init();
     ASSERT_EQ(rv, NS_OK);
 
-    rv = cache.Open();
+    rv = cache->Open();
     ASSERT_EQ(rv, NS_OK);
   });
 }
 
 // Tests start from here.
 TEST(UrlClassifierTableUpdateV4, FixLenghtPSetFullUpdate)
 {
   srand(time(NULL));
@@ -793,17 +794,17 @@ TEST(UrlClassifierTableUpdateV4, EmptyUp
 // This test ensure applying an empty update directly through update algorithm
 // should be correct.
 TEST(UrlClassifierTableUpdateV4, EmptyUpdate2)
 {
   // Setup LookupCache with initial data
   _PrefixArray array;
   CreateRandomSortedPrefixArray(100, 4, 4, array);
   CreateRandomSortedPrefixArray(10, 5, 32, array);
-  UniquePtr<LookupCacheV4> cache = SetupLookupCache<LookupCacheV4>(array);
+  RefPtr<LookupCacheV4> cache = SetupLookupCache<LookupCacheV4>(array);
 
   // Setup TableUpdate object with only checksum from previous update(initial data).
   nsCString checksum;
   CalculateCheckSum(array, checksum);
   std::string stdChecksum;
   stdChecksum.assign(const_cast<char*>(checksum.BeginReading()), checksum.Length());
 
   RefPtr<TableUpdateV4> tableUpdate = new TableUpdateV4(GTEST_TABLE);