Trim code from sync's ServerConnectionManager

chrome/browser/sync/internal_api/sync_manager.cc

Index: chrome/browser/sync/internal_api/sync_manager.cc
diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc
index ff6ad0778c1b4acd30d0f87c7f9d56b6746a4c02..6bb25437dc666df807f334cab55d57eb5c05c3fe 100644
--- a/chrome/browser/sync/internal_api/sync_manager.cc
+++ b/chrome/browser/sync/internal_api/sync_manager.cc
@@ -681,9 +681,6 @@ SyncManager::SyncManager(const std::string& name)
 
 SyncManager::Status::Status()
     : summary(INVALID),
-      authenticated(false),
-      server_up(false),
-      server_reachable(false),
       notifications_enabled(false),
       notifications_received(0),
       unsynced_count(0),
@@ -1154,7 +1151,7 @@ void SyncManager::SyncInternal::UpdateCredentials(
         credentials.email, credentials.sync_token);
     if (!setup_for_test_mode_ && initialized_) {
       if (scheduler())
-        scheduler()->OnCredentialsUpdated();
+        scheduler()->OnServerConnectionErrorFixed();

[rlarocque, 2012/02/08 01:57:29]

This bypasses the check of "connection_code_ == SYNC_AUTH_ERROR" in
SyncScheduler::OnCredentialsUpdated().  Although this looks dangerous, I think
it's perfectly safe.  The fact that UpdateCredentials() returns true only when
the auth token actually changed should ensure that we don't accidentally end up
in a loop pinging the server without exponential backoff.

     }
   }
 }
@@ -1721,14 +1718,16 @@ void SyncManager::SyncInternal::OnIPAddressChanged() {
 
 void SyncManager::SyncInternal::OnIPAddressChangedImpl() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (scheduler())
-    scheduler()->OnConnectionStatusChange();
+
+  if (scheduler() && (connection_manager()->server_status() ==
+                      browser_sync::HttpResponse::CONNECTION_UNAVAILABLE)) {

[rlarocque, 2012/02/08 01:57:29]

I've moved the check of the most recent server_status to the
ServerConnectionManager.  I know that you and Tim were arguing against this
move, but I was hoping that seeing it in this context where it allows us to
remove lots of code might change your mind.  

Let me know what you think.  I can revert this change if necessary.  

+    scheduler()->OnServerConnectionErrorFixed();
+  }
 }
 
 void SyncManager::SyncInternal::OnServerConnectionEvent(
     const ServerConnectionEvent& event) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  allstatus_.HandleServerConnectionEvent(event);
   if (event.connection_code ==
       browser_sync::HttpResponse::SERVER_CONNECTION_OK) {
     FOR_EACH_OBSERVER(SyncManager::Observer, observers_,