Fetch incremental updates. Store new state in V4Store.

components/safe_browsing_db/v4_store.h

Index: components/safe_browsing_db/v4_store.h
diff --git a/components/safe_browsing_db/v4_store.h b/components/safe_browsing_db/v4_store.h
index 7be3f72f20a158cf89973db533b9c89dffb5a2a9..f86d99f48548ab7f3482fe0d579db101fc0090fa 100644
--- a/components/safe_browsing_db/v4_store.h
+++ b/components/safe_browsing_db/v4_store.h
@@ -8,11 +8,16 @@
 #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"
 
 namespace safe_browsing {
 
 class V4Store;
 
+typedef base::Callback<void(std::unique_ptr<V4Store>)>
+    UpdatedStoreReadyCallback;
+
 // Factory for creating V4Store. Tests implement this factory to create fake
 // stores for testing.
 class V4StoreFactory {
@@ -32,20 +37,35 @@ class V4Store {
           const base::FilePath& store_path);
   virtual ~V4Store();
 
+  const base::FilePath& store_path() const { return store_path_; }
+
+  const std::string& state() const { return state_; }
+
+  void ApplyUpdate(const ListUpdateResponse&,
+                   const scoped_refptr<base::SingleThreadTaskRunner>&,
+                   UpdatedStoreReadyCallback);

[Scott Hess - ex-Googler, 2016/06/17 22:53:43]

While you're still early days, maybe try to keep this alphabetized.

[vakh (use Gerrit instead), 2016/06/20 22:28:43]

Done.

In each visibility section:
Constructur/Destructor
Sorted simple methods
Sorted other methods

+
   // Reset internal state and delete the backing file.
   virtual bool Reset();
 
-  const base::FilePath& store_path() const {
-    return store_path_;
-  }
+  std::string DebugString() const;

[Scott Hess - ex-Googler, 2016/06/17 22:53:43]

Is it necessary to have debug-output stuff in here?

[vakh (use Gerrit instead), 2016/06/20 22:28:43]

Useful for debugging. Not linked into the binary unless used.
As I mentioned in the other file, I find them useful right now but probably
won't be very useful in about 3-6 months. I could add a crbug to remove these in
6 months.

 
  protected:
   const scoped_refptr<base::SequencedTaskRunner> task_runner_;
   const base::FilePath store_path_;
+  std::string state_;

[Scott Hess - ex-Googler, 2016/06/17 22:53:43]

Is this the entire state of the store?  Or is it some sort of etag-type hash
blob?  Probably worth a comment.

[vakh (use Gerrit instead), 2016/06/20 22:28:43]

Done.

+
+ private:
+  // This constructor only copies the |store_path_| and the |task_runner_|
+  // information from |other|. The hash prefixes are not copied since they'll be
+  // different from |other|.

[Scott Hess - ex-Googler, 2016/06/17 22:53:43]

This feels like you're using the copy constructor as a convenience.  Anyplace
that can reach this private declaration can also reach the instance variables, I
think that just explicitly passing the task runner and store path is going to be
much easier to understand at that point than some comment in some other file.

[vakh (use Gerrit instead), 2016/06/20 22:28:43]

Done.

+  V4Store(const V4Store& other);
 
-  DISALLOW_COPY_AND_ASSIGN(V4Store);
+  std::string GetHumanReadableState() const;
 };
 
+std::ostream& operator<<(std::ostream& os, const V4Store& store);

[Scott Hess - ex-Googler, 2016/06/17 22:53:43]

I like this even less than the version on the list identifier.

[vakh (use Gerrit instead), 2016/06/20 22:28:43]

The UpdateListIdentifier is a struct so its members were visible. Not the case
here so I need the DebugString method.

[Scott Hess - ex-Googler, 2016/06/21 21:03:44]

Acknowledged.

+
 }  // namespace safe_browsing
 
 #endif  // COMPONENTS_SAFE_BROWSING_DB_V4_STORE_H_