Sync: Add DeviceInfo's ChangeProcessor

chrome/browser/sync/glue/sync_backend_host.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "build/build_config.h"
 
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/metrics/histogram.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/timer.h"
 #include "base/tracked_objects.h"
 #include "base/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/token_service.h"
 #include "chrome/browser/signin/token_service_factory.h"
 #include "chrome/browser/sync/glue/bridged_invalidator.h"
 #include "chrome/browser/sync/glue/change_processor.h"
 #include "chrome/browser/sync/glue/chrome_encryptor.h"
 #include "chrome/browser/sync/glue/chrome_sync_notification_bridge.h"
 #include "chrome/browser/sync/glue/device_info.h"
 #include "chrome/browser/sync/glue/sync_backend_registrar.h"
+#include "chrome/browser/sync/glue/synced_device_tracker.h"
+#include "chrome/browser/sync/glue/device_info.h"

[Nicolas Zea, 2012/11/16 19:42:27]

abc order

[rlarocque, 2012/11/17 00:07:21]

That file was included twice, actually.  Must have been a bad merge.  Fixed.

 #include "chrome/browser/sync/invalidations/invalidator_storage.h"
 #include "chrome/browser/sync/sync_prefs.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/common/content_client.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "jingle/notifier/base/notification_method.h"
@@ skipping 115 matching lines @@
   void DoSetEncryptionPassphrase(const std::string& passphrase,
                                  bool is_explicit);
 
   // Called to decrypt the pending keys.
   void DoSetDecryptionPassphrase(const std::string& passphrase);
 
   // Called to turn on encryption of all sync data as well as
   // reencrypt everything.
   void DoEnableEncryptEverything();
 
-  // Called to load sync encryption state and re-encrypt any types
-  // needing encryption as necessary.
-  void DoAssociateNigori();
+  // Called to perform tasks which require the control data to be downloaded.
+  // This includes refreshing encryption, setting up the device info change
+  // processor, etc.
+  void DoInitialProcessControlTypes();
+
+  void FinishInitialProcessControlTypes();

[Nicolas Zea, 2012/11/16 19:42:27]

Comment about why these two are separated?

[rlarocque, 2012/11/17 00:07:21]

Done.

 
   // The shutdown order is a bit complicated:
   // 1) From |sync_thread_|, invoke the syncapi Shutdown call to do
   //    a final SaveChanges, and close sqlite handles.
   // 2) Then, from |frontend_loop_|, halt the sync_thread_ (which is
   //    a blocking call). This causes syncapi thread-exit handlers
   //    to run and make use of cached pointers to various components
   //    owned implicitly by us.
   // 3) Destroy this Core. That will delete syncapi components in a
   //    safe order because the thread that was using them has exited
@@ skipping 14 matching lines @@
       const base::Callback<void(syncer::ModelTypeSet)>& ready_task);
   void DoRetryConfiguration(
       const base::Closure& retry_callback);
 
   // Set the base request context to use when making HTTP calls.
   // This method will add a reference to the context to persist it
   // on the IO thread. Must be removed from IO thread.
 
   syncer::SyncManager* sync_manager() { return sync_manager_.get(); }
 
+  SyncedDeviceTracker* synced_device_tracker() {
+    return synced_device_tracker_.get();
+  }
+
   // Delete the sync data folder to cleanup backend data.  Happens the first
   // time sync is enabled for a user (to prevent accidentally reusing old
   // sync databases), as well as shutdown when you're no longer syncing.
   void DeleteSyncDataFolder();
 
  private:
   friend class base::RefCountedThreadSafe<SyncBackendHost::Core>;
   friend class SyncBackendHostForProfileSyncTest;
 
   virtual ~Core();
@@ skipping 32 matching lines @@
   // Our parent's notification bridge (not owned).  Non-NULL only
   // between calls to DoInitialize() and DoShutdown().
   ChromeSyncNotificationBridge* chrome_sync_notification_bridge_;
 
   // The timer used to periodically call SaveChanges.
   scoped_ptr<base::RepeatingTimer<Core> > save_changes_timer_;
 
   // Our encryptor, which uses Chrome's encryption functions.
   ChromeEncryptor encryptor_;
 
+  // A special ChangeProcessor that tracks the DEVICE_INFO type for us.
+  scoped_ptr<SyncedDeviceTracker> synced_device_tracker_;
+
   // The top-level syncapi entry point.  Lives on the sync thread.
   scoped_ptr<syncer::SyncManager> sync_manager_;
 
   // Whether or not we registered with |sync_manager_| as an invalidation
   // handler.  Necessary since we may end up trying to unregister before we
   // register in tests (in synchronous initialization mode).
   //
   // TODO(akalin): Fix this behavior (see http://crbug.com/140354).
   bool registered_as_invalidation_handler_;
 
@@ skipping 486 matching lines @@
 void SyncBackendHost::GetModelSafeRoutingInfo(
     syncer::ModelSafeRoutingInfo* out) const {
   if (initialized()) {
     CHECK(registrar_.get());
     registrar_->GetModelSafeRoutingInfo(out);
   } else {
     NOTREACHED();
   }
 }
 
+SyncedDeviceTracker* SyncBackendHost::GetSyncedDeviceTrackerForTest() {
+  return core_->synced_device_tracker();
+}
+
 void SyncBackendHost::InitCore(const DoInitializeOptions& options) {
   sync_thread_.message_loop()->PostTask(FROM_HERE,
       base::Bind(&SyncBackendHost::Core::DoInitialize, core_.get(), options));
 }
 
 void SyncBackendHost::RequestConfigureSyncer(
     syncer::ConfigureReason reason,
     syncer::ModelTypeSet types_to_config,
     const syncer::ModelSafeRoutingInfo& routing_info,
     const base::Callback<void(syncer::ModelTypeSet)>& ready_task,
@@ skipping 298 matching lines @@
 #endif
 
   sync_manager_ = options.sync_manager_factory->CreateSyncManager(name_);
   sync_manager_->AddObserver(this);
   sync_manager_->Init(
       sync_data_folder_path_,
       options.event_handler,
       options.service_url.host() + options.service_url.path(),
       options.service_url.EffectiveIntPort(),
       options.service_url.SchemeIsSecure(),
-      BrowserThread::GetBlockingPool(),
       options.make_http_bridge_factory_fn.Run().Pass(),
       options.workers,
       options.extensions_activity_monitor,
       options.registrar /* as SyncManager::ChangeDelegate */,
       options.credentials,
       scoped_ptr<syncer::Invalidator>(new BridgedInvalidator(
           options.chrome_sync_notification_bridge,
           options.invalidator_factory->CreateInvalidator(),
           kDefaultInvalidatorState)),
       options.restored_key_for_bootstrapping,
@@ skipping 43 matching lines @@
     sync_manager_->UpdateRegisteredInvalidationIds(this, ids);
   }
 }
 
 void SyncBackendHost::Core::DoStartSyncing(
     const syncer::ModelSafeRoutingInfo& routing_info) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   sync_manager_->StartSyncingNormally(routing_info);
 }
 
-void SyncBackendHost::Core::DoAssociateNigori() {
-  DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  sync_manager_->GetEncryptionHandler()->Init();
-  host_.Call(FROM_HERE,
-             &SyncBackendHost::HandleInitializationCompletedOnFrontendLoop,
-             true);
-}
-
 void SyncBackendHost::Core::DoSetEncryptionPassphrase(
     const std::string& passphrase,
     bool is_explicit) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   sync_manager_->GetEncryptionHandler()->SetEncryptionPassphrase(
       passphrase, is_explicit);
 }
 
+void SyncBackendHost::Core::DoInitialProcessControlTypes() {
+  DCHECK_EQ(MessageLoop::current(), sync_loop_);
+
+  DVLOG(1) << "Initilalizing Control Types";
+
+  // Initialize encryption.
+  sync_manager_->GetEncryptionHandler()->Init();
+
+  // Initialize device info.
+  synced_device_tracker_.reset(
+      new SyncedDeviceTracker(sync_manager_->cache_guid()));
+  if (sync_manager_->GetUserShare()) { // NULL in some tests.
+    DVLOG(1) << "Initializing DeviceInfo type";
+    registrar_->ActivateDataType(syncer::DEVICE_INFO,

[Nicolas Zea, 2012/11/16 19:42:27]

Why not activate the datatype after initialization succeeds? Or is it that we
expect it to never fail?

[rlarocque, 2012/11/17 00:07:21]

It made more sense to me to do it after the activation was complete.  I see
InitLocalDeviceInfo() as an operation that can only be performed on an active
data type, in the same way that only an active BookmarkChangeProcessor can
respond to bookmark change notifications.

Also, I think it relies on some the ChangeProcessor::share_handle() function,
which I believe won't work until the type is activated.

Maybe InitLocalDeviceInfo() is a bad name?

[Nicolas Zea, 2012/11/17 00:31:01]

InitLocalDeviceInfo seems to me like an "AssociateModels" step, which typically
is what must succeed before we activate a datatype. If there's a chance of that
failing we shouldn't be auto-activating the type.

At the moment, it seems that's not the case. But in case we start doing
something fancier in the future, I think I'd prefer moving ActivateDatatype to
DoFinishInitialProcessControlTypes, which you could eventually extend with a
success bool if you need to.

Given that it's both (effectively) a model associator and a change processor, I
think it's fine to pass in the user share into the device tracker directly at
construction time (which would happen here). This is also how the sync
encryption handler works.

+                                 syncer::GROUP_PASSIVE,
+                                 synced_device_tracker_.get(),
+                                 sync_manager_->GetUserShare());
+    synced_device_tracker_->InitLocalDeviceInfo(
+        base::Bind(&SyncBackendHost::Core::FinishInitialProcessControlTypes,
+                   this));
+  } else {
+    DVLOG(1) << "Skipping initialization of DeviceInfo";
+    FinishInitialProcessControlTypes();
+  }
+}
+
+void SyncBackendHost::Core::FinishInitialProcessControlTypes() {

[Nicolas Zea, 2012/11/16 19:42:27]

I think the naming convention prefers DoFinishInitialProcessControlTypes?
Although, given that this is only calling
HandleInitializationCompletedOnFrontendLoop, DoHandleInitialization... might be
more appropriate.

[rlarocque, 2012/11/17 00:07:21]

I think you're right.  I renamed it.

+  host_.Call(
+      FROM_HERE,
+      &SyncBackendHost::HandleInitializationCompletedOnFrontendLoop,
+      true);
+}
+
 void SyncBackendHost::Core::DoSetDecryptionPassphrase(
     const std::string& passphrase) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   sync_manager_->GetEncryptionHandler()->SetDecryptionPassphrase(
       passphrase);
 }
 
 void SyncBackendHost::Core::DoEnableEncryptEverything() {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   sync_manager_->GetEncryptionHandler()->EnableEncryptEverything();
 }
 
 void SyncBackendHost::Core::DoStopSyncManagerForShutdown(
     const base::Closure& closure) {
   if (sync_manager_.get()) {
     sync_manager_->StopSyncingForShutdown(closure);
   } else {
     sync_loop_->PostTask(FROM_HERE, closure);
   }
 }
 
 void SyncBackendHost::Core::DoShutdown(bool sync_disabled) {
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
+  // It's safe to do this even if the type was never activated.
+  registrar_->DeactivateDataType(syncer::DEVICE_INFO);
+  synced_device_tracker_.reset();
+
   DoDestroySyncManager();
 
   chrome_sync_notification_bridge_ = NULL;
   registrar_ = NULL;
 
   if (sync_disabled)
     DeleteSyncDataFolder();
 
   sync_loop_ = NULL;
 
@@ skipping 125 matching lines @@
 
   // Run initialization state machine.
   switch (initialization_state_) {
     case NOT_INITIALIZED:
       // This configuration should result in a download request if the nigori
       // type's initial_sync_ended bit is unset.  If the download request
       // contains progress markers, there is a risk that the server will try to
       // trigger migration.  That would be disastrous, so we must rely on the
       // sync manager to ensure that this type never has both progress markers
       // and !initial_sync_ended.
-      initialization_state_ = DOWNLOADING_NIGORI;
+      initialization_state_ = DOWNLOADING_CONTROL_TYPES;
       ConfigureDataTypes(
           syncer::CONFIGURE_REASON_NEW_CLIENT,
           syncer::ModelTypeSet(syncer::ControlTypes()),
           syncer::ModelTypeSet(),
           // Calls back into this function.
           base::Bind(
               &SyncBackendHost::
                   HandleNigoriConfigurationCompletedOnFrontendLoop,
               weak_ptr_factory_.GetWeakPtr()),
           base::Bind(&SyncBackendHost::OnNigoriDownloadRetry,
                      weak_ptr_factory_.GetWeakPtr()));
       break;
-    case DOWNLOADING_NIGORI:
-      initialization_state_ = ASSOCIATING_NIGORI;
-      // Triggers OnEncryptedTypesChanged() and OnEncryptionComplete()
-      // if necessary.
-      sync_thread_.message_loop()->PostTask(
-          FROM_HERE,
-          base::Bind(&SyncBackendHost::Core::DoAssociateNigori,
-                     core_.get()));
+    case DOWNLOADING_CONTROL_TYPES:
+      initialization_state_ = PROCESSING_CONTROL_TYPES;
+      // Updates encryption and other metadata.  Will call
+      // OnEncryptedTypesChanged() and OnEncryptionComplete() if necessary.
+      InitialProcessControlTypes(
+          base::Bind(
+              &SyncBackendHost::
+                  HandleInitializationCompletedOnFrontendLoop,
+              weak_ptr_factory_.GetWeakPtr(), true));
       break;
-    case ASSOCIATING_NIGORI:
+    case PROCESSING_CONTROL_TYPES:
       initialization_state_ = INITIALIZED;
       // Now that we've downloaded the nigori node, we can see if there are any
       // experimental types to enable. This should be done before we inform
       // the frontend to ensure they're visible in the customize screen.
       AddExperimentalTypes();
       frontend_->OnBackendInitialized(js_backend_,
                                       debug_info_listener_,
                                       true);
       js_backend_.Reset();
       break;
@@ skipping 158 matching lines @@
 
   frontend_->OnConnectionStatusChange(status);
 }
 
 void SyncBackendHost::HandleNigoriConfigurationCompletedOnFrontendLoop(
     const syncer::ModelTypeSet failed_configuration_types) {
   HandleInitializationCompletedOnFrontendLoop(
       failed_configuration_types.Empty());
 }
 
+void SyncBackendHost::InitialProcessControlTypes(
+    const base::Closure& done_callback) {
+  DCHECK_EQ(MessageLoop::current(), frontend_loop_);
+  // Then forward the request to the sync thread.
+  sync_thread_.message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&SyncBackendHost::Core::DoInitialProcessControlTypes,
+                 core_.get()));
+}
+
 #undef SDVLOG
 
 #undef SLOG
 
 }  // namespace browser_sync