Bug 1296819 - Skip applying TableUpdateV4 to avoid premature update codes being run. draft
authorHenry Chang <hchang@mozilla.com>
Mon, 22 Aug 2016 15:43:05 +0800
changeset 403817 43af7d09e51b304c1156a4884bf031d4db66aad4
parent 403581 f97a056ae6235de7855fd8aaa04fb1c8d183bd06
child 529009 095b65b70499d2c895a39358b66c01a5622db7bb
push id27020
push userhchang@mozilla.com
push dateMon, 22 Aug 2016 08:28:17 +0000
bugs1296819
milestone51.0a1
Bug 1296819 - Skip applying TableUpdateV4 to avoid premature update codes being run. MozReview-Commit-ID: Cn1d9EvU2Bt
toolkit/components/url-classifier/Classifier.cpp
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -23,16 +23,42 @@ extern mozilla::LazyLogModule gUrlClassi
 
 #define STORE_DIRECTORY      NS_LITERAL_CSTRING("safebrowsing")
 #define TO_DELETE_DIR_SUFFIX NS_LITERAL_CSTRING("-to_delete")
 #define BACKUP_DIR_SUFFIX    NS_LITERAL_CSTRING("-backup")
 
 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:
+  ScopedUpdatesClearer(nsTArray<TableUpdate*>* aUpdates)
+    : mUpdates(aUpdates)
+  {
+  }
+
+  ~ScopedUpdatesClearer()
+  {
+    for (auto update : *mUpdates) {
+      delete update;
+    }
+    mUpdates->Clear();
+  }
+
+private:
+  nsTArray<TableUpdate*> *mUpdates;
+};
+
+} // 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);
@@ -291,38 +317,55 @@ Classifier::ApplyUpdates(nsTArray<TableU
 {
   Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_CL_UPDATE_TIME> timer;
 
   PRIntervalTime clockStart = 0;
   if (LOG_ENABLED()) {
     clockStart = PR_IntervalNow();
   }
 
-  LOG(("Backup before update."));
+  nsresult rv;
 
-  nsresult rv = BackupTables();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  LOG(("Applying %d table updates.", aUpdates->Length()));
+  {
+    ScopedUpdatesClearer scopedUpdatesClearer(aUpdates);
 
-  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());
-      rv = UpdateHashStore(aUpdates, updateTable);
-      if (NS_FAILED(rv)) {
-        if (rv != NS_ERROR_OUT_OF_MEMORY) {
-          Reset();
-        }
-        return rv;
+    // In order to prevent any premature update codes from being run
+    // against V4 updates, we bail out as early as possible if any
+    // of the updates is for V4.
+    for (auto update : *aUpdates) {
+      if (update && TableUpdate::Cast<TableUpdateV4>(update)) {
+        LOG(("V4 update is not supported yet."));
+        // TODO: Bug 1283009 - Supports applying table udpate V4.
+        return NS_ERROR_NOT_IMPLEMENTED;
       }
     }
-  }
-  aUpdates->Clear();
+
+    LOG(("Backup before update."));
+
+    rv = BackupTables();
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    LOG(("Applying %d 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());
+        rv = UpdateHashStore(aUpdates, updateTable);
+        if (NS_FAILED(rv)) {
+          if (rv != NS_ERROR_OUT_OF_MEMORY) {
+            Reset();
+          }
+          return rv;
+        }
+      }
+    }
+
+  } // End of scopedUpdatesClearer scope.
 
   rv = RegenActiveTables();
   NS_ENSURE_SUCCESS(rv, rv);
 
   LOG(("Cleaning up backups."));
 
   // Move the backup directory away (signaling the transaction finished
   // successfully). This is atomic.
@@ -344,26 +387,25 @@ Classifier::ApplyUpdates(nsTArray<TableU
   return NS_OK;
 }
 
 nsresult
 Classifier::ApplyFullHashes(nsTArray<TableUpdate*>* aUpdates)
 {
   LOG(("Applying %d 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);
     NS_ENSURE_SUCCESS(rv, rv);
 
     aUpdates->ElementAt(i) = nullptr;
-    delete update;
   }
-  aUpdates->Clear();
 
   return NS_OK;
 }
 
 nsresult
 Classifier::MarkSpoiled(nsTArray<nsCString>& aTables)
 {
   for (uint32_t i = 0; i < aTables.Length(); i++) {
@@ -586,17 +628,16 @@ Classifier::CheckValidUpdate(nsTArray<Ta
   uint32_t validupdates = 0;
 
   for (uint32_t i = 0; i < aUpdates->Length(); i++) {
     TableUpdate *update = aUpdates->ElementAt(i);
     if (!update || !update->TableName().Equals(aTable))
       continue;
     if (update->Empty()) {
       aUpdates->ElementAt(i) = nullptr;
-      delete update;
       continue;
     }
     validupdates++;
   }
 
   if (!validupdates) {
     // This can happen if the update was only valid for one table.
     return false;
@@ -662,17 +703,16 @@ Classifier::UpdateHashStore(nsTArray<Tab
       LOG(("  %d sub chunks", updateV2->SubChunks().Length()));
       LOG(("  %d sub prefixes", updateV2->SubPrefixes().Length()));
       LOG(("  %d sub completions", updateV2->SubCompletes().Length()));
       LOG(("  %d add expirations", updateV2->AddExpirations().Length()));
       LOG(("  %d sub expirations", updateV2->SubExpirations().Length()));
     }
 
     aUpdates->ElementAt(i) = nullptr;
-    delete update;
   }
 
   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()));