Many small sync changes centered around AllStatus

* Removed num_consecutive_errors and consecutive_transient_error
counters.  These aren't very useful because they tell us nothing about
the cause of the errors.  They lead to confusing bugs, with titles like
"Need a graceful way to recover from a non-zero 'Max consecutive error
count'" (most users commenting on this bug were being throttled).

Further adding to the confusion is a bug in AllStatus that caused this
counter to be reset after every sync cycle, so the value displayed was
the maximum number of consecutive errors *for the most recent sync
cycle*.

* Removed SyncInternal::RaiseAuthNeededEvent().  This method was
undefined.

* Removed notifiable commits counter.  We no longer use P2P
notifications, so the client no longer has any say about whether or not
a commit is "notifiable".  The terminology is out of date and the code
to maintain the count seemed out of place in SyncManager.

* Added some counters to report on the number of commit cycles.  These
counters should provide all the information that used to be made
available through the notifiable commits counter, and more.

BUG=98346
TEST=


Review URL: http://codereview.chromium.org/9269025

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

6 files changed, 1 insertions, 89 deletions

diff --git a/chrome/browser/sync/sessions/session_state.cc b/chrome/browser/sync/sessions/session_state.cc
index d9392f2..18f4e59 100644
--- a/chrome/browser/sync/sessions/session_state.cc
+++ b/chrome/browser/sync/sessions/session_state.cc
@@ -92,8 +92,6 @@ DictionaryValue* DownloadProgressMarkersToValue(
 
 ErrorCounters::ErrorCounters()
     : num_conflicting_commits(0),
-      consecutive_transient_error_commits(0),
-      consecutive_errors(0),
       last_download_updates_result(UNSET),
       last_post_commit_result(UNSET),
       last_process_commit_response_result(UNSET) {
@@ -102,9 +100,6 @@ ErrorCounters::ErrorCounters()
 DictionaryValue* ErrorCounters::ToValue() const {
   DictionaryValue* value = new DictionaryValue();
   value->SetInteger("numConflictingCommits", num_conflicting_commits);
-  value->SetInteger("consecutiveTransientErrorCommits",
-                consecutive_transient_error_commits);
-  value->SetInteger("consecutiveErrors", consecutive_errors);
   return value;
 }
 
diff --git a/chrome/browser/sync/sessions/session_state.h b/chrome/browser/sync/sessions/session_state.h
index 72bcb00..7b96ec8 100644
--- a/chrome/browser/sync/sessions/session_state.h
+++ b/chrome/browser/sync/sessions/session_state.h
@@ -92,15 +92,6 @@ struct ErrorCounters {
 
   int num_conflicting_commits;
 
-  // Number of commits hitting transient errors since the last successful
-  // commit.
-  int consecutive_transient_error_commits;
-
-  // Incremented when get_updates fails, commit fails, and when hitting
-  // transient errors. When any of these succeed, this counter is reset.
-  // TODO(chron): Reduce number of weird counters we use.
-  int consecutive_errors;
-
   // Any protocol errors that we received during this sync session.
   SyncProtocolError sync_protocol_error;
 
diff --git a/chrome/browser/sync/sessions/session_state_unittest.cc b/chrome/browser/sync/sessions/session_state_unittest.cc
index 3220dd1..244ece8e 100644
--- a/chrome/browser/sync/sessions/session_state_unittest.cc
+++ b/chrome/browser/sync/sessions/session_state_unittest.cc
@@ -72,17 +72,11 @@ TEST_F(SessionStateTest, SyncerStatusToValue) {
 TEST_F(SessionStateTest, ErrorCountersToValue) {
   ErrorCounters counters;
   counters.num_conflicting_commits = 1;
-  counters.consecutive_transient_error_commits = 5;
-  counters.consecutive_errors = 3;
 
   scoped_ptr<DictionaryValue> value(counters.ToValue());
-  EXPECT_EQ(3u, value->size());
+  EXPECT_EQ(1u, value->size());
   ExpectDictIntegerValue(counters.num_conflicting_commits,
                          *value, "numConflictingCommits");
-  ExpectDictIntegerValue(counters.consecutive_transient_error_commits,
-                         *value, "consecutiveTransientErrorCommits");
-  ExpectDictIntegerValue(counters.consecutive_errors,
-                         *value, "consecutiveErrors");
 }
 
 TEST_F(SessionStateTest, DownloadProgressMarkersToValue) {
diff --git a/chrome/browser/sync/sessions/status_controller.cc b/chrome/browser/sync/sessions/status_controller.cc
index 95c8c4f..ca9903d 100644
--- a/chrome/browser/sync/sessions/status_controller.cc
+++ b/chrome/browser/sync/sessions/status_controller.cc
@@ -127,26 +127,6 @@ void StatusController::reset_num_conflicting_commits() {
     shared_.error.mutate()->num_conflicting_commits = 0;
 }
 
-void StatusController::set_num_consecutive_transient_error_commits(int value) {
-  if (shared_.error.value().consecutive_transient_error_commits !=
-      value) {
-    shared_.error.mutate()->consecutive_transient_error_commits =
-        value;
-  }
-}
-
-void StatusController::increment_num_consecutive_transient_error_commits_by(
-    int value) {
-  set_num_consecutive_transient_error_commits(
-      shared_.error.value().consecutive_transient_error_commits +
-      value);
-}
-
-void StatusController::set_num_consecutive_errors(int value) {
-  if (shared_.error.value().consecutive_errors != value)
-    shared_.error.mutate()->consecutive_errors = value;
-}
-
 void StatusController::set_num_server_changes_remaining(
     int64 changes_remaining) {
   if (shared_.num_server_changes_remaining.value() != changes_remaining)
@@ -174,16 +154,6 @@ void StatusController::set_unsynced_handles(
   }
 }
 
-void StatusController::increment_num_consecutive_errors() {
-  set_num_consecutive_errors(
-      shared_.error.value().consecutive_errors + 1);
-}
-
-void StatusController::increment_num_consecutive_errors_by(int value) {
-  set_num_consecutive_errors(
-      shared_.error.value().consecutive_errors + value);
-}
-
 void StatusController::increment_num_successful_bookmark_commits() {
   set_num_successful_bookmark_commits(
       shared_.syncer_status.value().num_successful_bookmark_commits + 1);
diff --git a/chrome/browser/sync/sessions/status_controller.h b/chrome/browser/sync/sessions/status_controller.h
index b072577..651e90c6 100644
--- a/chrome/browser/sync/sessions/status_controller.h
+++ b/chrome/browser/sync/sessions/status_controller.h
@@ -207,11 +207,6 @@ class StatusController {
   // A toolbelt full of methods for updating counters and flags.
   void increment_num_conflicting_commits_by(int value);
   void reset_num_conflicting_commits();
-  void set_num_consecutive_transient_error_commits(int value);
-  void increment_num_consecutive_transient_error_commits_by(int value);
-  void set_num_consecutive_errors(int value);
-  void increment_num_consecutive_errors();
-  void increment_num_consecutive_errors_by(int value);
   void set_num_server_changes_remaining(int64 changes_remaining);
   void set_invalid_store(bool invalid_store);
   void set_num_successful_bookmark_commits(int value);
diff --git a/chrome/browser/sync/sessions/status_controller_unittest.cc b/chrome/browser/sync/sessions/status_controller_unittest.cc
index 1b3c17f..a207893 100644
--- a/chrome/browser/sync/sessions/status_controller_unittest.cc
+++ b/chrome/browser/sync/sessions/status_controller_unittest.cc
@@ -28,25 +28,6 @@ TEST_F(StatusControllerTest, GetsDirty) {
   status.increment_num_conflicting_commits_by(1);
   EXPECT_TRUE(status.TestAndClearIsDirty());
 
-  status.set_num_consecutive_transient_error_commits(1);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-
-  status.increment_num_consecutive_transient_error_commits_by(1);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-  status.increment_num_consecutive_transient_error_commits_by(0);
-  EXPECT_FALSE(status.TestAndClearIsDirty());
-
-  status.set_num_consecutive_errors(10);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-  status.set_num_consecutive_errors(10);
-  EXPECT_FALSE(status.TestAndClearIsDirty());  // Only dirty if value changed.
-  status.increment_num_consecutive_errors();
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-  status.increment_num_consecutive_errors_by(1);
-  EXPECT_TRUE(status.TestAndClearIsDirty());
-  status.increment_num_consecutive_errors_by(0);
-  EXPECT_FALSE(status.TestAndClearIsDirty());
-
   status.set_num_server_changes_remaining(30);
   EXPECT_TRUE(status.TestAndClearIsDirty());
 
@@ -98,20 +79,6 @@ TEST_F(StatusControllerTest, ReadYourWrites) {
   status.increment_num_conflicting_commits_by(1);
   EXPECT_EQ(1, status.error().num_conflicting_commits);
 
-  status.set_num_consecutive_transient_error_commits(6);
-  EXPECT_EQ(6, status.error().consecutive_transient_error_commits);
-  status.increment_num_consecutive_transient_error_commits_by(1);
-  EXPECT_EQ(7, status.error().consecutive_transient_error_commits);
-  status.increment_num_consecutive_transient_error_commits_by(0);
-  EXPECT_EQ(7, status.error().consecutive_transient_error_commits);
-
-  status.set_num_consecutive_errors(8);
-  EXPECT_EQ(8, status.error().consecutive_errors);
-  status.increment_num_consecutive_errors();
-  EXPECT_EQ(9, status.error().consecutive_errors);
-  status.increment_num_consecutive_errors_by(2);
-  EXPECT_EQ(11, status.error().consecutive_errors);
-
   status.set_num_server_changes_remaining(13);
   EXPECT_EQ(13, status.num_server_changes_remaining());