[Sync tests] Avoid usages of verifier profile in preferences tests

There were only few minor expectations relying on verifier profile in
preferences tests. This CL removes them together with verifier profile
notion in preferences_helper.

Bug: 1137746
Change-Id: I7d74ad99feaf1dfa7c4a0eb856fcf94263a681bc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2467521
Auto-Submit: Maksim Moskvitin <mmoskvitin@google.com>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#816524}

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

@@ -149,7 +149,6 @@ class MigrationTest : public SyncTest  {
         // boolean pref clobbers the local value), so it doesn't work
         // for anything but single-client tests.
         ASSERT_EQ(1, num_clients());
-        ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton));
         ChangeBooleanPref(0, prefs::kShowHomeButton);
         break;
       case MODIFY_BOOKMARK:

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

@@ -27,10 +27,6 @@ PrefService* GetPrefs(int index) {
   return test()->GetProfile(index)->GetPrefs();
 }
 
-PrefService* GetVerifierPrefs() {
-  return test()->verifier()->GetPrefs();
-}
-
 user_prefs::PrefRegistrySyncable* GetRegistry(Profile* profile) {
   // TODO(tschumann): Not sure what's the cleanest way to avoid this deprecated
   // call is. Ideally we could use a servicification integration test.
@@ -43,67 +39,29 @@ user_prefs::PrefRegistrySyncable* GetRegistry(Profile* profile) {
 void ChangeBooleanPref(int index, const char* pref_name) {
   bool new_value = !GetPrefs(index)->GetBoolean(pref_name);
   GetPrefs(index)->SetBoolean(pref_name, new_value);
-  if (test()->use_verifier())
-    GetVerifierPrefs()->SetBoolean(pref_name, new_value);
 }
 
 void ChangeIntegerPref(int index, const char* pref_name, int new_value) {
   GetPrefs(index)->SetInteger(pref_name, new_value);
-  if (test()->use_verifier())
-    GetVerifierPrefs()->SetInteger(pref_name, new_value);
-}
-
-void ChangeInt64Pref(int index, const char* pref_name, int64_t new_value) {
-  GetPrefs(index)->SetInt64(pref_name, new_value);
-  if (test()->use_verifier())
-    GetVerifierPrefs()->SetInt64(pref_name, new_value);
-}
-
-void ChangeDoublePref(int index, const char* pref_name, double new_value) {
-  GetPrefs(index)->SetDouble(pref_name, new_value);
-  if (test()->use_verifier())
-    GetVerifierPrefs()->SetDouble(pref_name, new_value);
 }
 
 void ChangeStringPref(int index,
                       const char* pref_name,
                       const std::string& new_value) {
   GetPrefs(index)->SetString(pref_name, new_value);
-  if (test()->use_verifier())
-    GetVerifierPrefs()->SetString(pref_name, new_value);
 }
 
 void ClearPref(int index, const char* pref_name) {
   GetPrefs(index)->ClearPref(pref_name);
-  if (test()->use_verifier())
-    GetVerifierPrefs()->ClearPref(pref_name);
-}
-
-void ChangeFilePathPref(int index,
-                        const char* pref_name,
-                        const base::FilePath& new_value) {
-  GetPrefs(index)->SetFilePath(pref_name, new_value);
-  if (test()->use_verifier())
-    GetVerifierPrefs()->SetFilePath(pref_name, new_value);
 }
 
 void ChangeListPref(int index,
                     const char* pref_name,
                     const base::ListValue& new_value) {
-  {
-    ListPrefUpdate update(GetPrefs(index), pref_name);
-    base::ListValue* list = update.Get();
-    for (auto it = new_value.begin(); it != new_value.end(); ++it) {
-      list->Append(it->CreateDeepCopy());
-    }
-  }
-
-  if (test()->use_verifier()) {
-    ListPrefUpdate update_verifier(GetVerifierPrefs(), pref_name);
-    base::ListValue* list_verifier = update_verifier.Get();
-    for (auto it = new_value.begin(); it != new_value.end(); ++it) {
-      list_verifier->Append(it->CreateDeepCopy());
-    }
+  ListPrefUpdate update(GetPrefs(index), pref_name);
+  base::ListValue* list = update.Get();
+  for (const auto& it : new_value) {
+    list->Append(it.CreateDeepCopy());
   }
 }
 
@@ -123,13 +81,8 @@ scoped_refptr<PrefStore> BuildPrefStoreFromPrefsFile(Profile* profile) {
 }
 
 bool BooleanPrefMatches(const char* pref_name) {
-  bool reference_value;
-  if (test()->use_verifier()) {
-    reference_value = GetVerifierPrefs()->GetBoolean(pref_name);
-  } else {
-    reference_value = GetPrefs(0)->GetBoolean(pref_name);
-  }
-  for (int i = 0; i < test()->num_clients(); ++i) {
+  bool reference_value = GetPrefs(0)->GetBoolean(pref_name);
+  for (int i = 1; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetBoolean(pref_name)) {
       DVLOG(1) << "Boolean preference " << pref_name << " mismatched in"
                << " profile " << i << ".";
@@ -140,13 +93,8 @@ bool BooleanPrefMatches(const char* pref_name) {
 }
 
 bool IntegerPrefMatches(const char* pref_name) {
-  int reference_value;
-  if (test()->use_verifier()) {
-    reference_value = GetVerifierPrefs()->GetInteger(pref_name);
-  } else {
-    reference_value = GetPrefs(0)->GetInteger(pref_name);
-  }
-  for (int i = 0; i < test()->num_clients(); ++i) {
+  int reference_value = GetPrefs(0)->GetInteger(pref_name);
+  for (int i = 1; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetInteger(pref_name)) {
       DVLOG(1) << "Integer preference " << pref_name << " mismatched in"
                << " profile " << i << ".";
@@ -156,48 +104,9 @@ bool IntegerPrefMatches(const char* pref_name) {
   return true;
 }
 
-bool Int64PrefMatches(const char* pref_name) {
-  int64_t reference_value;
-  if (test()->use_verifier()) {
-    reference_value = GetVerifierPrefs()->GetInt64(pref_name);
-  } else {
-    reference_value = GetPrefs(0)->GetInt64(pref_name);
-  }
-  for (int i = 0; i < test()->num_clients(); ++i) {
-    if (reference_value != GetPrefs(i)->GetInt64(pref_name)) {
-      DVLOG(1) << "Integer preference " << pref_name << " mismatched in"
-               << " profile " << i << ".";
-      return false;
-    }
-  }
-  return true;
-}
-
-bool DoublePrefMatches(const char* pref_name) {
-  double reference_value;
-  if (test()->use_verifier()) {
-    reference_value = GetVerifierPrefs()->GetDouble(pref_name);
-  } else {
-    reference_value = GetPrefs(0)->GetDouble(pref_name);
-  }
-  for (int i = 0; i < test()->num_clients(); ++i) {
-    if (reference_value != GetPrefs(i)->GetDouble(pref_name)) {
-      DVLOG(1) << "Double preference " << pref_name << " mismatched in"
-               << " profile " << i << ".";
-      return false;
-    }
-  }
-  return true;
-}
-
 bool StringPrefMatches(const char* pref_name) {
-  std::string reference_value;
-  if (test()->use_verifier()) {
-    reference_value = GetVerifierPrefs()->GetString(pref_name);
-  } else {
-    reference_value = GetPrefs(0)->GetString(pref_name);
-  }
-  for (int i = 0; i < test()->num_clients(); ++i) {
+  std::string reference_value = GetPrefs(0)->GetString(pref_name);
+  for (int i = 1; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetString(pref_name)) {
       DVLOG(1) << "String preference " << pref_name << " mismatched in"
                << " profile " << i << ".";
@@ -208,12 +117,6 @@ bool StringPrefMatches(const char* pref_name) {
 }
 
 bool ClearedPrefMatches(const char* pref_name) {
-  if (test()->use_verifier()) {
-    if (GetVerifierPrefs()->GetUserPrefValue(pref_name)) {
-      return false;
-    }
-  }
-
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (GetPrefs(i)->GetUserPrefValue(pref_name)) {
       DVLOG(1) << "Preference " << pref_name << " isn't cleared in"
@@ -224,31 +127,9 @@ bool ClearedPrefMatches(const char* pref_name) {
   return true;
 }
 
-bool FilePathPrefMatches(const char* pref_name) {
-  base::FilePath reference_value;
-  if (test()->use_verifier()) {
-    reference_value = GetVerifierPrefs()->GetFilePath(pref_name);
-  } else {
-    reference_value = GetPrefs(0)->GetFilePath(pref_name);
-  }
-  for (int i = 0; i < test()->num_clients(); ++i) {
-    if (reference_value != GetPrefs(i)->GetFilePath(pref_name)) {
-      DVLOG(1) << "base::FilePath preference " << pref_name << " mismatched in"
-               << " profile " << i << ".";
-      return false;
-    }
-  }
-  return true;
-}
-
 bool ListPrefMatches(const char* pref_name) {
-  const base::ListValue* reference_value;
-  if (test()->use_verifier()) {
-    reference_value = GetVerifierPrefs()->GetList(pref_name);
-  } else {
-    reference_value = GetPrefs(0)->GetList(pref_name);
-  }
-  for (int i = 0; i < test()->num_clients(); ++i) {
+  const base::ListValue* reference_value = GetPrefs(0)->GetList(pref_name);
+  for (int i = 1; i < test()->num_clients(); ++i) {
     if (!reference_value->Equals(GetPrefs(i)->GetList(pref_name))) {
       DVLOG(1) << "List preference " << pref_name << " mismatched in"
                << " profile " << i << ".";
@@ -261,15 +142,12 @@ bool ListPrefMatches(const char* pref_name) {
 }  // namespace preferences_helper
 
 PrefMatchChecker::PrefMatchChecker(const char* path) : path_(path) {
-  if (test()->use_verifier()) {
-    RegisterPrefListener(preferences_helper::GetVerifierPrefs());
-  }
   for (int i = 0; i < test()->num_clients(); ++i) {
     RegisterPrefListener(preferences_helper::GetPrefs(i));
   }
 }
 
-PrefMatchChecker::~PrefMatchChecker() {}
+PrefMatchChecker::~PrefMatchChecker() = default;
 
 const char* PrefMatchChecker::GetPath() const {
   return path_;

chrome/browser/sync/test/integration/preferences_helper.h

@@ -30,54 +30,29 @@ namespace preferences_helper {
 // Used to access the preferences within a particular sync profile.
 PrefService* GetPrefs(int index);
 
-// Used to access the preferences within the verifier sync profile.
-PrefService* GetVerifierPrefs();
-
 // Provides access to the syncable pref registy of a profile.
 user_prefs::PrefRegistrySyncable* GetRegistry(Profile* profile);
 
 // Inverts the value of the boolean preference with name |pref_name| in the
-// profile with index |index|. Also inverts its value in |verifier| if
-// DisableVerifier() hasn't been called.
+// profile with index |index|.
 void ChangeBooleanPref(int index, const char* pref_name);
 
 // Changes the value of the integer preference with name |pref_name| in the
-// profile with index |index| to |new_value|. Also changes its value in
-// |verifier| if DisableVerifier() hasn't been called.
+// profile with index |index| to |new_value|.
 void ChangeIntegerPref(int index, const char* pref_name, int new_value);
 
-// Changes the value of the int64_t preference with name |pref_name| in the
-// profile with index |index| to |new_value|. Also changes its value in
-// |verifier| if DisableVerifier() hasn't been called.
-void ChangeInt64Pref(int index, const char* pref_name, int64_t new_value);
-
-// Changes the value of the double preference with name |pref_name| in the
-// profile with index |index| to |new_value|. Also changes its value in
-// |verifier| if DisableVerifier() hasn't been called.
-void ChangeDoublePref(int index, const char* pref_name, double new_value);
-
 // Changes the value of the string preference with name |pref_name| in the
-// profile with index |index| to |new_value|. Also changes its value in
-// |verifier| if DisableVerifier() hasn't been called.
+// profile with index |index| to |new_value|.
 void ChangeStringPref(int index,
                       const char* pref_name,
                       const std::string& new_value);
 
 // Clears the value of the preference with name |pref_name| in the profile with
-// index |index|. Also changes its value in |verifier| if DisableVerifier()
-// hasn't been called.
+// index |index|.
 void ClearPref(int index, const char* pref_name);
 
-// Changes the value of the file path preference with name |pref_name| in the
-// profile with index |index| to |new_value|. Also changes its value in
-// |verifier| if DisableVerifier() hasn't been called.
-void ChangeFilePathPref(int index,
-                        const char* pref_name,
-                        const base::FilePath& new_value);
-
 // Changes the value of the list preference with name |pref_name| in the
-// profile with index |index| to |new_value|. Also changes its value in
-// |verifier| if DisableVerifier() hasn't been called.
+// profile with index |index| to |new_value|.
 void ChangeListPref(int index,
                     const char* pref_name,
                     const base::ListValue& new_value);
@@ -87,38 +62,19 @@ void ChangeListPref(int index,
 scoped_refptr<PrefStore> BuildPrefStoreFromPrefsFile(Profile* profile);
 
 // Used to verify that the boolean preference with name |pref_name| has the
-// same value across all profiles. Also checks |verifier| if DisableVerifier()
-// hasn't been called.
+// same value across all profiles.
 bool BooleanPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
 
 // Used to verify that the integer preference with name |pref_name| has the
-// same value across all profiles. Also checks |verifier| if DisableVerifier()
-// hasn't been called.
+// same value across all profiles.
 bool IntegerPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
 
-// Used to verify that the int64_t preference with name |pref_name| has the
-// same value across all profiles. Also checks |verifier| if DisableVerifier()
-// hasn't been called.
-bool Int64PrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
-
-// Used to verify that the double preference with name |pref_name| has the
-// same value across all profiles. Also checks |verifier| if DisableVerifier()
-// hasn't been called.
-bool DoublePrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
-
 // Used to verify that the string preference with name |pref_name| has the
-// same value across all profiles. Also checks |verifier| if DisableVerifier()
-// hasn't been called.
+// same value across all profiles.
 bool StringPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
 
-// Used to verify that the file path preference with name |pref_name| has the
-// same value across all profiles. Also checks |verifier| if DisableVerifier()
-// hasn't been called.
-bool FilePathPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
-
 // Used to verify that the list preference with name |pref_name| has the
-// same value across all profiles. Also checks |verifier| if DisableVerifier()
-// hasn't been called.
+// same value across all profiles.
 bool ListPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
 
 }  // namespace preferences_helper

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

@@ -8,17 +8,20 @@
 #include "chrome/browser/sync/test/integration/preferences_helper.h"
 #include "chrome/browser/sync/test/integration/updated_progress_marker_checker.h"
 #include "chromeos/constants/chromeos_features.h"
+#include "components/prefs/pref_service.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/driver/sync_service.h"
 #include "components/sync/driver/sync_user_settings.h"
 #include "content/public/test/browser_test.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using preferences_helper::ChangeStringPref;
-using preferences_helper::StringPrefMatches;
-
 namespace {
 
+using preferences_helper::ChangeStringPref;
+using preferences_helper::GetPrefs;
+using testing::Eq;
+
 class SingleClientOsPreferencesSyncTest : public OsSyncTest {
  public:
   SingleClientOsPreferencesSyncTest() : OsSyncTest(SINGLE_CLIENT) {}
@@ -30,11 +33,11 @@ IN_PROC_BROWSER_TEST_F(SingleClientOsPreferencesSyncTest, Sanity) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
 
   // Shelf alignment is a Chrome OS only preference.
-  ASSERT_TRUE(StringPrefMatches(ash::prefs::kShelfAlignment));
   ChangeStringPref(/*profile_index=*/0, ash::prefs::kShelfAlignment,
                    ash::kShelfAlignmentRight);
-  ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
-  EXPECT_TRUE(StringPrefMatches(ash::prefs::kShelfAlignment));
+  EXPECT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
+  EXPECT_THAT(GetPrefs(/*index=*/0)->GetString(ash::prefs::kShelfAlignment),
+              Eq(ash::kShelfAlignmentRight));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientOsPreferencesSyncTest,

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

@@ -21,21 +21,21 @@
 #include "content/public/test/browser_test.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
-using preferences_helper::BooleanPrefMatches;
+namespace {
+
 using preferences_helper::BuildPrefStoreFromPrefsFile;
 using preferences_helper::ChangeBooleanPref;
+using preferences_helper::GetPrefs;
 using preferences_helper::GetRegistry;
-using user_prefs::PrefRegistrySyncable;
 using testing::Eq;
+using testing::Ne;
 using testing::NotNull;
-
-namespace {
+using user_prefs::PrefRegistrySyncable;
 
 class SingleClientPreferencesSyncTest : public SyncTest {
  public:
   SingleClientPreferencesSyncTest() : SyncTest(SINGLE_CLIENT) {}
-
-  ~SingleClientPreferencesSyncTest() override {}
+  ~SingleClientPreferencesSyncTest() override = default;
 
   // If non-empty, |contents| will be written to the Preferences file of the
   // profile at |index| before that Profile object is created.
@@ -69,10 +69,13 @@ class SingleClientPreferencesSyncTest : public SyncTest {
 
 IN_PROC_BROWSER_TEST_F(SingleClientPreferencesSyncTest, Sanity) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+
+  const bool kDefaultValue =
+      GetPrefs(/*index=*/0)->GetBoolean(prefs::kHomePageIsNewTabPage);
   ChangeBooleanPref(0, prefs::kHomePageIsNewTabPage);
-  ASSERT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
-  EXPECT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+  EXPECT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
+  EXPECT_THAT(GetPrefs(/*index=*/0)->GetBoolean(prefs::kHomePageIsNewTabPage),
+              Ne(kDefaultValue));
 }
 
 IN_PROC_BROWSER_TEST_F(SingleClientPreferencesSyncTest,