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

components/safe_browsing_db/v4_database.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_SAFE_BROWSING_DB_V4_DATABASE_H_
 #define COMPONENTS_SAFE_BROWSING_DB_V4_DATABASE_H_
 
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
 #include "components/safe_browsing_db/v4_protocol_manager_util.h"
 #include "components/safe_browsing_db/v4_store.h"
 
 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.
 typedef base::Callback<void()> DatabaseUpdatedCallback;
 
 // Maps the ListIdentifiers to their corresponding in-memory stores, which
 // contain the hash prefixes for that ListIdentifier as well as manage their
 // storage on disk.
 typedef base::hash_map<ListIdentifier, std::unique_ptr<V4Store>> StoreMap;
 
@@ skipping 76 matching lines @@
   std::unique_ptr<StoreStateMap> GetStoreStateMap();
 
   // Searches for a hash prefix matching the |full_hash| in stores in the
   // database, filtered by |stores_to_check|, and returns the identifier of the
   // store along with the matching hash prefix in |matched_hash_prefix_map|.
   virtual void GetStoresMatchingFullHash(
       const FullHash& full_hash,
       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,
              std::unique_ptr<StoreMap> store_map);
 
  private:
   friend class V4DatabaseTest;
   FRIEND_TEST_ALL_PREFIXES(V4DatabaseTest, TestSetupDatabaseWithFakeStores);
   FRIEND_TEST_ALL_PREFIXES(V4DatabaseTest,
                            TestSetupDatabaseWithFakeStoresFailsReset);
@@ skipping 17 matching lines @@
       const ListInfos& list_infos,
       const scoped_refptr<base::SingleThreadTaskRunner>& callback_task_runner,
       NewDatabaseReadyCallback callback);
 
   // Callback called when a new store has been created and is ready to be used.
   // This method updates the store_map_ to point to the new store, which causes
   // the old store to get deleted.
   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.
   const std::unique_ptr<StoreMap> store_map_;
 
   DatabaseUpdatedCallback db_updated_callback_;
 
   // The factory that controls the creation of V4Store objects.
   static V4StoreFactory* factory_;
 
   // The number of stores for which the update request is pending. When this
   // goes down to 0, that indicates that the database has updated all the stores
   // that needed updating and is ready for the next update. It should only be
   // accessed on the IO thread.
   int pending_store_updates_;
 
   DISALLOW_COPY_AND_ASSIGN(V4Database);
 };
 
 }  // namespace safe_browsing
 
 #endif  // COMPONENTS_SAFE_BROWSING_DB_V4_DATABASE_H_