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

sync/notifier/invalidator_factory.cc

Index: sync/notifier/invalidator_factory.cc
diff --git a/sync/notifier/invalidator_factory.cc b/sync/notifier/invalidator_factory.cc
index 3f09f5f9d6a8e330cc80842f8773636bb051ee22..4c894bd23a4aec166eed68b5276c25707869ad39 100644
--- a/sync/notifier/invalidator_factory.cc
+++ b/sync/notifier/invalidator_factory.cc
@@ -6,7 +6,9 @@
 
 #include <string>
 
+#include "base/base64.h"
 #include "base/logging.h"
+#include "base/rand_util.h"
 #include "jingle/notifier/listener/push_client.h"
 #include "sync/notifier/invalidator.h"
 #include "sync/notifier/non_blocking_invalidator.h"
@@ -17,6 +19,7 @@ namespace {
 
 Invalidator* CreateDefaultInvalidator(
     const notifier::NotifierOptions& notifier_options,
+    const std::string& invalidator_client_id,
     const InvalidationStateMap& initial_invalidation_state_map,
     const std::string& invalidation_bootstrap_data,
     const WeakHandle<InvalidationStateTracker>& invalidation_state_tracker,
@@ -28,14 +31,24 @@ Invalidator* CreateDefaultInvalidator(
     // on this behaviour.  See crbug.com/97780.
     return new P2PInvalidator(
         notifier::PushClient::CreateDefault(notifier_options),
+        invalidator_client_id,
         NOTIFY_ALL);
   }
 
   return new NonBlockingInvalidator(
-      notifier_options, initial_invalidation_state_map,
+      notifier_options, invalidator_client_id, initial_invalidation_state_map,
       invalidation_bootstrap_data, invalidation_state_tracker, client_info);
 }
 
+std::string GenerateInvalidatorClientId() {
+  // Generate a GUID with 128 bits worth of base64-encoded randomness.
+  // This format is similar to that of sync's cache_guid.
+  const int kGuidBytes = 128 / 8;
+  std::string guid;
+  base::Base64Encode(base::RandBytesAsString(kGuidBytes), &guid);
+  return guid;
+}
+
 }  // namespace
 
 // TODO(akalin): Remove the dependency on jingle if OS_ANDROID is defined.
@@ -45,16 +58,28 @@ InvalidatorFactory::InvalidatorFactory(
     const base::WeakPtr<InvalidationStateTracker>&
         invalidation_state_tracker)
     : notifier_options_(notifier_options),
-      client_info_(client_info),
-      initial_invalidation_state_map_(
-          invalidation_state_tracker.get() ?
-          invalidation_state_tracker->GetAllInvalidationStates() :
-          InvalidationStateMap()),
-      invalidation_bootstrap_data_(
-          invalidation_state_tracker.get() ?
-          invalidation_state_tracker->GetBootstrapData() :
-          std::string()),
-      invalidation_state_tracker_(invalidation_state_tracker) {
+      client_info_(client_info) {
+  if (!invalidation_state_tracker.get()) {
+    return;
+  }
+
+  // TODO(rlarocque): This is not the most obvious place for client ID
+  // generation code.  We should try to find a better place for it when we
+  // refactor the invalidator into its own service.
+  if (invalidation_state_tracker->GetInvalidatorClientId().empty()) {
+    // If the client ID changes, the stored client state must be cleared.
+    invalidation_state_tracker->Clear();
+    invalidation_state_tracker->SetInvalidatorClientId(

[Nicolas Zea, 2013/03/20 21:17:33]

why not have the clear call part of the SetInvalidatorClientId call?

[rlarocque, 2013/03/20 23:25:33]

That's a great idea!  It should help prevent bugs like crbug.com/175847.

+        GenerateInvalidatorClientId());
+  }
+
+  initial_invalidation_state_map_ =
+      invalidation_state_tracker->GetAllInvalidationStates();
+  invalidator_client_id_ =
+      invalidation_state_tracker->GetInvalidatorClientId();
+  invalidation_bootstrap_data_ = invalidation_state_tracker->GetBootstrapData();
+  invalidation_state_tracker_ = WeakHandle<InvalidationStateTracker>(
+      invalidation_state_tracker);
 }
 
 InvalidatorFactory::~InvalidatorFactory() {
@@ -67,10 +92,16 @@ Invalidator* InvalidatorFactory::CreateInvalidator() {
   return NULL;
 #else
   return CreateDefaultInvalidator(notifier_options_,
+                                  invalidator_client_id_,
                                   initial_invalidation_state_map_,
                                   invalidation_bootstrap_data_,
                                   invalidation_state_tracker_,
                                   client_info_);
 #endif
 }
+
+std::string InvalidatorFactory::GetInvalidatorClientId() const {
+  return invalidator_client_id_;
+}
+
 }  // namespace syncer