Trim code form sync's ServerConnectionManager

- Remove 'authenticated' from SyncManager::Status.  This variable was
set to true iff the last attempt to contact the sync server resulted in
a 200 OK response.  There are lots of problems with this approach.
See also: crbug.com/117611.

The about:sync page also displays auth errors with
MakeSyncAuthErrorText(), which is much more likely to be accurate

- Remove server_up from SyncManager::Status.  It used to be set to true
iff the last server response was SERVER_CONNECTION_OK.  That doesn't
make a lot of sense and isn't very useful.

- Remove server_reachable from SyncManager::Status.  This was toggled
only when ServerConnectionManager::CheckServerReachable() failed, which
didn't happen very often.  CheckServerReachable() has since been
removed, so I think it now gets set only at startup and on the first
successful contact with the server.

- Remove server_reachable from ServerConnectionEvent and
ServerConnectionManager, for similar reasons.

- Remove the methods CheckTime(), IsServerReachable(),
IsUserAuthenticated(), and CheckServerReachable(),
SetServerParameters(), server_reachable() from ServerConnectionManager.
These methods were not used anywhere.  Also removed
IsGoodReplyFromServer(), whose last remaining callsite has been removed
in this commit.

- Remove the server_connection_ok_ flag from SyncScheduler.

- Hard-code AllStatus' online status to true.  See TODO comment.

- Remove OnCredentialsUpdated() and OnConnectionStatusChange() from
SyncScheduler.  Their functionality has been moved to the SyncManager.

- Rearrange some code in MockConnectionManager.  Apparently there used
to be such a thing as an "AUTHENTICATE" request, which we had unit tests
for.  That no longer exists, so I've rewritten some parts of this mock
class to better relfect ServerConnectionManager's current authentication
mechanisms.

- Update various unit tests in response to these changes.

- Update some integration tests to verify backoff behaviour when the
sync server is unreachable.  This replaces the old behaviour of
verifying that certain flags were set in the sync snapshot.

- Reduce the number of retries required to verify backoff.  This is to
help ensure that our integration tests fall within the 45s time limit.

- Disable notifications when disabling network connectivity in
integration tests.  This improves the reliability of exponential backoff
verification because the notifications would sometimes interfere with
backoff logic.


BUG=110954,105739,98346,106262
TEST=


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

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

10 files changed, 61 insertions, 230 deletions

diff --git a/sync/engine/net/server_connection_manager.cc b/sync/engine/net/server_connection_manager.cc
index 94d01f2..dcbe861 100644
--- a/sync/engine/net/server_connection_manager.cc
+++ b/sync/engine/net/server_connection_manager.cc
@@ -152,8 +152,7 @@ int ServerConnectionManager::Connection::ReadResponse(string* out_buffer,
 ScopedServerStatusWatcher::ScopedServerStatusWatcher(
     ServerConnectionManager* conn_mgr, HttpResponse* response)
     : conn_mgr_(conn_mgr),
-      response_(response),
-      server_reachable_(conn_mgr->server_reachable_) {
+      response_(response) {
   response->server_status = conn_mgr->server_status_;
 }
 
@@ -163,9 +162,6 @@ ScopedServerStatusWatcher::~ScopedServerStatusWatcher() {
     conn_mgr_->NotifyStatusChanged();
     return;
   }
-  // Notify if we've gone on or offline.
-  if (server_reachable_ != conn_mgr_->server_reachable_)
-    conn_mgr_->NotifyStatusChanged();
 }
 
 ServerConnectionManager::ServerConnectionManager(
@@ -180,7 +176,6 @@ ServerConnectionManager::ServerConnectionManager(
       proto_sync_path_(kSyncServerSyncPath),
       get_time_path_(kSyncServerGetTimePath),
       server_status_(HttpResponse::NONE),
-      server_reachable_(false),
       terminated_(false),
       active_connection_(NULL) {
 }
@@ -215,7 +210,7 @@ void ServerConnectionManager::NotifyStatusChanged() {
   DCHECK(thread_checker_.CalledOnValidThread());
   FOR_EACH_OBSERVER(ServerConnectionEventListener, listeners_,
      OnServerConnectionEvent(
-         ServerConnectionEvent(server_status_, server_reachable_)));
+         ServerConnectionEvent(server_status_)));
 }
 
 bool ServerConnectionManager::PostBufferWithCachedAuth(
@@ -259,86 +254,11 @@ bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params,
   if (post.get()->ReadBufferResponse(
       &params->buffer_out, &params->response, true)) {
     params->response.server_status = HttpResponse::SERVER_CONNECTION_OK;
-    server_reachable_ = true;
     return true;
   }
   return false;
 }
 
-bool ServerConnectionManager::CheckTime(int32* out_time) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-
-  // Verify that the server really is reachable by checking the time. We need
-  // to do this because of wifi interstitials that intercept messages from the
-  // client and return HTTP OK instead of a redirect.
-  HttpResponse response;
-  ScopedServerStatusWatcher watcher(this, &response);
-  string post_body = "command=get_time";
-
-  for (int i = 0 ; i < 3;  i++) {
-    ScopedConnectionHelper post(this, MakeActiveConnection());
-    if (!post.get())
-      break;
-
-    // Note that the server's get_time path doesn't require authentication.
-    string get_time_path =
-        MakeSyncServerPath(kSyncServerGetTimePath, post_body);
-    DVLOG(1) << "Requesting get_time from:" << get_time_path;
-
-    string blank_post_body;
-    bool ok = post.get()->Init(get_time_path.c_str(), blank_post_body,
-        blank_post_body, &response);
-    if (!ok) {
-      DVLOG(1) << "Unable to check the time";
-      continue;
-    }
-    string time_response;
-    time_response.resize(
-        static_cast<string::size_type>(response.content_length));
-    ok = post.get()->ReadDownloadResponse(&response, &time_response);
-    if (!ok || string::npos !=
-        time_response.find_first_not_of("0123456789")) {
-      LOG(ERROR) << "unable to read a non-numeric response from get_time:"
-            << time_response;
-      continue;
-    }
-    *out_time = atoi(time_response.c_str());
-    DVLOG(1) << "Server was reachable.";
-    return true;
-  }
-  return false;
-}
-
-bool ServerConnectionManager::IsServerReachable() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  int32 time;
-  return CheckTime(&time);
-}
-
-bool ServerConnectionManager::IsUserAuthenticated() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  return IsGoodReplyFromServer(server_status_);
-}
-
-bool ServerConnectionManager::CheckServerReachable() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  const bool server_is_reachable = IsServerReachable();
-  if (server_reachable_ != server_is_reachable) {
-    server_reachable_ = server_is_reachable;
-    NotifyStatusChanged();
-  }
-  return server_is_reachable;
-}
-
-void ServerConnectionManager::SetServerParameters(const string& server_url,
-                                                  int port,
-                                                  bool use_ssl) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  sync_server_ = server_url;
-  sync_server_port_ = port;
-  use_ssl_ = use_ssl;
-}
-
 // Returns the current server parameters in server_url and port.
 void ServerConnectionManager::GetServerParameters(string* server_url,
                                                   int* port,
diff --git a/sync/engine/net/server_connection_manager.h b/sync/engine/net/server_connection_manager.h
index 6347223..2121d8e 100644
--- a/sync/engine/net/server_connection_manager.h
+++ b/sync/engine/net/server_connection_manager.h
@@ -60,10 +60,6 @@ struct HttpResponse {
     // minimal changes prevail for the moment.  Fix this! Bug 35060.
     SYNC_AUTH_ERROR,
 
-    // All the following connection codes are valid responses from the server.
-    // Means the server is up.  If you update this list, be sure to also update
-    // IsGoodReplyFromServer().
-
     // SERVER_CONNECTION_OK is returned when request was handled correctly.
     SERVER_CONNECTION_OK,
 
@@ -96,16 +92,10 @@ struct HttpResponse {
       ServerConnectionCode code);
 };
 
-inline bool IsGoodReplyFromServer(HttpResponse::ServerConnectionCode code) {
-  return code >= HttpResponse::SERVER_CONNECTION_OK;
-}
-
 struct ServerConnectionEvent {
   HttpResponse::ServerConnectionCode connection_code;
-  bool server_reachable;
-  ServerConnectionEvent(HttpResponse::ServerConnectionCode code,
-                         bool server_reachable) :
-      connection_code(code), server_reachable(server_reachable) {}
+  explicit ServerConnectionEvent(HttpResponse::ServerConnectionCode code) :
+      connection_code(code) {}
 };
 
 class ServerConnectionEventListener {
@@ -127,7 +117,6 @@ class ScopedServerStatusWatcher : public base::NonThreadSafe {
  private:
   ServerConnectionManager* const conn_mgr_;
   HttpResponse* const response_;
-  bool server_reachable_;
   DISALLOW_COPY_AND_ASSIGN(ScopedServerStatusWatcher);
 };
 
@@ -201,20 +190,6 @@ class ServerConnectionManager {
   virtual bool PostBufferWithCachedAuth(PostBufferParams* params,
                                         ScopedServerStatusWatcher* watcher);
 
-  // Checks the time on the server. Returns false if the request failed. |time|
-  // is an out parameter that stores the value returned from the server.
-  virtual bool CheckTime(int32* out_time);
-
-  // Returns true if sync_server_ is reachable. This method verifies that the
-  // server is pingable and that traffic can be sent to and from it.
-  virtual bool IsServerReachable();
-
-  // Returns true if user has been successfully authenticated.
-  virtual bool IsUserAuthenticated();
-
-  // Updates status and broadcasts events on change.
-  bool CheckServerReachable();
-
   void AddListener(ServerConnectionEventListener* listener);
   void RemoveListener(ServerConnectionEventListener* listener);
 
@@ -225,18 +200,8 @@ class ServerConnectionManager {
     return server_status_;
   }
 
-  inline bool server_reachable() const { return server_reachable_; }
-
   const std::string client_id() const { return client_id_; }
 
-  // This changes the server info used by the connection manager. This allows
-  // a single client instance to talk to different backing servers. This is
-  // typically called during / after authentication so that the server url
-  // can be a function of the user's login id.
-  void SetServerParameters(const std::string& server_url,
-                           int port,
-                           bool use_ssl);
-
   // Returns the current server parameters in server_url, port and use_ssl.
   void GetServerParameters(std::string* server_url,
                            int* port,
@@ -280,6 +245,10 @@ class ServerConnectionManager {
     }
   }
 
+  bool HasInvalidAuthToken() {
+    return auth_token_.empty();
+  }
+
   const std::string auth_token() const {
     DCHECK(thread_checker_.CalledOnValidThread());
     return auth_token_;
@@ -330,7 +299,7 @@ class ServerConnectionManager {
   std::string proto_sync_path_;
   std::string get_time_path_;
 
-  // The auth token to use in authenticated requests. Set by the AuthWatcher.
+  // The auth token to use in authenticated requests.
   std::string auth_token_;
 
   // The previous auth token that is invalid now.
@@ -339,7 +308,6 @@ class ServerConnectionManager {
   ObserverList<ServerConnectionEventListener> listeners_;
 
   HttpResponse::ServerConnectionCode server_status_;
-  bool server_reachable_;
 
   base::ThreadChecker thread_checker_;
 
diff --git a/sync/engine/sync_scheduler.cc b/sync/engine/sync_scheduler.cc
index f03b727..87304ea 100644
--- a/sync/engine/sync_scheduler.cc
+++ b/sync/engine/sync_scheduler.cc
@@ -195,7 +195,6 @@ SyncScheduler::SyncScheduler(const std::string& name,
       mode_(NORMAL_MODE),
       // Start with assuming everything is fine with the connection.
       // At the end of the sync cycle we would have the correct status.
-      server_connection_ok_(true),
       connection_code_(HttpResponse::SERVER_CONNECTION_OK),
       delay_provider_(new DelayProvider()),
       syncer_(syncer),
@@ -231,9 +230,7 @@ void SyncScheduler::OnConnectionStatusChange() {
 }
 
 void SyncScheduler::OnServerConnectionErrorFixed() {
-  DCHECK(!server_connection_ok_);
   connection_code_ = HttpResponse::SERVER_CONNECTION_OK;
-  server_connection_ok_ = true;
   PostTask(FROM_HERE, "DoCanaryJob",
            base::Bind(&SyncScheduler::DoCanaryJob,
                       weak_ptr_factory_.GetWeakPtr()));
@@ -245,32 +242,8 @@ void SyncScheduler::UpdateServerConnectionManagerStatus(
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
   SDVLOG(2) << "New server connection code: "
             << HttpResponse::GetServerConnectionCodeString(code);
-  bool old_server_connection_ok = server_connection_ok_;
 
   connection_code_ = code;
-
-  // Note, be careful when adding cases here because if the SyncScheduler
-  // thinks there is no valid connection as determined by this method, it
-  // will drop out of *all* forward progress sync loops (it won't poll and it
-  // will queue up Talk notifications but not actually call SyncShare) until
-  // some external action causes a ServerConnectionManager to broadcast that
-  // a valid connection has been re-established
-  if (HttpResponse::CONNECTION_UNAVAILABLE == code ||
-      HttpResponse::SYNC_AUTH_ERROR == code) {
-    server_connection_ok_ = false;
-    SDVLOG(2) << "Sync auth error or unavailable connection: "
-              << "server connection is down";
-  } else if (HttpResponse::SERVER_CONNECTION_OK == code) {
-    server_connection_ok_ = true;
-    SDVLOG(2) << "Sync server connection is ok: "
-              << "server connection is up, doing canary job";
-  }
-
-  if (old_server_connection_ok != server_connection_ok_) {
-    const char* transition =
-        server_connection_ok_ ? "down -> up" : "up -> down";
-    SDVLOG(2) << "Server connection changed: " << transition;
-  }
 }
 
 void SyncScheduler::Start(Mode mode, const base::Closure& callback) {
@@ -403,10 +376,10 @@ SyncScheduler::JobProcessDecision SyncScheduler::DecideOnJob(
     return DROP;
   }
 
-  if (server_connection_ok_)
+  if (!session_context_->connection_manager()->HasInvalidAuthToken())
     return CONTINUE;
 
-  SDVLOG(2) << "Bad server connection. Using that to decide on job.";
+  SDVLOG(2) << "No valid auth token. Using that to decide on job.";
   return job.purpose == SyncSessionJob::NUDGE ? SAVE : DROP;
 }
 
@@ -873,15 +846,14 @@ void SyncScheduler::FinishSyncSessionJob(const SyncSessionJob& job) {
   }
   last_sync_session_end_time_ = now;
 
-  // Now update the status of the connection from SCM. We need this
-  // to decide whether we need to save/run future jobs. The notifications
-  // from SCM are not reliable.
+  // Now update the status of the connection from SCM. We need this to decide
+  // whether we need to save/run future jobs. The notifications from SCM are not
+  // reliable.
+  //
   // TODO(rlarocque): crbug.com/110954
-  // We should get rid of the notifications and
-  // it is probably not needed to maintain this status variable
-  // in 2 places. We should query it directly from SCM when needed.
-  // But that would need little more refactoring(including a method to
-  // query if the auth token is invalid) from SCM side.
+  // We should get rid of the notifications and it is probably not needed to
+  // maintain this status variable in 2 places. We should query it directly from
+  // SCM when needed.
   ServerConnectionManager* scm = session_context_->connection_manager();
   UpdateServerConnectionManagerStatus(scm->server_status());
 
diff --git a/sync/engine/sync_scheduler.h b/sync/engine/sync_scheduler.h
index 8540484..d5ff4c0 100644
--- a/sync/engine/sync_scheduler.h
+++ b/sync/engine/sync_scheduler.h
@@ -346,7 +346,7 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
                                 SyncerStep* start,
                                 SyncerStep* end);
 
-  // Used to update |server_connection_ok_|, see below.
+  // Used to update |connection_code_|, see below.
   void UpdateServerConnectionManagerStatus(
       HttpResponse::ServerConnectionCode code);
 
@@ -394,9 +394,6 @@ class SyncScheduler : public sessions::SyncSession::Delegate {
   // since the nudges could be for different types. Current impl doesn't care.
   base::TimeTicks last_sync_session_end_time_;
 
-  // Have we observed a valid server connection?
-  bool server_connection_ok_;
-
   // The latest connection code we got while trying to connect.
   HttpResponse::ServerConnectionCode connection_code_;
 
diff --git a/sync/engine/sync_scheduler_unittest.cc b/sync/engine/sync_scheduler_unittest.cc
index d90ce23..a48d67f 100644
--- a/sync/engine/sync_scheduler_unittest.cc
+++ b/sync/engine/sync_scheduler_unittest.cc
@@ -1124,8 +1124,9 @@ TEST_F(SyncSchedulerTest, DISABLED_NoConfigDuringNormal) {
 // break things when a connection is detected.
 TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
   connection()->SetServerNotReachable();
+  connection()->UpdateConnectionStatus();
   EXPECT_CALL(*syncer(), SyncShare(_,_,_))
-    .WillOnce(Invoke(sessions::test_util::SimulateDownloadUpdatesFailed))
+    .WillOnce(Invoke(sessions::test_util::SimulateConnectionFailure))
     .WillOnce(QuitLoopNowAction());
   StartSyncScheduler(SyncScheduler::NORMAL_MODE);
   MessageLoop::current()->RunAllPending();
@@ -1136,6 +1137,7 @@ TEST_F(SyncSchedulerTest, StartWhenNotConnected) {
   MessageLoop::current()->RunAllPending();
 
   connection()->SetServerReachable();
+  connection()->UpdateConnectionStatus();
   scheduler()->OnConnectionStatusChange();
   MessageLoop::current()->RunAllPending();
 }
diff --git a/sync/engine/sync_scheduler_whitebox_unittest.cc b/sync/engine/sync_scheduler_whitebox_unittest.cc
index 35a0955..33ddb29 100644
--- a/sync/engine/sync_scheduler_whitebox_unittest.cc
+++ b/sync/engine/sync_scheduler_whitebox_unittest.cc
@@ -34,7 +34,6 @@ class SyncSchedulerWhiteboxTest : public testing::Test {
     routes[syncable::NIGORI] = GROUP_PASSIVE;
     registrar_.reset(new FakeModelSafeWorkerRegistrar(routes));
     connection_.reset(new MockConnectionManager(NULL));
-    connection_->SetServerReachable();
     context_ =
         new SyncSessionContext(
             connection_.get(), dir_maker_.directory(),
@@ -58,10 +57,6 @@ class SyncSchedulerWhiteboxTest : public testing::Test {
     scheduler_->last_sync_session_end_time_ = ticks;
   }
 
-  void SetServerConnection(bool connected) {
-    scheduler_->server_connection_ok_ = connected;
-  }
-
   void ResetWaitInterval() {
     scheduler_->wait_interval_.reset();
   }
@@ -90,7 +85,6 @@ class SyncSchedulerWhiteboxTest : public testing::Test {
   void InitializeSyncerOnNormalMode() {
     SetMode(SyncScheduler::NORMAL_MODE);
     ResetWaitInterval();
-    SetServerConnection(true);
     SetLastSyncedTime(base::TimeTicks::Now());
   }
 
diff --git a/sync/sessions/test_util.cc b/sync/sessions/test_util.cc
index 7e9b54b..387dbf6 100644
--- a/sync/sessions/test_util.cc
+++ b/sync/sessions/test_util.cc
@@ -26,6 +26,12 @@ void SimulateCommitFailed(sessions::SyncSession* session,
       SERVER_RETURN_TRANSIENT_ERROR);
 }
 
+void SimulateConnectionFailure(sessions::SyncSession* session,
+                               SyncerStep begin, SyncerStep end) {
+  session->mutable_status_controller()->set_last_download_updates_result(
+      NETWORK_CONNECTION_UNAVAILABLE);
+}
+
 void SimulateSuccess(sessions::SyncSession* session,
                      SyncerStep begin, SyncerStep end) {
   if (session->HasMoreToSync()) {
diff --git a/sync/sessions/test_util.h b/sync/sessions/test_util.h
index b8ecf8f..aad02e2 100644
--- a/sync/sessions/test_util.h
+++ b/sync/sessions/test_util.h
@@ -22,6 +22,8 @@ void SimulateDownloadUpdatesFailed(sessions::SyncSession* session,
                                    SyncerStep begin, SyncerStep end);
 void SimulateCommitFailed(sessions::SyncSession* session,
                           SyncerStep begin, SyncerStep end);
+void SimulateConnectionFailure(sessions::SyncSession* session,
+                          SyncerStep begin, SyncerStep end);
 void SimulateSuccess(sessions::SyncSession* session,
                      SyncerStep begin, SyncerStep end);
 void SimulateThrottledImpl(sessions::SyncSession* session,
diff --git a/sync/test/engine/mock_connection_manager.cc b/sync/test/engine/mock_connection_manager.cc
index f73b14e..ab95bce 100644
--- a/sync/test/engine/mock_connection_manager.cc
+++ b/sync/test/engine/mock_connection_manager.cc
@@ -36,8 +36,11 @@ using syncable::MODEL_TYPE_COUNT;
 using syncable::ModelType;
 using syncable::WriteTransaction;
 
+static char kValidAuthToken[] = "AuthToken";
+
 MockConnectionManager::MockConnectionManager(syncable::Directory* directory)
     : ServerConnectionManager("unused", 0, false, "version"),
+      server_reachable_(true),
       conflict_all_commits_(false),
       conflict_n_commits_(0),
       next_new_id_(10000),
@@ -55,8 +58,8 @@ MockConnectionManager::MockConnectionManager(syncable::Directory* directory)
       next_position_in_parent_(2),
       use_legacy_bookmarks_protocol_(false),
       num_get_updates_requests_(0) {
-  server_reachable_ = true;
   SetNewTimestamp(0);
+  set_auth_token(kValidAuthToken);
 }
 
 MockConnectionManager::~MockConnectionManager() {
@@ -97,6 +100,17 @@ bool MockConnectionManager::PostBufferToPath(PostBufferParams* params,
     WriteTransaction wt(FROM_HERE, syncable::UNITTEST, directory_);
   }
 
+  if (auth_token.empty()) {
+    params->response.server_status = HttpResponse::SYNC_AUTH_ERROR;
+    return false;
+  }
+
+  if (auth_token != kValidAuthToken) {
+    // Simulate server-side auth failure.
+    params->response.server_status = HttpResponse::SYNC_AUTH_ERROR;
+    InvalidateAndClearAuthToken();
+  }
+
   if (fail_next_postbuffer_) {
     fail_next_postbuffer_ = false;
     return false;
@@ -129,8 +143,6 @@ bool MockConnectionManager::PostBufferToPath(PostBufferParams* params,
     ProcessCommit(&post, &response);
   } else if (post.message_contents() == ClientToServerMessage::GET_UPDATES) {
     ProcessGetUpdates(&post, &response);
-  } else if (post.message_contents() == ClientToServerMessage::AUTHENTICATE) {
-    ProcessAuthenticate(&post, &response, auth_token);
   } else if (post.message_contents() == ClientToServerMessage::CLEAR_DATA) {
     ProcessClearData(&post, &response);
   } else {
@@ -164,14 +176,6 @@ bool MockConnectionManager::PostBufferToPath(PostBufferParams* params,
   return result;
 }
 
-bool MockConnectionManager::IsServerReachable() {
-  return true;
-}
-
-bool MockConnectionManager::IsUserAuthenticated() {
-  return true;
-}
-
 sync_pb::GetUpdatesResponse* MockConnectionManager::GetUpdateResponse() {
   if (update_queue_.empty()) {
     NextUpdateBatch();
@@ -460,35 +464,6 @@ void MockConnectionManager::ProcessClearData(ClientToServerMessage* csm,
   response->set_error_code(clear_user_data_response_errortype_);
 }
 
-void MockConnectionManager::ProcessAuthenticate(
-    ClientToServerMessage* csm,
-    ClientToServerResponse* response,
-    const std::string& auth_token) {
-  ASSERT_EQ(csm->message_contents(), ClientToServerMessage::AUTHENTICATE);
-  EXPECT_FALSE(auth_token.empty());
-
-  if (auth_token != valid_auth_token_) {
-    response->set_error_code(SyncEnums::AUTH_INVALID);
-    return;
-  }
-
-  response->set_error_code(SyncEnums::SUCCESS);
-  response->mutable_authenticate()->CopyFrom(auth_response_);
-  auth_response_.Clear();
-}
-
-void MockConnectionManager::SetAuthenticationResponseInfo(
-    const std::string& valid_auth_token,
-    const std::string& user_display_name,
-    const std::string& user_display_email,
-    const std::string& user_obfuscated_id) {
-  valid_auth_token_ = valid_auth_token;
-  sync_pb::UserIdentification* user = auth_response_.mutable_user();
-  user->set_display_name(user_display_name);
-  user->set_email(user_display_email);
-  user->set_obfuscated_id(user_obfuscated_id);
-}
-
 bool MockConnectionManager::ShouldConflictThisCommit() {
   bool conflict = false;
   if (conflict_all_commits_) {
@@ -641,19 +616,17 @@ sync_pb::DataTypeProgressMarker const*
 }
 
 void MockConnectionManager::SetServerReachable() {
-  server_status_ = HttpResponse::SERVER_CONNECTION_OK;
   server_reachable_ = true;
-
-  FOR_EACH_OBSERVER(ServerConnectionEventListener, listeners_,
-     OnServerConnectionEvent(
-         ServerConnectionEvent(server_status_, server_reachable_)));
 }
 
 void MockConnectionManager::SetServerNotReachable() {
-  server_status_ = HttpResponse::CONNECTION_UNAVAILABLE;
   server_reachable_ = false;
+}
 
-  FOR_EACH_OBSERVER(ServerConnectionEventListener, listeners_,
-     OnServerConnectionEvent(
-         ServerConnectionEvent(server_status_, server_reachable_)));
+void MockConnectionManager::UpdateConnectionStatus() {
+  if (!server_reachable_) {
+    server_status_ = HttpResponse::CONNECTION_UNAVAILABLE;
+  } else {
+    server_status_ = HttpResponse::SERVER_CONNECTION_OK;
+  }
 }
diff --git a/sync/test/engine/mock_connection_manager.h b/sync/test/engine/mock_connection_manager.h
index c3202a5..a2b5060 100644
--- a/sync/test/engine/mock_connection_manager.h
+++ b/sync/test/engine/mock_connection_manager.h
@@ -41,9 +41,6 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
       const std::string& auth_token,
       browser_sync::ScopedServerStatusWatcher* watcher) OVERRIDE;
 
-  virtual bool IsServerReachable() OVERRIDE;
-  virtual bool IsUserAuthenticated() OVERRIDE;
-
   // Control of commit response.
   void SetMidCommitCallback(const base::Closure& callback);
   void SetMidCommitObserver(MidCommitObserver* observer);
@@ -120,12 +117,6 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
   // issue multiple requests during a sync cycle.
   void NextUpdateBatch();
 
-  // For AUTHENTICATE responses.
-  void SetAuthenticationResponseInfo(const std::string& valid_auth_token,
-                                     const std::string& user_display_name,
-                                     const std::string& user_display_email,
-                                     const std::string& user_obfuscated_id);
-
   void FailNextPostBufferToPathCall() { fail_next_postbuffer_ = true; }
 
   void SetClearUserDataResponseStatus(sync_pb::SyncEnums::ErrorType errortype);
@@ -219,6 +210,12 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
 
   void SetServerNotReachable();
 
+  // Updates our internal state as if we had attempted a connection.  Does not
+  // send notifications as a real connection attempt would.  This is useful in
+  // cases where we're mocking out most of the code that performs network
+  // requests.
+  void UpdateConnectionStatus();
+
   // Return by copy to be thread-safe.
   const std::string store_birthday() {
     base::AutoLock lock(store_birthday_lock_);
@@ -244,9 +241,6 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
   // Functions to handle the various types of server request.
   void ProcessGetUpdates(sync_pb::ClientToServerMessage* csm,
                          sync_pb::ClientToServerResponse* response);
-  void ProcessAuthenticate(sync_pb::ClientToServerMessage* csm,
-                           sync_pb::ClientToServerResponse* response,
-                           const std::string& auth_token);
   void ProcessCommit(sync_pb::ClientToServerMessage* csm,
                      sync_pb::ClientToServerResponse* response_buffer);
   void ProcessClearData(sync_pb::ClientToServerMessage* csm,
@@ -281,6 +275,9 @@ class MockConnectionManager : public browser_sync::ServerConnectionManager {
           sync_pb::DataTypeProgressMarker>& filter,
       syncable::ModelType value);
 
+  // When false, we pretend to have network connectivity issues.
+  bool server_reachable_;
+
   // All IDs that have been committed.
   std::vector<syncable::Id> committed_ids_;