sync: abort active HTTP requests on shutdown.

chrome/browser/sync/engine/net/server_connection_manager.cc

Index: chrome/browser/sync/engine/net/server_connection_manager.cc
diff --git a/chrome/browser/sync/engine/net/server_connection_manager.cc b/chrome/browser/sync/engine/net/server_connection_manager.cc
index 9f0b5518e63878e8c9b34964d66da28b0d4d3e5e..bceb18713f1e6e192b78d169917e8f2cdb6e9edd 100644
--- a/chrome/browser/sync/engine/net/server_connection_manager.cc
+++ b/chrome/browser/sync/engine/net/server_connection_manager.cc
@@ -58,7 +58,14 @@ const char* HttpResponse::GetServerConnectionCodeString(
 
 #undef ENUM_CASE
 
-bool ServerConnectionManager::Post::ReadBufferResponse(
+ServerConnectionManager::Connection::Connection(
+    ServerConnectionManager* scm) : scm_(scm) {
+}
+
+ServerConnectionManager::Connection::~Connection() {
+}
+
+bool ServerConnectionManager::Connection::ReadBufferResponse(
     string* buffer_out,
     HttpResponse* response,
     bool require_response) {
@@ -79,7 +86,7 @@ bool ServerConnectionManager::Post::ReadBufferResponse(
   return true;
 }
 
-bool ServerConnectionManager::Post::ReadDownloadResponse(
+bool ServerConnectionManager::Connection::ReadDownloadResponse(
     HttpResponse* response,
     string* buffer_out) {
   const int64 bytes_read = ReadResponse(buffer_out,
@@ -94,6 +101,21 @@ bool ServerConnectionManager::Post::ReadDownloadResponse(
   return true;
 }
 
+ServerConnectionManager::ScopedConnectionHelper::ScopedConnectionHelper(
+    ServerConnectionManager* manager, Connection* connection)
+    : manager_(manager), connection_(connection) {}
+
+ServerConnectionManager::ScopedConnectionHelper::~ScopedConnectionHelper() {
+  if (connection_.get())
+    manager_->OnConnectionDestroyed(connection_.get());
+  connection_.reset();
+}
+
+ServerConnectionManager::Connection*
+ServerConnectionManager::ScopedConnectionHelper::get() {
+  return connection_.get();
+}
+
 namespace {
 
 string StripTrailingSlash(const string& s) {
@@ -108,7 +130,7 @@ string StripTrailingSlash(const string& s) {
 }  // namespace
 
 // TODO(chron): Use a GURL instead of string concatenation.
-string ServerConnectionManager::Post::MakeConnectionURL(
+string ServerConnectionManager::Connection::MakeConnectionURL(
     const string& sync_server,
     const string& path,
     bool use_ssl) const {
@@ -120,8 +142,8 @@ string ServerConnectionManager::Post::MakeConnectionURL(
   return connection_url;
 }
 
-int ServerConnectionManager::Post::ReadResponse(string* out_buffer,
-                                                int length) {
+int ServerConnectionManager::Connection::ReadResponse(string* out_buffer,
+                                                      int length) {
   int bytes_read = buffer_.length();
   CHECK(length <= bytes_read);
   out_buffer->assign(buffer_);
@@ -160,14 +182,39 @@ ServerConnectionManager::ServerConnectionManager(
       get_time_path_(kSyncServerGetTimePath),
       error_count_(0),
       server_status_(HttpResponse::NONE),
-      server_reachable_(false) {
+      server_reachable_(false),
+      terminated_(false),
+      active_connection_(NULL) {
 }
 
 ServerConnectionManager::~ServerConnectionManager() {
 }
 
+ServerConnectionManager::Connection*
+ServerConnectionManager::MakeActiveConnection() {
+  base::AutoLock lock(terminate_connection_lock_);
+  DCHECK(!active_connection_);
+  if (terminated_)
+    return NULL;
+
+  active_connection_ = MakeConnection();
+  return active_connection_;
+}
+
+void ServerConnectionManager::OnConnectionDestroyed(Connection* connection) {
+  DCHECK(connection);
+  base::AutoLock lock(terminate_connection_lock_);
+  // |active_connection_| can be NULL already if it was aborted. Also,
+  // it can legitimately be a different Connection object if a new Connection
+  // was created after a previous one was Aborted and destroyed.
+  if (active_connection_ != connection)
+    return;
+
+  active_connection_ = NULL;
+}
+
 void ServerConnectionManager::NotifyStatusChanged() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   FOR_EACH_OBSERVER(ServerConnectionEventListener, listeners_,
      OnServerConnectionEvent(
          ServerConnectionEvent(server_status_, server_reachable_)));
@@ -175,7 +222,7 @@ void ServerConnectionManager::NotifyStatusChanged() {
 
 bool ServerConnectionManager::PostBufferWithCachedAuth(
     PostBufferParams* params, ScopedServerStatusWatcher* watcher) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   string path =
       MakeSyncServerPath(proto_sync_path(), MakeSyncQueryString(client_id_));
   return PostBufferToPath(params, path, auth_token(), watcher);
@@ -184,7 +231,7 @@ bool ServerConnectionManager::PostBufferWithCachedAuth(
 bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params,
     const string& path, const string& auth_token,
     ScopedServerStatusWatcher* watcher) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(watcher != NULL);
 
   if (auth_token.empty()) {
@@ -192,9 +239,18 @@ bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params,
     return false;
   }
 
-  scoped_ptr<Post> post(MakePost());
-  bool ok = post->Init(path.c_str(), auth_token, params->buffer_in,
-                       &params->response);
+  // When our connection object falls out of scope, it clears itself from
+  // active_connection_.
+  ScopedConnectionHelper post(this, MakeActiveConnection());
+  if (!post.get()) {
+    params->response.server_status = HttpResponse::CONNECTION_UNAVAILABLE;
+    return false;
+  }
+
+  // Note that |post| may be aborted by now, which will just cause Init to fail
+  // with CONNECTION_UNAVAILABLE.
+  bool ok = post.get()->Init(
+      path.c_str(), auth_token, params->buffer_in, &params->response);
 
   if (params->response.server_status == HttpResponse::SYNC_AUTH_ERROR) {
     InvalidateAndClearAuthToken();
@@ -205,7 +261,8 @@ bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params,
     return false;
   }
 
-  if (post->ReadBufferResponse(&params->buffer_out, &params->response, true)) {
+  if (post.get()->ReadBufferResponse(
+      &params->buffer_out, &params->response, true)) {
     params->response.server_status = HttpResponse::SERVER_CONNECTION_OK;
     server_reachable_ = true;
     return true;
@@ -214,7 +271,8 @@ bool ServerConnectionManager::PostBufferToPath(PostBufferParams* params,
 }
 
 bool ServerConnectionManager::CheckTime(int32* out_time) {
-  DCHECK(CalledOnValidThread());
+  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.
@@ -223,7 +281,9 @@ bool ServerConnectionManager::CheckTime(int32* out_time) {
   string post_body = "command=get_time";
 
   for (int i = 0 ; i < 3;  i++) {
-    scoped_ptr<Post> post(MakePost());
+    ScopedConnectionHelper post(this, MakeActiveConnection());
+    if (!post.get())
+      break;
 
     // Note that the server's get_time path doesn't require authentication.
     string get_time_path =
@@ -231,7 +291,7 @@ bool ServerConnectionManager::CheckTime(int32* out_time) {
     VLOG(1) << "Requesting get_time from:" << get_time_path;
 
     string blank_post_body;
-    bool ok = post->Init(get_time_path.c_str(), blank_post_body,
+    bool ok = post.get()->Init(get_time_path.c_str(), blank_post_body,
         blank_post_body, &response);
     if (!ok) {
       VLOG(1) << "Unable to check the time";
@@ -240,7 +300,7 @@ bool ServerConnectionManager::CheckTime(int32* out_time) {
     string time_response;
     time_response.resize(
         static_cast<string::size_type>(response.content_length));
-    ok = post->ReadDownloadResponse(&response, &time_response);
+    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:"
@@ -256,18 +316,18 @@ bool ServerConnectionManager::CheckTime(int32* out_time) {
 }
 
 bool ServerConnectionManager::IsServerReachable() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   int32 time;
   return CheckTime(&time);
 }
 
 bool ServerConnectionManager::IsUserAuthenticated() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   return IsGoodReplyFromServer(server_status_);
 }
 
 bool ServerConnectionManager::CheckServerReachable() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   const bool server_is_reachable = IsServerReachable();
   if (server_reachable_ != server_is_reachable) {
     server_reachable_ = server_is_reachable;
@@ -277,7 +337,7 @@ bool ServerConnectionManager::CheckServerReachable() {
 }
 
 bool ServerConnectionManager::IncrementErrorCount() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   error_count_++;
 
   if (error_count_ > kMaxConnectionErrorsBeforeReset) {
@@ -298,7 +358,7 @@ bool ServerConnectionManager::IncrementErrorCount() {
 void ServerConnectionManager::SetServerParameters(const string& server_url,
                                                   int port,
                                                   bool use_ssl) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   sync_server_ = server_url;
   sync_server_port_ = port;
   use_ssl_ = use_ssl;
@@ -333,20 +393,32 @@ std::string ServerConnectionManager::GetServerHost() const {
 
 void ServerConnectionManager::AddListener(
     ServerConnectionEventListener* listener) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   listeners_.AddObserver(listener);
 }
 
 void ServerConnectionManager::RemoveListener(
     ServerConnectionEventListener* listener) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(thread_checker_.CalledOnValidThread());
   listeners_.RemoveObserver(listener);
 }
 
-ServerConnectionManager::Post* ServerConnectionManager::MakePost() {
+ServerConnectionManager::Connection* ServerConnectionManager::MakeConnection()
+{
   return NULL;  // For testing.
 }
 
+void ServerConnectionManager::TerminateAllIO() {
+  base::AutoLock lock(terminate_connection_lock_);
+  terminated_ = true;
+  if (active_connection_)
+    active_connection_->Abort();
+
+  // Sever our ties to this connection object. Note that it still may exist,
+  // since we don't own it, but it has been neutered.
+  active_connection_ = NULL;
+}
+
 bool FillMessageWithShareDetails(sync_pb::ClientToServerMessage* csm,
                                  syncable::DirectoryManager* manager,
                                  const std::string& share) {