[Sync] Moving DataTypeState to be a proto so that it can easily be

serialized to storage by model types.

BUG=543405

Review URL: https://codereview.chromium.org/1678343002

Cr-Commit-Position: refs/heads/master@{#374667}

11 files changed, 55 insertions, 74 deletions

diff --git a/sync/internal_api/public/activation_context.h b/sync/internal_api/public/activation_context.h
index 798d851..48d9476 100644
--- a/sync/internal_api/public/activation_context.h
+++ b/sync/internal_api/public/activation_context.h
@@ -11,6 +11,7 @@
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/model_type_processor.h"
 #include "sync/internal_api/public/non_blocking_sync_common.h"
+#include "sync/protocol/data_type_state.pb.h"
 
 namespace syncer_v2 {
 
@@ -21,7 +22,7 @@ struct SYNC_EXPORT ActivationContext {
   ~ActivationContext();
 
   // Initial DataTypeState at the moment of activation.
-  DataTypeState data_type_state;
+  sync_pb::DataTypeState data_type_state;
 
   // Pending updates from the previous session.
   // TODO(stanisc): crbug.com/529498: should remove pending updates.
diff --git a/sync/internal_api/public/model_type_processor.h b/sync/internal_api/public/model_type_processor.h
index 121055f..297b48f 100644
--- a/sync/internal_api/public/model_type_processor.h
+++ b/sync/internal_api/public/model_type_processor.h
@@ -8,6 +8,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/non_blocking_sync_common.h"
+#include "sync/protocol/data_type_state.pb.h"
 
 namespace syncer_v2 {
 class CommitQueue;
@@ -24,13 +25,13 @@ class SYNC_EXPORT ModelTypeProcessor {
   // Informs this object that some of its commit requests have been
   // successfully serviced.
   virtual void OnCommitCompleted(
-      const DataTypeState& type_state,
+      const sync_pb::DataTypeState& type_state,
       const CommitResponseDataList& response_list) = 0;
 
   // Informs this object that there are some incoming updates is should
   // handle.
   virtual void OnUpdateReceived(
-      const DataTypeState& type_state,
+      const sync_pb::DataTypeState& type_state,
       const UpdateResponseDataList& response_list,
       const UpdateResponseDataList& pending_updates) = 0;
 };
diff --git a/sync/internal_api/public/non_blocking_sync_common.cc b/sync/internal_api/public/non_blocking_sync_common.cc
index f76895c..a8ad64f 100644
--- a/sync/internal_api/public/non_blocking_sync_common.cc
+++ b/sync/internal_api/public/non_blocking_sync_common.cc
@@ -6,10 +6,6 @@
 
 namespace syncer_v2 {
 
-DataTypeState::DataTypeState() {}
-
-DataTypeState::~DataTypeState() {}
-
 CommitRequestData::CommitRequestData() {}
 
 CommitRequestData::~CommitRequestData() {}
diff --git a/sync/internal_api/public/non_blocking_sync_common.h b/sync/internal_api/public/non_blocking_sync_common.h
index 799dfca..b12ff01 100644
--- a/sync/internal_api/public/non_blocking_sync_common.h
+++ b/sync/internal_api/public/non_blocking_sync_common.h
@@ -19,32 +19,6 @@ namespace syncer_v2 {
 
 static const int64_t kUncommittedVersion = -1;
 
-// Data-type global state that must be accessed and updated on the sync thread,
-// but persisted on or through the model thread.
-struct SYNC_EXPORT DataTypeState {
-  DataTypeState();
-  ~DataTypeState();
-
-  // The latest progress markers received from the server.
-  sync_pb::DataTypeProgressMarker progress_marker;
-
-  // A data type context.  Sent to the server in every commit or update
-  // request.  May be updated by either by responses from the server or
-  // requests made on the model thread.  The interpretation of this value may
-  // be data-type specific.  Many data types ignore it.
-  sync_pb::DataTypeContext type_context;
-
-  // This value is set if this type's data should be encrypted on the server.
-  // If this key changes, the client will need to re-commit all of its local
-  // data to the server using the new encryption key.
-  std::string encryption_key_name;
-
-  // This flag is set to true when the first download cycle is complete.  The
-  // ModelTypeProcessor should not attempt to commit any items until this
-  // flag is set.
-  bool initial_sync_done = false;
-};
-
 struct SYNC_EXPORT CommitRequestData {
   CommitRequestData();
   ~CommitRequestData();
diff --git a/sync/internal_api/public/shared_model_type_processor.h b/sync/internal_api/public/shared_model_type_processor.h
index 855e964..0d8ba15 100644
--- a/sync/internal_api/public/shared_model_type_processor.h
+++ b/sync/internal_api/public/shared_model_type_processor.h
@@ -20,6 +20,7 @@
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/model_type_processor.h"
 #include "sync/internal_api/public/non_blocking_sync_common.h"
+#include "sync/protocol/data_type_state.pb.h"
 #include "sync/protocol/sync.pb.h"
 
 namespace syncer_v2 {
@@ -85,9 +86,9 @@ class SYNC_EXPORT SharedModelTypeProcessor : public ModelTypeProcessor,
 
   // ModelTypeProcessor implementation.
   void OnConnect(scoped_ptr<CommitQueue> worker) override;
-  void OnCommitCompleted(const DataTypeState& type_state,
+  void OnCommitCompleted(const sync_pb::DataTypeState& type_state,
                          const CommitResponseDataList& response_list) override;
-  void OnUpdateReceived(const DataTypeState& type_state,
+  void OnUpdateReceived(const sync_pb::DataTypeState& type_state,
                         const UpdateResponseDataList& response_list,
                         const UpdateResponseDataList& pending_updates) override;
 
@@ -101,7 +102,7 @@ class SYNC_EXPORT SharedModelTypeProcessor : public ModelTypeProcessor,
   void FinishStart();
 
   // Handle the first update received from the server after being enabled.
-  void OnInitialUpdateReceived(const DataTypeState& type_state,
+  void OnInitialUpdateReceived(const sync_pb::DataTypeState& type_state,
                                const UpdateResponseDataList& response_list,
                                const UpdateResponseDataList& pending_updates);
 
@@ -114,7 +115,7 @@ class SYNC_EXPORT SharedModelTypeProcessor : public ModelTypeProcessor,
   void ClearTransientSyncState();
 
   syncer::ModelType type_;
-  DataTypeState data_type_state_;
+  sync_pb::DataTypeState data_type_state_;
 
   // Stores the start callback in between Start() and FinishStart().
   StartCallback start_callback_;
diff --git a/sync/internal_api/public/simple_metadata_change_list.cc b/sync/internal_api/public/simple_metadata_change_list.cc
index c1bb25f..999eda6 100644
--- a/sync/internal_api/public/simple_metadata_change_list.cc
+++ b/sync/internal_api/public/simple_metadata_change_list.cc
@@ -11,7 +11,7 @@ SimpleMetadataChangeList::SimpleMetadataChangeList() {}
 SimpleMetadataChangeList::~SimpleMetadataChangeList() {}
 
 void SimpleMetadataChangeList::UpdateDataTypeState(
-    const DataTypeState& data_type_state) {
+    const sync_pb::DataTypeState& data_type_state) {
   // TODO(skym): Implementation.
 }
 
diff --git a/sync/internal_api/public/simple_metadata_change_list.h b/sync/internal_api/public/simple_metadata_change_list.h
index aca0e69..f302b12 100644
--- a/sync/internal_api/public/simple_metadata_change_list.h
+++ b/sync/internal_api/public/simple_metadata_change_list.h
@@ -5,6 +5,8 @@
 #ifndef SYNC_INTERNAL_API_PUBLIC_SIMPLE_METADATA_CHANGE_LIST_H_
 #define SYNC_INTERNAL_API_PUBLIC_SIMPLE_METADATA_CHANGE_LIST_H_
 
+#include <string>
+
 #include "sync/api/metadata_change_list.h"
 #include "sync/api/model_type_store.h"
 #include "sync/base/sync_export.h"
@@ -19,7 +21,8 @@ class SYNC_EXPORT SimpleMetadataChangeList : public MetadataChangeList {
   SimpleMetadataChangeList();
   ~SimpleMetadataChangeList() override;
 
-  void UpdateDataTypeState(const DataTypeState& data_type_state) override;
+  void UpdateDataTypeState(
+      const sync_pb::DataTypeState& data_type_state) override;
   void ClearDataTypeState() override;
   void UpdateMetadata(const std::string& client_tag,
                       const sync_pb::EntityMetadata& metadata) override;
diff --git a/sync/internal_api/public/test/fake_metadata_change_list.h b/sync/internal_api/public/test/fake_metadata_change_list.h
index 6359602..3941f0a 100644
--- a/sync/internal_api/public/test/fake_metadata_change_list.h
+++ b/sync/internal_api/public/test/fake_metadata_change_list.h
@@ -5,10 +5,12 @@
 #ifndef SYNC_INTERNAL_API_PUBLIC_TEST_FAKE_METADATA_CHANGE_LIST_H_
 #define SYNC_INTERNAL_API_PUBLIC_TEST_FAKE_METADATA_CHANGE_LIST_H_
 
+#include <string>
 #include <vector>
 
 #include "sync/api/metadata_change_list.h"
 #include "sync/internal_api/public/non_blocking_sync_common.h"
+#include "sync/protocol/data_type_state.pb.h"
 #include "sync/protocol/entity_metadata.pb.h"
 
 namespace syncer_v2 {
@@ -22,7 +24,8 @@ class FakeMetadataChangeList : public MetadataChangeList {
   FakeMetadataChangeList();
   ~FakeMetadataChangeList() override;
 
-  void UpdateDataTypeState(const DataTypeState& data_type_state) override;
+  void UpdateDataTypeState(
+      const sync_pb::DataTypeState& data_type_state) override;
   void ClearDataTypeState() override;
   void UpdateMetadata(const std::string& client_tag,
                       const sync_pb::EntityMetadata& metadata) override;
@@ -41,7 +44,7 @@ class FakeMetadataChangeList : public MetadataChangeList {
 
     Action action;
     std::string tag;
-    DataTypeState data_type_state;
+    sync_pb::DataTypeState data_type_state;
     sync_pb::EntityMetadata metadata;
   };
 
diff --git a/sync/internal_api/shared_model_type_processor.cc b/sync/internal_api/shared_model_type_processor.cc
index dbf7bf8..4b6b983 100644
--- a/sync/internal_api/shared_model_type_processor.cc
+++ b/sync/internal_api/shared_model_type_processor.cc
@@ -26,9 +26,9 @@ class ModelTypeProcessorProxy : public ModelTypeProcessor {
   ~ModelTypeProcessorProxy() override;
 
   void OnConnect(scoped_ptr<CommitQueue> worker) override;
-  void OnCommitCompleted(const DataTypeState& type_state,
+  void OnCommitCompleted(const sync_pb::DataTypeState& type_state,
                          const CommitResponseDataList& response_list) override;
-  void OnUpdateReceived(const DataTypeState& type_state,
+  void OnUpdateReceived(const sync_pb::DataTypeState& type_state,
                         const UpdateResponseDataList& response_list,
                         const UpdateResponseDataList& pending_updates) override;
 
@@ -51,7 +51,7 @@ void ModelTypeProcessorProxy::OnConnect(scoped_ptr<CommitQueue> worker) {
 }
 
 void ModelTypeProcessorProxy::OnCommitCompleted(
-    const DataTypeState& type_state,
+    const sync_pb::DataTypeState& type_state,
     const CommitResponseDataList& response_list) {
   processor_task_runner_->PostTask(
       FROM_HERE, base::Bind(&ModelTypeProcessor::OnCommitCompleted, processor_,
@@ -59,7 +59,7 @@ void ModelTypeProcessorProxy::OnCommitCompleted(
 }
 
 void ModelTypeProcessorProxy::OnUpdateReceived(
-    const DataTypeState& type_state,
+    const sync_pb::DataTypeState& type_state,
     const UpdateResponseDataList& response_list,
     const UpdateResponseDataList& pending_updates) {
   processor_task_runner_->PostTask(
@@ -101,7 +101,7 @@ void SharedModelTypeProcessor::OnMetadataLoaded(
   DCHECK(entities_.empty());
   DCHECK(!IsConnected());
 
-  if (batch->GetDataTypeState().initial_sync_done) {
+  if (batch->GetDataTypeState().initial_sync_done()) {
     EntityMetadataMap metadata_map(batch->TakeAllMetadata());
     for (auto it = metadata_map.begin(); it != metadata_map.end(); it++) {
       entities_.insert(std::make_pair(
@@ -114,7 +114,7 @@ void SharedModelTypeProcessor::OnMetadataLoaded(
     // we have data but no metadata?
   } else {
     // First time syncing; initialize metadata.
-    data_type_state_.progress_marker.set_data_type_id(
+    data_type_state_.mutable_progress_marker()->set_data_type_id(
         GetSpecificsFieldNumberFromModelType(type_));
   }
 
@@ -274,7 +274,7 @@ void SharedModelTypeProcessor::FlushPendingCommitRequests() {
     return;
 
   // Don't send anything if the type is not ready to handle commits.
-  if (!data_type_state_.initial_sync_done)
+  if (!data_type_state_.initial_sync_done())
     return;
 
   // TODO(rlarocque): Do something smarter than iterate here.
@@ -292,7 +292,7 @@ void SharedModelTypeProcessor::FlushPendingCommitRequests() {
 }
 
 void SharedModelTypeProcessor::OnCommitCompleted(
-    const DataTypeState& type_state,
+    const sync_pb::DataTypeState& type_state,
     const CommitResponseDataList& response_list) {
   scoped_ptr<MetadataChangeList> change_list =
       service_->CreateMetadataChangeList();
@@ -311,9 +311,10 @@ void SharedModelTypeProcessor::OnCommitCompleted(
                    << " type: " << type_ << " client_tag: " << client_tag_hash;
       return;
     } else {
-      it->second->ReceiveCommitResponse(
-          response_data.id, response_data.sequence_number,
-          response_data.response_version, data_type_state_.encryption_key_name);
+      it->second->ReceiveCommitResponse(response_data.id,
+                                        response_data.sequence_number,
+                                        response_data.response_version,
+                                        data_type_state_.encryption_key_name());
       // TODO(stanisc): crbug.com/573333: Delete case.
       // This might be the right place to clear a metadata entry that has
       // been deleted locally and confirmed deleted by the server.
@@ -329,11 +330,10 @@ void SharedModelTypeProcessor::OnCommitCompleted(
 }
 
 void SharedModelTypeProcessor::OnUpdateReceived(
-    const DataTypeState& data_type_state,
+    const sync_pb::DataTypeState& data_type_state,
     const UpdateResponseDataList& response_list,
     const UpdateResponseDataList& pending_updates) {
-
-  if (!data_type_state_.initial_sync_done) {
+  if (!data_type_state_.initial_sync_done()) {
     OnInitialUpdateReceived(data_type_state, response_list, pending_updates);
   }
 
@@ -342,8 +342,9 @@ void SharedModelTypeProcessor::OnUpdateReceived(
   EntityChangeList entity_changes;
 
   metadata_changes->UpdateDataTypeState(data_type_state);
-  bool got_new_encryption_requirements = data_type_state_.encryption_key_name !=
-                                         data_type_state.encryption_key_name;
+  bool got_new_encryption_requirements =
+      data_type_state_.encryption_key_name() !=
+      data_type_state.encryption_key_name();
   data_type_state_ = data_type_state;
 
   for (auto list_it = response_list.begin(); list_it != response_list.end();
@@ -401,14 +402,14 @@ void SharedModelTypeProcessor::OnUpdateReceived(
 
     // If the received entity has out of date encryption, we schedule another
     // commit to fix it.
-    if (data_type_state_.encryption_key_name !=
+    if (data_type_state_.encryption_key_name() !=
         response_data.encryption_key_name) {
       DVLOG(2) << ModelTypeToString(type_) << ": Requesting re-encrypt commit "
                << response_data.encryption_key_name << " -> "
-               << data_type_state_.encryption_key_name;
+               << data_type_state_.encryption_key_name();
       auto it2 = entities_.find(client_tag_hash);
       it2->second->UpdateDesiredEncryptionKey(
-          data_type_state_.encryption_key_name);
+          data_type_state_.encryption_key_name());
     }
   }
 
@@ -435,7 +436,7 @@ void SharedModelTypeProcessor::OnUpdateReceived(
   if (got_new_encryption_requirements) {
     for (auto it = entities_.begin(); it != entities_.end(); ++it) {
       it->second->UpdateDesiredEncryptionKey(
-          data_type_state_.encryption_key_name);
+          data_type_state_.encryption_key_name());
     }
   }
 
@@ -448,7 +449,7 @@ void SharedModelTypeProcessor::OnUpdateReceived(
 }
 
 void SharedModelTypeProcessor::OnInitialUpdateReceived(
-    const DataTypeState& data_type_state,
+    const sync_pb::DataTypeState& data_type_state,
     const UpdateResponseDataList& response_list,
     const UpdateResponseDataList& pending_updates) {
   // TODO(maxbogue): crbug.com/569675: Generate metadata for all entities.
diff --git a/sync/internal_api/shared_model_type_processor_unittest.cc b/sync/internal_api/shared_model_type_processor_unittest.cc
index 741cfda..776179a 100644
--- a/sync/internal_api/shared_model_type_processor_unittest.cc
+++ b/sync/internal_api/shared_model_type_processor_unittest.cc
@@ -16,6 +16,7 @@
 #include "sync/internal_api/public/non_blocking_sync_common.h"
 #include "sync/internal_api/public/test/fake_metadata_change_list.h"
 #include "sync/internal_api/public/test/fake_model_type_service.h"
+#include "sync/protocol/data_type_state.pb.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/syncable_util.h"
 #include "sync/test/engine/mock_commit_queue.h"
@@ -168,7 +169,7 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
   MockCommitQueue* mock_queue_;
   scoped_ptr<MockCommitQueue> mock_queue_ptr_;
 
-  DataTypeState data_type_state_;
+  sync_pb::DataTypeState data_type_state_;
 
   // The last received EntityChangeList.
   scoped_ptr<EntityChangeList> entity_change_list_;
@@ -188,7 +189,7 @@ SharedModelTypeProcessorTest::SharedModelTypeProcessorTest()
 SharedModelTypeProcessorTest::~SharedModelTypeProcessorTest() {}
 
 void SharedModelTypeProcessorTest::InitializeToReadyState() {
-  data_type_state_.initial_sync_done = true;
+  data_type_state_.set_initial_sync_done(true);
   OnMetadataLoaded();
   Start();
   // TODO(maxbogue): crbug.com/569642: Remove this once entity data is loaded
@@ -261,7 +262,7 @@ void SharedModelTypeProcessorTest::DeleteItem(const std::string& tag,
 }
 
 void SharedModelTypeProcessorTest::OnInitialSyncDone() {
-  data_type_state_.initial_sync_done = true;
+  data_type_state_.set_initial_sync_done(true);
   UpdateResponseDataList empty_update_list;
 
   // TODO(stanisc): crbug/569645: replace this with loading the initial state
@@ -352,7 +353,7 @@ void SharedModelTypeProcessorTest::SuccessfulCommitResponse(
 
 void SharedModelTypeProcessorTest::UpdateDesiredEncryptionKey(
     const std::string& key_name) {
-  data_type_state_.encryption_key_name = key_name;
+  data_type_state_.set_encryption_key_name(key_name);
   type_processor()->OnUpdateReceived(data_type_state_, UpdateResponseDataList(),
                                      UpdateResponseDataList());
 }
@@ -561,7 +562,7 @@ TEST_F(SharedModelTypeProcessorTest, CreateAndModifyWithOverrides) {
   entity_data->specifics.mutable_preference()->set_value("value2");
   entity_data->non_unique_name = "name2";
   entity_data->client_tag_hash = "hash";
-  // TODO (skym): Consider removing this. The ID should never be changed by the
+  // TODO(skym): Consider removing this. The ID should never be changed by the
   // client once established.
   entity_data->id = "cid2";
 
@@ -588,7 +589,7 @@ TEST_F(SharedModelTypeProcessorTest, CreateAndModifyWithOverrides) {
   const FakeMetadataChangeList::Record& record2 = change_list.GetNthRecord(1);
   EXPECT_EQ(FakeMetadataChangeList::UPDATE_METADATA, record2.action);
   EXPECT_EQ("tag1", record2.tag);
-  // TODO (skym): Is this correct?
+  // TODO(skym): Is this correct?
   EXPECT_EQ("cid1", record2.metadata.server_id());
   EXPECT_EQ("hash", record2.metadata.client_tag_hash());
 
@@ -919,11 +920,11 @@ TEST_F(SharedModelTypeProcessorTest, DISABLED_Disable) {
   // Once we're ready to commit, all three local items should consider
   // themselves uncommitted and pending for commit.
   // TODO(maxbogue): crbug.com/569645: Fix when data is loaded.
-   EXPECT_EQ(1U, GetNumCommitRequestLists());
-   EXPECT_EQ(3U, GetNthCommitRequestList(0).size());
-   EXPECT_TRUE(HasCommitRequestForTag("tag1"));
-   EXPECT_TRUE(HasCommitRequestForTag("tag2"));
-   EXPECT_TRUE(HasCommitRequestForTag("tag3"));
+  EXPECT_EQ(1U, GetNumCommitRequestLists());
+  EXPECT_EQ(3U, GetNthCommitRequestList(0).size());
+  EXPECT_TRUE(HasCommitRequestForTag("tag1"));
+  EXPECT_TRUE(HasCommitRequestForTag("tag2"));
+  EXPECT_TRUE(HasCommitRequestForTag("tag3"));
 }
 
 // Test receipt of pending updates.
diff --git a/sync/internal_api/test/fake_metadata_change_list.cc b/sync/internal_api/test/fake_metadata_change_list.cc
index 407066d..ceba2cb 100644
--- a/sync/internal_api/test/fake_metadata_change_list.cc
+++ b/sync/internal_api/test/fake_metadata_change_list.cc
@@ -17,7 +17,7 @@ FakeMetadataChangeList::Record::Record() {}
 FakeMetadataChangeList::Record::~Record() {}
 
 void FakeMetadataChangeList::UpdateDataTypeState(
-    const DataTypeState& data_type_state) {
+    const sync_pb::DataTypeState& data_type_state) {
   Record record;
   record.action = UPDATE_DATA_TYPE_STATE;
   record.data_type_state = data_type_state;