[Sync] Fix ConfigureDataTypes bug that leads to extra nudge

chrome/browser/sync/glue/sync_backend_host_unittest.cc

Index: chrome/browser/sync/glue/sync_backend_host_unittest.cc
diff --git a/chrome/browser/sync/glue/sync_backend_host_unittest.cc b/chrome/browser/sync/glue/sync_backend_host_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..f02f23a93b7ad258fbf4d44e22ed794e3884fa94
--- /dev/null
+++ b/chrome/browser/sync/glue/sync_backend_host_unittest.cc
@@ -0,0 +1,182 @@
+// 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/glue/sync_backend_host.h"
+
+#include <cstddef>
+
+#include "base/message_loop.h"
+#include "base/scoped_ptr.h"
+#include "chrome/browser/sync/engine/model_safe_worker.h"
+#include "chrome/browser/sync/engine/syncapi.h"
+#include "chrome/browser/sync/glue/data_type_controller.h"
+#include "chrome/browser/sync/syncable/model_type.h"
+#include "chrome/test/testing_profile.h"
+#include "content/browser/browser_thread.h"
+#include "googleurl/src/gurl.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+// TODO(akalin): Remove this once we fix the TODO below.
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/common/pref_names.h"
+
+namespace browser_sync {
+
+namespace {
+
+class MockSyncFrontend : public SyncFrontend {

[tim (not reviewing), 2011/04/05 04:56:01]

I've been waiting for MockSyncFrontend day for a very long time.
April 4, forevermore.

+ public:
+  virtual ~MockSyncFrontend() {}
+
+  MOCK_METHOD0(OnBackendInitialized, void());
+  MOCK_METHOD0(OnSyncCycleCompleted, void());
+  MOCK_METHOD0(OnAuthError, void());
+  MOCK_METHOD0(OnStopSyncingPermanently, void());
+  MOCK_METHOD0(OnClearServerDataSucceeded, void());
+  MOCK_METHOD0(OnClearServerDataFailed, void());
+  MOCK_METHOD1(OnPassphraseRequired, void(bool));
+  MOCK_METHOD0(OnPassphraseAccepted, void());
+  MOCK_METHOD1(OnEncryptionComplete, void(const syncable::ModelTypeSet&));
+};
+
+}  // namespace
+
+class SyncBackendHostTest : public testing::Test {
+ protected:
+  SyncBackendHostTest()
+      : ui_thread_(BrowserThread::UI, &ui_loop_) {}
+
+ private:
+  MessageLoop ui_loop_;
+  BrowserThread ui_thread_;
+};
+
+TEST_F(SyncBackendHostTest, InitShutdown) {
+  TestingProfile profile;
+  profile.CreateRequestContext();
+
+  SyncBackendHost backend(&profile);
+
+  // TODO(akalin): Handle this in SyncBackendHost instead of in
+  // ProfileSyncService.

[tim (not reviewing), 2011/04/05 04:56:01]

what you lose is the convenience / peace of mind of having all sync prefs in one
place;  looking at the browser_main initial register prefs calls, that seemed to
be the pattern (at least at the time).

[akalin, 2011/04/05 09:55:45]

I'm more concerned with there being a magic incantation needed before creating a
SBH.  Maybe some sort of RegisterSyncPrefs() function?  Anyway, modified TODO.

+  PrefService* pref_service = profile.GetPrefs();
+  pref_service->RegisterStringPref(prefs::kEncryptionBootstrapToken, "");
+
+  MockSyncFrontend mock_frontend;
+  sync_api::SyncCredentials credentials;
+  credentials.email = "user@example.com";
+  credentials.sync_token = "sync_token";
+  backend.Initialize(&mock_frontend,
+                     GURL("http://www.example.com"),
+                     syncable::ModelTypeSet(),
+                     profile.GetRequestContext(),
+                     credentials,
+                     true);
+  backend.Shutdown(false);
+}
+
+TEST_F(SyncBackendHostTest, GetPendingConfigModeState) {
+  // Empty.
+  {
+    DataTypeController::TypeMap data_type_controllers;
+    syncable::ModelTypeSet types;
+    ModelSafeRoutingInfo routing_info;
+
+    scoped_ptr<SyncBackendHost::PendingConfigureDataTypesState>
+        state(SyncBackendHost::GetPendingConfigModeState(
+            data_type_controllers, types, NULL, &routing_info));
+    EXPECT_TRUE(routing_info.empty());
+    EXPECT_FALSE(state->ready_task.get());
+    EXPECT_EQ(types, state->initial_types);
+    EXPECT_FALSE(state->deleted_type);
+    EXPECT_TRUE(state->added_types.none());
+  }
+
+  // No enabled types.
+  {
+    DataTypeController::TypeMap data_type_controllers;
+    data_type_controllers[syncable::BOOKMARKS] = NULL;
+    syncable::ModelTypeSet types;
+    ModelSafeRoutingInfo routing_info;
+
+    scoped_ptr<SyncBackendHost::PendingConfigureDataTypesState>
+        state(SyncBackendHost::GetPendingConfigModeState(
+            data_type_controllers, types, NULL, &routing_info));
+    EXPECT_TRUE(routing_info.empty());
+    EXPECT_FALSE(state->ready_task.get());
+    EXPECT_EQ(types, state->initial_types);
+    EXPECT_FALSE(state->deleted_type);
+    EXPECT_TRUE(state->added_types.none());
+  }
+
+  // Add type.
+  {
+    DataTypeController::TypeMap data_type_controllers;
+    data_type_controllers[syncable::BOOKMARKS] = NULL;
+    syncable::ModelTypeSet types;
+    types.insert(syncable::BOOKMARKS);
+    ModelSafeRoutingInfo routing_info;
+
+    scoped_ptr<SyncBackendHost::PendingConfigureDataTypesState>
+        state(SyncBackendHost::GetPendingConfigModeState(
+            data_type_controllers, types, NULL, &routing_info));
+
+    ModelSafeRoutingInfo expected_routing_info;
+    expected_routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
+    EXPECT_EQ(expected_routing_info, routing_info);
+    EXPECT_FALSE(state->ready_task.get());
+    EXPECT_EQ(types, state->initial_types);
+    EXPECT_FALSE(state->deleted_type);
+
+    syncable::ModelTypeBitSet expected_added_types;
+    expected_added_types.set(syncable::BOOKMARKS);
+    EXPECT_EQ(expected_added_types, state->added_types);
+  }
+
+  // Add existing type.
+  {
+    DataTypeController::TypeMap data_type_controllers;
+    data_type_controllers[syncable::BOOKMARKS] = NULL;
+    syncable::ModelTypeSet types;
+    types.insert(syncable::BOOKMARKS);
+    ModelSafeRoutingInfo routing_info;
+    routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
+    ModelSafeRoutingInfo expected_routing_info = routing_info;
+
+    scoped_ptr<SyncBackendHost::PendingConfigureDataTypesState>
+        state(SyncBackendHost::GetPendingConfigModeState(
+            data_type_controllers, types, NULL, &routing_info));
+
+    EXPECT_EQ(expected_routing_info, routing_info);
+    EXPECT_FALSE(state->ready_task.get());
+    EXPECT_EQ(types, state->initial_types);
+    EXPECT_FALSE(state->deleted_type);
+    EXPECT_TRUE(state->added_types.none());
+  }
+
+  // Delete type.
+  {
+    DataTypeController::TypeMap data_type_controllers;
+    data_type_controllers[syncable::BOOKMARKS] = NULL;
+    syncable::ModelTypeSet types;
+    ModelSafeRoutingInfo routing_info;
+    routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
+
+    scoped_ptr<SyncBackendHost::PendingConfigureDataTypesState>
+        state(SyncBackendHost::GetPendingConfigModeState(
+            data_type_controllers, types, NULL, &routing_info));
+
+    ModelSafeRoutingInfo expected_routing_info;
+    EXPECT_EQ(expected_routing_info, routing_info);
+    EXPECT_FALSE(state->ready_task.get());
+    EXPECT_EQ(types, state->initial_types);
+    EXPECT_TRUE(state->deleted_type);
+    EXPECT_TRUE(state->added_types.none());
+  }
+}
+
+// TODO(akalin): Write more SyncBackendHost unit tests.
+
+}  // namespace browser_sync