PVer4: Test checksum on startup outside the hotpath of DB load

components/safe_browsing_db/v4_database.h

Index: components/safe_browsing_db/v4_database.h
diff --git a/components/safe_browsing_db/v4_database.h b/components/safe_browsing_db/v4_database.h
index bddba598850d4b5750b1274981242f1112a5a1be..06707d3df841f2e9e775e2cdbb83cf29cc1a1358 100644
--- a/components/safe_browsing_db/v4_database.h
+++ b/components/safe_browsing_db/v4_database.h
@@ -17,9 +17,17 @@ namespace safe_browsing {
 
 class V4Database;
 
+// Scheduled when the database has been read from disk and is ready to process
+// resource reputation requests.
 typedef base::Callback<void(std::unique_ptr<V4Database>)>
     NewDatabaseReadyCallback;
 
+// Scheduled when the checksum for all the stores in the database has been
+// verified to match the expected value. Stores for which the checksum did not
+// match are passed as the argument and need to be reset.
+typedef base::Callback<void(const std::vector<ListIdentifier>&)>
+    DatabaseReadyForUpdatesCallback;
+
 // This callback is scheduled once the database has finished processing the
 // update requests for all stores and is ready to process the next set of update
 // requests.
@@ -116,8 +124,18 @@ class V4Database {
       const StoresToCheck& stores_to_check,
       StoreAndHashPrefixes* matched_store_and_full_hashes);
 
-  // Deletes the current database and creates a new one.
-  virtual bool ResetDatabase();
+  // Resets the stores in |stores_to_reset| to an empty state. This is done if
+  // the checksum of the prefix hashes in lexicographical order, doesn't match

[Nathan Parker, 2016/10/07 23:24:28]

nit: too much detail for here. Could just say "...This is done if the checksum
doesn't match the expected value"

[vakh (use Gerrit instead), 2016/10/10 17:42:33]

Done.

+  // the expected checksum read from disk.
+  void ResetStores(const std::vector<ListIdentifier>& stores_to_reset);

[Nathan Parker, 2016/10/07 23:24:28]

nit: I'd put ResetStores after VerifyChecksum since that's the order of
operation, and it makes the comments simpler to understand.

[vakh (use Gerrit instead), 2016/10/10 17:42:33]

Yes, but I'd prefer to keep the sorted order. The functions are literally next
to each other so it's not too bad.

+
+  // Schedules verification of the checksum of each store read from disk on task
+  // runner. If the checksum doesn't match, that store is passed to the
+  // |db_ready_for_updates_callback|. At the end,
+  // |db_ready_for_updates_callback| is scheduled (on the same thread as it was
+  // called) to indicate that the database updates can now be scheduled.
+  void VerifyChecksum(
+      DatabaseReadyForUpdatesCallback db_ready_for_updates_callback);
 
  protected:
   V4Database(const scoped_refptr<base::SequencedTaskRunner>& db_task_runner,
@@ -155,6 +173,15 @@ class V4Database {
   void UpdatedStoreReady(ListIdentifier identifier,
                          std::unique_ptr<V4Store> store);
 
+  // Verifies the checksum of each store read from disk on task runner. If the
+  // checksum doesn't match, that store is passed to the

[Nathan Parker, 2016/10/07 23:24:28]

nit-picky nit: Redundant comments, likely to diverge in the future...
This could just say "See VerifyChecksum()," since that describes it all.

[vakh (use Gerrit instead), 2016/10/10 17:42:33]

Done.

+  // |db_ready_for_updates_callback| to be reset. At the end,
+  // |db_ready_for_updates_callback| is scheduled on |callback_task_runner| to
+  // indicate that the database updates can now be scheduled.
+  void VerifyChecksumOnTaskRunner(
+      const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner,
+      DatabaseReadyForUpdatesCallback db_ready_for_updates_callback);
+
   const scoped_refptr<base::SequencedTaskRunner> db_task_runner_;
 
   // Map of ListIdentifier to the V4Store.