Revert 222154 "sync: Gracefully handle very early shutdown"

trunk/src/sync/engine/net/server_connection_manager.cc

Index: trunk/src/sync/engine/net/server_connection_manager.cc
===================================================================
--- trunk/src/sync/engine/net/server_connection_manager.cc	(revision 222204)
+++ trunk/src/sync/engine/net/server_connection_manager.cc	(working copy)
@@ -16,7 +16,6 @@
 #include "net/http/http_status_code.h"
 #include "sync/engine/net/url_translator.h"
 #include "sync/engine/syncer.h"
-#include "sync/internal_api/public/base/cancelation_signal.h"
 #include "sync/protocol/sync.pb.h"
 #include "sync/syncable/directory.h"
 #include "url/gurl.h"
@@ -115,32 +114,13 @@
 }
 
 ServerConnectionManager::ScopedConnectionHelper::ScopedConnectionHelper(
-    CancelationSignal* signaller, scoped_ptr<Connection> connection)
-    : cancelation_signal_(signaller), connection_(connection.Pass()) {
-  // Special early return for tests.
-  if (!connection_.get())
-    return;
+    ServerConnectionManager* manager, Connection* connection)
+    : manager_(manager), connection_(connection) {}
 
-  if (!cancelation_signal_->TryRegisterHandler(this)) {
-    connection_.reset();
-  }
-}
-
-// This function may be called from another thread.
-void ServerConnectionManager::ScopedConnectionHelper::OnStopRequested() {
-  DCHECK(connection_);
-  connection_->Abort();
-}
-
 ServerConnectionManager::ScopedConnectionHelper::~ScopedConnectionHelper() {
-  // We should be registered iff connection_.get() != NULL.
-  if (connection_.get()) {
-    // It is important that this be called before this destructor completes.
-    // Until the unregistration is complete, it's possible that the virtual
-    // OnStopRequested() function may be called from a different thread.  We
-    // need to unregister it before destruction modifies our vptr.
-    cancelation_signal_->UnregisterHandler(this);
-  }
+  if (connection_)
+    manager_->OnConnectionDestroyed(connection_.get());
+  connection_.reset();
 }
 
 ServerConnectionManager::Connection*
@@ -197,20 +177,43 @@
     const string& server,
     int port,
     bool use_ssl,
-    bool use_oauth2_token,
-    CancelationSignal* cancelation_signal)
+    bool use_oauth2_token)
     : sync_server_(server),
       sync_server_port_(port),
       use_ssl_(use_ssl),
       use_oauth2_token_(use_oauth2_token),
       proto_sync_path_(kSyncServerSyncPath),
       server_status_(HttpResponse::NONE),
-      cancelation_signal_(cancelation_signal) {
+      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;
+}
+
 bool ServerConnectionManager::SetAuthToken(const std::string& auth_token) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (previously_invalidated_token != auth_token) {
@@ -275,7 +278,7 @@
 
   // When our connection object falls out of scope, it clears itself from
   // active_connection_.
-  ScopedConnectionHelper post(cancelation_signal_, MakeConnection());
+  ScopedConnectionHelper post(this, MakeActiveConnection());
   if (!post.get()) {
     params->response.server_status = HttpResponse::CONNECTION_UNAVAILABLE;
     return false;
@@ -340,12 +343,22 @@
   listeners_.RemoveObserver(listener);
 }
 
-scoped_ptr<ServerConnectionManager::Connection>
-ServerConnectionManager::MakeConnection()
+ServerConnectionManager::Connection* ServerConnectionManager::MakeConnection()
 {
-  return scoped_ptr<Connection>();  // For testing.
+  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;
+}
+
 std::ostream& operator << (std::ostream& s, const struct HttpResponse& hr) {
   s << " Response Code (bogus on error): " << hr.response_code;
   s << " Content-Length (bogus on error): " << hr.content_length;