sync: Refactor TwoClientPreferencesSyncTest

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

Index: chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc b/chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc
index a16191c80a46e0d96072085a57fe464ffe4a7d44..da9ee8b565783efba316d0f400ebddbe33f284b1 100644
--- a/chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc
+++ b/chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc
@@ -2,59 +2,45 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/values.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/browser/translate/translate_tab_helper.h"
 #include "chrome/common/pref_names.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/sync_driver/pref_names.h"
-#include "components/translate/core/browser/translate_prefs.h"
 #include "components/translate/core/common/translate_pref_names.h"
 
 using preferences_helper::AppendStringPref;
+using preferences_helper::AwaitBooleanPrefMatches;
+using preferences_helper::AwaitIntegerPrefMatches;
+using preferences_helper::AwaitListPrefMatches;
+using preferences_helper::AwaitStringPrefMatches;
 using preferences_helper::BooleanPrefMatches;
 using preferences_helper::ChangeBooleanPref;
 using preferences_helper::ChangeIntegerPref;
-using preferences_helper::ChangeInt64Pref;
 using preferences_helper::ChangeListPref;
 using preferences_helper::ChangeStringPref;
-using preferences_helper::GetPrefs;
-using preferences_helper::IntegerPrefMatches;
-using preferences_helper::Int64PrefMatches;
-using preferences_helper::ListPrefMatches;
-using preferences_helper::StringPrefMatches;
-using sync_integration_test_util::AwaitCommitActivityCompletion;
 
 class TwoClientPreferencesSyncTest : public SyncTest {
  public:
   TwoClientPreferencesSyncTest() : SyncTest(TWO_CLIENT) {}
   virtual ~TwoClientPreferencesSyncTest() {}
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(TwoClientPreferencesSyncTest);
-};
-
-class LegacyTwoClientPreferencesSyncTest : public SyncTest {
- public:
-  LegacyTwoClientPreferencesSyncTest() : SyncTest(TWO_CLIENT_LEGACY) {}
-  virtual ~LegacyTwoClientPreferencesSyncTest() {}
+  virtual bool TestUsesSelfNotifications() OVERRIDE { return false; }
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(LegacyTwoClientPreferencesSyncTest);
+  DISALLOW_COPY_AND_ASSIGN(TwoClientPreferencesSyncTest);
 };
 
 // TCM ID - 7306186.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kHomePageIsNewTabPage) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kHomePageIsNewTabPage));
 
   ChangeBooleanPref(0, prefs::kHomePageIsNewTabPage);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kHomePageIsNewTabPage));
 }
 
 // TCM ID - 7260488.
@@ -62,25 +48,23 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, Race) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   DisableVerifier();
 
-  ASSERT_TRUE(StringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 
   ChangeStringPref(0, prefs::kHomePage, "http://www.google.com/0");
   ChangeStringPref(1, prefs::kHomePage,"http://www.google.com/1");
-  ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(StringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 }
 
 // TCM ID - 3649278.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kPasswordManagerEnabled) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(
+      password_manager::prefs::kPasswordManagerEnabled));
 
   ChangeBooleanPref(0, password_manager::prefs::kPasswordManagerEnabled);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(
+      password_manager::prefs::kPasswordManagerEnabled));
 }
 
 // TCM ID - 3699293.
@@ -90,66 +74,21 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
   DisableVerifier();
 
   ASSERT_TRUE(
-      BooleanPrefMatches(sync_driver::prefs::kSyncKeepEverythingSynced));
-  ASSERT_TRUE(BooleanPrefMatches(sync_driver::prefs::kSyncThemes));
+      AwaitBooleanPrefMatches(sync_driver::prefs::kSyncKeepEverythingSynced));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(sync_driver::prefs::kSyncThemes));
 
   GetClient(0)->DisableSyncForDatatype(syncer::THEMES);
   ASSERT_FALSE(
       BooleanPrefMatches(sync_driver::prefs::kSyncKeepEverythingSynced));
 }
 
-// TCM ID - 3661290.

[rlarocque, 2014/06/04 01:14:44]

I believe these two fall under the category of disable tests, which we decided
weren't worth testing on a type-by-type basis.

[pval...(no longer on Chromium), 2014/06/04 18:06:31]

roger

-IN_PROC_BROWSER_TEST_F(LegacyTwoClientPreferencesSyncTest, DisablePreferences) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  DisableVerifier();
-
-  ASSERT_TRUE(BooleanPrefMatches(sync_driver::prefs::kSyncPreferences));
-  ASSERT_TRUE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
-
-  GetClient(1)->DisableSyncForDatatype(syncer::PREFERENCES);
-  ChangeBooleanPref(0, password_manager::prefs::kPasswordManagerEnabled);
-  ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
-  ASSERT_FALSE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
-
-  GetClient(1)->EnableSyncForDatatype(syncer::PREFERENCES);
-  ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
-}
-
-// TCM ID - 3664292.
-IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, DisableSync) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  DisableVerifier();
-
-  ASSERT_TRUE(BooleanPrefMatches(sync_driver::prefs::kSyncPreferences));
-  ASSERT_TRUE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton));
-
-  GetClient(1)->DisableSyncForAllDatatypes();
-  ChangeBooleanPref(0, password_manager::prefs::kPasswordManagerEnabled);
-  ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
-  ASSERT_FALSE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
-
-  ChangeBooleanPref(1, prefs::kShowHomeButton);
-  ASSERT_FALSE(BooleanPrefMatches(prefs::kShowHomeButton));
-
-  GetClient(1)->EnableSyncForAllDatatypes();
-  ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(
-      BooleanPrefMatches(password_manager::prefs::kPasswordManagerEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton));
-}
-
 // TCM ID - 3604297.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, SignInDialog) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   DisableVerifier();
 
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
+
   ASSERT_TRUE(BooleanPrefMatches(sync_driver::prefs::kSyncPreferences));
   ASSERT_TRUE(BooleanPrefMatches(sync_driver::prefs::kSyncBookmarks));
   ASSERT_TRUE(BooleanPrefMatches(sync_driver::prefs::kSyncThemes));
@@ -169,7 +108,12 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, SignInDialog) {
   GetClient(0)->DisableSyncForDatatype(syncer::THEMES);
   GetClient(1)->EnableSyncForDatatype(syncer::THEMES);
 
-  ASSERT_TRUE(AwaitQuiescence());
+  // This pref is syncable and prefs sync is enabled.
+  ChangeStringPref(0, prefs::kHomePage, "http://news.google.com");
+
+  // Wait until the syncable pref is synced, then expect the rest are
+  // still out of sync.
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 
   ASSERT_FALSE(BooleanPrefMatches(sync_driver::prefs::kSyncPreferences));
   ASSERT_FALSE(BooleanPrefMatches(sync_driver::prefs::kSyncBookmarks));
@@ -183,94 +127,93 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, SignInDialog) {
 // TCM ID - 3666296.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kShowBookmarkBar) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowBookmarkBar));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kShowBookmarkBar));
 
   ChangeBooleanPref(0, prefs::kShowBookmarkBar);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowBookmarkBar));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kShowBookmarkBar));
 }
 
 // TCM ID - 3611311.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kCheckDefaultBrowser) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   DisableVerifier();
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kCheckDefaultBrowser));
 
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kCheckDefaultBrowser));
-
+  // This pref is not syncable.
   ChangeBooleanPref(0, prefs::kCheckDefaultBrowser);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+
+  // This pref is syncable.

[pval...(no longer on Chromium), 2014/06/04 18:06:31]

Why have a syncable pref here at all? What about just verifying that no sync
cycle was initiated?

This question applies to other non-syncable pref tests here as well.

[rlarocque, 2014/06/04 18:24:40]

It's in accordance with the idea that sync cycles are internal to the sync
engine.  An integration test should not directly observe them.

Verifying that no sync cycle is initiated is hard.  We have nudge delays that
prevent things from syncing for a while after a change is made.  If we did an
EXPECT() right away, or even an EXPECT() after a short sleep, it might not
actually prove anything.

The idea here is that if a syncable pref has synced, then it's probably safe to
infer that a sync cycle has happened and that the syncer could have synced the
non-syncable pref by now if it wanted to.

This is the hack that I referred to in the comment at the top of the review
request.

[pval...(no longer on Chromium), 2014/06/04 23:00:41]

Ok, I think this is good.

+  ChangeStringPref(0, prefs::kHomePage, "http://news.google.com");
+
+  // Wait until the syncable pref is synced, then expect that the non-syncable
+  // one is still out of sync.
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
   ASSERT_FALSE(BooleanPrefMatches(prefs::kCheckDefaultBrowser));
 }
 
 // TCM ID - 3628298.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kHomePage) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(StringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 
   ChangeStringPref(0, prefs::kHomePage, "http://news.google.com");
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(StringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 }
 
 // TCM ID - 7297269.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kShowHomeButton) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kShowHomeButton));
 
   ChangeBooleanPref(0, prefs::kShowHomeButton);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kShowHomeButton));
 }
 
 // TCM ID - 3710285.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kEnableTranslate) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableTranslate));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kEnableTranslate));
 
   ChangeBooleanPref(0, prefs::kEnableTranslate);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableTranslate));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kEnableTranslate));
 }
 
 // TCM ID - 3664293.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kAutofillEnabled) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(autofill::prefs::kAutofillEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(autofill::prefs::kAutofillEnabled));
 
   ChangeBooleanPref(0, autofill::prefs::kAutofillEnabled);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(autofill::prefs::kAutofillEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(autofill::prefs::kAutofillEnabled));
 }
 
 // TCM ID - 3632259.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kURLsToRestoreOnStartup) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(IntegerPrefMatches(prefs::kRestoreOnStartup));
-  ASSERT_TRUE(ListPrefMatches(prefs::kURLsToRestoreOnStartup));
+  ASSERT_TRUE(AwaitIntegerPrefMatches(prefs::kRestoreOnStartup));
+  ASSERT_TRUE(AwaitListPrefMatches(prefs::kURLsToRestoreOnStartup));
 
   ChangeIntegerPref(0, prefs::kRestoreOnStartup, 0);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(IntegerPrefMatches(prefs::kRestoreOnStartup));
+  ASSERT_TRUE(AwaitIntegerPrefMatches(prefs::kRestoreOnStartup));
 
   base::ListValue urls;
   urls.Append(base::Value::CreateStringValue("http://www.google.com/"));
   urls.Append(base::Value::CreateStringValue("http://www.flickr.com/"));
   ChangeIntegerPref(0, prefs::kRestoreOnStartup, 4);
   ChangeListPref(0, prefs::kURLsToRestoreOnStartup, urls);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(IntegerPrefMatches(prefs::kRestoreOnStartup));
-  ASSERT_TRUE(ListPrefMatches(prefs::kURLsToRestoreOnStartup));
+  ASSERT_TRUE(AwaitIntegerPrefMatches(prefs::kRestoreOnStartup));
+  ASSERT_TRUE(AwaitListPrefMatches(prefs::kURLsToRestoreOnStartup));
 }
 
 // TCM ID - 3684287.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kRestoreOnStartup) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(IntegerPrefMatches(prefs::kRestoreOnStartup));
+  ASSERT_TRUE(AwaitIntegerPrefMatches(prefs::kRestoreOnStartup));
 
   ChangeIntegerPref(0, prefs::kRestoreOnStartup, 1);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(IntegerPrefMatches(prefs::kRestoreOnStartup));
+  ASSERT_TRUE(AwaitIntegerPrefMatches(prefs::kRestoreOnStartup));
 }
 
 // TCM ID - 3703314.
@@ -278,20 +221,19 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, Privacy) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   DisableVerifier();
 
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kAlternateErrorPagesEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kSearchSuggestEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kNetworkPredictionEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kSafeBrowsingEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kAlternateErrorPagesEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kSearchSuggestEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kNetworkPredictionEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kSafeBrowsingEnabled));
 
   ChangeBooleanPref(0, prefs::kAlternateErrorPagesEnabled);
   ChangeBooleanPref(0, prefs::kSearchSuggestEnabled);
   ChangeBooleanPref(0, prefs::kNetworkPredictionEnabled);
   ChangeBooleanPref(0, prefs::kSafeBrowsingEnabled);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kAlternateErrorPagesEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kSearchSuggestEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kNetworkPredictionEnabled));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kSafeBrowsingEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kAlternateErrorPagesEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kSearchSuggestEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kNetworkPredictionEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kSafeBrowsingEnabled));
 }
 
 // TCM ID - 3649279.
@@ -299,13 +241,13 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, ClearData) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   DisableVerifier();
 
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteBrowsingHistory));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteDownloadHistory));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteCache));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteCookies));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeletePasswords));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteFormData));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteHostedAppsData));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteBrowsingHistory));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteDownloadHistory));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteCache));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteCookies));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeletePasswords));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteFormData));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteHostedAppsData));
 
   ChangeBooleanPref(0, prefs::kDeleteBrowsingHistory);
   ChangeBooleanPref(0, prefs::kDeleteDownloadHistory);
@@ -314,68 +256,62 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, ClearData) {
   ChangeBooleanPref(0, prefs::kDeletePasswords);
   ChangeBooleanPref(0, prefs::kDeleteFormData);
   ChangeBooleanPref(0, prefs::kDeleteHostedAppsData);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteBrowsingHistory));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteDownloadHistory));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteCache));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteCookies));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeletePasswords));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteFormData));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kDeleteHostedAppsData));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteBrowsingHistory));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteDownloadHistory));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteCache));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteCookies));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeletePasswords));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteFormData));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kDeleteHostedAppsData));
 }
 
 // TCM ID - 3686300.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kWebKitUsesUniversalDetector) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kWebKitUsesUniversalDetector));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kWebKitUsesUniversalDetector));
 
   ChangeBooleanPref(0, prefs::kWebKitUsesUniversalDetector);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kWebKitUsesUniversalDetector));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kWebKitUsesUniversalDetector));
 }
 
 // TCM ID - 3673298.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kDefaultCharset) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(StringPrefMatches(prefs::kDefaultCharset));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kDefaultCharset));
 
   ChangeStringPref(0, prefs::kDefaultCharset, "Thai");
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(StringPrefMatches(prefs::kDefaultCharset));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kDefaultCharset));
 }
 
 // TCM ID - 3653296.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kBlockThirdPartyCookies) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kBlockThirdPartyCookies));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kBlockThirdPartyCookies));
 
   ChangeBooleanPref(0, prefs::kBlockThirdPartyCookies);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kBlockThirdPartyCookies));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kBlockThirdPartyCookies));
 }
 
 // TCM ID - 7297279.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kClearSiteDataOnExit) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kClearSiteDataOnExit));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kClearSiteDataOnExit));
 
   ChangeBooleanPref(0, prefs::kClearSiteDataOnExit);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kClearSiteDataOnExit));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kClearSiteDataOnExit));
 }
 
 // TCM ID - 7306184.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kSafeBrowsingEnabled) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kSafeBrowsingEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kSafeBrowsingEnabled));
 
   ChangeBooleanPref(0, prefs::kSafeBrowsingEnabled);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kSafeBrowsingEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kSafeBrowsingEnabled));
 }
 
 // TCM ID - 3624302.
@@ -384,11 +320,18 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   DisableVerifier();
 
-  ASSERT_TRUE(
-      BooleanPrefMatches(autofill::prefs::kAutofillAuxiliaryProfilesEnabled));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(
+      autofill::prefs::kAutofillAuxiliaryProfilesEnabled));
 
+  // This pref may be syncable.
   ChangeBooleanPref(0, autofill::prefs::kAutofillAuxiliaryProfilesEnabled);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+
+  // This pref is always syncable.
+  ChangeStringPref(0, prefs::kHomePage, "http://news.google.com");
+
+  // Wait for the syncable pref to propagate.
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 
   // kAutofillAuxiliaryProfilesEnabled is only synced on Mac.
 #if defined(OS_MACOSX)
@@ -403,126 +346,55 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
 // TCM ID - 3717298.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kPromptForDownload) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kPromptForDownload));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kPromptForDownload));
 
   ChangeBooleanPref(0, prefs::kPromptForDownload);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kPromptForDownload));
-}
-
-// TCM ID - 3729263.
-IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,

[rlarocque, 2014/06/04 01:14:44]

These tests would be particularly difficult to maintain going forward.  They'd
require their own set of status checkers.  They have no obvious hooks for
observers, so we'd probably have to trigger the checks based on sync cycles,
rather than observing the model directly.

If you'd like to keep them, it can be done.  I just figured I'd offer you the
choice to delete them now, rather than doing all the work to support them only
to find that everyone agrees they're redundant and should be deleted anyway.

[pval...(no longer on Chromium), 2014/06/04 18:06:31]

I'd rather delete costly/complicated tests now and revisit them in the future.
This deletion sounds find to me.

-                       kPrefTranslateLanguageBlacklist) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableTranslate));
-
-  scoped_ptr<TranslatePrefs> translate_client0_prefs(
-      TranslateTabHelper::CreateTranslatePrefs(GetPrefs(0)));
-  scoped_ptr<TranslatePrefs> translate_client1_prefs(
-      TranslateTabHelper::CreateTranslatePrefs(GetPrefs(1)));
-  ASSERT_FALSE(translate_client0_prefs->IsBlockedLanguage("fr"));
-  translate_client0_prefs->BlockLanguage("fr");
-  ASSERT_TRUE(translate_client0_prefs->IsBlockedLanguage("fr"));
-
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(translate_client1_prefs->IsBlockedLanguage("fr"));
-
-  translate_client0_prefs->UnblockLanguage("fr");
-  ASSERT_FALSE(translate_client0_prefs->IsBlockedLanguage("fr"));
-
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_FALSE(translate_client1_prefs->IsBlockedLanguage("fr"));
-}
-
-// TCM ID - 7307195.
-IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
-                       kPrefTranslateWhitelists) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableTranslate));
-
-  scoped_ptr<TranslatePrefs> translate_client0_prefs(
-      TranslateTabHelper::CreateTranslatePrefs(GetPrefs(0)));
-  scoped_ptr<TranslatePrefs> translate_client1_prefs(
-      TranslateTabHelper::CreateTranslatePrefs(GetPrefs(1)));
-  ASSERT_FALSE(translate_client0_prefs->IsLanguagePairWhitelisted("en", "bg"));
-  translate_client0_prefs->WhitelistLanguagePair("en", "bg");
-  ASSERT_TRUE(translate_client0_prefs->IsLanguagePairWhitelisted("en", "bg"));
-
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(translate_client1_prefs->IsLanguagePairWhitelisted("en", "bg"));
-
-  translate_client0_prefs->RemoveLanguagePairFromWhitelist("en", "bg");
-  ASSERT_FALSE(translate_client0_prefs->IsLanguagePairWhitelisted("en", "bg"));
-
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_FALSE(translate_client1_prefs->IsLanguagePairWhitelisted("en", "bg"));
-}
-
-// TCM ID - 3625298.
-IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
-                       kPrefTranslateSiteBlacklist) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableTranslate));
-
-  GURL url("http://www.google.com");
-  std::string host(url.host());
-  scoped_ptr<TranslatePrefs> translate_client0_prefs(
-      TranslateTabHelper::CreateTranslatePrefs(GetPrefs(0)));
-  scoped_ptr<TranslatePrefs> translate_client1_prefs(
-      TranslateTabHelper::CreateTranslatePrefs(GetPrefs(1)));
-  ASSERT_FALSE(translate_client0_prefs->IsSiteBlacklisted(host));
-  translate_client0_prefs->BlacklistSite(host);
-  ASSERT_TRUE(translate_client0_prefs->IsSiteBlacklisted(host));
-
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(translate_client1_prefs->IsSiteBlacklisted(host));
-
-  translate_client0_prefs->RemoveSiteFromBlacklist(host);
-  ASSERT_FALSE(translate_client0_prefs->IsSiteBlacklisted(host));
-
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_FALSE(translate_client1_prefs->IsSiteBlacklisted(host));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kPromptForDownload));
 }
 
 // TCM ID - 6515252.
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        kExtensionsUIDeveloperMode) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kExtensionsUIDeveloperMode));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kExtensionsUIDeveloperMode));
 
   ChangeBooleanPref(0, prefs::kExtensionsUIDeveloperMode);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kExtensionsUIDeveloperMode));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kExtensionsUIDeveloperMode));
 }
 
 // TCM ID - 7583816
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kAcceptLanguages) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   DisableVerifier();
-  ASSERT_TRUE(StringPrefMatches(prefs::kAcceptLanguages));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kAcceptLanguages));
 
   AppendStringPref(0, prefs::kAcceptLanguages, ",ar");
   AppendStringPref(1, prefs::kAcceptLanguages, ",fr");
-  ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(StringPrefMatches(prefs::kAcceptLanguages));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kAcceptLanguages));
 
   ChangeStringPref(0, prefs::kAcceptLanguages, "en-US");
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(StringPrefMatches(prefs::kAcceptLanguages));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kAcceptLanguages));
 
   ChangeStringPref(0, prefs::kAcceptLanguages, "ar,en-US");
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(StringPrefMatches(prefs::kAcceptLanguages));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kAcceptLanguages));
 }
 
 // TCM ID - 7590682
 #if defined(TOOLKIT_GTK)
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kUsesSystemTheme) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kUsesSystemTheme));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kUsesSystemTheme));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
+
+  // This pref is syncable.
+  ChangeStringPref(0, prefs::kHomePage, "http://news.google.com");
 
+  // This pref is not syncable.
   ChangeBooleanPref(0, prefs::kUsesSystemTheme);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
+
+  // Wait until the syncable pref is synced, then expect that the non-syncable
+  // one is still out of sync.
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
   ASSERT_FALSE(BooleanPrefMatches(prefs::kUsesSystemTheme));
 }
 #endif  // TOOLKIT_GTK
@@ -532,15 +404,13 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kUsesSystemTheme) {
 // Disabled, http://crbug.com/351159 .
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, DISABLED_kTapToClickEnabled) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kTapToClickEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kTapToClickEnabled));
 
   ChangeBooleanPref(0, prefs::kTapToClickEnabled);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kTapToClickEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kTapToClickEnabled));
 
   ChangeBooleanPref(1, prefs::kTapToClickEnabled);
-  ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kTapToClickEnabled));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kTapToClickEnabled));
 }
 #endif  // OS_CHROMEOS
 
@@ -548,83 +418,42 @@ IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, DISABLED_kTapToClickEnabled
 #if defined(OS_CHROMEOS)
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest, kEnableAutoScreenLock) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableAutoScreenLock));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kEnableAutoScreenLock));
 
   ChangeBooleanPref(0, prefs::kEnableAutoScreenLock);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableAutoScreenLock));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kEnableAutoScreenLock));
 
   ChangeBooleanPref(1, prefs::kEnableAutoScreenLock);
-  ASSERT_TRUE(GetClient(1)->AwaitMutualSyncCycleCompletion(GetClient(0)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kEnableAutoScreenLock));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kEnableAutoScreenLock));
 }
 #endif  // OS_CHROMEOS
 
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
-                       SingleClientEnabledEncryption) {

[rlarocque, 2014/06/04 01:14:43]

These tests are a not very useful.  Here's what I think may have happened:

- At some point in the past, EnableEncryption would enable encryption but would
not wait for it.
- Then something changed and EnableEncryption was setup so that it would always
wait for encryption to be completed.  I'm not sure if this happened before or
after we started untangling the ProfileSyncServiceHarness state machine
hairball.  I'm guessing before.
- Today, the IsEncryptionComplete() is redundant. 
ASSERT_TRUE(EnableEncryption(0)) by definition will not return until
IsEncryptionComplete(0) is true.

So if we remove the redundant lines from SingleClientEnabledEncryption, and
replaced the MutualSyncCycleCompletion() check with something based on a
StatusChecker, we would have:

ASSERT_TRUE(SetupSync());
ASSERT_TRUE(EnableEncryption(0);
ASSERT_TRUE(AwaitEncryptionComplete(1));

Which doesn't look much like a preferences test.  It's also functionally
identical to the similarly refactor-able BothClientsEnabledEncryption.

Since none of this makes sense to me, I decided to delete most of these tests
and leave the two that looked least trivial.

[pval...(no longer on Chromium), 2014/06/04 18:06:31]

roger

-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-
-  ASSERT_TRUE(EnableEncryption(0));
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(IsEncryptionComplete(0));
-  ASSERT_TRUE(IsEncryptionComplete(1));
-}
-
-IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
-                       SingleClientEnabledEncryptionAndChanged) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
-
-  ChangeBooleanPref(0, prefs::kHomePageIsNewTabPage);
-  ASSERT_TRUE(EnableEncryption(0));
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(IsEncryptionComplete(0));
-  ASSERT_TRUE(IsEncryptionComplete(1));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
-}
-
-IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
-                       BothClientsEnabledEncryption) {
-  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-
-  ASSERT_TRUE(EnableEncryption(0));
-  ASSERT_TRUE(EnableEncryption(1));
-  ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(IsEncryptionComplete(0));
-  ASSERT_TRUE(IsEncryptionComplete(1));
-}
-
-IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
                        SingleClientEnabledEncryptionBothChanged) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
-  ASSERT_TRUE(StringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 
   ASSERT_TRUE(EnableEncryption(0));
   ChangeBooleanPref(0, prefs::kHomePageIsNewTabPage);
   ChangeStringPref(1, prefs::kHomePage, "http://www.google.com/1");
-  ASSERT_TRUE(AwaitQuiescence());
-  ASSERT_TRUE(IsEncryptionComplete(0));
-  ASSERT_TRUE(IsEncryptionComplete(1));
-  ASSERT_TRUE(BooleanPrefMatches(
-      prefs::kHomePageIsNewTabPage));
-  ASSERT_TRUE(StringPrefMatches(prefs::kHomePage));
+  ASSERT_TRUE(AwaitEncryptionComplete(0));
+  ASSERT_TRUE(AwaitEncryptionComplete(1));
+  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+  ASSERT_TRUE(AwaitStringPrefMatches(prefs::kHomePage));
 }
 
 IN_PROC_BROWSER_TEST_F(TwoClientPreferencesSyncTest,
-                       SingleClientEnabledEncryptionAndChangedMultipleTimes) {
+                       BothClientsEnabledEncryptionAndChangedMultipleTimes) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kHomePageIsNewTabPage));
 
   ChangeBooleanPref(0, prefs::kHomePageIsNewTabPage);
   ASSERT_TRUE(EnableEncryption(0));
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(IsEncryptionComplete(0));
-  ASSERT_TRUE(IsEncryptionComplete(1));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kHomePageIsNewTabPage));
+  ASSERT_TRUE(EnableEncryption(1));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kHomePageIsNewTabPage));
 
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kShowHomeButton));
   ChangeBooleanPref(0, prefs::kShowHomeButton);
-  ASSERT_TRUE(GetClient(0)->AwaitMutualSyncCycleCompletion(GetClient(1)));
-  ASSERT_TRUE(BooleanPrefMatches(prefs::kShowHomeButton));
+  ASSERT_TRUE(AwaitBooleanPrefMatches(prefs::kShowHomeButton));
 }