sync: Change progress marker checking in tests

Introduces two new StatusChangeCheckers: UpdatedProgressMarkerChecker
and QuiesceStatusChangeChecker.

The first, UpdatedProgressMarkerChecker, is a simple checker for
ensuring progress markers are "fully synced".  It's a bit flaky, but no
flakier than the functions that preceded it.

The second, QuiesceStatusChangeChecker, is intended to be a replacement
for AwaitQuiescence and many other Await* methods.  The logic is
different, but the heuristic used is essentially the same.  We want to
wait until all clients pass the 'HasLatestProgressMarkers()' check, then
make sure that their progress markers match each other.  The new
implementation is a bit simpler, since it takes advantage of the
transitive properties of equality rather than doing O(N^2) comparisons.

The QuiesceStatusChangeChecker is made more complicated by the fact that
the HasLatestProgressMarkers() is not very reliable.  This becomes a
problem in practice if we call that function on some "Profile A" in
response to an event that happened in "Profile B".  So the naive
implementation of QuiesceStatusChangeChecker won't work.

The implementation in this CL uses a system of observers to make the
calls to HasLatestProgressMarker() safer.  It's an ugly hack, but IMHO
no uglier than the existing set of hacks.

BUG=323380

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

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

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

@@ -11,7 +11,6 @@
 #include <sstream>
 #include <vector>
 
-#include "base/base64.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
@@ -32,8 +31,10 @@
 #include "chrome/browser/sync/backend_migrator.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/sync/test/integration/p2p_invalidation_forwarder.h"
+#include "chrome/browser/sync/test/integration/quiesce_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/status_change_checker.h"
+#include "chrome/browser/sync/test/integration/updated_progress_marker_checker.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/sync_driver/data_type_controller.h"
@@ -172,7 +173,6 @@ ProfileSyncServiceHarness::ProfileSyncServiceHarness(
     P2PInvalidationService* p2p_invalidation_service)
     : profile_(profile),
       service_(ProfileSyncServiceFactory::GetForProfile(profile)),
-      progress_marker_partner_(NULL),
       username_(username),
       password_(password),
       oauth2_refesh_token_number_(0),
@@ -353,14 +353,8 @@ bool ProfileSyncServiceHarness::AwaitBackendInitialized() {
 // an in-process C++ server, this function can be reimplemented without relying
 // on progress markers.
 bool ProfileSyncServiceHarness::AwaitCommitActivityCompletion() {
-  DVLOG(1) << GetClientInfoString("AwaitCommitActivityCompletion");
-  CallbackStatusChecker latest_progress_markers_checker(
-      service(),
-      base::Bind(&ProfileSyncServiceHarness::HasLatestProgressMarkers,
-                 base::Unretained(this)),
-      "HasLatestProgressMarkers");
-  AwaitStatusChange(&latest_progress_markers_checker);
-  return HasLatestProgressMarkers();
+  UpdatedProgressMarkerChecker progress_marker_checker(service());
+  return AwaitStatusChange(&progress_marker_checker);
 }
 
 bool ProfileSyncServiceHarness::AwaitSyncDisabled() {
@@ -384,78 +378,37 @@ bool ProfileSyncServiceHarness::AwaitSyncSetupCompletion() {
 
 bool ProfileSyncServiceHarness::AwaitMutualSyncCycleCompletion(
     ProfileSyncServiceHarness* partner) {
-  DVLOG(1) << GetClientInfoString("AwaitMutualSyncCycleCompletion");
-  if (!AwaitCommitActivityCompletion())
-    return false;
-  return partner->WaitUntilProgressMarkersMatch(this);
+  std::vector<ProfileSyncServiceHarness*> harnesses;
+  harnesses.push_back(this);
+  harnesses.push_back(partner);
+  return AwaitQuiescence(harnesses);
 }
 
 bool ProfileSyncServiceHarness::AwaitGroupSyncCycleCompletion(
     std::vector<ProfileSyncServiceHarness*>& partners) {
-  DVLOG(1) << GetClientInfoString("AwaitGroupSyncCycleCompletion");
-  if (!AwaitCommitActivityCompletion())
-    return false;
-  bool return_value = true;
-  for (std::vector<ProfileSyncServiceHarness*>::iterator it =
-      partners.begin(); it != partners.end(); ++it) {
-    if ((this != *it) && (!(*it)->IsSyncDisabled())) {
-      return_value = return_value &&
-          (*it)->WaitUntilProgressMarkersMatch(this);
-    }
-  }
-  return return_value;
+  return AwaitQuiescence(partners);
 }
 
 // static
 bool ProfileSyncServiceHarness::AwaitQuiescence(
     std::vector<ProfileSyncServiceHarness*>& clients) {
-  DVLOG(1) << "AwaitQuiescence.";
-  bool return_value = true;
-  for (std::vector<ProfileSyncServiceHarness*>::iterator it =
-      clients.begin(); it != clients.end(); ++it) {
-    if (!(*it)->IsSyncDisabled()) {
-      return_value = return_value &&
-          (*it)->AwaitGroupSyncCycleCompletion(clients);
-    }
+  std::vector<ProfileSyncService*> services;
+  if (clients.empty()) {
+    return true;
   }
-  return return_value;
-}
 
-bool ProfileSyncServiceHarness::WaitUntilProgressMarkersMatch(
-    ProfileSyncServiceHarness* partner) {
-  DVLOG(1) << GetClientInfoString("WaitUntilProgressMarkersMatch");
-
-  // TODO(rsimha): Replace the mechanism of matching up progress markers with
-  // one that doesn't require every client to have the same progress markers.
-  DCHECK(!progress_marker_partner_);
-  progress_marker_partner_ = partner;
-  bool return_value = false;
-  if (MatchesPartnerClient()) {
-    // Progress markers already match; don't wait.
-    return_value = true;
-  } else {
-    partner->service()->AddObserver(this);
-    CallbackStatusChecker matches_other_client_checker(
-        service(),
-        base::Bind(&ProfileSyncServiceHarness::MatchesPartnerClient,
-                   base::Unretained(this)),
-        "MatchesPartnerClient");
-    return_value = AwaitStatusChange(&matches_other_client_checker);
-    partner->service()->RemoveObserver(this);
+  for (std::vector<ProfileSyncServiceHarness*>::iterator it = clients.begin();
+       it != clients.end(); ++it) {
+    services.push_back((*it)->service());
   }
-  progress_marker_partner_ = NULL;
-  return return_value;
+  QuiesceStatusChangeChecker checker(services);
+  return clients[0]->AwaitStatusChange(&checker);
 }
 
 bool ProfileSyncServiceHarness::AwaitStatusChange(
     StatusChangeChecker* checker) {
   DVLOG(1) << GetClientInfoString("AwaitStatusChange");
 
-  if (IsSyncDisabled()) {
-    LOG(ERROR) << "Sync disabled for " << profile_debug_name_ << ".";
-    return false;
-  }
-
   DCHECK(checker);
   if (checker->IsExitConditionSatisfied()) {
     DVLOG(1) << GetClientInfoString("AwaitStatusChange exiting early because "
@@ -519,14 +472,6 @@ bool ProfileSyncServiceHarness::HasAuthError() const {
              GoogleServiceAuthError::REQUEST_CANCELED;
 }
 
-// TODO(sync): Remove this method once we stop relying on self notifications and
-// comparing progress markers.
-bool ProfileSyncServiceHarness::HasLatestProgressMarkers() const {
-  const SyncSessionSnapshot& snap = GetLastSessionSnapshot();
-  return snap.model_neutral_state().num_successful_commits == 0 &&
-         !service()->HasUnsyncedItems();
-}
-
 void ProfileSyncServiceHarness::FinishSyncSetup() {
   service()->SetSetupInProgress(false);
   service()->SetSyncSetupCompleted();
@@ -536,43 +481,6 @@ bool ProfileSyncServiceHarness::AutoStartEnabled() {
   return service()->auto_start_enabled();
 }
 
-bool ProfileSyncServiceHarness::MatchesPartnerClient() const {
-  DCHECK(progress_marker_partner_);
-
-  // Only look for a match if we have at least one enabled datatype in
-  // common with the partner client.
-  const syncer::ModelTypeSet common_types =
-      Intersection(service()->GetActiveDataTypes(),
-                   progress_marker_partner_->service()->GetActiveDataTypes());
-
-  DVLOG(2) << profile_debug_name_ << ", "
-           << progress_marker_partner_->profile_debug_name_
-           << ": common types are "
-           << syncer::ModelTypeSetToString(common_types);
-
-  for (syncer::ModelTypeSet::Iterator i = common_types.First();
-       i.Good(); i.Inc()) {
-    const std::string marker = GetSerializedProgressMarker(i.Get());
-    const std::string partner_marker =
-        progress_marker_partner_->GetSerializedProgressMarker(i.Get());
-    if (marker != partner_marker) {
-      if (VLOG_IS_ON(2)) {
-        std::string marker_base64, partner_marker_base64;
-        base::Base64Encode(marker, &marker_base64);
-        base::Base64Encode(partner_marker, &partner_marker_base64);
-        DVLOG(2) << syncer::ModelTypeToString(i.Get()) << ": "
-                 << profile_debug_name_ << " progress marker = "
-                 << marker_base64 << ", "
-                 << progress_marker_partner_->profile_debug_name_
-                 << " partner progress marker = "
-                 << partner_marker_base64;
-      }
-      return false;
-    }
-  }
-  return true;
-}
-
 SyncSessionSnapshot ProfileSyncServiceHarness::GetLastSessionSnapshot() const {
   DCHECK(service() != NULL) << "Sync service has not yet been set up.";
   if (service()->sync_initialized()) {

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

@@ -88,10 +88,6 @@ class ProfileSyncServiceHarness
   // true if sync setup is complete.
   bool AwaitSyncSetupCompletion();
 
-  // Blocks the caller until this harness has observed that the sync engine
-  // has downloaded all the changes seen by the |partner| harness's client.
-  bool WaitUntilProgressMarkersMatch(ProfileSyncServiceHarness* partner);
-
   // Calling this acts as a barrier and blocks the caller until |this| and
   // |partner| have both completed a sync cycle.  When calling this method,
   // the |partner| should be the passive responder who responds to the actions
@@ -187,10 +183,6 @@ class ProfileSyncServiceHarness
   // available), annotated with |message|. Useful for logging.
   std::string GetClientInfoString(const std::string& message) const;
 
-  // Returns true if this client has downloaded all the items that the
-  // other client has.
-  bool MatchesPartnerClient() const;
-
  private:
   ProfileSyncServiceHarness(
       Profile* profile,
@@ -210,10 +202,6 @@ class ProfileSyncServiceHarness
   // found.
   std::string GetSerializedProgressMarker(syncer::ModelType model_type) const;
 
-  // Returns true if a client has nothing left to commit and its progress
-  // markers are up to date.
-  bool HasLatestProgressMarkers() const;
-
   // Gets detailed status from |service_| in pretty-printable form.
   std::string GetServiceStatus();
 
@@ -226,10 +214,6 @@ class ProfileSyncServiceHarness
   // An bridge between the ProfileSyncService and P2PInvalidationService.
   scoped_ptr<P2PInvalidationForwarder> p2p_invalidation_forwarder_;
 
-  // The harness of the client whose update progress marker we're expecting
-  // eventually match.
-  ProfileSyncServiceHarness* progress_marker_partner_;
-
   // Credentials used for GAIA authentication.
   std::string username_;
   std::string password_;

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

+// Copyright 2014 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/quiesce_status_change_checker.h"
+
+#include "base/format_macros.h"
+#include "base/scoped_observer.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
+#include "sync/internal_api/public/sessions/sync_session_snapshot.h"
+
+namespace {
+
+// Returns true if this service is disabled.
+bool IsSyncDisabled(ProfileSyncService* service) {
+  return !service->setup_in_progress() && !service->HasSyncSetupCompleted();
+}
+
+// Returns true if these services have matching progress markers.
+bool ProgressMarkersMatch(const ProfileSyncService* service1,
+                          const ProfileSyncService* service2) {
+  const syncer::ModelTypeSet common_types =
+      Intersection(service1->GetActiveDataTypes(),
+                   service2->GetActiveDataTypes());
+
+  const syncer::sessions::SyncSessionSnapshot& snap1 =
+      service1->GetLastSessionSnapshot();
+  const syncer::sessions::SyncSessionSnapshot& snap2 =
+      service2->GetLastSessionSnapshot();
+
+  for (syncer::ModelTypeSet::Iterator type_it = common_types.First();
+       type_it.Good(); type_it.Inc()) {
+    // Look up the progress markers.  Fail if either one is missing.
+    syncer::ProgressMarkerMap::const_iterator pm_it1 =
+        snap1.download_progress_markers().find(type_it.Get());
+    if (pm_it1 == snap1.download_progress_markers().end()) {
+      return false;
+    }
+
+    syncer::ProgressMarkerMap::const_iterator pm_it2 =
+        snap2.download_progress_markers().find(type_it.Get());
+    if (pm_it2 == snap2.download_progress_markers().end()) {
+      return false;
+    }
+
+    // Fail if any of them don't match.
+    if (pm_it1->second != pm_it2->second) {
+      return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace
+
+// A helper class to keep an eye on a particular ProfileSyncService's
+// "HasLatestProgressMarkers()" state.
+//
+// This is a work-around for the HasLatestProgressMarkers check's inherent
+// flakiness.  It's not safe to check that condition whenever we want.  The
+// safest time to check it is when the ProfileSyncService emits an
+// OnStateChanged() event.  This class waits for those events and updates its
+// cached HasLatestProgressMarkers state every time that event occurs.
+//
+// See the comments in UpdatedProgressMarkerChecker for more details.
+//
+// The long-term plan is to deprecate this hack by replacing all its usees with
+// more reliable status checkers.
+class ProgressMarkerWatcher : public ProfileSyncServiceObserver {
+ public:
+  ProgressMarkerWatcher(
+      ProfileSyncService* service,
+      QuiesceStatusChangeChecker* quiesce_checker);
+  virtual ~ProgressMarkerWatcher();
+  virtual void OnStateChanged() OVERRIDE;
+
+  bool HasLatestProgressMarkers();
+  bool IsSyncDisabled();
+
+ private:
+  void UpdateHasLatestProgressMarkers();
+
+  ProfileSyncService* service_;
+  QuiesceStatusChangeChecker* quiesce_checker_;
+  ScopedObserver<ProfileSyncService, ProgressMarkerWatcher> scoped_observer_;
+  bool probably_has_latest_progress_markers_;
+};
+
+ProgressMarkerWatcher::ProgressMarkerWatcher(
+    ProfileSyncService* service,
+    QuiesceStatusChangeChecker* quiesce_checker)
+  : service_(service),
+    quiesce_checker_(quiesce_checker),
+    scoped_observer_(this),
+    probably_has_latest_progress_markers_(false) {
+  scoped_observer_.Add(service);
+  UpdateHasLatestProgressMarkers();
+}
+
+ProgressMarkerWatcher::~ProgressMarkerWatcher() { }
+
+void ProgressMarkerWatcher::OnStateChanged() {
+  UpdateHasLatestProgressMarkers();
+  quiesce_checker_->OnServiceStateChanged(service_);
+}
+
+void ProgressMarkerWatcher::UpdateHasLatestProgressMarkers() {
+  if (IsSyncDisabled()) {
+    probably_has_latest_progress_markers_ = false;
+    return;
+  }
+
+  // This is the same progress marker check as used by the
+  // UpdatedProgressMarkerChecker.  It has the samed drawbacks and potential for
+  // flakiness.  See the comment in
+  // UpdatedProgressMarkerChecker::IsExitConditionSatisfied() for more
+  // information.
+  //
+  // The QuiesceStatusChangeChecker attempts to work around the limitations of
+  // this progress marker checking method.  It tries to update the progress
+  // marker status only in the OnStateChanged() callback, where the snapshot is
+  // freshest.
+  //
+  // It also checks the progress marker status when it is first initialized, and
+  // that's where it's most likely that we could return a false positive.  We
+  // need to check these service at startup, since not every service is
+  // guaranteed to generate OnStateChanged() events while we're waiting for
+  // quiescence.
+  const syncer::sessions::SyncSessionSnapshot& snap =
+      service_->GetLastSessionSnapshot();
+  probably_has_latest_progress_markers_ =
+      snap.model_neutral_state().num_successful_commits == 0 &&
+      !service_->HasUnsyncedItems();
+}
+
+bool ProgressMarkerWatcher::HasLatestProgressMarkers() {
+  return probably_has_latest_progress_markers_;
+}
+
+bool ProgressMarkerWatcher::IsSyncDisabled() {
+  return ::IsSyncDisabled(service_);
+}
+
+QuiesceStatusChangeChecker::QuiesceStatusChangeChecker(
+    std::vector<ProfileSyncService*> services)
+  : services_(services), harness_(NULL) {
+  DCHECK_LE(1U, services_.size());
+  for (size_t i = 0; i < services_.size(); ++i) {
+    observers_.push_back(new ProgressMarkerWatcher(services[i], this));
+  }
+}
+
+QuiesceStatusChangeChecker::~QuiesceStatusChangeChecker() {}
+
+bool QuiesceStatusChangeChecker::IsExitConditionSatisfied() {
+  // Check that all progress markers are up to date.
+  for (ScopedVector<ProgressMarkerWatcher>::const_iterator it =
+       observers_.begin(); it != observers_.end(); ++it) {
+    if ((*it)->IsSyncDisabled()) {
+      continue;  // Skip disabled services.
+    }
+
+    if (!(*it)->HasLatestProgressMarkers()) {
+      VLOG(1) << "Not quiesced: Progress markers are old.";
+      return false;
+    }
+  }
+
+  std::vector<ProfileSyncService*> enabled_services;
+  for (std::vector<ProfileSyncService*>::const_iterator it = services_.begin();
+       it != services_.end(); ++it) {
+    if (!IsSyncDisabled(*it)) {
+      enabled_services.push_back(*it);
+    }
+  }
+
+  // Return true if we have nothing to compare against.
+  if (enabled_services.size() <= 1) {
+    return true;
+  }
+
+  std::vector<ProfileSyncService*>::const_iterator it1 =
+      enabled_services.begin();
+  std::vector<ProfileSyncService*>::const_iterator it2 =
+      enabled_services.begin();
+  it2++;
+
+  while (it2 != enabled_services.end()) {
+    // Return false if there is a progress marker mismatch.
+    if (!ProgressMarkersMatch(*it1, *it2)) {
+      VLOG(1) << "Not quiesced: Progress marker mismatch.";
+      return false;
+    }
+    it1++;
+    it2++;
+  }
+
+  return true;
+}
+
+std::string QuiesceStatusChangeChecker::GetDebugMessage() const {
+  return base::StringPrintf("Waiting for quiescence of %" PRIuS " clients",
+                            services_.size());
+}
+
+
+void QuiesceStatusChangeChecker::InitObserver(
+    ProfileSyncServiceHarness* harness) {
+  harness_ = harness;
+}
+
+void QuiesceStatusChangeChecker::UninitObserver(
+    ProfileSyncServiceHarness* harness) {
+  harness_ = NULL;
+}
+
+void QuiesceStatusChangeChecker::OnServiceStateChanged(
+    ProfileSyncService* service) {
+  harness_->OnStateChanged();
+}

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

+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_SYNC_TEST_INTEGRATION_QUIESCE_STATUS_CHANGE_CHECKER_H_
+#define CHROME_BROWSER_SYNC_TEST_INTEGRATION_QUIESCE_STATUS_CHANGE_CHECKER_H_
+
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_vector.h"
+#include "chrome/browser/sync/test/integration/status_change_checker.h"
+
+class ProfileSyncService;
+class ProgressMarkerWatcher;
+class ProfileSyncServiceHarness;
+
+// Waits until all provided clients have finished committing any unsynced items
+// and downloading each others' udpates.
+//
+// This requires that "self-notifications" be enabled.  Otherwise the clients
+// will not fetch the latest progress markers on their own, and the latest
+// progress markers are needed to confirm that clients are in sync.
+//
+// There is a race condition here.  If we manage to perform the check at
+// precisely the wrong time, we could end up seeing stale snapshot state
+// (crbug.com/95742), which would make us think that the client has finished
+// syncing when it hasn't.  In practice, this race is rare enough that it
+// doesn't cause test failures.
+class QuiesceStatusChangeChecker : public StatusChangeChecker {
+ public:
+  explicit QuiesceStatusChangeChecker(
+      std::vector<ProfileSyncService*> services);
+  virtual ~QuiesceStatusChangeChecker();
+
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+  virtual std::string GetDebugMessage() const OVERRIDE;
+  virtual void InitObserver(ProfileSyncServiceHarness* harness) OVERRIDE;
+  virtual void UninitObserver(ProfileSyncServiceHarness* harness) OVERRIDE;
+
+  void OnServiceStateChanged(ProfileSyncService* service);
+
+ private:
+  std::vector<ProfileSyncService*> services_;
+  ScopedVector<ProgressMarkerWatcher> observers_;
+  ProfileSyncServiceHarness* harness_;
+
+  DISALLOW_COPY_AND_ASSIGN(QuiesceStatusChangeChecker);
+};
+
+#endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_QUIESCE_STATUS_CHANGE_CHECKER_H_

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

@@ -5,7 +5,7 @@
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 
 #include "chrome/browser/sync/profile_sync_service.h"
-#include "chrome/browser/sync/profile_sync_service_observer.h"
+#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
 
 SingleClientStatusChangeChecker::SingleClientStatusChangeChecker(
     ProfileSyncService* service) : service_(service) {}
@@ -13,11 +13,11 @@ SingleClientStatusChangeChecker::SingleClientStatusChangeChecker(
 SingleClientStatusChangeChecker::~SingleClientStatusChangeChecker() {}
 
 void SingleClientStatusChangeChecker::InitObserver(
-    ProfileSyncServiceObserver* obs) {
+    ProfileSyncServiceHarness* obs) {
   service()->AddObserver(obs);
 }
 
 void SingleClientStatusChangeChecker::UninitObserver(
-    ProfileSyncServiceObserver* obs) {
+    ProfileSyncServiceHarness* obs) {
   service()->RemoveObserver(obs);
 }

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

@@ -9,7 +9,7 @@
 #include "chrome/browser/sync/test/integration/status_change_checker.h"
 
 class ProfileSyncService;
-class ProfileSyncServiceObserver;
+class ProfileSyncServiceHarness;
 
 // This class provides some common functionality for StatusChangeCheckers that
 // observe only one ProfileSyncService.  This class is abstract.  Its
@@ -22,8 +22,8 @@ class SingleClientStatusChangeChecker : public StatusChangeChecker {
   // StatusChangeChecker implementations and stubs.
   virtual bool IsExitConditionSatisfied() = 0;
   virtual std::string GetDebugMessage() const = 0;
-  virtual void InitObserver(ProfileSyncServiceObserver*) OVERRIDE;
-  virtual void UninitObserver(ProfileSyncServiceObserver*) OVERRIDE;
+  virtual void InitObserver(ProfileSyncServiceHarness* obs) OVERRIDE;
+  virtual void UninitObserver(ProfileSyncServiceHarness* obs) OVERRIDE;
 
  protected:
   ProfileSyncService* service() { return service_; }

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

@@ -7,7 +7,7 @@
 
 #include <string>
 
-class ProfileSyncServiceObserver;
+class ProfileSyncServiceHarness;
 
 // Interface for a helper class that can be used to check if a desired change in
 // the state of the sync engine has taken place. Used by the desktop sync
@@ -29,8 +29,8 @@ class StatusChangeChecker {
   // or "AwaitMigrationDone(BOOKMARKS)".
   virtual std::string GetDebugMessage() const = 0;
 
-  virtual void InitObserver(ProfileSyncServiceObserver*) = 0;
-  virtual void UninitObserver(ProfileSyncServiceObserver*) = 0;
+  virtual void InitObserver(ProfileSyncServiceHarness*) = 0;
+  virtual void UninitObserver(ProfileSyncServiceHarness*) = 0;
 
  protected:
   virtual ~StatusChangeChecker();

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

+// Copyright 2014 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/updated_progress_marker_checker.h"
+
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "sync/internal_api/public/sessions/sync_session_snapshot.h"
+
+UpdatedProgressMarkerChecker::UpdatedProgressMarkerChecker(
+    ProfileSyncService* service) : SingleClientStatusChangeChecker(service) {}
+
+UpdatedProgressMarkerChecker::~UpdatedProgressMarkerChecker() {}
+
+bool UpdatedProgressMarkerChecker::IsExitConditionSatisfied() {
+  // Checks to see if our self-notify sync cycle has completed and
+  // there's nothing to commit.
+  //
+  // If we assume that no one else is committing at this time and that the
+  // current client did not commit anything in its previous sync cycle, then
+  // this client has the latest progress markers.
+  //
+  // The !service()->HasUnsyncedItems() check makes sure that we have nothing to
+  // commit.
+  //
+  // There is a subtle race condition here.  While committing items, the syncer
+  // will unset the IS_UNSYNCED bits in the directory.  However, the evidence of
+  // this current sync cycle won't be available from GetLastSessionSnapshot()
+  // until the sync cycle completes.  If we query this condition between the
+  // commit response processing and the end of the sync cycle, we could return a
+  // false positive.
+  //
+  // In practice, this doesn't happen very often because we only query the
+  // status when the waiting first starts and when we receive notification of a
+  // sync session complete or other significant event from the
+  // ProfileSyncService.  If we're calling this right after the sync session
+  // completes, then the snapshot is much more likely to be up to date.
+  const syncer::sessions::SyncSessionSnapshot& snap =
+      service()->GetLastSessionSnapshot();
+  return snap.model_neutral_state().num_successful_commits == 0 &&
+         !service()->HasUnsyncedItems();
+}
+
+std::string UpdatedProgressMarkerChecker::GetDebugMessage() const {
+  return "Waiting for progress markers";
+}

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

+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_SYNC_TEST_INTEGRATION_UPDATED_PROGRESS_MARKER_CHECKER_H_
+#define CHROME_BROWSER_SYNC_TEST_INTEGRATION_UPDATED_PROGRESS_MARKER_CHECKER_H_
+
+#include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
+
+// Waits until the latest progress markers are available.
+//
+// There are several limitations to this checker:
+// - It assumes that this client is the only one committing at this time.
+// - It relies on the test-only 'self-notify' to trigger an extra GetUpdate
+//   cycle after every commit.
+// - It's flaky.  In some rare cases, the IsExitConditionSatisifed() call could
+//   return a false positive.  See comments in the .cc file for details.
+//
+// Because of these limitations, we intend to eventually migrate all tests off
+// of this checker.  Please do not use it in new tests.
+class UpdatedProgressMarkerChecker : public SingleClientStatusChangeChecker {
+ public:
+  explicit UpdatedProgressMarkerChecker(ProfileSyncService* service);
+  virtual ~UpdatedProgressMarkerChecker();
+
+  virtual bool IsExitConditionSatisfied() OVERRIDE;
+  virtual std::string GetDebugMessage() const OVERRIDE;
+};
+
+#endif  // CHROME_BROWSER_SYNC_TEST_INTEGRATION_UPDATED_PROGRESS_MARKER_CHECKER_H_

chrome/chrome_tests.gypi

@@ -2372,6 +2372,8 @@
         'browser/sync/test/integration/preferences_helper.h',
         'browser/sync/test/integration/profile_sync_service_harness.cc',
         'browser/sync/test/integration/profile_sync_service_harness.h',
+        'browser/sync/test/integration/quiesce_status_change_checker.cc',
+        'browser/sync/test/integration/quiesce_status_change_checker.h',
         'browser/sync/test/integration/retry_verifier.cc',
         'browser/sync/test/integration/retry_verifier.h',
         'browser/sync/test/integration/search_engines_helper.cc',
@@ -2396,6 +2398,8 @@
         'browser/sync/test/integration/themes_helper.h',
         'browser/sync/test/integration/typed_urls_helper.cc',
         'browser/sync/test/integration/typed_urls_helper.h',
+        'browser/sync/test/integration/updated_progress_marker_checker.cc',
+        'browser/sync/test/integration/updated_progress_marker_checker.h',
       ],
       'conditions': [
         ['OS=="mac"', {