Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2233)

Unified Diff: chrome/browser/sync/engine/net/server_connection_manager.h

Issue 7841013: sync: take 2 at aborting active HTTP requests. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: initial Created 9 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/sync/engine/net/server_connection_manager.h
diff --git a/chrome/browser/sync/engine/net/server_connection_manager.h b/chrome/browser/sync/engine/net/server_connection_manager.h
index 50f033d28c4a33e6479c6572739fa3eb09061523..9fc9d74cccba47e0e73aed4e9ce4dc8b5e1f3d90 100644
--- a/chrome/browser/sync/engine/net/server_connection_manager.h
+++ b/chrome/browser/sync/engine/net/server_connection_manager.h
@@ -10,9 +10,11 @@
#include <string>
#include "base/atomicops.h"
+#include "base/memory/scoped_ptr.h"
#include "base/observer_list.h"
#include "base/string_util.h"
#include "base/threading/non_thread_safe.h"
+#include "base/threading/thread_checker.h"
#include "base/synchronization/lock.h"
#include "chrome/browser/sync/syncable/syncable_id.h"
#include "chrome/common/net/http_return.h"
@@ -138,7 +140,7 @@ class ScopedServerStatusWatcher : public base::NonThreadSafe {
// Use this class to interact with the sync server.
// The ServerConnectionManager currently supports POSTing protocol buffers.
//
-class ServerConnectionManager : public base::NonThreadSafe {
+class ServerConnectionManager {
public:
// buffer_in - will be POSTed
// buffer_out - string will be overwritten with response
@@ -151,11 +153,10 @@ class ServerConnectionManager : public base::NonThreadSafe {
// Abstract class providing network-layer functionality to the
// ServerConnectionManager. Subclasses implement this using an HTTP stack of
// their choice.
- class Post {
+ class Connection {
public:
- explicit Post(ServerConnectionManager* scm) : scm_(scm) {
- }
- virtual ~Post() { }
+ explicit Connection(ServerConnectionManager* scm);
+ virtual ~Connection();
// Called to initialize and perform an HTTP POST.
virtual bool Init(const char* path,
@@ -163,6 +164,10 @@ class ServerConnectionManager : public base::NonThreadSafe {
const std::string& payload,
HttpResponse* response) = 0;
+ // Immediately abandons a pending HTTP POST request and unblocks caller
+ // in Init.
+ virtual void Abort() = 0;
+
bool ReadBufferResponse(std::string* buffer_out, HttpResponse* response,
bool require_response);
bool ReadDownloadResponse(HttpResponse* response, std::string* buffer_out);
@@ -222,7 +227,7 @@ class ServerConnectionManager : public base::NonThreadSafe {
inline std::string user_agent() const { return user_agent_; }
inline HttpResponse::ServerConnectionCode server_status() const {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
return server_status_;
}
@@ -245,19 +250,25 @@ class ServerConnectionManager : public base::NonThreadSafe {
std::string GetServerHost() const;
- // Factory method to create a Post object we can use for communication with
- // the server.
- virtual Post* MakePost();
+ // Factory method to create an Connection object we can use for
+ // communication with the server.
+ virtual Connection* MakeConnection();
+
+ // Aborts any active HTTP POST request.
+ // We expect this to get called on a different thread than the valid
+ // ThreadChecker thread, as we want to kill any pending http traffic without
+ // having to wait for the request to complete.
+ void TerminateAllIO();
void set_client_id(const std::string& client_id) {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(client_id_.empty());
client_id_.assign(client_id);
}
// Returns true if the auth token is succesfully set and false otherwise.
bool set_auth_token(const std::string& auth_token) {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
if (previously_invalidated_token != auth_token) {
auth_token_.assign(auth_token);
previously_invalidated_token = std::string();
@@ -267,7 +278,7 @@ class ServerConnectionManager : public base::NonThreadSafe {
}
void InvalidateAndClearAuthToken() {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
// Copy over the token to previous invalid token.
if (!auth_token_.empty()) {
previously_invalidated_token.assign(auth_token_);
@@ -276,7 +287,7 @@ class ServerConnectionManager : public base::NonThreadSafe {
}
const std::string auth_token() const {
- DCHECK(CalledOnValidThread());
+ DCHECK(thread_checker_.CalledOnValidThread());
return auth_token_;
}
@@ -301,6 +312,15 @@ class ServerConnectionManager : public base::NonThreadSafe {
const std::string& auth_token,
ScopedServerStatusWatcher* watcher);
+ // Helper to check terminated flags and build a Connection object, installing
+ // it as the |active_connection_|. If this ServerConnectionManager has been
+ // terminated, this will return NULL.
+ Connection* MakeActiveConnection();
+
+ // Called by Connection objects as they are destroyed to allow the
+ // ServerConnectionManager to cleanup active connections.
+ void OnConnectionDestroyed(Connection* connection);
+
// The sync_server_ is the server that requests will be made to.
std::string sync_server_;
@@ -333,10 +353,40 @@ class ServerConnectionManager : public base::NonThreadSafe {
HttpResponse::ServerConnectionCode server_status_;
bool server_reachable_;
+ base::ThreadChecker thread_checker_;
+
+ // Protects all variables below to allow bailing out of active connections.
+ base::Lock terminate_connection_lock_;
+
+ // If true, we've been told to terminate IO and expect to be destroyed
+ // shortly. No future network requests will be made.
+ bool terminated_;
+
+ // A non-owning pointer to any active http connection, so that we can abort
+ // it if necessary.
+ Connection* active_connection_;
+
private:
- friend class Post;
+ friend class Connection;
friend class ScopedServerStatusWatcher;
+ // A class to help deal with cleaning up active Connection objects when (for
+ // ex) multiple early-exits are present in some scope. ScopedConnectionHelper
+ // informs the ServerConnectionManager before the Connection object it takes
+ // ownership of is destroyed.
+ class ScopedConnectionHelper {
+ public:
+ // |manager| must outlive this. Takes ownership of |connection|.
+ ScopedConnectionHelper(ServerConnectionManager* manager,
+ Connection* connection);
+ ~ScopedConnectionHelper();
+ Connection* get();
+ private:
+ ServerConnectionManager* manager_;
+ scoped_ptr<Connection> connection_;
+ DISALLOW_COPY_AND_ASSIGN(ScopedConnectionHelper);
+ };
+
void NotifyStatusChanged();
DISALLOW_COPY_AND_ASSIGN(ServerConnectionManager);

Powered by Google App Engine
This is Rietveld 408576698