sync: hook up ServerConnectionManager <> SyncerThread2 and tie up more loose ends

chrome/browser/sync/engine/syncapi.cc

 // Copyright (c) 2011 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 "chrome/browser/sync/engine/syncapi.h"
 
 #include <algorithm>
 #include <bitset>
 #include <iomanip>
 #include <list>
@@ skipping 54 matching lines @@
 #include "content/browser/browser_thread.h"
 #include "net/base/network_change_notifier.h"
 
 using browser_sync::AllStatus;
 using browser_sync::Cryptographer;
 using browser_sync::KeyParams;
 using browser_sync::ModelSafeRoutingInfo;
 using browser_sync::ModelSafeWorker;
 using browser_sync::ModelSafeWorkerRegistrar;
 using browser_sync::ServerConnectionEvent;
+using browser_sync::ServerConnectionEvent2;
+using browser_sync::ServerConnectionEventListener;
 using browser_sync::SyncEngineEvent;
 using browser_sync::SyncEngineEventListener;
 using browser_sync::Syncer;
 using browser_sync::SyncerThread;
 using browser_sync::SyncerThreadAdapter;
 using browser_sync::kNigoriTag;
 using browser_sync::sessions::SyncSessionContext;
 using std::list;
 using std::hex;
 using std::string;
@@ skipping 1018 matching lines @@
   return unencrypted_;
 }
 
 //////////////////////////////////////////////////////////////////////////
 // SyncManager's implementation: SyncManager::SyncInternal
 class SyncManager::SyncInternal
     : public net::NetworkChangeNotifier::IPAddressObserver,
       public sync_notifier::SyncNotifierObserver,
       public browser_sync::ChannelEventHandler<syncable::DirectoryChangeEvent>,
       public browser_sync::JsBackend,
-      public SyncEngineEventListener {
+      public SyncEngineEventListener,
+      public ServerConnectionEventListener {
   static const int kDefaultNudgeDelayMilliseconds;
   static const int kPreferencesNudgeDelayMilliseconds;
  public:
   explicit SyncInternal(SyncManager* sync_manager)
       : core_message_loop_(NULL),
         parent_router_(NULL),
         sync_manager_(sync_manager),
         registrar_(NULL),
         initialized_(false),
         ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
@@ skipping 176 matching lines @@
       DCHECK(false) << "ScopedDirLookup failed when checking initial sync";
       syncable::AutofillMigrationDebugInfo null_value = {0};
       return null_value;
     }
     return lookup->get_autofill_migration_debug_info();
   }
 
   // SyncEngineEventListener implementation.
   virtual void OnSyncEngineEvent(const SyncEngineEvent& event);
 
+  // ServerConnectionEventListener implementation.
+  virtual void OnServerConnectionEvent(const ServerConnectionEvent2& event);
+
   // browser_sync::JsBackend implementation.
   virtual void SetParentJsEventRouter(browser_sync::JsEventRouter* router);
   virtual void RemoveParentJsEventRouter();
   virtual const browser_sync::JsEventRouter* GetParentJsEventRouter() const;
   virtual void ProcessMessage(const std::string& name,
                               const browser_sync::JsArgList& args,
                               const browser_sync::JsEventHandler* sender);
 
  private:
   // Helper to call OnAuthError when no authentication credentials are
@@ skipping 281 matching lines @@
 
 void SyncManager::RequestClearServerData() {
   if (data_->syncer_thread())
     data_->syncer_thread()->NudgeSyncer(0, SyncerThread::kClearPrivateData);
 }
 
 void SyncManager::RequestConfig(const syncable::ModelTypeBitSet& types) {
   if (!data_->syncer_thread())
     return;
   // It is an error for this to be called if new_impl is null.
+  StartConfigurationMode(NULL);
+  data_->syncer_thread()->new_impl()->ScheduleConfig(types);
+}
+
+void SyncManager::StartConfigurationMode(ModeChangeCallback* callback) {
+  if (!data_->syncer_thread())
+    return;
+  // It is an error for this to be called if new_impl is null.
   data_->syncer_thread()->new_impl()->Start(
-      browser_sync::s3::SyncerThread::CONFIGURATION_MODE);
-  data_->syncer_thread()->new_impl()->ScheduleConfig(types);
+      browser_sync::s3::SyncerThread::CONFIGURATION_MODE, callback);
 }
 
 const std::string& SyncManager::GetAuthenticatedUsername() {
   DCHECK(data_);
   return data_->username_for_share();
 }
 
 bool SyncManager::SyncInternal::Init(
     const FilePath& database_location,
     const std::string& sync_server_and_path,
@@ skipping 15 matching lines @@
   setup_for_test_mode_ = setup_for_test_mode;
 
   sync_notifier_.reset(sync_notifier);
   sync_notifier_->AddObserver(this);
 
   share_.dir_manager.reset(new DirectoryManager(database_location));
 
   connection_manager_.reset(new SyncAPIServerConnectionManager(
       sync_server_and_path, port, use_ssl, user_agent, post_factory));
 
-  connection_manager_hookup_.reset(
-      NewEventListenerHookup(connection_manager()->channel(), this,
-          &SyncManager::SyncInternal::HandleServerConnectionEvent));
+  net::NetworkChangeNotifier::AddIPAddressObserver(this);
 
-  net::NetworkChangeNotifier::AddIPAddressObserver(this);
+  bool new_syncer_thread = CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kNewSyncerThread);
+
+  if (new_syncer_thread) {
+    connection_manager()->AddListener(this);
+  } else {
+    connection_manager_hookup_.reset(
+        NewEventListenerHookup(connection_manager()->channel(), this,
+            &SyncManager::SyncInternal::HandleServerConnectionEvent));
+  }
+
   // TODO(akalin): CheckServerReachable() can block, which may cause jank if we
   // try to shut down sync.  Fix this.
   core_message_loop_->PostTask(FROM_HERE,
       method_factory_.NewRunnableMethod(&SyncInternal::CheckServerReachable));
 
   // Test mode does not use a syncer context or syncer thread.
   if (!setup_for_test_mode_) {
     // Build a SyncSessionContext and store the worker in it.
     VLOG(1) << "Sync is bringing up SyncSessionContext.";
     std::vector<SyncEngineEventListener*> listeners;
     listeners.push_back(&allstatus_);
     listeners.push_back(this);
     SyncSessionContext* context = new SyncSessionContext(
         connection_manager_.get(),
         dir_manager(),
         model_safe_worker_registrar,
         listeners);
     context->set_account_name(credentials.email);
     // The SyncerThread takes ownership of |context|.
     syncer_thread_.reset(new SyncerThreadAdapter(context,
-        CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kNewSyncerThread)));
+        new_syncer_thread));
   }
 
   bool signed_in = SignIn(credentials);
 
   // Do this once the directory is opened.
   BootstrapEncryption(restored_key_for_bootstrapping);
   return signed_in;
 }
 
 void SyncManager::SyncInternal::BootstrapEncryption(
@@ skipping 433 matching lines @@
       kChromeOSNetworkChangeReactionDelayHackMsec);
 #else
   OnIPAddressChangedImpl();
 #endif  // defined(OS_CHROMEOS)
 }
 
 void SyncManager::SyncInternal::OnIPAddressChangedImpl() {
   // TODO(akalin): CheckServerReachable() can block, which may cause
   // jank if we try to shut down sync.  Fix this.
   connection_manager()->CheckServerReachable();
   sync_manager_->RequestNudge();

[Nicolas Zea, 2011/03/25 18:53:03]

Do we make a new connection to the server on an ip address change? If so, your
OnServerConnectionEvent in SyncerThread should be able to handle the nudge,
instead of here, right?

[tim (not reviewing), 2011/03/28 20:02:25]

We don't but see TODO(tim) in syncer_thread2.h that discusses reacting to a
change there.

 }
 
 // Listen to model changes, filter out ones initiated by the sync API, and
 // saves the rest (hopefully just backend Syncer changes resulting from
 // ApplyUpdates) to data_->changelist.
 void SyncManager::SyncInternal::HandleChannelEvent(
     const syncable::DirectoryChangeEvent& event) {
   if (event.todo == syncable::DirectoryChangeEvent::TRANSACTION_COMPLETE) {
     // Safe to perform slow I/O operations now, go ahead and commit.
     HandleTransactionCompleteChangeEvent(event);
@@ skipping 25 matching lines @@
   // Call commit
   for (int i = 0; i < syncable::MODEL_TYPE_COUNT; ++i) {
     if (model_has_change_.test(i)) {
       FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                         OnChangesComplete(syncable::ModelTypeFromInt(i)));
       model_has_change_.reset(i);
     }
   }
 }
 
+void SyncManager::SyncInternal::OnServerConnectionEvent(
+    const ServerConnectionEvent2& event) {
+  ServerConnectionEvent legacy;
+  legacy.what_happened = ServerConnectionEvent::STATUS_CHANGED;
+  legacy.connection_code = event.connection_code;
+  legacy.server_reachable = event.server_reachable;
+  HandleServerConnectionEvent(legacy);
+}
+
 void SyncManager::SyncInternal::HandleServerConnectionEvent(
     const ServerConnectionEvent& event) {
   allstatus_.HandleServerConnectionEvent(event);
   if (event.what_happened == ServerConnectionEvent::STATUS_CHANGED) {
     if (event.connection_code ==
         browser_sync::HttpResponse::SERVER_CONNECTION_OK) {
       FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                         OnAuthError(AuthError::None()));
     }
 
@@ skipping 493 matching lines @@
 void SyncManager::TriggerOnIncomingNotificationForTest(
     const syncable::ModelTypeBitSet& model_types) {
   syncable::ModelTypePayloadMap model_types_with_payloads =
       syncable::ModelTypePayloadMapFromBitSet(model_types,
           std::string());
 
   data_->OnIncomingNotification(model_types_with_payloads);
 }
 
 }  // namespace sync_api