Fix the false-positive detection of commit errors

chrome/browser/sync/sessions/sync_session.cc

Index: chrome/browser/sync/sessions/sync_session.cc
diff --git a/chrome/browser/sync/sessions/sync_session.cc b/chrome/browser/sync/sessions/sync_session.cc
index f399d7d7904ed1fee8a5425a8641cfaaab04675e..b210aed622ac680113e823669ca56d74607ff7af 100644
--- a/chrome/browser/sync/sessions/sync_session.cc
+++ b/chrome/browser/sync/sessions/sync_session.cc
@@ -141,7 +141,27 @@ bool SyncSession::HasMoreToSync() const {
       status->conflicts_resolved();
       // Or, we have conflicting updates, but we're making progress on
       // resolving them...
-  }
+}
+
+bool SyncSession::WasUnableToMakeProgress() const {

[rlarocque, 2011/09/20 20:10:03]

This name sucks.  I should call it WasAbleToMakeProgress(), because
!WasAbleToMakeProgress() is more readable than WasUnableToMakeProgress().  

+  VLOG(1) << "Was unable to make progress?";
+  const StatusController* status = status_controller_.get();
+
+  // In the future, we should be able to report more about the cause of the
+  // errors and return an enum value.  Right now that's not possible because
+  // our error detection is based on state that can tell us only that an
+  // error exists, but not much else.
+
+  // We were unable to download all updates, for some unknown reason

[tim (not reviewing), 2011/09/20 16:21:57]

style nit - end comments with periods

+  if (status->num_server_changes_remaining() > 0)
+    return true;
+
+  // Lots of errors could cause this
+  if (status->error().consecutive_errors > 0)

[tim (not reviewing), 2011/09/20 16:21:57]

Are all these errors "all or nothing"?  I'm wondering how / if we can enforce
that going forward...

[rlarocque, 2011/09/20 20:10:03]

In the future, I was hoping we could replace this with a GetError() function
that returns an enum value informing us of what kind of error we experienced, or
what the root cause was, or some data structure that contains both.  

Right now I'm not sure how to define an error of any kind.  I also don't know
what the set of error categories would look like.  So I decided to start simple.
 We can base this function on top of GetError() in the future, if we ever figure
out how to implement it.  

+    return true;
+
+  return false;
+}
 
 }  // namespace sessions
 }  // namespace browser_sync