[Sync] Refactor sync configuration logic.

sync/internal_api/syncapi_unittest.cc

Index: sync/internal_api/syncapi_unittest.cc
diff --git a/sync/internal_api/syncapi_unittest.cc b/sync/internal_api/syncapi_unittest.cc
index e8fa2bd7f26eb9d7acdc206f91a4ee764e260524..cfe2f7724fa38de38de7c5f7813e916bc386f33e 100644
--- a/sync/internal_api/syncapi_unittest.cc
+++ b/sync/internal_api/syncapi_unittest.cc
@@ -24,6 +24,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "sync/engine/nigori_util.h"
+#include "sync/engine/sync_scheduler.h"
 #include "sync/internal_api/change_record.h"
 #include "sync/internal_api/http_post_provider_factory.h"
 #include "sync/internal_api/http_post_provider_interface.h"
@@ -720,6 +721,35 @@ class SyncNotifierMock : public sync_notifier::SyncNotifier {
   MOCK_METHOD1(SendNotification, void(syncable::ModelTypeSet));
 };
 
+class TestSyncManager : public SyncManager {
+ public:
+  TestSyncManager()
+    : SyncManager("Test sync manager"),
+      result_(true) {}
+
+  browser_sync::ConfigureParams get_config_params() const { return params_; }
+  void set_config_result(bool result) { result_ = result; }
+
+ protected:
+  // SyncManager implementation.
+  virtual bool DoConfigureSyncer(
+      const browser_sync::ConfigureParams& params) OVERRIDE {
+    params_ = params;
+    if (params.ready_task.is_null()) {
+      ADD_FAILURE();
+      return result_;
+    }
+
+    if (result_)
+      params.ready_task.Run();
+    return result_;
+  }
+
+ private:
+  browser_sync::ConfigureParams params_;
+  bool result_;
+};
+
 }  // namespace
 
 class SyncManagerTest : public testing::Test,
@@ -738,7 +768,6 @@ class SyncManagerTest : public testing::Test,
 
   SyncManagerTest()
       : sync_notifier_mock_(NULL),
-        sync_manager_("Test sync manager"),
         sync_notifier_observer_(NULL),
         update_enabled_types_call_count_(0) {}
 
@@ -939,7 +968,7 @@ class SyncManagerTest : public testing::Test,
  protected:
   FakeEncryptor encryptor_;
   TestUnrecoverableErrorHandler handler_;
-  SyncManager sync_manager_;
+  TestSyncManager sync_manager_;
   WeakHandle<JsBackend> js_backend_;
   StrictMock<SyncManagerObserverMock> observer_;
   sync_notifier::SyncNotifierObserver* sync_notifier_observer_;
@@ -2507,4 +2536,60 @@ TEST_F(SyncManagerTest, SetPreviouslyEncryptedSpecifics) {
   }
 }
 
+namespace {
+void SetBool(bool* called) {

[rlarocque, 2012/06/09 01:44:35]

Same comment as in the other file: I'd prefer to see this implemented with
mocks.

[Nicolas Zea, 2012/06/11 23:05:20]

See other response.

+  *called = true;
+}
+}  // namespace
+
+// Test that the configuration params are properly sent to DoConfigureSyncer.
+TEST_F(SyncManagerTest, BasicConfiguration) {
+  ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION;
+  syncable::ModelTypeSet types_to_config(syncable::BOOKMARKS,
+                                         syncable::PREFERENCES);
+  browser_sync::ModelSafeRoutingInfo new_routing_info;
+  GetModelSafeRoutingInfo(&new_routing_info);
+  sync_manager_.set_config_result(true);
+
+  bool ready_task = false;
+  bool retry_task = false;
+  sync_manager_.ConfigureSyncer(reason,
+                                types_to_config,
+                                new_routing_info,
+                                base::Bind(&SetBool, &ready_task),
+                                base::Bind(&SetBool, &retry_task));
+  EXPECT_TRUE(ready_task);
+  EXPECT_FALSE(retry_task);
+  browser_sync::ConfigureParams params = sync_manager_.get_config_params();
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
+            params.source);
+  EXPECT_TRUE(types_to_config.Equals(params.types_to_config));
+  EXPECT_EQ(new_routing_info, params.routing_info);
+}
+
+// Test that the retry callback is invoked on configuration failure.
+TEST_F(SyncManagerTest, ConfigurationRetry) {
+  ConfigureReason reason = CONFIGURE_REASON_RECONFIGURATION;
+  syncable::ModelTypeSet types_to_config(syncable::BOOKMARKS,
+                                         syncable::PREFERENCES);
+  browser_sync::ModelSafeRoutingInfo new_routing_info;
+  GetModelSafeRoutingInfo(&new_routing_info);
+  sync_manager_.set_config_result(false);
+
+  bool ready_task = false;
+  bool retry_task = false;
+  sync_manager_.ConfigureSyncer(reason,
+                                types_to_config,
+                                new_routing_info,
+                                base::Bind(&SetBool, &ready_task),
+                                base::Bind(&SetBool, &retry_task));
+  EXPECT_FALSE(ready_task);
+  EXPECT_TRUE(retry_task);
+  browser_sync::ConfigureParams params = sync_manager_.get_config_params();
+  EXPECT_EQ(sync_pb::GetUpdatesCallerInfo::RECONFIGURATION,
+            params.source);
+  EXPECT_TRUE(types_to_config.Equals(params.types_to_config));
+  EXPECT_EQ(new_routing_info, params.routing_info);
+}
+
 }  // namespace browser_sync