Bug 1434206 - Keep TableUpdate objects in smart pointers. r?gcp draft
authorFrancois Marier <francois@mozilla.com>
Fri, 01 Jun 2018 15:48:48 -0700
changeset 805421 efdd28ddff6a77fbac97f70720424a09f1646f68
parent 805420 70eb311be2baf190bb74da4aac32542c08476644
child 805422 4fd6aa3f48122572764b3a89725bdede902e2fd6
push id112654
push userfmarier@mozilla.com
push dateThu, 07 Jun 2018 20:10:46 +0000
reviewersgcp
bugs1434206
milestone62.0a1
Bug 1434206 - Keep TableUpdate objects in smart pointers. r?gcp Manually keeping tabs on the lifetime of these objects is a pain and is the likely source of some of our crashes. I suspect we might also be leaking memory. This change creates an explicit copy of the main array into the update thread to avoid using a non-thread-safe shared data structure. This is a shallow copy. Only the pointers to the TableUpdates are copied, which means one pointer per list (e.g. 5 in total for google4 in a new profile). MozReview-Commit-ID: 221d6GkKt0M
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/Classifier.h
toolkit/components/url-classifier/HashStore.cpp
toolkit/components/url-classifier/HashStore.h
toolkit/components/url-classifier/LookupCacheV4.cpp
toolkit/components/url-classifier/LookupCacheV4.h
toolkit/components/url-classifier/ProtocolParser.cpp
toolkit/components/url-classifier/ProtocolParser.h
toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
toolkit/components/url-classifier/nsUrlClassifierDBService.h
toolkit/components/url-classifier/tests/gtest/Common.cpp
toolkit/components/url-classifier/tests/gtest/TestFailUpdate.cpp
toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -36,43 +36,16 @@ extern mozilla::LazyLogModule gUrlClassi
 #define BACKUP_DIR_SUFFIX    NS_LITERAL_CSTRING("-backup")
 #define UPDATING_DIR_SUFFIX  NS_LITERAL_CSTRING("-updating")
 
 #define METADATA_SUFFIX      NS_LITERAL_CSTRING(".metadata")
 
 namespace mozilla {
 namespace safebrowsing {
 
-namespace {
-
-// A scoped-clearer for nsTArray<TableUpdate*>.
-// The owning elements will be deleted and the array itself
-// will be cleared on exiting the scope.
-class ScopedUpdatesClearer {
-public:
-  explicit ScopedUpdatesClearer(nsTArray<TableUpdate*> *aUpdates)
-    : mUpdatesArrayRef(aUpdates)
-  {
-    for (auto update : *aUpdates) {
-      mUpdatesPointerHolder.AppendElement(update);
-    }
-  }
-
-  ~ScopedUpdatesClearer()
-  {
-    mUpdatesArrayRef->Clear();
-  }
-
-private:
-  nsTArray<TableUpdate*>* mUpdatesArrayRef;
-  nsTArray<UniquePtr<TableUpdate>> mUpdatesPointerHolder;
-};
-
-} // End of unnamed namespace.
-
 void
 Classifier::SplitTables(const nsACString& str, nsTArray<nsCString>& tables)
 {
   tables.Clear();
 
   nsACString::const_iterator begin, iter, end;
   str.BeginReading(begin);
   str.EndReading(end);
@@ -703,17 +676,17 @@ void Classifier::FlushAndDisableAsyncUpd
     return;
   }
 
   mUpdateThread->Shutdown();
   mUpdateThread = nullptr;
 }
 
 nsresult
-Classifier::AsyncApplyUpdates(nsTArray<TableUpdate*>* aUpdates,
+Classifier::AsyncApplyUpdates(const TableUpdateArray& aUpdates,
                               const AsyncUpdateCallback& aCallback)
 {
   LOG(("Classifier::AsyncApplyUpdates"));
 
   if (!mUpdateThread) {
     LOG(("Async update has already been disabled."));
     return NS_ERROR_FAILURE;
   }
@@ -736,19 +709,29 @@ Classifier::AsyncApplyUpdates(nsTArray<T
   nsCOMPtr<nsIThread> callerThread = NS_GetCurrentThread();
   MOZ_ASSERT(callerThread != mUpdateThread);
 
   nsCOMPtr<nsIRunnable> bgRunnable =
     NS_NewRunnableFunction("safebrowsing::Classifier::AsyncApplyUpdates", [=] {
       MOZ_ASSERT(NS_GetCurrentThread() == mUpdateThread,
                  "MUST be on update thread");
 
-      LOG(("Step 1. ApplyUpdatesBackground on update thread."));
+      nsresult bgRv;
       nsCString failedTableName;
-      nsresult bgRv = ApplyUpdatesBackground(aUpdates, failedTableName);
+      TableUpdateArray updates;
+
+      // Make a copy of the array since we'll be removing entries as
+      // we process them on the background thread.
+      if (updates.AppendElements(aUpdates, fallible)) {
+        LOG(("Step 1. ApplyUpdatesBackground on update thread."));
+        bgRv = ApplyUpdatesBackground(updates, failedTableName);
+      } else {
+        LOG(("Step 1. Not enough memory to run ApplyUpdatesBackground on update thread."));
+        bgRv = NS_ERROR_OUT_OF_MEMORY;
+      }
 
       nsCOMPtr<nsIRunnable> fgRunnable = NS_NewRunnableFunction(
         "safebrowsing::Classifier::AsyncApplyUpdates", [=] {
           MOZ_ASSERT(NS_GetCurrentThread() == callerThread,
                      "MUST be on caller thread");
 
           LOG(("Step 2. ApplyUpdatesForeground on caller thread"));
           nsresult rv = ApplyUpdatesForeground(bgRv, failedTableName);
@@ -760,92 +743,88 @@ Classifier::AsyncApplyUpdates(nsTArray<T
         });
       callerThread->Dispatch(fgRunnable, NS_DISPATCH_NORMAL);
     });
 
   return mUpdateThread->Dispatch(bgRunnable, NS_DISPATCH_NORMAL);
 }
 
 nsresult
-Classifier::ApplyUpdatesBackground(nsTArray<TableUpdate*>* aUpdates,
+Classifier::ApplyUpdatesBackground(TableUpdateArray& aUpdates,
                                    nsACString& aFailedTableName)
 {
   // |mUpdateInterrupted| is guaranteed to have been unset.
   // If |mUpdateInterrupted| is set at any point, Reset() must have
   // been called then we need to interrupt the update process.
   // We only add checkpoints for non-trivial tasks.
 
-  if (!aUpdates || aUpdates->Length() == 0) {
+  if (aUpdates.IsEmpty()) {
     return NS_OK;
   }
 
   nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
     do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
 
   nsCString provider;
   // Assume all TableUpdate objects should have the same provider.
-  urlUtil->GetTelemetryProvider((*aUpdates)[0]->TableName(), provider);
+  urlUtil->GetTelemetryProvider(aUpdates[0]->TableName(), provider);
 
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_CL_KEYED_UPDATE_TIME>
     keyedTimer(provider);
 
   PRIntervalTime clockStart = 0;
   if (LOG_ENABLED()) {
     clockStart = PR_IntervalNow();
   }
 
   nsresult rv;
 
-  {
-    ScopedUpdatesClearer scopedUpdatesClearer(aUpdates);
+  // Check point 1: Copying file takes time so we check here.
+  if (mUpdateInterrupted) {
+    LOG(("Update is interrupted. Don't copy files."));
+    return NS_OK;
+  }
 
-    {
-      // Check point 1: Copying file takes time so we check here.
-      if (mUpdateInterrupted) {
-        LOG(("Update is interrupted. Don't copy files."));
-        return NS_OK;
-      }
+  rv = CopyInUseDirForUpdate(); // i.e. mUpdatingDirectory will be setup.
+  if (NS_FAILED(rv)) {
+    LOG(("Failed to copy in-use directory for update."));
+    return rv;
+  }
 
-      rv = CopyInUseDirForUpdate(); // i.e. mUpdatingDirectory will be setup.
-      if (NS_FAILED(rv)) {
-        LOG(("Failed to copy in-use directory for update."));
-        return rv;
-      }
+  LOG(("Applying %zu table updates.", aUpdates.Length()));
+
+  for (uint32_t i = 0; i < aUpdates.Length(); i++) {
+    RefPtr<TableUpdate> update = aUpdates[i];
+    if (!update) {
+      // Previous UpdateHashStore() may have consumed this update..
+      continue;
     }
 
-    LOG(("Applying %zu table updates.", aUpdates->Length()));
-
-    for (uint32_t i = 0; i < aUpdates->Length(); i++) {
-      // Previous UpdateHashStore() may have consumed this update..
-      if ((*aUpdates)[i]) {
-        // Run all updates for one table
-        nsCString updateTable(aUpdates->ElementAt(i)->TableName());
-
-        // Check point 2: Processing downloaded data takes time.
-        if (mUpdateInterrupted) {
-          LOG(("Update is interrupted. Stop building new tables."));
-          return NS_OK;
-        }
+    // Run all updates for one table
+    nsAutoCString updateTable(update->TableName());
 
-        // Will update the mirrored in-memory and on-disk databases.
-        if (TableUpdate::Cast<TableUpdateV2>((*aUpdates)[i])) {
-          rv = UpdateHashStore(aUpdates, updateTable);
-        } else {
-          rv = UpdateTableV4(aUpdates, updateTable);
-        }
-
-        if (NS_FAILED(rv)) {
-          aFailedTableName = updateTable;
-          RemoveUpdateIntermediaries();
-          return rv;
-        }
-      }
+    // Check point 2: Processing downloaded data takes time.
+    if (mUpdateInterrupted) {
+      LOG(("Update is interrupted. Stop building new tables."));
+      return NS_OK;
     }
 
-  } // End of scopedUpdatesClearer scope.
+    // Will update the mirrored in-memory and on-disk databases.
+    if (TableUpdate::Cast<TableUpdateV2>(update)) {
+      rv = UpdateHashStore(aUpdates, updateTable);
+    } else {
+      rv = UpdateTableV4(aUpdates, updateTable);
+    }
+
+    if (NS_FAILED(rv)) {
+      aFailedTableName = updateTable;
+      RemoveUpdateIntermediaries();
+      return rv;
+    }
+  }
 
   if (LOG_ENABLED()) {
     PRIntervalTime clockEnd = PR_IntervalNow();
     LOG(("update took %dms\n",
          PR_IntervalToMilliseconds(clockEnd - clockStart)));
   }
 
   return rv;
@@ -869,33 +848,30 @@ Classifier::ApplyUpdatesForeground(nsres
   }
   if (NS_ERROR_OUT_OF_MEMORY != aBackgroundRv) {
     ResetTables(Clear_All, nsTArray<nsCString> { nsCString(aFailedTableName) });
   }
   return aBackgroundRv;
 }
 
 nsresult
-Classifier::ApplyFullHashes(nsTArray<TableUpdate*>* aUpdates)
+Classifier::ApplyFullHashes(TableUpdateArray& aUpdates)
 {
   MOZ_ASSERT(NS_GetCurrentThread() != mUpdateThread,
              "ApplyFullHashes() MUST NOT be called on update thread");
   MOZ_ASSERT(!NS_IsMainThread(),
              "ApplyFullHashes() must be called on the classifier worker thread.");
 
-  LOG(("Applying %zu table gethashes.", aUpdates->Length()));
+  LOG(("Applying %zu table gethashes.", aUpdates.Length()));
 
-  ScopedUpdatesClearer scopedUpdatesClearer(aUpdates);
-  for (uint32_t i = 0; i < aUpdates->Length(); i++) {
-    TableUpdate *update = aUpdates->ElementAt(i);
-
-    nsresult rv = UpdateCache(update);
+  for (uint32_t i = 0; i < aUpdates.Length(); i++) {
+    nsresult rv = UpdateCache(aUpdates[i]);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    aUpdates->ElementAt(i) = nullptr;
+    aUpdates[i] = nullptr;
   }
 
   return NS_OK;
 }
 
 void
 Classifier::GetCacheInfo(const nsACString& aTable,
                          nsIUrlClassifierCacheInfo** aCache)
@@ -1155,29 +1131,30 @@ Classifier::RecoverBackups()
     rv = SetupPathNames();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return NS_OK;
 }
 
 bool
-Classifier::CheckValidUpdate(nsTArray<TableUpdate*>* aUpdates,
+Classifier::CheckValidUpdate(TableUpdateArray& aUpdates,
                              const nsACString& aTable)
 {
   // take the quick exit if there is no valid update for us
   // (common case)
   uint32_t validupdates = 0;
 
-  for (uint32_t i = 0; i < aUpdates->Length(); i++) {
-    TableUpdate *update = aUpdates->ElementAt(i);
-    if (!update || !update->TableName().Equals(aTable))
+  for (uint32_t i = 0; i < aUpdates.Length(); i++) {
+    RefPtr<TableUpdate> update = aUpdates[i];
+    if (!update || !update->TableName().Equals(aTable)) {
       continue;
+    }
     if (update->Empty()) {
-      aUpdates->ElementAt(i) = nullptr;
+      aUpdates[i] = nullptr;
       continue;
     }
     validupdates++;
   }
 
   if (!validupdates) {
     // This can happen if the update was only valid for one table.
     return false;
@@ -1197,17 +1174,17 @@ Classifier::GetProvider(const nsACString
 
   return NS_SUCCEEDED(rv) ? provider : EmptyCString();
 }
 
 /*
  * This will consume+delete updates from the passed nsTArray.
 */
 nsresult
-Classifier::UpdateHashStore(nsTArray<TableUpdate*>* aUpdates,
+Classifier::UpdateHashStore(TableUpdateArray& aUpdates,
                             const nsACString& aTable)
 {
   if (nsUrlClassifierDBService::ShutdownHasStarted()) {
     return NS_ERROR_UC_UPDATE_SHUTDOWNING;
   }
 
   LOG(("Classifier::UpdateHashStore(%s)", PromiseFlatCString(aTable).get()));
 
@@ -1233,23 +1210,23 @@ Classifier::UpdateHashStore(nsTArray<Tab
   rv = lookupCache->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++) {
-    TableUpdate *update = aUpdates->ElementAt(i);
+  for (uint32_t i = 0; i < aUpdates.Length(); i++) {
+    RefPtr<TableUpdate> update = aUpdates[i];
     if (!update || !update->TableName().Equals(store.TableName())) {
       continue;
     }
 
-    TableUpdateV2* updateV2 = TableUpdate::Cast<TableUpdateV2>(update);
+    RefPtr<TableUpdateV2> updateV2 = TableUpdate::Cast<TableUpdateV2>(update);
     NS_ENSURE_TRUE(updateV2, NS_ERROR_UC_UPDATE_UNEXPECTED_VERSION);
 
     rv = store.ApplyUpdate(updateV2);
     NS_ENSURE_SUCCESS(rv, rv);
 
     applied++;
 
     LOG(("Applied update to table %s:", store.TableName().get()));
@@ -1257,17 +1234,17 @@ Classifier::UpdateHashStore(nsTArray<Tab
     LOG(("  %zu add prefixes", updateV2->AddPrefixes().Length()));
     LOG(("  %zu add completions", updateV2->AddCompletes().Length()));
     LOG(("  %d sub chunks", updateV2->SubChunks().Length()));
     LOG(("  %zu sub prefixes", updateV2->SubPrefixes().Length()));
     LOG(("  %zu sub completions", updateV2->SubCompletes().Length()));
     LOG(("  %d add expirations", updateV2->AddExpirations().Length()));
     LOG(("  %d sub expirations", updateV2->SubExpirations().Length()));
 
-    aUpdates->ElementAt(i) = nullptr;
+    aUpdates[i] = nullptr;
   }
 
   LOG(("Applied %d update(s) to %s.", applied, store.TableName().get()));
 
   rv = store.Rebuild();
   NS_ENSURE_SUCCESS(rv, rv);
 
   LOG(("Table %s now has:", store.TableName().get()));
@@ -1293,17 +1270,17 @@ Classifier::UpdateHashStore(nsTArray<Tab
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
 
   LOG(("Successfully updated %s", store.TableName().get()));
 
   return NS_OK;
 }
 
 nsresult
-Classifier::UpdateTableV4(nsTArray<TableUpdate*>* aUpdates,
+Classifier::UpdateTableV4(TableUpdateArray& aUpdates,
                           const nsACString& aTable)
 {
   MOZ_ASSERT(!NS_IsMainThread(),
              "UpdateTableV4 must be called on the classifier worker thread.");
   if (nsUrlClassifierDBService::ShutdownHasStarted()) {
     return NS_ERROR_UC_UPDATE_SHUTDOWNING;
   }
 
@@ -1323,23 +1300,23 @@ Classifier::UpdateTableV4(nsTArray<Table
 
   // 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;
   PrefixStringMap* input = &prefixes1;
   PrefixStringMap* output = &prefixes2;
 
   TableUpdateV4* lastAppliedUpdate = nullptr;
-  for (uint32_t i = 0; i < aUpdates->Length(); i++) {
-    TableUpdate *update = aUpdates->ElementAt(i);
+  for (uint32_t i = 0; i < aUpdates.Length(); i++) {
+    RefPtr<TableUpdate> update = aUpdates[i];
     if (!update || !update->TableName().Equals(aTable)) {
       continue;
     }
 
-    auto updateV4 = TableUpdate::Cast<TableUpdateV4>(update);
+    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);
       if (NS_FAILED(rv)) {
         return rv;
@@ -1366,17 +1343,17 @@ Classifier::UpdateTableV4(nsTArray<Table
       }
 
       input->Clear();
     }
 
     // Keep track of the last applied update.
     lastAppliedUpdate = updateV4;
 
-    aUpdates->ElementAt(i) = nullptr;
+    aUpdates[i] = nullptr;
   }
 
   rv = lookupCache->Build(*output);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_BUILD_PREFIX_FAILURE);
 
   rv = lookupCache->WriteFile();
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
 
@@ -1387,42 +1364,42 @@ Classifier::UpdateTableV4(nsTArray<Table
   }
 
   LOG(("Successfully updated %s\n", PromiseFlatCString(aTable).get()));
 
   return NS_OK;
 }
 
 nsresult
-Classifier::UpdateCache(TableUpdate* aUpdate)
+Classifier::UpdateCache(RefPtr<TableUpdate> aUpdate)
 {
   if (!aUpdate) {
     return NS_OK;
   }
 
   nsAutoCString table(aUpdate->TableName());
   LOG(("Classifier::UpdateCache(%s)", table.get()));
 
   LookupCache *lookupCache = GetLookupCache(table);
   if (!lookupCache) {
     return NS_ERROR_FAILURE;
   }
 
   auto lookupV2 = LookupCache::Cast<LookupCacheV2>(lookupCache);
   if (lookupV2) {
-    auto updateV2 = TableUpdate::Cast<TableUpdateV2>(aUpdate);
+    RefPtr<TableUpdateV2> updateV2 = TableUpdate::Cast<TableUpdateV2>(aUpdate);
     lookupV2->AddGethashResultToCache(updateV2->AddCompletes(),
                                       updateV2->MissPrefixes());
   } else {
     auto lookupV4 = LookupCache::Cast<LookupCacheV4>(lookupCache);
     if (!lookupV4) {
       return NS_ERROR_FAILURE;
     }
 
-    auto updateV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
+    RefPtr<TableUpdateV4> updateV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
     lookupV4->AddFullHashResponseToCache(updateV4->FullHashResponse());
   }
 
 #if defined(DEBUG)
   lookupCache->DumpCache();
 #endif
 
   return NS_OK;
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -59,35 +59,33 @@ public:
    */
   nsresult Check(const nsACString& aSpec,
                  const nsACString& tables,
                  LookupResultArray& aResults);
 
   /**
    * Asynchronously apply updates to the in-use databases. When the
    * update is complete, the caller can be notified by |aCallback|, which
-   * will occur on the caller thread. Note that the ownership of
-   * |aUpdates| will be transferred. This design is inherited from the
-   * previous sync update function (ApplyUpdates) which has been removed.
+   * will occur on the caller thread.
    */
   using AsyncUpdateCallback = std::function<void(nsresult)>;
-  nsresult AsyncApplyUpdates(nsTArray<TableUpdate*>* aUpdates,
+  nsresult AsyncApplyUpdates(const TableUpdateArray& aUpdates,
                              const AsyncUpdateCallback& aCallback);
 
   /**
    * Wait until the ongoing async update is finished and callback
    * is fired. Once this function returns, AsyncApplyUpdates is
    * no longer available.
    */
   void FlushAndDisableAsyncUpdate();
 
   /**
    * Apply full hashes retrived from gethash to cache.
    */
-  nsresult ApplyFullHashes(nsTArray<TableUpdate*>* aUpdates);
+  nsresult ApplyFullHashes(TableUpdateArray& aUpdates);
 
   /*
    * 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,
                             uint32_t aCount,
@@ -145,47 +143,47 @@ private:
 
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
   already_AddRefed<nsIFile> GetFailedUpdateDirectroy();
   nsresult DumpFailedUpdate();
 #endif
 
   nsresult ScanStoreDir(nsIFile* aDirectory, nsTArray<nsCString>& aTables);
 
-  nsresult UpdateHashStore(nsTArray<TableUpdate*>* aUpdates,
+  nsresult UpdateHashStore(TableUpdateArray& aUpdates,
                            const nsACString& aTable);
 
-  nsresult UpdateTableV4(nsTArray<TableUpdate*>* aUpdates,
+  nsresult UpdateTableV4(TableUpdateArray& aUpdates,
                          const nsACString& aTable);
 
-  nsresult UpdateCache(TableUpdate* aUpdates);
+  nsresult UpdateCache(RefPtr<TableUpdate> aUpdates);
 
   LookupCache *GetLookupCacheForUpdate(const nsACString& aTable) {
     return GetLookupCache(aTable, true);
   }
 
   LookupCache *GetLookupCacheFrom(const nsACString& aTable,
                                   nsTArray<LookupCache*>& aLookupCaches,
                                   nsIFile* aRootStoreDirectory);
 
 
-  bool CheckValidUpdate(nsTArray<TableUpdate*>* aUpdates,
+  bool CheckValidUpdate(TableUpdateArray& aUpdates,
                         const nsACString& aTable);
 
   nsresult LoadMetadata(nsIFile* aDirectory, nsACString& aResult);
 
   static nsCString GetProvider(const nsACString& aTableName);
 
   /**
    * The "background" part of ApplyUpdates. Once the background update
    * is called, the foreground update has to be called along with the
    * background result no matter whether the background update is
    * successful or not.
    */
-  nsresult ApplyUpdatesBackground(nsTArray<TableUpdate*>* aUpdates,
+  nsresult ApplyUpdatesBackground(TableUpdateArray& aUpdates,
                                   nsACString& aFailedTableName);
 
   /**
    * The "foreground" part of ApplyUpdates. The in-use data (in-memory and
    * on-disk) will be touched so this MUST be mutually exclusive to other
    * member functions.
    *
    * If |aBackgroundRv| is successful, the return value is the result of
--- a/toolkit/components/url-classifier/HashStore.cpp
+++ b/toolkit/components/url-classifier/HashStore.cpp
@@ -614,17 +614,17 @@ Merge(ChunkSet* aStoreChunks,
     return NS_ERROR_OUT_OF_MEMORY;
 
   EntrySort(*aStorePrefixes);
 
   return NS_OK;
 }
 
 nsresult
-HashStore::ApplyUpdate(TableUpdateV2 *aUpdate)
+HashStore::ApplyUpdate(RefPtr<TableUpdateV2> aUpdate)
 {
   MOZ_ASSERT(mTableName.Equals(aUpdate->TableName()));
 
   nsresult rv = mAddExpirations.Merge(aUpdate->AddExpirations());
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = mSubExpirations.Merge(aUpdate->SubExpirations());
   NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/url-classifier/HashStore.h
+++ b/toolkit/components/url-classifier/HashStore.h
@@ -3,21 +3,21 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef HashStore_h__
 #define HashStore_h__
 
 #include "Entries.h"
 #include "ChunkSet.h"
 
-#include "chromium/safebrowsing.pb.h"
 #include "nsString.h"
 #include "nsTArray.h"
 #include "nsIFile.h"
 #include "nsIFileStreams.h"
+#include "nsISupports.h"
 #include "nsCOMPtr.h"
 #include "nsClassHashtable.h"
 #include <string>
 
 namespace mozilla {
 namespace safebrowsing {
 
 // The abstract class of TableUpdateV2 and TableUpdateV4. This
@@ -25,35 +25,40 @@ namespace safebrowsing {
 // with v2 and v4 instance.
 class TableUpdate {
 public:
   TableUpdate(const nsACString& aTable)
     : mTable(aTable)
   {
   }
 
-  virtual ~TableUpdate() {}
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(TableUpdate);
 
   // To be overriden.
   virtual bool Empty() const = 0;
 
   // Common interfaces.
   const nsCString& TableName() const { return mTable; }
 
   template<typename T>
   static T* Cast(TableUpdate* aThat) {
     return (T::TAG == aThat->Tag() ? reinterpret_cast<T*>(aThat) : nullptr);
   }
 
+protected:
+  virtual ~TableUpdate() {}
+
 private:
   virtual int Tag() const = 0;
 
   const nsCString mTable;
 };
 
+typedef nsTArray<RefPtr<TableUpdate>> TableUpdateArray;
+
 // A table update is built from a single update chunk from the server. As the
 // protocol parser processes each chunk, it constructs a table update with the
 // new hashes.
 class TableUpdateV2 : public TableUpdate {
 public:
   explicit TableUpdateV2(const nsACString& aTable)
     : TableUpdate(aTable) {}
 
@@ -215,17 +220,17 @@ public:
 
   // =======
   // Updates
   // =======
   // Begin the update process.  Reads the store into memory.
   nsresult BeginUpdate();
 
   // Imports the data from a TableUpdate.
-  nsresult ApplyUpdate(TableUpdateV2 *aUpdate);
+  nsresult ApplyUpdate(RefPtr<TableUpdateV2> aUpdate);
 
   // Process expired chunks
   nsresult Expire();
 
   // Rebuild the store, Incorporating all the applied updates.
   nsresult Rebuild();
 
   // Write the current state of the store to disk.
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -256,17 +256,17 @@ UpdateChecksum(nsICryptoHash* aCrypto, c
   aCrypto->Update(reinterpret_cast<uint8_t*>(const_cast<char*>(
                   aPrefix.BeginReading())),
                   aPrefix.Length());
 }
 
 // Please see https://bug1287058.bmoattachments.org/attachment.cgi?id=8795366
 // for detail about partial update algorithm.
 nsresult
-LookupCacheV4::ApplyUpdate(TableUpdateV4* aTableUpdate,
+LookupCacheV4::ApplyUpdate(RefPtr<TableUpdateV4> aTableUpdate,
                            PrefixStringMap& aInputMap,
                            PrefixStringMap& aOutputMap)
 {
   MOZ_ASSERT(aOutputMap.IsEmpty());
 
   nsCOMPtr<nsICryptoHash> crypto;
   nsresult rv = InitCrypto(crypto);
   if (NS_FAILED(rv)) {
--- a/toolkit/components/url-classifier/LookupCacheV4.h
+++ b/toolkit/components/url-classifier/LookupCacheV4.h
@@ -32,17 +32,17 @@ public:
   virtual bool IsEmpty() const override;
 
   nsresult Build(PrefixStringMap& aPrefixMap);
 
   nsresult GetPrefixes(PrefixStringMap& aPrefixMap);
   nsresult GetFixedLengthPrefixes(FallibleTArray<uint32_t>& aPrefixes);
 
   // ApplyUpdate will merge data stored in aTableUpdate with prefixes in aInputMap.
-  nsresult ApplyUpdate(TableUpdateV4* aTableUpdate,
+  nsresult ApplyUpdate(RefPtr<TableUpdateV4> aTableUpdate,
                        PrefixStringMap& aInputMap,
                        PrefixStringMap& aOutputMap);
 
   nsresult AddFullHashResponseToCache(const FullHashResponseMap& aResponseMap);
 
   nsresult WriteMetadata(TableUpdateV4* aTableUpdate);
   nsresult LoadMetadata(nsACString& aState, nsACString& aChecksum);
 
--- a/toolkit/components/url-classifier/ProtocolParser.cpp
+++ b/toolkit/components/url-classifier/ProtocolParser.cpp
@@ -73,26 +73,16 @@ ParseChunkRange(nsACString::const_iterat
 ProtocolParser::ProtocolParser()
   : mUpdateStatus(NS_OK)
   , mUpdateWaitSec(0)
 {
 }
 
 ProtocolParser::~ProtocolParser()
 {
-  CleanupUpdates();
-}
-
-void
-ProtocolParser::CleanupUpdates()
-{
-  for (uint32_t i = 0; i < mTableUpdates.Length(); i++) {
-    delete mTableUpdates[i];
-  }
-  mTableUpdates.Clear();
 }
 
 nsresult
 ProtocolParser::Begin(const nsACString& aTable,
                       const nsTArray<nsCString>& aUpdateTables)
 {
   // ProtocolParser objects should never be reused.
   MOZ_ASSERT(mPending.IsEmpty());
@@ -104,30 +94,30 @@ ProtocolParser::Begin(const nsACString& 
   if (!aTable.IsEmpty()) {
     SetCurrentTable(aTable);
   }
   SetRequestedTables(aUpdateTables);
 
   return NS_OK;
 }
 
-TableUpdate *
+RefPtr<TableUpdate>
 ProtocolParser::GetTableUpdate(const nsACString& aTable)
 {
   for (uint32_t i = 0; i < mTableUpdates.Length(); i++) {
     if (aTable.Equals(mTableUpdates[i]->TableName())) {
       return mTableUpdates[i];
     }
   }
 
   // We free automatically on destruction, ownership of these
   // updates can be transferred to DBServiceWorker, which passes
   // them back to Classifier when doing the updates, and that
   // will free them.
-  TableUpdate *update = CreateTableUpdate(aTable);
+  RefPtr<TableUpdate> update = CreateTableUpdate(aTable);
   mTableUpdates.AppendElement(update);
   return update;
 }
 
 ///////////////////////////////////////////////////////////////////////
 // ProtocolParserV2
 
 ProtocolParserV2::ProtocolParserV2()
@@ -138,17 +128,17 @@ ProtocolParserV2::ProtocolParserV2()
 
 ProtocolParserV2::~ProtocolParserV2()
 {
 }
 
 void
 ProtocolParserV2::SetCurrentTable(const nsACString& aTable)
 {
-  auto update = GetTableUpdate(aTable);
+  RefPtr<TableUpdate> update = GetTableUpdate(aTable);
   mTableUpdate = TableUpdate::Cast<TableUpdateV2>(update);
 }
 
 nsresult
 ProtocolParserV2::AppendStream(const nsACString& aData)
 {
   if (NS_FAILED(mUpdateStatus))
     return mUpdateStatus;
@@ -693,17 +683,17 @@ ProtocolParserV2::ProcessHostAddComplete
     *aStart += COMPLETE_SIZE;
   }
 
   return NS_OK;
 }
 
 nsresult
 ProtocolParserV2::ProcessHostSubComplete(uint8_t aNumEntries,
-                                       const nsACString& aChunk, uint32_t* aStart)
+                                         const nsACString& aChunk, uint32_t* aStart)
 {
   MOZ_ASSERT(mTableUpdate);
   NS_ASSERTION(mChunkState.hashSize == COMPLETE_SIZE,
                "ProcessHostSubComplete should only be called for complete hashes.");
 
   if (aNumEntries == 0) {
     // this is totally comprehensible.
     NS_WARNING("Expected > 0 entries for a 32-byte hash sub.");
@@ -743,17 +733,17 @@ ProtocolParserV2::NextLine(nsACString& a
   if (newline == kNotFound) {
     return false;
   }
   aLine.Assign(Substring(mPending, 0, newline));
   mPending.Cut(0, newline + 1);
   return true;
 }
 
-TableUpdate*
+RefPtr<TableUpdate>
 ProtocolParserV2::CreateTableUpdate(const nsACString& aTableName) const
 {
   return new TableUpdateV2(aTableName);
 }
 
 ///////////////////////////////////////////////////////////////////////
 // ProtocolParserProtobuf
 
@@ -768,17 +758,17 @@ ProtocolParserProtobuf::~ProtocolParserP
 void
 ProtocolParserProtobuf::SetCurrentTable(const nsACString& aTable)
 {
   // Should never occur.
   MOZ_ASSERT_UNREACHABLE("SetCurrentTable shouldn't be called");
 }
 
 
-TableUpdate*
+RefPtr<TableUpdate>
 ProtocolParserProtobuf::CreateTableUpdate(const nsACString& aTableName) const
 {
   return new TableUpdateV4(aTableName);
 }
 
 nsresult
 ProtocolParserProtobuf::AppendStream(const nsACString& aData)
 {
--- a/toolkit/components/url-classifier/ProtocolParser.h
+++ b/toolkit/components/url-classifier/ProtocolParser.h
@@ -43,52 +43,47 @@ public:
   virtual nsresult AppendStream(const nsACString& aData) = 0;
 
   uint32_t UpdateWaitSec() { return mUpdateWaitSec; }
 
   // Notify that the inbound data is ready for parsing if progressive
   // parsing is not supported, for example in V4.
   virtual void End() = 0;
 
-  // Forget the table updates that were created by this pass.  It
-  // becomes the caller's responsibility to free them.  This is shitty.
-  TableUpdate *GetTableUpdate(const nsACString& aTable);
+  RefPtr<TableUpdate> GetTableUpdate(const nsACString& aTable);
   void ForgetTableUpdates() { mTableUpdates.Clear(); }
-  const nsTArray<TableUpdate*>& GetTableUpdates() { return mTableUpdates; }
+  const TableUpdateArray& GetTableUpdates() { return mTableUpdates; }
 
   // These are only meaningful to V2. Since they were originally public,
   // moving them to ProtocolParserV2 requires a dymamic cast in the call
   // sites. As a result, we will leave them until we remove support
   // for V2 entirely..
   virtual const nsTArray<ForwardedUpdate> &Forwards() const { return mForwards; }
   bool ResetRequested() const { return !mTablesToReset.IsEmpty(); }
   const nsTArray<nsCString>& TablesToReset() const { return mTablesToReset; }
 
 protected:
-  virtual TableUpdate* CreateTableUpdate(const nsACString& aTableName) const = 0;
+  virtual RefPtr<TableUpdate> CreateTableUpdate(const nsACString& aTableName) const = 0;
 
   nsCString mPending;
   nsresult mUpdateStatus;
 
   // Keep track of updates to apply before passing them to the DBServiceWorkers.
-  nsTArray<TableUpdate*> mTableUpdates;
+  TableUpdateArray mTableUpdates;
 
   nsTArray<ForwardedUpdate> mForwards;
 
   // The table names that were requested from the client.
   nsTArray<nsCString> mRequestedTables;
 
   // The table names that failed to update and need to be reset.
   nsTArray<nsCString> mTablesToReset;
 
   // How long we should wait until the next update.
   uint32_t mUpdateWaitSec;
-
-private:
-  void CleanupUpdates();
 };
 
 /**
  * Helpers to parse the "shavar", "digest256" and "simple" list formats.
  */
 class ProtocolParserV2 final : public ProtocolParser {
 public:
   ProtocolParserV2();
@@ -103,17 +98,17 @@ public:
 
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
   // Unfortunately we have to override to return mRawUpdate which
   // will not be modified during the parsing, unlike mPending.
   virtual nsCString GetRawTableUpdates() const override { return mRawUpdate; }
 #endif
 
 private:
-  virtual TableUpdate* CreateTableUpdate(const nsACString& aTableName) const override;
+  virtual RefPtr<TableUpdate> CreateTableUpdate(const nsACString& aTableName) const override;
 
   nsresult ProcessControl(bool* aDone);
   nsresult ProcessExpirations(const nsCString& aLine);
   nsresult ProcessChunkControl(const nsCString& aLine);
   nsresult ProcessForward(const nsCString& aLine);
   nsresult AddForward(const nsACString& aUrl);
   nsresult ProcessChunk(bool* done);
   // Remove this, it's only used for testing
@@ -156,17 +151,17 @@ private:
     uint32_t num;
     uint32_t hashSize;
     uint32_t length;
     void Clear() { num = 0; hashSize = 0; length = 0; }
   };
   ChunkState mChunkState;
 
   // Updates to apply to the current table being parsed.
-  TableUpdateV2 *mTableUpdate;
+  RefPtr<TableUpdateV2> mTableUpdate;
 
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
   nsCString mRawUpdate; // Keep a copy of mPending before it's processed.
 #endif
 };
 
 // Helpers to parse the "proto" list format.
 class ProtocolParserProtobuf final : public ProtocolParser {
@@ -179,17 +174,17 @@ public:
 
   virtual void SetCurrentTable(const nsACString& aTable) override;
   virtual nsresult AppendStream(const nsACString& aData) override;
   virtual void End() override;
 
 private:
   virtual ~ProtocolParserProtobuf();
 
-  virtual TableUpdate* CreateTableUpdate(const nsACString& aTableName) const override;
+  virtual RefPtr<TableUpdate> CreateTableUpdate(const nsACString& aTableName) const override;
 
   // For parsing update info.
   nsresult ProcessOneResponse(const ListUpdateResponse& aResponse,
                               nsACString& aListName);
 
   nsresult ProcessAdditionOrRemoval(TableUpdateV4& aTableUpdate,
                                     const ThreatEntrySetList& aUpdate,
                                     bool aIsAddition);
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -33,17 +33,16 @@
 #include "mozilla/ErrorNames.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/Logging.h"
 #include "prnetdb.h"
 #include "Entries.h"
-#include "HashStore.h"
 #include "Classifier.h"
 #include "ProtocolParser.h"
 #include "mozilla/Attributes.h"
 #include "nsIPrincipal.h"
 #include "Classifier.h"
 #include "ProtocolParser.h"
 #include "nsContentUtils.h"
 #include "mozilla/dom/ContentChild.h"
@@ -415,16 +414,18 @@ nsUrlClassifierDBServiceWorker::BeginUpd
 
   nsresult rv = OpenDb();
   if (NS_FAILED(rv)) {
     NS_ERROR("Unable to open SafeBrowsing database");
     return NS_ERROR_FAILURE;
   }
 
   mUpdateStatus = NS_OK;
+  MOZ_ASSERT(mTableUpdates.IsEmpty(),
+             "mTableUpdates should have been cleared in FinishUpdate()");
   mUpdateObserver = observer;
   Classifier::SplitTables(tables, mUpdateTables);
 
   return NS_OK;
 }
 
 // Called from the stream updater.
 NS_IMETHODIMP
@@ -594,39 +595,41 @@ nsUrlClassifierDBServiceWorker::FinishUp
   MOZ_ASSERT(!mProtocolParser, "Should have been nulled out in FinishStream() "
                                "or never created.");
 
   NS_ENSURE_STATE(mUpdateObserver);
 
   if (NS_FAILED(mUpdateStatus)) {
     LOG(("nsUrlClassifierDBServiceWorker::FinishUpdate() Not running "
          "ApplyUpdate() since the update has already failed."));
+    mTableUpdates.Clear();
     return NotifyUpdateObserver(mUpdateStatus);
   }
 
   if (mTableUpdates.IsEmpty()) {
     LOG(("Nothing to update. Just notify update observer."));
     return NotifyUpdateObserver(NS_OK);
   }
 
   RefPtr<nsUrlClassifierDBServiceWorker> self = this;
-  nsresult rv = mClassifier->AsyncApplyUpdates(&mTableUpdates,
+  nsresult rv = mClassifier->AsyncApplyUpdates(mTableUpdates,
                                                [=] (nsresult aRv) -> void {
 #ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
     if (NS_FAILED(aRv) &&
         NS_ERROR_OUT_OF_MEMORY != aRv &&
         NS_ERROR_UC_UPDATE_SHUTDOWNING != aRv) {
       self->mClassifier->DumpRawTableUpdates(mRawTableUpdates);
     }
     // Invalidate the raw table updates.
     self->mRawTableUpdates = EmptyCString();
 #endif
 
     self->NotifyUpdateObserver(aRv);
   });
+  mTableUpdates.Clear(); // Classifier is working on its copy.
 
   if (NS_FAILED(rv)) {
     LOG(("Failed to start async update. Notify immediately."));
     NotifyUpdateObserver(rv);
   }
 
   return rv;
 }
@@ -865,17 +868,17 @@ nsUrlClassifierDBServiceWorker::CacheCom
       if (!s.IsEmpty()) {
         s += ",";
       }
       s += tables[i];
     }
     LOG(("Active tables: %s", s.get()));
   }
 
-  nsTArray<TableUpdate*> updates;
+  TableUpdateArray updates;
 
   for (uint32_t i = 0; i < resultsPtr->Length(); i++) {
     bool activeTable = false;
     CacheResult* result = resultsPtr->ElementAt(i).get();
 
     for (uint32_t table = 0; table < tables.Length(); table++) {
       if (tables[table].Equals(result->table)) {
         activeTable = true;
@@ -883,44 +886,44 @@ nsUrlClassifierDBServiceWorker::CacheCom
       }
     }
     if (activeTable) {
       nsAutoPtr<ProtocolParser> pParse;
       pParse = result->Ver() == CacheResult::V2 ?
                  static_cast<ProtocolParser*>(new ProtocolParserV2()) :
                  static_cast<ProtocolParser*>(new ProtocolParserProtobuf());
 
-      TableUpdate* tu = pParse->GetTableUpdate(result->table);
+      RefPtr<TableUpdate> tu = pParse->GetTableUpdate(result->table);
 
       rv = CacheResultToTableUpdate(result, tu);
       if (NS_FAILED(rv)) {
         // We can bail without leaking here because ForgetTableUpdates
         // hasn't been called yet.
         return rv;
       }
       updates.AppendElement(tu);
       pParse->ForgetTableUpdates();
     } else {
       LOG(("Completion received, but table %s is not active, so not caching.",
            result->table.get()));
     }
-   }
-
-  rv = mClassifier->ApplyFullHashes(&updates);
+  }
+
+  rv = mClassifier->ApplyFullHashes(updates);
   if (NS_SUCCEEDED(rv)) {
     mLastResults = std::move(resultsPtr);
   }
   return rv;
 }
 
 nsresult
 nsUrlClassifierDBServiceWorker::CacheResultToTableUpdate(CacheResult* aCacheResult,
-                                                         TableUpdate* aUpdate)
+                                                         RefPtr<TableUpdate> aUpdate)
 {
-  auto tuV2 = TableUpdate::Cast<TableUpdateV2>(aUpdate);
+  RefPtr<TableUpdateV2> tuV2 = TableUpdate::Cast<TableUpdateV2>(aUpdate);
   if (tuV2) {
     const CacheResultV2* result = CacheResult::Cast<CacheResultV2>(aCacheResult);
     MOZ_ASSERT(result);
 
     if (result->miss) {
       return tuV2->NewMissPrefix(result->prefix);
     } else {
       LOG(("CacheCompletion hash %X, Addchunk %d", result->completion.ToUint32(),
@@ -929,17 +932,17 @@ nsUrlClassifierDBServiceWorker::CacheRes
       nsresult rv = tuV2->NewAddComplete(result->addChunk, result->completion);
       if (NS_FAILED(rv)) {
         return rv;
       }
       return tuV2->NewAddChunk(result->addChunk);
     }
   }
 
-  auto tuV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
+  RefPtr<TableUpdateV4> tuV4 = TableUpdate::Cast<TableUpdateV4>(aUpdate);
   if (tuV4) {
     const CacheResultV4* result = CacheResult::Cast<CacheResultV4>(aCacheResult);
     MOZ_ASSERT(result);
 
     if (LOG_ENABLED()) {
       const FullHashExpiryCache& fullHashes = result->response.fullHashes;
       for (auto iter = fullHashes.ConstIter(); !iter.Done(); iter.Next()) {
         Completion completion;
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -20,16 +20,17 @@
 #include "nsToolkitCompsCID.h"
 #include "nsICryptoHMAC.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/TimeStamp.h"
 
 #include "Entries.h"
 #include "LookupCache.h"
+#include "HashStore.h"
 
 // GCC < 6.1 workaround, see bug 1329593
 #if defined(XP_WIN) && defined(__MINGW32__)
 #define GCC_MANGLING_WORKAROUND __stdcall
 #else
 #define GCC_MANGLING_WORKAROUND
 #endif
 
@@ -68,17 +69,16 @@ using namespace mozilla::safebrowsing;
 class nsUrlClassifierDBServiceWorker;
 class nsIThread;
 class nsIURI;
 class UrlClassifierDBServiceWorkerProxy;
 namespace mozilla {
 namespace safebrowsing {
 class Classifier;
 class ProtocolParser;
-class TableUpdate;
 
 nsresult
 TablesToResponse(const nsACString& tables);
 
 } // namespace safebrowsing
 } // namespace mozilla
 
 // This is a proxy class that just creates a background thread and delegates
@@ -262,32 +262,30 @@ private:
                     nsIUrlClassifierLookupCallback* c);
 
   nsresult AddNoise(const Prefix aPrefix,
                     const nsCString tableName,
                     uint32_t aCount,
                     LookupResultArray& results);
 
   nsresult CacheResultToTableUpdate(CacheResult* aCacheResult,
-                                    TableUpdate* aUpdate);
+                                    RefPtr<TableUpdate> aUpdate);
 
   bool IsSameAsLastResults(const CacheResultArray& aResult) const;
 
   nsAutoPtr<mozilla::safebrowsing::Classifier> mClassifier;
   // The class that actually parses the update chunks.
   nsAutoPtr<ProtocolParser> mProtocolParser;
 
   // Directory where to store the SB databases.
   nsCOMPtr<nsIFile> mCacheDir;
 
   RefPtr<nsUrlClassifierDBService> mDBService;
 
-  // XXX: maybe an array of autoptrs.  Or maybe a class specifically
-  // storing a series of updates.
-  nsTArray<mozilla::safebrowsing::TableUpdate*> mTableUpdates;
+  TableUpdateArray mTableUpdates;
 
   uint32_t mUpdateWaitSec;
 
   // Stores the last results that triggered a table update.
   nsAutoPtr<CacheResultArray> mLastResults;
 
   nsresult mUpdateStatus;
   nsTArray<nsCString> mUpdateTables;
--- a/toolkit/components/url-classifier/tests/gtest/Common.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/Common.cpp
@@ -20,17 +20,17 @@ void RunTestInNewThread(Function&& aFunc
   nsCOMPtr<nsIThread> testingThread;
   nsresult rv =
     NS_NewNamedThread("Testing Thread", getter_AddRefs(testingThread), r);
   ASSERT_EQ(rv, NS_OK);
   testingThread->Shutdown();
 }
 
 nsresult SyncApplyUpdates(Classifier* aClassifier,
-                          nsTArray<TableUpdate*>* aUpdates)
+                          TableUpdateArray& aUpdates)
 {
   // We need to spin a new thread specifically because the callback
   // will be on the caller thread. If we call Classifier::AsyncApplyUpdates
   // and wait on the same thread, this function will never return.
 
   nsresult ret = NS_ERROR_FAILURE;
   bool done = false;
   auto onUpdateComplete = [&done, &ret](nsresult rv) {
@@ -79,17 +79,17 @@ GetFile(const nsTArray<nsString>& path)
   }
 
   for (uint32_t i = 0; i < path.Length(); i++) {
     file->Append(path[i]);
   }
   return file.forget();
 }
 
-void ApplyUpdate(nsTArray<TableUpdate*>& updates)
+void ApplyUpdate(TableUpdateArray& updates)
 {
   nsCOMPtr<nsIFile> file;
   NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file));
 
   UniquePtr<Classifier> classifier(new Classifier());
   classifier->Open(*file);
 
   {
@@ -97,22 +97,22 @@ void ApplyUpdate(nsTArray<TableUpdate*>&
     // because nsIUrlClassifierDBService will not run in advance
     // in gtest.
     nsresult rv;
     nsCOMPtr<nsIUrlClassifierUtils> dummy =
       do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID, &rv);
       ASSERT_TRUE(NS_SUCCEEDED(rv));
   }
 
-  SyncApplyUpdates(classifier.get(), &updates);
+  SyncApplyUpdates(classifier.get(), updates);
 }
 
 void ApplyUpdate(TableUpdate* update)
 {
-  nsTArray<TableUpdate*> updates = { update };
+  TableUpdateArray updates = { update };
   ApplyUpdate(updates);
 }
 
 void
 PrefixArrayToPrefixStringMap(const nsTArray<nsCString>& prefixArray,
                              PrefixStringMap& out)
 {
   out.Clear();
--- a/toolkit/components/url-classifier/tests/gtest/TestFailUpdate.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestFailUpdate.cpp
@@ -35,57 +35,57 @@ void CheckFileExist(const char* table, c
 
 TEST(UrlClassifierFailUpdate, CheckTableReset)
 {
   const bool FULL_UPDATE = true;
   const bool PARTIAL_UPDATE = false;
 
   // Apply V2 update
   {
-    auto update = new TableUpdateV2(NS_LITERAL_CSTRING(V2_TABLE));
+    RefPtr<TableUpdateV2> update = new TableUpdateV2(NS_LITERAL_CSTRING(V2_TABLE));
     Unused << update->NewAddChunk(1);
 
     ApplyUpdate(update);
 
     // A successful V2 update should create .pset & .sbstore files
     CheckFileExist(V2_TABLE, kFilesInV2, true);
   }
 
   // Helper function to generate table update data
-  auto func = [](TableUpdateV4* update, bool full, const char* str) {
+  auto func = [](RefPtr<TableUpdateV4> update, bool full, const char* str) {
     update->SetFullUpdate(full);
     nsCString prefix(str);
     update->NewPrefixes(prefix.Length(), prefix);
   };
 
   // Apply V4 update for table1
   {
-    auto update = new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE1));
+    RefPtr<TableUpdateV4> update = new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE1));
     func(update, FULL_UPDATE, "test_prefix");
 
     ApplyUpdate(update);
 
     // A successful V4 update should create .pset & .metadata files
     CheckFileExist(V4_TABLE1, kFilesInV4, true);
   }
 
   // Apply V4 update for table2
   {
-    auto update = new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE2));
+    RefPtr<TableUpdateV4> update = new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE2));
     func(update, FULL_UPDATE, "test_prefix");
 
     ApplyUpdate(update);
 
     CheckFileExist(V4_TABLE2, kFilesInV4, true);
   }
 
   // Apply V4 update with the same prefix in previous full udpate
   // This should cause an update error.
   {
-    auto update = new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE1));
+    RefPtr<TableUpdateV4> update = new TableUpdateV4(NS_LITERAL_CSTRING(V4_TABLE1));
     func(update, PARTIAL_UPDATE, "test_prefix");
 
     ApplyUpdate(update);
 
     // A fail update should remove files for that table
     CheckFileExist(V4_TABLE1, kFilesInV4, false);
 
     // A fail update should NOT remove files for the other tables
--- a/toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
+++ b/toolkit/components/url-classifier/tests/gtest/TestUrlClassifierTableUpdateV4.cpp
@@ -122,19 +122,19 @@ CreateRandomRemovalIndices(uint32_t N,
 }
 
 // Function to generate TableUpdateV4.
 static void
 GenerateUpdateData(bool fullUpdate,
                    PrefixStringMap& add,
                    nsTArray<uint32_t>* removal,
                    nsCString* checksum,
-                   nsTArray<TableUpdate*>& tableUpdates)
+                   TableUpdateArray& tableUpdates)
 {
-  TableUpdateV4* tableUpdate = new TableUpdateV4(GTEST_TABLE);
+  RefPtr<TableUpdateV4> tableUpdate = new TableUpdateV4(GTEST_TABLE);
   tableUpdate->SetFullUpdate(fullUpdate);
 
   for (auto iter = add.ConstIter(); !iter.Done(); iter.Next()) {
     nsCString* pstring = iter.Data();
     tableUpdate->NewPrefixes(iter.Key(), *pstring);
   }
 
   if (removal) {
@@ -183,30 +183,30 @@ Clear()
   NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file));
 
   UniquePtr<Classifier> classifier(new Classifier());
   classifier->Open(*file);
   classifier->Reset();
 }
 
 static void
-testUpdateFail(nsTArray<TableUpdate*>& tableUpdates)
+testUpdateFail(TableUpdateArray& tableUpdates)
 {
   nsCOMPtr<nsIFile> file;
   NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file));
 
   UniquePtr<Classifier> classifier(new Classifier());
   classifier->Open(*file);
 
-  nsresult rv = SyncApplyUpdates(classifier.get(), &tableUpdates);
+  nsresult rv = SyncApplyUpdates(classifier.get(), tableUpdates);
   ASSERT_TRUE(NS_FAILED(rv));
 }
 
 static void
-testUpdate(nsTArray<TableUpdate*>& tableUpdates,
+testUpdate(TableUpdateArray& tableUpdates,
            PrefixStringMap& expected)
 {
   nsCOMPtr<nsIFile> file;
   NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, getter_AddRefs(file));
 
   {
     // Force nsIUrlClassifierUtils loading on main thread
     // because nsIUrlClassifierDBService will not run in advance
@@ -215,38 +215,38 @@ testUpdate(nsTArray<TableUpdate*>& table
     nsCOMPtr<nsIUrlClassifierUtils> dummy =
       do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID, &rv);
       ASSERT_TRUE(NS_SUCCEEDED(rv));
   }
 
   UniquePtr<Classifier> classifier(new Classifier());
   classifier->Open(*file);
 
-  nsresult rv = SyncApplyUpdates(classifier.get(), &tableUpdates);
+  nsresult rv = SyncApplyUpdates(classifier.get(), tableUpdates);
   ASSERT_TRUE(rv == NS_OK);
   VerifyPrefixSet(expected);
 }
 
 static void
 testFullUpdate(PrefixStringMap& add, nsCString* checksum)
 {
-  nsTArray<TableUpdate*> tableUpdates;
+  TableUpdateArray tableUpdates;
 
   GenerateUpdateData(true, add, nullptr, checksum, tableUpdates);
 
   testUpdate(tableUpdates, add);
 }
 
 static void
 testPartialUpdate(PrefixStringMap& add,
                   nsTArray<uint32_t>* removal,
                   nsCString* checksum,
                   PrefixStringMap& expected)
 {
-  nsTArray<TableUpdate*> tableUpdates;
+  TableUpdateArray tableUpdates;
   GenerateUpdateData(false, add, removal, checksum, tableUpdates);
 
   testUpdate(tableUpdates, expected);
 }
 
 static void
 testOpenLookupCache()
 {
@@ -295,17 +295,17 @@ TEST(UrlClassifierTableUpdateV4, Variabl
   testFullUpdate(map, &checksum);
 
   Clear();
 }
 
 // This test contain both variable length prefix set and fixed-length prefix set
 TEST(UrlClassifierTableUpdateV4, MixedPSetFullUpdate)
 {
-   _PrefixArray array;
+  _PrefixArray array;
   PrefixStringMap map;
   nsCString checksum;
 
   CreateRandomSortedPrefixArray(5000, 4, 4, array);
   CreateRandomSortedPrefixArray(1000, 5, 32, array);
   PrefixArrayToPrefixStringMap(array, map);
   CalculateCheckSum(array, checksum);
 
@@ -415,17 +415,17 @@ TEST(UrlClassifierTableUpdateV4, Partial
     testFullUpdate(fMap, &checksum);
   }
 
   // Apply a partial update which contains a prefix in previous full update.
   // This should cause an update error.
   {
     _PrefixArray pArray;
     PrefixStringMap pMap;
-    nsTArray<TableUpdate*> tableUpdates;
+    TableUpdateArray tableUpdates;
 
     // Pick one prefix from full update prefix and add it to partial update.
     // This should result a failure when call ApplyUpdates.
     pArray.AppendElement(fArray[rand() % fArray.Length()]);
     CreateRandomSortedPrefixArray(200, 4, 32, pArray);
     PrefixArrayToPrefixStringMap(pArray, pMap);
 
     GenerateUpdateData(false, pMap, nullptr, nullptr, tableUpdates);
@@ -492,17 +492,17 @@ TEST(UrlClassifierTableUpdateV4, Partial
 
 // Test one tableupdate array contains full update and multiple partial updates.
 TEST(UrlClassifierTableUpdateV4, MultipleTableUpdates)
 {
   _PrefixArray fArray, pArray, mergedArray;
   PrefixStringMap fMap, pMap, mergedMap;
   nsCString checksum;
 
-  nsTArray<TableUpdate*> tableUpdates;
+  TableUpdateArray tableUpdates;
 
   // Generate first full udpate
   CreateRandomSortedPrefixArray(10000, 4, 4, fArray);
   CreateRandomSortedPrefixArray(2000, 5, 32, fArray);
   PrefixArrayToPrefixStringMap(fArray, fMap);
   CalculateCheckSum(fArray, checksum);
 
   GenerateUpdateData(true, fMap, nullptr, &checksum, tableUpdates);
@@ -564,17 +564,17 @@ TEST(UrlClassifierTableUpdateV4, Multipl
   }
 
   // Apply multiple partial updates in one table update
   {
     _PrefixArray pArray, mergedArray;
     PrefixStringMap pMap, mergedMap;
     nsCString checksum;
     nsTArray<uint32_t> removal;
-    nsTArray<TableUpdate*> tableUpdates;
+    TableUpdateArray tableUpdates;
 
     // Generate first partial update
     CreateRandomSortedPrefixArray(3000, 4, 4, pArray);
     CreateRandomSortedPrefixArray(1000, 5, 32, pArray);
     RemoveIntersection(fArray, pArray);
     PrefixArrayToPrefixStringMap(pArray, pMap);
 
     // Remove 1/5 of elements of original prefix set.
@@ -631,17 +631,17 @@ TEST(UrlClassifierTableUpdateV4, Removal
   }
 
   // Apply a partial update with removal indice array larger than
   // old prefix set(fArray). This should cause an error.
   {
     _PrefixArray pArray;
     PrefixStringMap pMap;
     nsTArray<uint32_t> removal;
-    nsTArray<TableUpdate*> tableUpdates;
+    TableUpdateArray tableUpdates;
 
     CreateRandomSortedPrefixArray(200, 4, 32, pArray);
     RemoveIntersection(fArray, pArray);
     PrefixArrayToPrefixStringMap(pArray, pMap);
 
     for (uint32_t i = 0; i < fArray.Length() + 1 ;i++) {
       removal.AppendElement(i);
     }
@@ -668,17 +668,17 @@ TEST(UrlClassifierTableUpdateV4, Checksu
     testFullUpdate(fMap, &checksum);
   }
 
   // Apply a partial update with incorrect checksum
   {
     _PrefixArray pArray;
     PrefixStringMap pMap;
     nsCString checksum;
-    nsTArray<TableUpdate*> tableUpdates;
+    TableUpdateArray tableUpdates;
 
     CreateRandomSortedPrefixArray(200, 4, 32, pArray);
     PrefixArrayToPrefixStringMap(pArray, pMap);
 
     // Checksum should be calculated with both old prefix set and add prefix set,
     // here we only calculate checksum with add prefix set to check if applyUpdate
     // will return failure.
     CalculateCheckSum(pArray, checksum);
@@ -790,28 +790,28 @@ TEST(UrlClassifierTableUpdateV4, EmptyUp
   Clear();
 }
 
 // This test ensure applying an empty update directly through update algorithm
 // should be correct.
 TEST(UrlClassifierTableUpdateV4, EmptyUpdate2)
 {
   // Setup LookupCache with initial data
-   _PrefixArray array;
+  _PrefixArray array;
   CreateRandomSortedPrefixArray(100, 4, 4, array);
   CreateRandomSortedPrefixArray(10, 5, 32, array);
   UniquePtr<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());
 
-  UniquePtr<TableUpdateV4> tableUpdate = MakeUnique<TableUpdateV4>(GTEST_TABLE);
+  RefPtr<TableUpdateV4> tableUpdate = new TableUpdateV4(GTEST_TABLE);
   tableUpdate->NewChecksum(stdChecksum);
 
   // Apply update directly through LookupCache interface
   PrefixStringMap input, output;
   PrefixArrayToPrefixStringMap(array, input);
   nsresult rv = cache->ApplyUpdate(tableUpdate.get(), input, output);
 
   ASSERT_TRUE(rv == NS_OK);