Refactor password sync integration tests

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

 // Copyright (c) 2012 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/test/integration/passwords_helper.h"
 
 #include "base/compiler_specific.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/time/time.h"
 #include "chrome/browser/password_manager/password_store_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/test/integration/multi_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
+#include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/sync_datatype_helper.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/password_manager/core/browser/password_form_data.h"
 #include "components/password_manager/core/browser/password_store.h"
 #include "components/password_manager/core/browser/password_store_consumer.h"
 
 using autofill::PasswordForm;
 using password_manager::PasswordStore;
 using sync_datatype_helper::test;
 
@@ skipping 143 matching lines @@
          it != forms_b.end(); ++it) {
       LOG(ERROR) << *it << std::endl;
     }
   }
   return result;
 }
 
 bool AllProfilesContainSamePasswordFormsAsVerifier() {
   for (int i = 0; i < test()->num_clients(); ++i) {
     if (!ProfileContainsSamePasswordFormsAsVerifier(i)) {
-      LOG(ERROR) << "Profile " << i << " does not contain the same password"
-                                       " forms as the verifier.";
+      LOG(INFO) << "Profile " << i << " does not contain the same password"

[Nicolas Zea, 2014/05/06 00:16:37]

any reason this was changed?

[rlarocque, 2014/05/06 00:33:36]

ERROR isn't quite right anymore.  In some ways it was never right, but now it
actually matters.

Now that we use these functions in an Await() function, it's normal and expected
for this function to fail from time to time.  It's only an error when the
profiles don't match after 40 seconds of waiting.

[Nicolas Zea, 2014/05/06 18:42:15]

If it's expected for this to be called, I think it should probably be a
DVLOG(1), rather than printing non-useful messages.

[rlarocque, 2014/05/06 23:40:47]

Done.

+                                      " forms as the verifier.";
       return false;
     }
   }
   return true;
 }
 
 bool AllProfilesContainSamePasswordForms() {
   for (int i = 1; i < test()->num_clients(); ++i) {
     if (!ProfilesContainSamePasswordForms(0, i)) {
-      LOG(ERROR) << "Profile " << i << " does not contain the same password"
-                                       " forms as Profile 0.";
+      LOG(INFO) << "Profile " << i << " does not contain the same password"
+                                      " forms as Profile 0.";
       return false;
     }
   }
   return true;
 }
 
 namespace {
 
 // Helper class used in the implementation of
 // AwaitAllProfilesContainSamePasswordForms.
@@ skipping 55 matching lines @@
 }
 
 }  //  namespace
 
 bool AwaitAllProfilesContainSamePasswordForms() {
   SamePasswordFormsChecker checker;
   checker.Wait();
   return !checker.TimedOut();
 }
 
+namespace {
+
+// Helper class used in the implementation of
+// AwaitProfileContainSamePasswordFormsAsVerifier.
+class SamePasswordFormsAsVerifierChecker
+    : public SingleClientStatusChangeChecker {
+ public:
+  explicit SamePasswordFormsAsVerifierChecker(int index);
+  virtual ~SamePasswordFormsAsVerifierChecker();
+
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+  virtual std::string GetDebugMessage() const OVERRIDE;
+
+ private:
+  int index_;
+
+  bool in_progress_;
+  bool needs_recheck_;
+};
+
+SamePasswordFormsAsVerifierChecker::SamePasswordFormsAsVerifierChecker(int i)
+    : SingleClientStatusChangeChecker(
+          sync_datatype_helper::test()->GetSyncService(i)),
+      index_(i),
+      in_progress_(false),
+      needs_recheck_(false) {
+}
+
+SamePasswordFormsAsVerifierChecker::~SamePasswordFormsAsVerifierChecker() {
+}
+
+// This method uses the same re-entrancy prevention trick as
+// the SamePasswordFormsChecker.
+bool SamePasswordFormsAsVerifierChecker::IsExitConditionSatisfied() {
+  if (in_progress_) {
+    LOG(WARNING) << "Setting flag and returning early to prevent nesting.";
+    needs_recheck_ = true;
+    return false;
+  }
+
+  // Keep retrying until we get a good reading.
+  bool result = false;
+  in_progress_ = true;
+  do {
+    needs_recheck_ = false;
+    result = ProfileContainsSamePasswordFormsAsVerifier(index_);
+  } while (needs_recheck_);
+  in_progress_ = false;
+  return result;
+}
+
+std::string SamePasswordFormsAsVerifierChecker::GetDebugMessage() const {
+  return "Waiting for passwords to match verifier";
+}
+
+}  //  namespace
+
+bool AwaitProfileContainsSamePasswordFormsAsVerifier(int index) {
+  SamePasswordFormsAsVerifierChecker checker(index);
+  checker.Wait();
+  return !checker.TimedOut();
+}
+
 int GetPasswordCount(int index) {
   std::vector<PasswordForm> forms;
   GetLogins(GetPasswordStore(index), forms);
   return forms.size();
 }
 
 int GetVerifierPasswordCount() {
   std::vector<PasswordForm> verifier_forms;
   GetLogins(GetVerifierPasswordStore(), verifier_forms);
   return verifier_forms.size();
 }
 
 PasswordForm CreateTestPasswordForm(int index) {
   PasswordForm form;
   form.signon_realm = kFakeSignonRealm;
   form.origin = GURL(base::StringPrintf(kIndexedFakeOrigin, index));
   form.username_value =
       base::ASCIIToUTF16(base::StringPrintf("username%d", index));
   form.password_value =
       base::ASCIIToUTF16(base::StringPrintf("password%d", index));
   form.date_created = base::Time::Now();
   return form;
 }
 
 }  // namespace passwords_helper