[Sync] USS: Rename SharedModelTypeProcessor::Start and related methods.

Trying to make things clearer with the following renames:

- Start -> OnSyncStarting
- FinishStart -> ReadyToConnect
- OnConnect -> ConnectSync
- Stop -> DisconnectSync

BUG=584486

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

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

8 files changed, 63 insertions, 64 deletions

diff --git a/sync/internal_api/public/model_type_processor.h b/sync/internal_api/public/model_type_processor.h
index 297b48f..4ab4946 100644
--- a/sync/internal_api/public/model_type_processor.h
+++ b/sync/internal_api/public/model_type_processor.h
@@ -19,8 +19,11 @@ class SYNC_EXPORT ModelTypeProcessor {
   ModelTypeProcessor();
   virtual ~ModelTypeProcessor();
 
-  // Callback used to process the handshake response from the worker.
-  virtual void OnConnect(scoped_ptr<CommitQueue> commit_queue) = 0;
+  // Connect this processor to the sync engine via |commit_queue|. Once called,
+  // the processor will send any pending and future commits via this channel.
+  // This can only be called multiple times if the processor is disconnected
+  // (via the DataTypeController) in between.
+  virtual void ConnectSync(scoped_ptr<CommitQueue> commit_queue) = 0;
 
   // Informs this object that some of its commit requests have been
   // successfully serviced.
diff --git a/sync/internal_api/public/shared_model_type_processor.h b/sync/internal_api/public/shared_model_type_processor.h
index 0d8ba15..606a22b 100644
--- a/sync/internal_api/public/shared_model_type_processor.h
+++ b/sync/internal_api/public/shared_model_type_processor.h
@@ -40,16 +40,16 @@ class SYNC_EXPORT SharedModelTypeProcessor : public ModelTypeProcessor,
   typedef base::Callback<void(syncer::SyncError, scoped_ptr<ActivationContext>)>
       StartCallback;
 
-  // Called by DataTypeController to begins asynchronous operation of preparing
-  // the model to sync. Once the model is ready to be activated with Sync the
-  // callback will be invoked with the activation context. If the model is
-  // already ready it is safe to call the callback right away. Otherwise the
-  // callback needs to be stored and called when the model is ready.
-  void Start(StartCallback callback);
+  // Called by the DataTypeController to gather additional information needed
+  // before a CommitQueue object can be created for this model type. Once the
+  // metadata has been loaded, the info is collected and given to |callback|.
+  // Once called, this can only be called again if sync is disconnected.
+  void OnSyncStarting(StartCallback callback);
 
-  // Called by DataTypeController to inform the model that the sync is
-  // stopping for the model type.
-  void Stop();
+  // Disconnect this processor from the sync engine. Change metadata will
+  // continue being processed and persisted, but no commits can be made until
+  // the next time sync is connected.
+  void DisconnectSync();
 
   // Indicates that we no longer want to do any sync-related things for this
   // data type. Severs all ties to the sync thread, deletes all local sync
@@ -85,7 +85,7 @@ class SYNC_EXPORT SharedModelTypeProcessor : public ModelTypeProcessor,
   base::WeakPtr<SharedModelTypeProcessor> AsWeakPtrForUI();
 
   // ModelTypeProcessor implementation.
-  void OnConnect(scoped_ptr<CommitQueue> worker) override;
+  void ConnectSync(scoped_ptr<CommitQueue> worker) override;
   void OnCommitCompleted(const sync_pb::DataTypeState& type_state,
                          const CommitResponseDataList& response_list) override;
   void OnUpdateReceived(const sync_pb::DataTypeState& type_state,
@@ -99,7 +99,7 @@ class SYNC_EXPORT SharedModelTypeProcessor : public ModelTypeProcessor,
   using UpdateMap = std::map<std::string, scoped_ptr<UpdateResponseData>>;
 
   // Complete the start process.
-  void FinishStart();
+  void ReadyToConnect();
 
   // Handle the first update received from the server after being enabled.
   void OnInitialUpdateReceived(const sync_pb::DataTypeState& type_state,
@@ -117,7 +117,7 @@ class SYNC_EXPORT SharedModelTypeProcessor : public ModelTypeProcessor,
   syncer::ModelType type_;
   sync_pb::DataTypeState data_type_state_;
 
-  // Stores the start callback in between Start() and FinishStart().
+  // Stores the start callback in between OnSyncStarting() and ReadyToConnect().
   StartCallback start_callback_;
 
   // Indicates whether the metadata has finished loading.
diff --git a/sync/internal_api/shared_model_type_processor.cc b/sync/internal_api/shared_model_type_processor.cc
index 4b6b983..83a6a51 100644
--- a/sync/internal_api/shared_model_type_processor.cc
+++ b/sync/internal_api/shared_model_type_processor.cc
@@ -25,7 +25,7 @@ class ModelTypeProcessorProxy : public ModelTypeProcessor {
       const scoped_refptr<base::SequencedTaskRunner>& processor_task_runner);
   ~ModelTypeProcessorProxy() override;
 
-  void OnConnect(scoped_ptr<CommitQueue> worker) override;
+  void ConnectSync(scoped_ptr<CommitQueue> worker) override;
   void OnCommitCompleted(const sync_pb::DataTypeState& type_state,
                          const CommitResponseDataList& response_list) override;
   void OnUpdateReceived(const sync_pb::DataTypeState& type_state,
@@ -44,9 +44,9 @@ ModelTypeProcessorProxy::ModelTypeProcessorProxy(
 
 ModelTypeProcessorProxy::~ModelTypeProcessorProxy() {}
 
-void ModelTypeProcessorProxy::OnConnect(scoped_ptr<CommitQueue> worker) {
+void ModelTypeProcessorProxy::ConnectSync(scoped_ptr<CommitQueue> worker) {
   processor_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&ModelTypeProcessor::OnConnect, processor_,
+      FROM_HERE, base::Bind(&ModelTypeProcessor::ConnectSync, processor_,
                             base::Passed(std::move(worker))));
 }
 
@@ -81,17 +81,17 @@ SharedModelTypeProcessor::SharedModelTypeProcessor(syncer::ModelType type,
 
 SharedModelTypeProcessor::~SharedModelTypeProcessor() {}
 
-void SharedModelTypeProcessor::Start(StartCallback start_callback) {
+void SharedModelTypeProcessor::OnSyncStarting(StartCallback start_callback) {
   DCHECK(CalledOnValidThread());
   DCHECK(start_callback_.is_null());
   DCHECK(!IsConnected());
-  DVLOG(1) << "Starting " << ModelTypeToString(type_);
+  DVLOG(1) << "Sync is starting for " << ModelTypeToString(type_);
 
   start_callback_ = start_callback;
 
   if (is_metadata_loaded_) {
-    // The metadata was already loaded, so finish starting immediately.
-    FinishStart();
+    // The metadata was already loaded, so we are ready to connect.
+    ReadyToConnect();
   }
 }
 
@@ -121,12 +121,12 @@ void SharedModelTypeProcessor::OnMetadataLoaded(
   is_metadata_loaded_ = true;
 
   if (!start_callback_.is_null()) {
-    // If Start() was already called, finish starting.
-    FinishStart();
+    // If OnSyncStarting() was already called, we are now ready to connect.
+    ReadyToConnect();
   }
 }
 
-void SharedModelTypeProcessor::FinishStart() {
+void SharedModelTypeProcessor::ReadyToConnect() {
   DCHECK(CalledOnValidThread());
   DCHECK(is_metadata_loaded_);
   DCHECK(!start_callback_.is_null());
@@ -168,11 +168,11 @@ void SharedModelTypeProcessor::Disable() {
   service_->clear_change_processor();
 }
 
-void SharedModelTypeProcessor::Stop() {
+void SharedModelTypeProcessor::DisconnectSync() {
   DCHECK(CalledOnValidThread());
   DCHECK(IsConnected());
 
-  DVLOG(1) << "Stopping " << ModelTypeToString(type_);
+  DVLOG(1) << "Disconnecting sync for " << ModelTypeToString(type_);
   weak_ptr_factory_for_sync_.InvalidateWeakPtrs();
   worker_.reset();
 
@@ -185,7 +185,7 @@ SharedModelTypeProcessor::AsWeakPtrForUI() {
   return weak_ptr_factory_for_ui_.GetWeakPtr();
 }
 
-void SharedModelTypeProcessor::OnConnect(scoped_ptr<CommitQueue> worker) {
+void SharedModelTypeProcessor::ConnectSync(scoped_ptr<CommitQueue> worker) {
   DCHECK(CalledOnValidThread());
   DVLOG(1) << "Successfully connected " << ModelTypeToString(type_);
 
diff --git a/sync/internal_api/shared_model_type_processor_unittest.cc b/sync/internal_api/shared_model_type_processor_unittest.cc
index 776179a..b3ba7ea 100644
--- a/sync/internal_api/shared_model_type_processor_unittest.cc
+++ b/sync/internal_api/shared_model_type_processor_unittest.cc
@@ -57,20 +57,16 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
   // Initialize to a "ready-to-commit" state.
   void InitializeToReadyState();
 
+  // SharedModelTypeProcessor method wrappers.
   void OnMetadataLoaded();
-
-  // Start our SharedModelTypeProcessor, which will be unable to commit until it
-  // receives notification that initial sync has completed.
-  void Start();
-
-  // Stop and disconnect the CommitQueue from our SharedModelTypeProcessor.
-  void Stop();
+  void OnSyncStarting();
+  void DisconnectSync();
 
   // Disable sync for this SharedModelTypeProcessor.  Should cause sync state to
   // be discarded.
   void Disable();
 
-  // Restart sync after Stop() or Disable().
+  // Restart sync after DisconnectSync() or Disable().
   void Restart();
 
   // Local data modification.  Emulates signals from the model thread.
@@ -150,8 +146,8 @@ class SharedModelTypeProcessorTest : public ::testing::Test,
   int64_t GetServerVersion(const std::string& tag);
   void SetServerVersion(const std::string& tag, int64_t version);
 
-  void StartDone(syncer::SyncError error,
-                 scoped_ptr<ActivationContext> context);
+  void OnReadyToConnect(syncer::SyncError error,
+                        scoped_ptr<ActivationContext> context);
 
   // FakeModelTypeService overrides.
   std::string GetClientTag(const EntityData& entity_data) override;
@@ -191,7 +187,7 @@ SharedModelTypeProcessorTest::~SharedModelTypeProcessorTest() {}
 void SharedModelTypeProcessorTest::InitializeToReadyState() {
   data_type_state_.set_initial_sync_done(true);
   OnMetadataLoaded();
-  Start();
+  OnSyncStarting();
   // TODO(maxbogue): crbug.com/569642: Remove this once entity data is loaded
   // for the normal startup flow.
   UpdateResponseDataList empty_update_list;
@@ -205,13 +201,13 @@ void SharedModelTypeProcessorTest::OnMetadataLoaded() {
   metadata_batch_.reset(new MetadataBatch());
 }
 
-void SharedModelTypeProcessorTest::Start() {
-  type_processor()->Start(base::Bind(&SharedModelTypeProcessorTest::StartDone,
-                                     base::Unretained(this)));
+void SharedModelTypeProcessorTest::OnSyncStarting() {
+  type_processor()->OnSyncStarting(base::Bind(
+      &SharedModelTypeProcessorTest::OnReadyToConnect, base::Unretained(this)));
 }
 
-void SharedModelTypeProcessorTest::Stop() {
-  type_processor()->Stop();
+void SharedModelTypeProcessorTest::DisconnectSync() {
+  type_processor()->DisconnectSync();
   mock_queue_ = NULL;
   mock_queue_ptr_.reset();
 }
@@ -233,16 +229,16 @@ void SharedModelTypeProcessorTest::Restart() {
   mock_queue_ptr_.reset(new MockCommitQueue());
   mock_queue_ = mock_queue_ptr_.get();
   // Restart sync with the new CommitQueue.
-  Start();
+  OnSyncStarting();
 }
 
-void SharedModelTypeProcessorTest::StartDone(
+void SharedModelTypeProcessorTest::OnReadyToConnect(
     syncer::SyncError error,
     scoped_ptr<ActivationContext> context) {
   // Hand off ownership of |mock_queue_ptr_|, while keeping
   // an unsafe pointer to it.  This is why we can only connect once.
   DCHECK(mock_queue_ptr_);
-  context->type_processor->OnConnect(std::move(mock_queue_ptr_));
+  context->type_processor->ConnectSync(std::move(mock_queue_ptr_));
   // The context's type processor is a proxy; run the task it posted.
   sync_loop_.RunUntilIdle();
 }
@@ -832,7 +828,7 @@ TEST_F(SharedModelTypeProcessorTest, TwoIndependentItems) {
 // Verify that it waits until initial sync is complete before requesting
 // commits.
 TEST_F(SharedModelTypeProcessorTest, NoCommitsUntilInitialSyncDone) {
-  Start();
+  OnSyncStarting();
   OnMetadataLoaded();
 
   FakeMetadataChangeList change_list;
@@ -857,7 +853,7 @@ TEST_F(SharedModelTypeProcessorTest, NoCommitsUntilInitialSyncDone) {
 //
 // Creates items in various states of commit and verifies they re-attempt to
 // commit on reconnect.
-TEST_F(SharedModelTypeProcessorTest, Stop) {
+TEST_F(SharedModelTypeProcessorTest, Disconnect) {
   InitializeToReadyState();
 
   FakeMetadataChangeList change_list;
@@ -871,7 +867,7 @@ TEST_F(SharedModelTypeProcessorTest, Stop) {
   WriteItem("tag2", "value2", &change_list);
   EXPECT_TRUE(HasCommitRequestForTag("tag2"));
 
-  Stop();
+  DisconnectSync();
 
   // The third item is added after stopping.
   WriteItem("tag3", "value3", &change_list);
@@ -973,15 +969,15 @@ TEST_F(SharedModelTypeProcessorTest, DisableWithPendingUpdates) {
   EXPECT_FALSE(HasPendingUpdate("tag1"));
 }
 
-// Test that Stop does not clear pending update state.
-TEST_F(SharedModelTypeProcessorTest, StopWithPendingUpdates) {
+// Test that disconnecting does not clear pending update state.
+TEST_F(SharedModelTypeProcessorTest, DisconnectWithPendingUpdates) {
   InitializeToReadyState();
 
   PendingUpdateFromServer(5, "tag1", "value1", "key1");
   EXPECT_EQ(1U, GetNumPendingUpdates());
   ASSERT_TRUE(HasPendingUpdate("tag1"));
 
-  Stop();
+  DisconnectSync();
   Restart();
 
   EXPECT_EQ(1U, GetNumPendingUpdates());
diff --git a/sync/internal_api/sync_context_proxy_impl_unittest.cc b/sync/internal_api/sync_context_proxy_impl_unittest.cc
index 0319dd2..6058d67 100644
--- a/sync/internal_api/sync_context_proxy_impl_unittest.cc
+++ b/sync/internal_api/sync_context_proxy_impl_unittest.cc
@@ -48,13 +48,13 @@ class SyncContextProxyImplTest : public ::testing::Test, FakeModelTypeService {
   // function simulates such an event.
   void DisableSync() { registry_.reset(); }
 
-  void Start(SharedModelTypeProcessor* processor) {
-    processor->Start(base::Bind(&SyncContextProxyImplTest::StartDone,
-                                base::Unretained(this)));
+  void OnSyncStarting(SharedModelTypeProcessor* processor) {
+    processor->OnSyncStarting(base::Bind(
+        &SyncContextProxyImplTest::OnReadyToConnect, base::Unretained(this)));
   }
 
-  void StartDone(syncer::SyncError error,
-                 scoped_ptr<ActivationContext> context) {
+  void OnReadyToConnect(syncer::SyncError error,
+                        scoped_ptr<ActivationContext> context) {
     context_proxy_->ConnectTypeToSync(syncer::THEMES, std::move(context));
   }
 
@@ -82,7 +82,7 @@ class SyncContextProxyImplTest : public ::testing::Test, FakeModelTypeService {
 TEST_F(SyncContextProxyImplTest, FailToConnect1) {
   scoped_ptr<SharedModelTypeProcessor> processor = CreateModelTypeProcessor();
   DisableSync();
-  Start(processor.get());
+  OnSyncStarting(processor.get());
 
   base::RunLoop run_loop_;
   run_loop_.RunUntilIdle();
@@ -92,7 +92,7 @@ TEST_F(SyncContextProxyImplTest, FailToConnect1) {
 // Try to connect a type to a SyncContext as it shuts down.
 TEST_F(SyncContextProxyImplTest, FailToConnect2) {
   scoped_ptr<SharedModelTypeProcessor> processor = CreateModelTypeProcessor();
-  Start(processor.get());
+  OnSyncStarting(processor.get());
   DisableSync();
 
   base::RunLoop run_loop_;
@@ -103,7 +103,7 @@ TEST_F(SyncContextProxyImplTest, FailToConnect2) {
 // Tests the case where the type's sync proxy shuts down first.
 TEST_F(SyncContextProxyImplTest, TypeDisconnectsFirst) {
   scoped_ptr<SharedModelTypeProcessor> processor = CreateModelTypeProcessor();
-  Start(processor.get());
+  OnSyncStarting(processor.get());
 
   base::RunLoop run_loop_;
   run_loop_.RunUntilIdle();
@@ -115,7 +115,7 @@ TEST_F(SyncContextProxyImplTest, TypeDisconnectsFirst) {
 // Tests the case where the sync thread shuts down first.
 TEST_F(SyncContextProxyImplTest, SyncDisconnectsFirst) {
   scoped_ptr<SharedModelTypeProcessor> processor = CreateModelTypeProcessor();
-  Start(processor.get());
+  OnSyncStarting(processor.get());
 
   base::RunLoop run_loop_;
   run_loop_.RunUntilIdle();
diff --git a/sync/sessions/model_type_registry.cc b/sync/sessions/model_type_registry.cc
index 5806522..a4766db4 100644
--- a/sync/sessions/model_type_registry.cc
+++ b/sync/sessions/model_type_registry.cc
@@ -156,7 +156,7 @@ void ModelTypeRegistry::ConnectSyncTypeToWorker(
       worker->AsWeakPtr(), scoped_refptr<base::SequencedTaskRunner>(
                                base::ThreadTaskRunnerHandle::Get())));
 
-  type_processor->OnConnect(std::move(commit_queue_proxy));
+  type_processor->ConnectSync(std::move(commit_queue_proxy));
 
   DCHECK(update_handler_map_.find(type) == update_handler_map_.end());
   DCHECK(commit_contributor_map_.find(type) == commit_contributor_map_.end());
diff --git a/sync/test/engine/mock_model_type_processor.cc b/sync/test/engine/mock_model_type_processor.cc
index c22a847..7a6acf5 100644
--- a/sync/test/engine/mock_model_type_processor.cc
+++ b/sync/test/engine/mock_model_type_processor.cc
@@ -18,7 +18,7 @@ MockModelTypeProcessor::MockModelTypeProcessor() : is_synchronous_(true) {
 MockModelTypeProcessor::~MockModelTypeProcessor() {
 }
 
-void MockModelTypeProcessor::OnConnect(scoped_ptr<CommitQueue> commit_queue) {
+void MockModelTypeProcessor::ConnectSync(scoped_ptr<CommitQueue> commit_queue) {
   NOTREACHED();
 }
 
diff --git a/sync/test/engine/mock_model_type_processor.h b/sync/test/engine/mock_model_type_processor.h
index a5a168e..58600e9 100644
--- a/sync/test/engine/mock_model_type_processor.h
+++ b/sync/test/engine/mock_model_type_processor.h
@@ -37,7 +37,7 @@ class MockModelTypeProcessor : public ModelTypeProcessor {
   ~MockModelTypeProcessor() override;
 
   // Implementation of ModelTypeProcessor.
-  void OnConnect(scoped_ptr<CommitQueue> commit_queue) override;
+  void ConnectSync(scoped_ptr<CommitQueue> commit_queue) override;
   void OnCommitCompleted(const sync_pb::DataTypeState& type_state,
                          const CommitResponseDataList& response_list) override;
   void OnUpdateReceived(const sync_pb::DataTypeState& type_state,