Separate invalidator and sync client ID (part 1/2)

This change implements support for setting the sync client ID and
invalidator client ID separately.  The two IDs are managed separately
and both of them are sent up to the server.  This change includes some
additional changes to support the transition, such as a new field in
the about:sync UI.

At this point, the IDs are not different.  Both are initialized from the
same source: the sync database.  The work to store and manage the
invalidator's ID separate from sync was begun in r180907 and will be
finished in a separate commit.  We will not be able to complete that
work until the server supports separate IDs.

BUG=124142


Review URL: https://chromiumcodereview.appspot.com/12256033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@185721 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 76 insertions, 27 deletions

diff --git a/sync/engine/all_status.cc b/sync/engine/all_status.cc
index 12a5a09..10201f8 100644
--- a/sync/engine/all_status.cc
+++ b/sync/engine/all_status.cc
@@ -167,9 +167,15 @@ void AllStatus::SetKeystoreMigrationTime(const base::Time& migration_time) {
   status_.keystore_migration_time = migration_time;
 }
 
-void AllStatus::SetUniqueId(const std::string& guid) {
+void AllStatus::SetSyncId(const std::string& sync_id) {
   ScopedStatusLock lock(this);
-  status_.unique_id = guid;
+  status_.sync_id = sync_id;
+}
+
+void AllStatus::SetInvalidatorClientId(
+    const std::string& invalidator_client_id) {
+  ScopedStatusLock lock(this);
+  status_.invalidator_client_id = invalidator_client_id;
 }
 
 void AllStatus::IncrementNudgeCounter(NudgeSource source) {
diff --git a/sync/engine/all_status.h b/sync/engine/all_status.h
index 6d3e52b..76ce3073 100644
--- a/sync/engine/all_status.h
+++ b/sync/engine/all_status.h
@@ -58,7 +58,8 @@ class AllStatus : public SyncEngineEventListener {
   void SetHasKeystoreKey(bool has_keystore_key);
   void SetKeystoreMigrationTime(const base::Time& migration_time);
 
-  void SetUniqueId(const std::string& guid);
+  void SetSyncId(const std::string& sync_id);
+  void SetInvalidatorClientId(const std::string& invalidator_client_id);
 
   void IncrementNudgeCounter(NudgeSource source);
 
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index 4718952..3fe4c0d 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -128,7 +128,8 @@ class SyncSchedulerTest : public testing::Test {
             connection_.get(), directory(), workers,
             &extensions_activity_monitor_, throttled_data_type_tracker_.get(),
             std::vector<SyncEngineEventListener*>(), NULL, NULL,
-            true  /* enable keystore encryption */));
+            true,  // enable keystore encryption
+            "fake_invalidator_client_id"));
     context_->set_routing_info(routing_info_);
     context_->set_notifications_enabled(true);
     context_->set_account_name("Test");
diff --git a/sync/engine/sync_scheduler_whitebox_unittest.cc b/sync/engine/sync_scheduler_whitebox_unittest.cc
index 5a9d774..8b0f7ef 100644
--- a/sync/engine/sync_scheduler_whitebox_unittest.cc
+++ b/sync/engine/sync_scheduler_whitebox_unittest.cc
@@ -53,7 +53,8 @@ class SyncSchedulerWhiteboxTest : public testing::Test {
             workers, &extensions_activity_monitor_,
             throttled_data_type_tracker_.get(),
             std::vector<SyncEngineEventListener*>(), NULL, NULL,
-            true  /* enable keystore encryption */));
+            true,  // enable keystore encryption
+            "fake_invalidator_client_id"));
     context_->set_notifications_enabled(true);
     context_->set_account_name("Test");
     scheduler_.reset(
diff --git a/sync/engine/sync_session_job_unittest.cc b/sync/engine/sync_session_job_unittest.cc
index f19ade8..7a3f2ba 100644
--- a/sync/engine/sync_session_job_unittest.cc
+++ b/sync/engine/sync_session_job_unittest.cc
@@ -57,7 +57,8 @@ class SyncSessionJobTest : public testing::Test {
         std::vector<SyncEngineEventListener*>(),
         NULL,  // |debug_info_getter|
         NULL,  // |traffic_recorder|
-        true   /* |enable keystore encryption| */));
+        true,  // |enable keystore encryption|
+        "fake_invalidator_client_id"));
     context_->set_routing_info(routes_);
   }
 
diff --git a/sync/engine/syncer_proto_util.cc b/sync/engine/syncer_proto_util.cc
index fd856fc..47a17da 100644
--- a/sync/engine/syncer_proto_util.cc
+++ b/sync/engine/syncer_proto_util.cc
@@ -363,6 +363,7 @@ SyncerError SyncerProtoUtil::PostClientToServerMessage(
   AddBagOfChips(session->context()->directory(), msg);
   msg->set_api_key(google_apis::GetAPIKey());
   msg->mutable_client_status()->CopyFrom(session->context()->client_status());
+  msg->set_invalidator_client_id(session->context()->invalidator_client_id());
 
   syncable::Directory* dir = session->context()->directory();
 
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index b85f652..304dd5a 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -226,7 +226,8 @@ class SyncerTest : public testing::Test,
             mock_server_.get(), directory(), workers,
             &extensions_activity_monitor_, throttled_data_type_tracker_.get(),
             listeners, NULL, &traffic_recorder_,
-            true  /* enable keystore encryption */));
+            true,  // enable keystore encryption
+            "fake_invalidator_client_id"));
     context_->set_routing_info(routing_info);
     syncer_ = new Syncer();
     session_.reset(MakeSession());
diff --git a/sync/internal_api/internal_components_factory_impl.cc b/sync/internal_api/internal_components_factory_impl.cc
index 68d4c7b..ccc1951 100644
--- a/sync/internal_api/internal_components_factory_impl.cc
+++ b/sync/internal_api/internal_components_factory_impl.cc
@@ -41,13 +41,15 @@ InternalComponentsFactoryImpl::BuildContext(
     ThrottledDataTypeTracker* throttled_data_type_tracker,
     const std::vector<SyncEngineEventListener*>& listeners,
     sessions::DebugInfoGetter* debug_info_getter,
-    TrafficRecorder* traffic_recorder) {
+    TrafficRecorder* traffic_recorder,
+    const std::string& invalidation_client_id) {
   return scoped_ptr<sessions::SyncSessionContext>(
       new sessions::SyncSessionContext(
           connection_manager, directory, workers, monitor,
           throttled_data_type_tracker, listeners, debug_info_getter,
           traffic_recorder,
-          switches_.encryption_method == ENCRYPTION_KEYSTORE));
+          switches_.encryption_method == ENCRYPTION_KEYSTORE,
+          invalidation_client_id));
 }
 
 scoped_ptr<syncable::DirectoryBackingStore>
diff --git a/sync/internal_api/public/engine/sync_status.h b/sync/internal_api/public/engine/sync_status.h
index b8d49bf..b28d361 100644
--- a/sync/internal_api/public/engine/sync_status.h
+++ b/sync/internal_api/public/engine/sync_status.h
@@ -93,8 +93,11 @@ struct SYNC_EXPORT SyncStatus {
   // Per-datatype throttled status.
   ModelTypeSet throttled_types;
 
-  // The unique identifer for this client.
-  std::string unique_id;
+  // The unique identifer for the sync store.
+  std::string sync_id;
+
+  // The unique identifier for the invalidation client.
+  std::string invalidator_client_id;
 
   // Counters grouped by model type
   std::vector<int> num_entries_by_type;
diff --git a/sync/internal_api/public/internal_components_factory.h b/sync/internal_api/public/internal_components_factory.h
index aad95d1..f49f88a 100644
--- a/sync/internal_api/public/internal_components_factory.h
+++ b/sync/internal_api/public/internal_components_factory.h
@@ -76,7 +76,8 @@ class SYNC_EXPORT InternalComponentsFactory {
       ThrottledDataTypeTracker* throttled_data_type_tracker,
       const std::vector<SyncEngineEventListener*>& listeners,
       sessions::DebugInfoGetter* debug_info_getter,
-      TrafficRecorder* traffic_recorder) = 0;
+      TrafficRecorder* traffic_recorder,
+      const std::string& invalidator_client_id) = 0;
 
   virtual scoped_ptr<syncable::DirectoryBackingStore>
   BuildDirectoryBackingStore(
diff --git a/sync/internal_api/public/internal_components_factory_impl.h b/sync/internal_api/public/internal_components_factory_impl.h
index e69fa3d..197ce46 100644
--- a/sync/internal_api/public/internal_components_factory_impl.h
+++ b/sync/internal_api/public/internal_components_factory_impl.h
@@ -31,7 +31,8 @@ class SYNC_EXPORT InternalComponentsFactoryImpl
       ThrottledDataTypeTracker* throttled_data_type_tracker,
       const std::vector<SyncEngineEventListener*>& listeners,
       sessions::DebugInfoGetter* debug_info_getter,
-      TrafficRecorder* traffic_recorder) OVERRIDE;
+      TrafficRecorder* traffic_recorder,
+      const std::string& invalidator_client_id) OVERRIDE;
 
   virtual scoped_ptr<syncable::DirectoryBackingStore>
   BuildDirectoryBackingStore(
diff --git a/sync/internal_api/public/test/test_internal_components_factory.h b/sync/internal_api/public/test/test_internal_components_factory.h
index 41420ae..9db26e2 100644
--- a/sync/internal_api/public/test/test_internal_components_factory.h
+++ b/sync/internal_api/public/test/test_internal_components_factory.h
@@ -37,7 +37,8 @@ class TestInternalComponentsFactory : public InternalComponentsFactory {
       ThrottledDataTypeTracker* throttled_data_type_tracker,
       const std::vector<SyncEngineEventListener*>& listeners,
       sessions::DebugInfoGetter* debug_info_getter,
-      TrafficRecorder* traffic_recorder) OVERRIDE;
+      TrafficRecorder* traffic_recorder,
+      const std::string& invalidator_client_id) OVERRIDE;
 
   virtual scoped_ptr<syncable::DirectoryBackingStore>
   BuildDirectoryBackingStore(
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index d95ba0b..b0d826a 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -415,11 +415,17 @@ void SyncManagerImpl::Init(
   connection_manager_->set_client_id(directory()->cache_guid());
   connection_manager_->AddListener(this);
 
-  // Retrieve and set the sync notifier id.
-  std::string unique_id = directory()->cache_guid();
-  DVLOG(1) << "Read notification unique ID: " << unique_id;
-  allstatus_.SetUniqueId(unique_id);
-  invalidator_->SetUniqueId(unique_id);
+  std::string sync_id = directory()->cache_guid();
+
+  // TODO(rlarocque): The invalidator client ID should be independent from the
+  // sync client ID.  See crbug.com/124142.
+  const std::string invalidator_client_id = sync_id;
+
+  allstatus_.SetSyncId(sync_id);
+  allstatus_.SetInvalidatorClientId(invalidator_client_id);
+
+  DVLOG(1) << "Setting sync client ID: " << sync_id;
+  DVLOG(1) << "Setting invalidator client ID: " << invalidator_client_id;
 
   // Build a SyncSessionContext and store the worker in it.
   DVLOG(1) << "Sync is bringing up SyncSessionContext.";
@@ -434,7 +440,8 @@ void SyncManagerImpl::Init(
       &throttled_data_type_tracker_,
       listeners,
       &debug_info_event_listener_,
-      &traffic_recorder_).Pass();
+      &traffic_recorder_,
+      invalidator_client_id).Pass();
   session_context_->set_account_name(credentials.email);
   scheduler_ = internal_components_factory->BuildScheduler(
       name_, session_context_.get()).Pass();
diff --git a/sync/internal_api/test/test_internal_components_factory.cc b/sync/internal_api/test/test_internal_components_factory.cc
index 1fe161aa..dee381f 100644
--- a/sync/internal_api/test/test_internal_components_factory.cc
+++ b/sync/internal_api/test/test_internal_components_factory.cc
@@ -35,7 +35,8 @@ TestInternalComponentsFactory::BuildContext(
     ThrottledDataTypeTracker* throttled_data_type_tracker,
     const std::vector<SyncEngineEventListener*>& listeners,
     sessions::DebugInfoGetter* debug_info_getter,
-    TrafficRecorder* traffic_recorder) {
+    TrafficRecorder* traffic_recorder,
+    const std::string& invalidator_client_id) {
 
   // Tests don't wire up listeners.
   std::vector<SyncEngineEventListener*> empty_listeners;
@@ -44,7 +45,8 @@ TestInternalComponentsFactory::BuildContext(
           connection_manager, directory, workers, monitor,
           throttled_data_type_tracker, empty_listeners, debug_info_getter,
           traffic_recorder,
-          switches_.encryption_method == ENCRYPTION_KEYSTORE));
+          switches_.encryption_method == ENCRYPTION_KEYSTORE,
+          invalidator_client_id));
 
 }
 
diff --git a/sync/protocol/sync.proto b/sync/protocol/sync.proto
index 468da88..934021d 100644
--- a/sync/protocol/sync.proto
+++ b/sync/protocol/sync.proto
@@ -569,6 +569,11 @@ message ClientToServerMessage {
   // The client should set this on every message sent to the server, though its
   // member fields may often be unset.
   optional ClientStatus client_status = 13;
+
+  // The ID that our invalidation client used to identify itself to the server.
+  // Sending the ID here allows the server to not send notifications of our own
+  // changes to our invalidator.
+  optional string invalidator_client_id = 14;
 };
 
 message CommitResponse {
diff --git a/sync/sessions/sync_session_context.cc b/sync/sessions/sync_session_context.cc
index f4f625b..f931f4c 100644
--- a/sync/sessions/sync_session_context.cc
+++ b/sync/sessions/sync_session_context.cc
@@ -23,7 +23,8 @@ SyncSessionContext::SyncSessionContext(
     const std::vector<SyncEngineEventListener*>& listeners,
     DebugInfoGetter* debug_info_getter,
     TrafficRecorder* traffic_recorder,
-    bool keystore_encryption_enabled)
+    bool keystore_encryption_enabled,
+    const std::string& invalidator_client_id)
     : connection_manager_(connection_manager),
       directory_(directory),
       workers_(workers),
@@ -33,7 +34,8 @@ SyncSessionContext::SyncSessionContext(
       throttled_data_type_tracker_(throttled_data_type_tracker),
       debug_info_getter_(debug_info_getter),
       traffic_recorder_(traffic_recorder),
-      keystore_encryption_enabled_(keystore_encryption_enabled) {
+      keystore_encryption_enabled_(keystore_encryption_enabled),
+      invalidator_client_id_(invalidator_client_id) {
   std::vector<SyncEngineEventListener*>::const_iterator it;
   for (it = listeners.begin(); it != listeners.end(); ++it)
     listeners_.AddObserver(*it);
diff --git a/sync/sessions/sync_session_context.h b/sync/sessions/sync_session_context.h
index d47c3e2..755e574 100644
--- a/sync/sessions/sync_session_context.h
+++ b/sync/sessions/sync_session_context.h
@@ -56,7 +56,8 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
                      const std::vector<SyncEngineEventListener*>& listeners,
                      DebugInfoGetter* debug_info_getter,
                      TrafficRecorder* traffic_recorder,
-                     bool keystore_encryption_enabled);
+                     bool keystore_encryption_enabled,
+                     const std::string& invalidator_client_id);
 
   ~SyncSessionContext();
 
@@ -130,6 +131,10 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
     return client_status_;
   }
 
+  const std::string& invalidator_client_id() const {
+    return invalidator_client_id_;
+  }
+
  private:
   // Rather than force clients to set and null-out various context members, we
   // extend our encapsulation boundary to scoped helpers that take care of this
@@ -178,6 +183,12 @@ class SYNC_EXPORT_PRIVATE SyncSessionContext {
   // the experiment is not enabled.
   bool keystore_encryption_enabled_;
 
+  // This is a copy of the identifier the that the invalidations client used to
+  // register itself with the invalidations server during startup.  We need to
+  // provide this to the sync server when we make changes to enable it to
+  // prevent us from receiving notifications of changes we make ourselves.
+  const std::string invalidator_client_id_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncSessionContext);
 };
 
diff --git a/sync/sessions/sync_session_unittest.cc b/sync/sessions/sync_session_unittest.cc
index 008e893..bb4a3d5 100644
--- a/sync/sessions/sync_session_unittest.cc
+++ b/sync/sessions/sync_session_unittest.cc
@@ -64,7 +64,8 @@ class SyncSessionTest : public testing::Test,
             std::vector<SyncEngineEventListener*>(),
             NULL,
             NULL,
-            true  /* enable keystore encryption */));
+            true,  // enable keystore encryption
+            "fake_invalidator_client_id"));
     context_->set_routing_info(routes_);
 
     session_.reset(MakeSession());
diff --git a/sync/test/engine/syncer_command_test.h b/sync/test/engine/syncer_command_test.h
index d3870bb..7e3ff6e 100644
--- a/sync/test/engine/syncer_command_test.h
+++ b/sync/test/engine/syncer_command_test.h
@@ -127,7 +127,8 @@ class SyncerCommandTestBase : public testing::Test,
             std::vector<SyncEngineEventListener*>(),
             &mock_debug_info_getter_,
             &traffic_recorder_,
-            true  /* enable keystore encryption*/ ));
+            true,  // enable keystore encryption
+            "fake_invalidator_client_id"));
     context_->set_routing_info(routing_info_);
     context_->set_account_name(directory()->name());
     ClearSession();