[Smart Lock, Prefs reconciliation] Prefs migration logic for desktop platforms.

chrome/browser/sync/test/integration/single_client_password_manager_setting_migrater_service_sync_test.cc

Index: chrome/browser/sync/test/integration/single_client_password_manager_setting_migrater_service_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/single_client_password_manager_setting_migrater_service_sync_test.cc b/chrome/browser/sync/test/integration/single_client_password_manager_setting_migrater_service_sync_test.cc
new file mode 100644
index 0000000000000000000000000000000000000000..94dcd688cc34cbb7c6be3e4595010800ffcc7931
--- /dev/null
+++ b/chrome/browser/sync/test/integration/single_client_password_manager_setting_migrater_service_sync_test.cc
@@ -0,0 +1,161 @@
+// Copyright (c) 2015 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 "base/guid.h"
+#include "base/json/json_string_value_serializer.h"
+#include "base/prefs/pref_service.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/password_manager/password_manager_setting_migrater_service.h"
+#include "chrome/browser/prefs/pref_service_syncable.h"
+#include "chrome/browser/sync/test/integration/preferences_helper.h"
+#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
+#include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
+#include "chrome/browser/sync/test/integration/sync_test.h"
+#include "chrome/common/pref_names.h"
+#include "components/password_manager/core/common/password_manager_pref_names.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "sync/internal_api/public/base/model_type.h"
+#include "sync/protocol/preference_specifics.pb.h"
+#include "sync/protocol/priority_preference_specifics.pb.h"
+#include "sync/protocol/sync.pb.h"
+#include "sync/test/fake_server/unique_client_entity.h"
+
+using preferences_helper::AwaitBooleanPrefMatches;
+using preferences_helper::BooleanPrefMatches;
+using preferences_helper::ChangeBooleanPref;
+using preferences_helper::GetPrefs;
+
+namespace {
+
+void InjectDataToFakeServer(fake_server::FakeServer* fake_server,
+                            const std::string& name,
+                            bool value) {
+  std::string serialized;
+  JSONStringValueSerializer json(&serialized);
+  base::FundamentalValue bool_value(value);
+  json.Serialize(bool_value);
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::PreferenceSpecifics* pref = nullptr;
+  syncer::ModelType type;
+  if (name == password_manager::prefs::kPasswordManagerSavingEnabled) {
+    pref = specifics.mutable_preference();
+    type = syncer::PREFERENCES;
+  } else if (name == password_manager::prefs::kCredentialsEnableService) {
+    pref = specifics.mutable_priority_preference()->mutable_preference();
+    type = syncer::PRIORITY_PREFERENCES;
+  } else {
+    NOTREACHED() << "Wrong preference name: " << name;
+  }
+  pref->set_name(name);
+  pref->set_value(serialized);
+  const std::string id = "settings_reconciliation";
+  fake_server->InjectEntity(
+      fake_server::UniqueClientEntity::CreateForInjection(id, specifics));
+}
+
+}  // namespace
+
+class SingleClientPasswordManagerSettingMigraterServiceSyncTest
+    : public SyncTest {
+ public:
+  SingleClientPasswordManagerSettingMigraterServiceSyncTest()
+      : SyncTest(MULTIPLE_CLIENT) {}
+  ~SingleClientPasswordManagerSettingMigraterServiceSyncTest() override {}
+
+  void NotifyProfileAdded(int index) {
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_PROFILE_ADDED,
+        content::Source<Profile>(GetProfile(index)),
+        content::NotificationService::NoDetails());
+  }
+
+  bool TestUsesSelfNotifications() override { return false; }
+
+  void TestReconcileOnMergeDataAndStartSyncing(bool new_pref_local_value,
+                                               bool old_pref_local_value,
+                                               bool new_pref_sync_value,
+                                               bool old_pref_sync_value,
+                                               bool result_value) {
+    using namespace password_manager::prefs;
+    ASSERT_TRUE(SetupClients());
+    DisableVerifier();
+    GetPrefs(0)->SetBoolean(kCredentialsEnableService, new_pref_local_value);
+    GetPrefs(0)
+        ->SetBoolean(kPasswordManagerSavingEnabled, old_pref_local_value);
+
+    ASSERT_EQ(new_pref_local_value,
+              GetPrefs(0)->GetBoolean(kCredentialsEnableService));
+    ASSERT_EQ(old_pref_local_value,
+              GetPrefs(0)->GetBoolean(kPasswordManagerSavingEnabled));
+
+    NotifyProfileAdded(0);
+
+    InjectDataToFakeServer(GetFakeServer(), kCredentialsEnableService,
+                           new_pref_sync_value);
+    InjectDataToFakeServer(GetFakeServer(), kPasswordManagerSavingEnabled,
+                           old_pref_sync_value);
+
+    ASSERT_TRUE(SetupSync());
+    ASSERT_TRUE(sync_integration_test_util::AwaitCommitActivityCompletion(
+        GetSyncService((0))));
+    EXPECT_EQ(result_value, GetPrefs(0)->GetBoolean(kCredentialsEnableService));
+    EXPECT_EQ(result_value,
+              GetPrefs(0)->GetBoolean(kPasswordManagerSavingEnabled));
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(
+      SingleClientPasswordManagerSettingMigraterServiceSyncTest);
+};
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOnOnSyncOffOff) {
+  TestReconcileOnMergeDataAndStartSyncing(true, true, false, false, false);

[pval...(no longer on Chromium), 2015/09/08 22:21:18]

Calls like this are pretty confusing to read in the test cases. I understand
that this reduces code duplication, but I would recommend splitting up the
TestReconcileOnMergeDataAndStartSyncing method so it's more clear what each test
case is doing. I would also recommend adding a comment whenever passing boolean
params to indicate what they mean (you could also create constants for better
readability).

For example the test case could look like this:
AssertPrefValues(true /* kCredentialsEnableService */, true /*
kPasswordManagerSavingEnabled */);
InjectNewValues(false /* ... */, false /* ... */);
ASSERT_TRUE(SetupSync());
ASSERT_TRUE(sync_integration_test_util::AwaitCommitActivityCompletion(GetSyncService((0))));
AssertPrefValues(false /* kCredentialsEnableService */, false /*
kPasswordManagerSavingEnabled */);

+}
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOnOnSyncOnOff) {
+  TestReconcileOnMergeDataAndStartSyncing(true, true, true, false, false);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOnOnSyncOffOn) {
+  TestReconcileOnMergeDataAndStartSyncing(true, true, false, true, false);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOnOffSyncOnOn) {
+  TestReconcileOnMergeDataAndStartSyncing(true, false, true, true, true);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOnOffSyncOnOff) {
+  TestReconcileOnMergeDataAndStartSyncing(true, false, true, false, false);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOnOffSyncOffOn) {
+  TestReconcileOnMergeDataAndStartSyncing(true, false, false, true, false);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOffOffSyncOffOn) {
+  TestReconcileOnMergeDataAndStartSyncing(false, false, false, true, true);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    SingleClientPasswordManagerSettingMigraterServiceSyncTest,
+    LocalOffOffSyncOnOff) {
+  TestReconcileOnMergeDataAndStartSyncing(false, false, true, false, true);
+}