sync: Refactor TwoClientPreferencesSyncTest

Makes the TwoClientPreferencesSyncTests compatible with
reflection-blocking and enables reflection-blocking in its test harness.

Introduces new StatusCheckers that wait for preferences.  The new
preference status checkers work by observering preference changes rather
than sync cycle completion notifications.  Also introduces some new
status checkers to wait until encryption is enabled.

Removes DisablePreferences and DisableSync tests because we have decided
that there is not enough value in testing disable scenarios to bother
maintaining them for each type.

Removes several translation tests because they seem to not provide much
additional value and they would be particularly onerous to maintain.
Each one of these tests would have needed its own status checker or
a generic status-checker that makes use of an injected Closure.

Removes several encryption tests because they were redundant.

Removes or renames many other redundant tests.

BUG=97780,95742,351159

Review URL: https://codereview.chromium.org/342853004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282241 0039d316-1c4b-4281-b951-d872f2087c98

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

@@ -4,9 +4,10 @@
 
 #include "chrome/browser/sync/test/integration/preferences_helper.h"
 
+#include "base/prefs/pref_change_registrar.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
-#include "base/values.h"
+#include "base/strings/stringprintf.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/test/integration/multi_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
@@ -58,14 +59,6 @@ void ChangeStringPref(int index,
     GetVerifierPrefs()->SetString(pref_name, new_value);
 }
 
-void AppendStringPref(int index,
-                      const char* pref_name,
-                      const std::string& append_value) {
-  ChangeStringPref(index,
-                   pref_name,
-                   GetPrefs(index)->GetString(pref_name) + append_value);
-}
-
 void ChangeFilePathPref(int index,
                         const char* pref_name,
                         const base::FilePath& new_value) {
@@ -107,8 +100,8 @@ bool BooleanPrefMatches(const char* pref_name) {
   }
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetBoolean(pref_name)) {
-      LOG(ERROR) << "Boolean preference " << pref_name << " mismatched in"
-                 << " profile " << i << ".";
+      DVLOG(1) << "Boolean preference " << pref_name << " mismatched in"
+               << " profile " << i << ".";
       return false;
     }
   }
@@ -124,8 +117,8 @@ bool IntegerPrefMatches(const char* pref_name) {
   }
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetInteger(pref_name)) {
-      LOG(ERROR) << "Integer preference " << pref_name << " mismatched in"
-                 << " profile " << i << ".";
+      DVLOG(1) << "Integer preference " << pref_name << " mismatched in"
+               << " profile " << i << ".";
       return false;
     }
   }
@@ -141,8 +134,8 @@ bool Int64PrefMatches(const char* pref_name) {
   }
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetInt64(pref_name)) {
-      LOG(ERROR) << "Integer preference " << pref_name << " mismatched in"
-                 << " profile " << i << ".";
+      DVLOG(1) << "Integer preference " << pref_name << " mismatched in"
+               << " profile " << i << ".";
       return false;
     }
   }
@@ -158,8 +151,8 @@ bool DoublePrefMatches(const char* pref_name) {
   }
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetDouble(pref_name)) {
-      LOG(ERROR) << "Double preference " << pref_name << " mismatched in"
-                 << " profile " << i << ".";
+      DVLOG(1) << "Double preference " << pref_name << " mismatched in"
+               << " profile " << i << ".";
       return false;
     }
   }
@@ -175,8 +168,8 @@ bool StringPrefMatches(const char* pref_name) {
   }
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetString(pref_name)) {
-      LOG(ERROR) << "String preference " << pref_name << " mismatched in"
-                 << " profile " << i << ".";
+      DVLOG(1) << "String preference " << pref_name << " mismatched in"
+               << " profile " << i << ".";
       return false;
     }
   }
@@ -192,8 +185,8 @@ bool FilePathPrefMatches(const char* pref_name) {
   }
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (reference_value != GetPrefs(i)->GetFilePath(pref_name)) {
-      LOG(ERROR) << "base::FilePath preference " << pref_name
-                 << " mismatched in" << " profile " << i << ".";
+      DVLOG(1) << "base::FilePath preference " << pref_name << " mismatched in"
+               << " profile " << i << ".";
       return false;
     }
   }
@@ -209,8 +202,8 @@ bool ListPrefMatches(const char* pref_name) {
   }
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (!reference_value->Equals(GetPrefs(i)->GetList(pref_name))) {
-      LOG(ERROR) << "List preference " << pref_name << " mismatched in"
-                 << " profile " << i << ".";
+      DVLOG(1) << "List preference " << pref_name << " mismatched in"
+               << " profile " << i << ".";
       return false;
     }
   }
@@ -220,37 +213,173 @@ bool ListPrefMatches(const char* pref_name) {
 
 namespace {
 
-// Helper class used in the implementation of AwaitListPrefMatches.
-class ListPrefMatchStatusChecker : public MultiClientStatusChangeChecker {
+class PrefMatchChecker : public StatusChangeChecker {
  public:
-  explicit ListPrefMatchStatusChecker(const char* pref_name);
-  virtual ~ListPrefMatchStatusChecker();
+  explicit PrefMatchChecker(const char* path);
+  virtual ~PrefMatchChecker();
 
-  virtual bool IsExitConditionSatisfied() OVERRIDE;
+  // StatusChangeChecker implementation.
+  virtual bool IsExitConditionSatisfied() = 0;
   virtual std::string GetDebugMessage() const OVERRIDE;
+
+  // Wait for condition to become true.
+  void Wait();
+
+ protected:
+  const char* GetPath() const;
+
  private:
-  const char* pref_name_;
+  void RegisterPrefListener(PrefService* pref_service);
+
+  ScopedVector<PrefChangeRegistrar> pref_change_registrars_;
+  const char* path_;
+};
+
+PrefMatchChecker::PrefMatchChecker(const char* path) : path_(path) {
+}
+
+PrefMatchChecker::~PrefMatchChecker() {
+}
+
+std::string PrefMatchChecker::GetDebugMessage() const {
+  return base::StringPrintf("Waiting for pref '%s' to match", GetPath());
+}
+
+void PrefMatchChecker::Wait() {
+  if (test()->use_verifier()) {
+    RegisterPrefListener(GetVerifierPrefs());
+  }
+
+  for (int i = 0; i < test()->num_clients(); ++i) {
+    RegisterPrefListener(GetPrefs(i));
+  }
+
+  if (IsExitConditionSatisfied()) {
+    return;
+  }
+
+  StartBlockingWait();
+}
+
+const char* PrefMatchChecker::GetPath() const {
+  return path_;
+}
+
+void PrefMatchChecker::RegisterPrefListener(PrefService* pref_service) {
+  scoped_ptr<PrefChangeRegistrar> registrar(new PrefChangeRegistrar());
+  registrar->Init(pref_service);
+  registrar->Add(path_,
+                 base::Bind(&PrefMatchChecker::CheckExitCondition,
+                            base::Unretained(this)));
+  pref_change_registrars_.push_back(registrar.release());
+}
+
+// Helper class used in the implementation of AwaitListPrefMatches.
+class ListPrefMatchChecker : public PrefMatchChecker {
+ public:
+  explicit ListPrefMatchChecker(const char* path);
+  virtual ~ListPrefMatchChecker();
+
+  // Implementation of PrefMatchChecker.
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+};
+
+ListPrefMatchChecker::ListPrefMatchChecker(const char* path)
+    : PrefMatchChecker(path) {
+}
+
+ListPrefMatchChecker::~ListPrefMatchChecker() {
+}
+
+bool ListPrefMatchChecker::IsExitConditionSatisfied() {
+  return ListPrefMatches(GetPath());
+}
+
+// Helper class used in the implementation of AwaitBooleanPrefMatches.
+class BooleanPrefMatchChecker : public PrefMatchChecker {
+ public:
+  explicit BooleanPrefMatchChecker(const char* path);
+  virtual ~BooleanPrefMatchChecker();
+
+  // Implementation of PrefMatchChecker.
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+};
+
+BooleanPrefMatchChecker::BooleanPrefMatchChecker(const char* path)
+    : PrefMatchChecker(path) {
+}
+
+BooleanPrefMatchChecker::~BooleanPrefMatchChecker() {
+}
+
+bool BooleanPrefMatchChecker::IsExitConditionSatisfied() {
+  return BooleanPrefMatches(GetPath());
+}
+
+// Helper class used in the implementation of AwaitIntegerPrefMatches.
+class IntegerPrefMatchChecker : public PrefMatchChecker {
+ public:
+  explicit IntegerPrefMatchChecker(const char* path);
+  virtual ~IntegerPrefMatchChecker();
+
+  // Implementation of PrefMatchChecker.
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
 };
 
-ListPrefMatchStatusChecker::ListPrefMatchStatusChecker(const char* pref_name)
-    : MultiClientStatusChangeChecker(
-        sync_datatype_helper::test()->GetSyncServices()),
-      pref_name_(pref_name) {}
+IntegerPrefMatchChecker::IntegerPrefMatchChecker(const char* path)
+    : PrefMatchChecker(path) {
+}
+
+IntegerPrefMatchChecker::~IntegerPrefMatchChecker() {
+}
+
+bool IntegerPrefMatchChecker::IsExitConditionSatisfied() {
+  return IntegerPrefMatches(GetPath());
+}
+
+// Helper class used in the implementation of AwaitStringPrefMatches.
+class StringPrefMatchChecker : public PrefMatchChecker {
+ public:
+  explicit StringPrefMatchChecker(const char* path);
+  virtual ~StringPrefMatchChecker();
+
+  // Implementation of PrefMatchChecker.
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+};
 
-ListPrefMatchStatusChecker::~ListPrefMatchStatusChecker() {}
+StringPrefMatchChecker::StringPrefMatchChecker(const char* path)
+    : PrefMatchChecker(path) {
+}
 
-bool ListPrefMatchStatusChecker::IsExitConditionSatisfied() {
-  return ListPrefMatches(pref_name_);
+StringPrefMatchChecker::~StringPrefMatchChecker() {
 }
 
-std::string ListPrefMatchStatusChecker::GetDebugMessage() const {
-  return "Waiting for matching preferences";
+bool StringPrefMatchChecker::IsExitConditionSatisfied() {
+  return StringPrefMatches(GetPath());
 }
 
 }  //  namespace
 
 bool AwaitListPrefMatches(const char* pref_name) {
-  ListPrefMatchStatusChecker checker(pref_name);
+  ListPrefMatchChecker checker(pref_name);
+  checker.Wait();
+  return !checker.TimedOut();
+}
+
+bool AwaitBooleanPrefMatches(const char* pref_name) {
+  BooleanPrefMatchChecker checker(pref_name);
+  checker.Wait();
+  return !checker.TimedOut();
+}
+
+bool AwaitIntegerPrefMatches(const char* pref_name) {
+  IntegerPrefMatchChecker checker(pref_name);
+  checker.Wait();
+  return !checker.TimedOut();
+}
+
+bool AwaitStringPrefMatches(const char* pref_name) {
+  StringPrefMatchChecker checker(pref_name);
   checker.Wait();
   return !checker.TimedOut();
 }

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

@@ -47,14 +47,6 @@ void ChangeStringPref(int index,
                       const char* pref_name,
                       const std::string& new_value);
 
-// Modifies the value of the string preference with name |pref_name| in the
-// profile with index |index| by appending |append_value| to its current
-// value. Also changes its value in |verifier| if DisableVerifier() hasn't
-// been called.
-void AppendStringPref(int index,
-                      const char* pref_name,
-                      const std::string& append_value);
-
 // 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.
@@ -108,6 +100,18 @@ bool ListPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
 // to match in all profiles. Returns false if this operation times out.
 bool AwaitListPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
 
+// Blocks the test until the specified pref matches on all relevant clients or
+// a timeout occurs.  Returns false if it returns because of a timeout.
+bool AwaitBooleanPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+// Blocks the test until the specified pref matches on all relevant clients or
+// a timeout occurs.  Returns false if it returns because of a timeout.
+bool AwaitIntegerPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
+// Blocks the test until the specified pref matches on all relevant clients or
+// a timeout occurs.  Returns false if it returns because of a timeout.
+bool AwaitStringPrefMatches(const char* pref_name) WARN_UNUSED_RESULT;
+
 }  // namespace preferences_helper
 
 #endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_PREFERENCES_HELPER_H_

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

@@ -835,17 +835,20 @@ bool SyncTest::EnableEncryption(int index) {
   bool sync_everything = synced_datatypes.Equals(syncer::ModelTypeSet::All());
   service->OnUserChoseDatatypes(sync_everything, synced_datatypes);
 
-  // Wait some time to let the enryption finish.
-  EncryptionChecker checker(service);
-  checker.Wait();
-
-  return !checker.TimedOut();
+  return AwaitEncryptionComplete(index);
 }
 
 bool SyncTest::IsEncryptionComplete(int index) {
   return ::IsEncryptionComplete(GetClient(index)->service());
 }
 
+bool SyncTest::AwaitEncryptionComplete(int index) {
+  ProfileSyncService* service = GetClient(index)->service();
+  EncryptionChecker checker(service);
+  checker.Wait();
+  return !checker.TimedOut();
+}
+
 bool SyncTest::AwaitQuiescence() {
   return ProfileSyncServiceHarness::AwaitQuiescence(clients());
 }

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

@@ -190,6 +190,9 @@ class SyncTest : public InProcessBrowserTest {
   // Checks if encryption is complete for profile |index|.
   bool IsEncryptionComplete(int index);
 
+  // Waits until IsEncryptionComplete returns true or a timeout is reached.
+  bool AwaitEncryptionComplete(int index);
+
   // Blocks until all sync clients have completed their mutual sync cycles.
   // Returns true if a quiescent state was successfully reached.
   bool AwaitQuiescence();