summaryrefslogtreecommitdiffstats
path: root/net/http
diff options
context:
space:
mode:
authorwillchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-10-14 18:03:09 +0000
committerwillchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-10-14 18:03:09 +0000
commit2681688fc8449fe1cc6b2f66267fcf04f0327ea1 (patch)
tree424611ca258e9699de6b63349e1d0fba118144a3 /net/http
parent0f337f026ad2bca4d3eda9bb1cb8e5c9dc2dd7f3 (diff)
downloadchromium_src-2681688fc8449fe1cc6b2f66267fcf04f0327ea1.zip
chromium_src-2681688fc8449fe1cc6b2f66267fcf04f0327ea1.tar.gz
chromium_src-2681688fc8449fe1cc6b2f66267fcf04f0327ea1.tar.bz2
Cleanup StreamFactory && StreamRequest APIs.
Stop refcounting StreamRequest. Establish a clear ownership of the StreamRequest. Deletion implies cancellation. Use ScopedRunnableMethodFactory::NewRunnableMethod() instead of NewRunnableMethod(). Remove Start() from StreamRequest. This is an implementation detail of HttpStreamRequest, so it only exists there now. BUG=59103 TEST=existing, this is a pure refactor. Review URL: http://codereview.chromium.org/3746002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62581 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/http')
-rw-r--r--net/http/http_network_transaction.cc32
-rw-r--r--net/http/http_network_transaction.h6
-rw-r--r--net/http/http_stream_factory.cc15
-rw-r--r--net/http/http_stream_factory.h13
-rw-r--r--net/http/http_stream_request.cc86
-rw-r--r--net/http/http_stream_request.h32
-rw-r--r--net/http/stream_factory.h116
7 files changed, 122 insertions, 178 deletions
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index b5a5707..f4c12d0 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -198,11 +198,6 @@ HttpNetworkTransaction::~HttpNetworkTransaction() {
}
}
}
-
- if (stream_request_.get()) {
- stream_request_->Cancel();
- stream_request_ = NULL;
- }
}
int HttpNetworkTransaction::Start(const HttpRequestInfo* request_info,
@@ -598,13 +593,15 @@ int HttpNetworkTransaction::DoLoop(int result) {
int HttpNetworkTransaction::DoCreateStream() {
next_state_ = STATE_CREATE_STREAM_COMPLETE;
- session_->http_stream_factory()->RequestStream(request_,
- &ssl_config_,
- &proxy_info_,
- this,
- net_log_,
- session_,
- &stream_request_);
+ stream_request_.reset(
+ session_->http_stream_factory()->RequestStream(
+ request_,
+ &ssl_config_,
+ &proxy_info_,
+ session_,
+ this,
+ net_log_));
+ DCHECK(stream_request_.get());
return ERR_IO_PENDING;
}
@@ -615,7 +612,7 @@ int HttpNetworkTransaction::DoCreateStreamComplete(int result) {
}
// At this point we are done with the stream_request_.
- stream_request_ = NULL;
+ stream_request_.reset();
return result;
}
@@ -1073,12 +1070,9 @@ int HttpNetworkTransaction::HandleCertificateRequest(int error) {
stream_.reset();
}
- if (stream_request_.get()) {
- // The server is asking for a client certificate during the initial
- // handshake.
- stream_request_->Cancel();
- stream_request_ = NULL;
- }
+ // The server is asking for a client certificate during the initial
+ // handshake.
+ stream_request_.reset();
// If the user selected one of the certificate in client_certs for this
// server before, use it automatically.
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index 7ddee29..a8d371d 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -33,7 +33,7 @@ class SSLHostInfo;
struct HttpRequestInfo;
class HttpNetworkTransaction : public HttpTransaction,
- public StreamFactory::StreamRequestDelegate {
+ public StreamRequest::Delegate {
public:
explicit HttpNetworkTransaction(HttpNetworkSession* session);
@@ -58,7 +58,7 @@ class HttpNetworkTransaction : public HttpTransaction,
virtual uint64 GetUploadProgress() const;
virtual void SetSSLHostInfo(SSLHostInfo* host_info);
- // StreamRequestDelegate methods:
+ // StreamRequest::Delegate methods:
virtual void OnStreamReady(HttpStream* stream);
virtual void OnStreamFailed(int status);
virtual void OnCertificateError(int status, const SSLInfo& ssl_info);
@@ -211,7 +211,7 @@ class HttpNetworkTransaction : public HttpTransaction,
ProxyInfo proxy_info_;
- scoped_refptr<StreamFactory::StreamRequestJob> stream_request_;
+ scoped_ptr<StreamRequest> stream_request_;
scoped_ptr<HttpStream> stream_;
// True if we've validated the headers that the stream parser has returned.
diff --git a/net/http/http_stream_factory.cc b/net/http/http_stream_factory.cc
index 7a26282..9342564 100644
--- a/net/http/http_stream_factory.cc
+++ b/net/http/http_stream_factory.cc
@@ -44,17 +44,16 @@ HttpStreamFactory::HttpStreamFactory() {
HttpStreamFactory::~HttpStreamFactory() {
}
-void HttpStreamFactory::RequestStream(
+StreamRequest* HttpStreamFactory::RequestStream(
const HttpRequestInfo* request_info,
SSLConfig* ssl_config,
ProxyInfo* proxy_info,
- StreamFactory::StreamRequestDelegate* delegate,
- const BoundNetLog& net_log,
- const scoped_refptr<HttpNetworkSession>& session,
- scoped_refptr<StreamRequestJob>* stream) {
- DCHECK(stream != NULL);
- *stream = new HttpStreamRequest(this, session);
- (*stream)->Start(request_info, ssl_config, proxy_info, delegate, net_log);
+ HttpNetworkSession* session,
+ StreamRequest::Delegate* delegate,
+ const BoundNetLog& net_log) {
+ HttpStreamRequest* stream = new HttpStreamRequest(this, session);
+ stream->Start(request_info, ssl_config, proxy_info, delegate, net_log);
+ return stream;
}
void HttpStreamFactory::AddTLSIntolerantServer(const GURL& url) {
diff --git a/net/http/http_stream_factory.h b/net/http/http_stream_factory.h
index 08da415..b9f5586 100644
--- a/net/http/http_stream_factory.h
+++ b/net/http/http_stream_factory.h
@@ -30,13 +30,12 @@ class HttpStreamFactory : public StreamFactory,
virtual ~HttpStreamFactory();
// StreamFactory Interface
- virtual void RequestStream(const HttpRequestInfo* info,
- SSLConfig* ssl_config,
- ProxyInfo* proxy_info,
- StreamRequestDelegate* delegate,
- const BoundNetLog& net_log,
- const scoped_refptr<HttpNetworkSession>& session,
- scoped_refptr<StreamRequestJob>* stream);
+ virtual StreamRequest* RequestStream(const HttpRequestInfo* info,
+ SSLConfig* ssl_config,
+ ProxyInfo* proxy_info,
+ HttpNetworkSession* session,
+ StreamRequest::Delegate* delegate,
+ const BoundNetLog& net_log);
// TLS Intolerant Server API
void AddTLSIntolerantServer(const GURL& url);
diff --git a/net/http/http_stream_request.cc b/net/http/http_stream_request.cc
index 13c30e6..f43037e 100644
--- a/net/http/http_stream_request.cc
+++ b/net/http/http_stream_request.cc
@@ -47,7 +47,7 @@ GURL UpgradeUrlToHttps(const GURL& original_url) {
HttpStreamRequest::HttpStreamRequest(
HttpStreamFactory* factory,
- const scoped_refptr<HttpNetworkSession>& session)
+ HttpNetworkSession* session)
: request_info_(NULL),
proxy_info_(NULL),
ssl_config_(NULL),
@@ -67,7 +67,7 @@ HttpStreamRequest::HttpStreamRequest(
establishing_tunnel_(false),
was_alternate_protocol_available_(false),
was_npn_negotiated_(false),
- cancelled_(false) {
+ ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
if (factory->use_alternate_protocols())
alternate_protocol_mode_ = kUnspecified;
else
@@ -80,21 +80,24 @@ HttpStreamRequest::~HttpStreamRequest() {
// this stream at all.
if (next_state_ == STATE_WAITING_USER_ACTION) {
connection_->socket()->Disconnect();
- connection_->Reset();
connection_.reset();
}
if (pac_request_)
session_->proxy_service()->CancelPacRequest(pac_request_);
+
+ // The stream could be in a partial state. It is not reusable.
+ if (stream_.get() && next_state_ != STATE_DONE) {
+ stream_->Close(true /* not reusable */);
+ }
}
void HttpStreamRequest::Start(const HttpRequestInfo* request_info,
SSLConfig* ssl_config,
ProxyInfo* proxy_info,
- StreamFactory::StreamRequestDelegate* delegate,
+ Delegate* delegate,
const BoundNetLog& net_log) {
CHECK_EQ(STATE_NONE, next_state_);
- CHECK(!cancelled_);
request_info_ = request_info;
ssl_config_ = ssl_config;
@@ -106,29 +109,6 @@ void HttpStreamRequest::Start(const HttpRequestInfo* request_info,
DCHECK_EQ(ERR_IO_PENDING, rv);
}
-void HttpStreamRequest::Cancel() {
- cancelled_ = true;
-
- // If we were waiting for the user to take action, then the connection
- // is in a partially connected state. All we can do is close it at this
- // point.
- if (next_state_ == STATE_WAITING_USER_ACTION) {
- connection_->socket()->Disconnect();
- connection_->Reset();
- connection_.reset();
- }
-
- // The stream could be in a partial state. It is not reusable.
- if (stream_.get()) {
- stream_->Close(true);
- stream_.reset();
- }
-
- delegate_ = NULL;
-
- next_state_ = STATE_NONE;
-}
-
int HttpStreamRequest::RestartWithCertificate(X509Certificate* client_cert) {
ssl_config()->client_cert = client_cert;
ssl_config()->send_client_cert = true;
@@ -170,54 +150,39 @@ void HttpStreamRequest::GetSSLInfo() {
}
const HttpRequestInfo& HttpStreamRequest::request_info() const {
- DCHECK(!cancelled_); // Can't access this after cancellation.
return *request_info_;
}
ProxyInfo* HttpStreamRequest::proxy_info() const {
- DCHECK(!cancelled_); // Can't access this after cancellation.
return proxy_info_;
}
SSLConfig* HttpStreamRequest::ssl_config() const {
- DCHECK(!cancelled_); // Can't access this after cancellation.
return ssl_config_;
}
-void HttpStreamRequest::OnStreamReadyCallback(HttpStream* stream) {
- if (cancelled_) {
- // The delegate is gone. We need to cleanup the stream.
- delete stream;
- return;
- }
- delegate_->OnStreamReady(stream);
+void HttpStreamRequest::OnStreamReadyCallback() {
+ DCHECK(stream_.get());
+ delegate_->OnStreamReady(stream_.release());
}
void HttpStreamRequest::OnStreamFailedCallback(int result) {
- if (cancelled_)
- return;
delegate_->OnStreamFailed(result);
}
void HttpStreamRequest::OnCertificateErrorCallback(int result,
const SSLInfo& ssl_info) {
- if (cancelled_)
- return;
delegate_->OnCertificateError(result, ssl_info);
}
void HttpStreamRequest::OnNeedsProxyAuthCallback(
const HttpResponseInfo& response,
HttpAuthController* auth_controller) {
- if (cancelled_)
- return;
delegate_->OnNeedsProxyAuth(response, auth_controller);
}
void HttpStreamRequest::OnNeedsClientAuthCallback(
SSLCertRequestInfo* cert_info) {
- if (cancelled_)
- return;
delegate_->OnNeedsClientAuth(cert_info);
}
@@ -226,9 +191,6 @@ void HttpStreamRequest::OnIOComplete(int result) {
}
int HttpStreamRequest::RunLoop(int result) {
- if (cancelled_)
- return ERR_ABORTED;
-
result = DoLoop(result);
DCHECK(delegate_);
@@ -240,9 +202,9 @@ int HttpStreamRequest::RunLoop(int result) {
next_state_ = STATE_WAITING_USER_ACTION;
MessageLoop::current()->PostTask(
FROM_HERE,
- NewRunnableMethod(this,
- &HttpStreamRequest::OnCertificateErrorCallback,
- result, ssl_info_));
+ method_factory_.NewRunnableMethod(
+ &HttpStreamRequest::OnCertificateErrorCallback,
+ result, ssl_info_));
return ERR_IO_PENDING;
}
@@ -261,7 +223,7 @@ int HttpStreamRequest::RunLoop(int result) {
next_state_ = STATE_WAITING_USER_ACTION;
MessageLoop::current()->PostTask(
FROM_HERE,
- NewRunnableMethod(this,
+ method_factory_.NewRunnableMethod(
&HttpStreamRequest::OnNeedsProxyAuthCallback,
*tunnel_auth_response,
http_proxy_socket->auth_controller()));
@@ -271,28 +233,28 @@ int HttpStreamRequest::RunLoop(int result) {
case ERR_SSL_CLIENT_AUTH_CERT_NEEDED:
MessageLoop::current()->PostTask(
FROM_HERE,
- NewRunnableMethod(this,
- &HttpStreamRequest::OnNeedsClientAuthCallback,
- connection_->ssl_error_response_info().cert_request_info));
+ method_factory_.NewRunnableMethod(
+ &HttpStreamRequest::OnNeedsClientAuthCallback,
+ connection_->ssl_error_response_info().cert_request_info));
return ERR_IO_PENDING;
case ERR_IO_PENDING:
break;
case OK:
+ next_state_ = STATE_DONE;
MessageLoop::current()->PostTask(
FROM_HERE,
- NewRunnableMethod(this,
- &HttpStreamRequest::OnStreamReadyCallback,
- stream_.release()));
+ method_factory_.NewRunnableMethod(
+ &HttpStreamRequest::OnStreamReadyCallback));
return ERR_IO_PENDING;
default:
MessageLoop::current()->PostTask(
FROM_HERE,
- NewRunnableMethod(this,
- &HttpStreamRequest::OnStreamFailedCallback,
- result));
+ method_factory_.NewRunnableMethod(
+ &HttpStreamRequest::OnStreamFailedCallback,
+ result));
return ERR_IO_PENDING;
}
return result;
diff --git a/net/http/http_stream_request.h b/net/http/http_stream_request.h
index dcec385..0933272 100644
--- a/net/http/http_stream_request.h
+++ b/net/http/http_stream_request.h
@@ -6,6 +6,7 @@
#define NET_HTTP_HTTP_STREAM_REQUEST_H_
#include "base/scoped_ptr.h"
+#include "base/task.h"
#include "net/base/completion_callback.h"
#include "net/base/host_mapping_rules.h"
#include "net/base/ssl_config_service.h"
@@ -32,19 +33,24 @@ struct HttpRequestInfo;
// An HttpStreamRequest exists for each stream which is in progress of being
// created for the StreamFactory.
-class HttpStreamRequest : public StreamFactory::StreamRequestJob {
+class HttpStreamRequest : public StreamRequest {
public:
HttpStreamRequest(HttpStreamFactory* factory,
- const scoped_refptr<HttpNetworkSession>& session);
+ HttpNetworkSession* session);
virtual ~HttpStreamRequest();
+ // Start initiates the process of creating a new HttpStream.
+ // 3 parameters are passed in by reference. The caller asserts that the
+ // lifecycle of these parameters will remain valid until the stream is
+ // created, failed, or destroyed. In the first two cases, the delegate will
+ // be called to notify completion of the request.
+ void Start(const HttpRequestInfo* request_info,
+ SSLConfig* ssl_config,
+ ProxyInfo* proxy_info,
+ Delegate* delegate,
+ const BoundNetLog& net_log);
+
// StreamRequest interface
- virtual void Start(const HttpRequestInfo* request_info,
- SSLConfig* ssl_config,
- ProxyInfo* proxy_info,
- StreamFactory::StreamRequestDelegate* delegate,
- const BoundNetLog& net_log);
- virtual void Cancel();
virtual int RestartWithCertificate(X509Certificate* client_cert);
virtual int RestartTunnelWithProxyAuth(const string16& username,
const string16& password);
@@ -75,6 +81,7 @@ class HttpStreamRequest : public StreamFactory::StreamRequestJob {
STATE_CREATE_STREAM_COMPLETE,
STATE_DRAIN_BODY_FOR_AUTH_RESTART,
STATE_DRAIN_BODY_FOR_AUTH_RESTART_COMPLETE,
+ STATE_DONE,
STATE_NONE
};
@@ -83,7 +90,7 @@ class HttpStreamRequest : public StreamFactory::StreamRequestJob {
SSLConfig* ssl_config() const;
// Callbacks to the delegate.
- void OnStreamReadyCallback(HttpStream* stream);
+ void OnStreamReadyCallback();
void OnStreamFailedCallback(int result);
void OnCertificateErrorCallback(int result, const SSLInfo& ssl_info);
void OnNeedsProxyAuthCallback(const HttpResponseInfo& response_info,
@@ -163,7 +170,7 @@ class HttpStreamRequest : public StreamFactory::StreamRequestJob {
CompletionCallbackImpl<HttpStreamRequest> io_callback_;
scoped_ptr<ClientSocketHandle> connection_;
scoped_refptr<HttpStreamFactory> factory_;
- StreamFactory::StreamRequestDelegate* delegate_;
+ Delegate* delegate_;
BoundNetLog net_log_;
State next_state_;
ProxyService::PacRequest* pac_request_;
@@ -208,10 +215,7 @@ class HttpStreamRequest : public StreamFactory::StreamRequestJob {
// True if we negotiated NPN.
bool was_npn_negotiated_;
- // Indicates that this StreamRequest has been cancelled. Note that once
- // this has been cancelled, input parameters passed into the StreamRequest
- // can no longer be touched (as they belong to the requestor).
- bool cancelled_;
+ ScopedRunnableMethodFactory<HttpStreamRequest> method_factory_;
DISALLOW_COPY_AND_ASSIGN(HttpStreamRequest);
};
diff --git a/net/http/stream_factory.h b/net/http/stream_factory.h
index 9ec85f1..592ce5a 100644
--- a/net/http/stream_factory.h
+++ b/net/http/stream_factory.h
@@ -26,15 +26,19 @@ class X509Certificate;
struct HttpRequestInfo;
struct SSLConfig;
-// The StreamFactory defines an interface for creating usable HttpStreams.
-class StreamFactory {
+// The StreamRequest is the client's handle to the worker object which handles
+// the creation of an HttpStream. While the HttpStream is being created, this
+// object is the creator's handle for interacting with the HttpStream creation
+// process. The request is cancelled by deleting it, after which no callbacks
+// will be invoked.
+class StreamRequest {
public:
// The StreamRequestDelegate is a set of callback methods for a
// StreamRequestJob. Generally, only one of these methods will be
// called as a result of a stream request.
- class StreamRequestDelegate {
+ class Delegate {
public:
- virtual ~StreamRequestDelegate() {}
+ virtual ~Delegate() {}
// This is the success case.
// |stream| is now owned by the delegate.
@@ -66,72 +70,54 @@ class StreamFactory {
virtual void OnNeedsClientAuth(SSLCertRequestInfo* cert_info) = 0;
};
- // The StreamRequestJob is the worker object which handles the creation
- // of an HttpStream. While the HttpStream is being created, this job
- // is the creator's handle for interacting with the HttpStream creation
- // process.
- class StreamRequestJob : public base::RefCounted<StreamRequestJob> {
- public:
- virtual ~StreamRequestJob() {}
-
- // Start initiates the process of creating a new HttpStream.
- // 3 parameters are passed in by reference. The caller asserts that the
- // lifecycle of these parameters will remain valid until the stream is
- // created, failed, or until the caller calls Cancel() on the stream
- // request. In all cases, the delegate will be called to notify
- // completion of the request.
- virtual void Start(const HttpRequestInfo* request_info,
- SSLConfig* ssl_config,
- ProxyInfo* proxy_info,
- StreamRequestDelegate* delegate,
- const BoundNetLog& net_log) = 0;
-
- // Cancel can be used to abort the HttpStream creation. Once cancelled,
- // the delegates associated with the request will not be invoked.
- virtual void Cancel() = 0;
-
- // When a HttpStream creation process requires a SSL Certificate,
- // the delegate OnNeedsClientAuth handler will have been called.
- // It now becomes the delegate's responsibility to collect the certificate
- // (probably from the user), and then call this method to resume
- // the HttpStream creation process.
- // Ownership of |client_cert| remains with the StreamRequest. The
- // delegate can take a reference if needed beyond the lifetime of this
- // call.
- virtual int RestartWithCertificate(X509Certificate* client_cert) = 0;
-
- // When a HttpStream creation process is stalled due to necessity
- // of Proxy authentication credentials, the delegate OnNeedsProxyAuth
- // will have been called. It now becomes the delegate's responsibility
- // to collect the necessary credentials, and then call this method to
- // resume the HttpStream creation process.
- virtual int RestartTunnelWithProxyAuth(const string16& username,
- const string16& password) = 0;
-
- // Returns the LoadState for the request.
- virtual LoadState GetLoadState() const = 0;
-
- // Returns true if an AlternateProtocol for this request was available.
- virtual bool was_alternate_protocol_available() const = 0;
-
- // Returns true if TLS/NPN was negotiated for this stream.
- virtual bool was_npn_negotiated() const = 0;
-
- // Returns true if this stream is being fetched over SPDY.
- virtual bool using_spdy() const = 0;
- };
+ virtual ~StreamRequest() {}
+
+ // When a HttpStream creation process requires a SSL Certificate,
+ // the delegate OnNeedsClientAuth handler will have been called.
+ // It now becomes the delegate's responsibility to collect the certificate
+ // (probably from the user), and then call this method to resume
+ // the HttpStream creation process.
+ // Ownership of |client_cert| remains with the StreamRequest. The
+ // delegate can take a reference if needed beyond the lifetime of this
+ // call.
+ virtual int RestartWithCertificate(X509Certificate* client_cert) = 0;
+
+ // When a HttpStream creation process is stalled due to necessity
+ // of Proxy authentication credentials, the delegate OnNeedsProxyAuth
+ // will have been called. It now becomes the delegate's responsibility
+ // to collect the necessary credentials, and then call this method to
+ // resume the HttpStream creation process.
+ virtual int RestartTunnelWithProxyAuth(const string16& username,
+ const string16& password) = 0;
+
+ // Returns the LoadState for the request.
+ virtual LoadState GetLoadState() const = 0;
+
+ // Returns true if an AlternateProtocol for this request was available.
+ virtual bool was_alternate_protocol_available() const = 0;
+
+ // Returns true if TLS/NPN was negotiated for this stream.
+ virtual bool was_npn_negotiated() const = 0;
+
+ // Returns true if this stream is being fetched over SPDY.
+ virtual bool using_spdy() const = 0;
+};
+// The StreamFactory defines an interface for creating usable HttpStreams.
+class StreamFactory {
+ public:
virtual ~StreamFactory() {}
// Request a stream.
// Will callback to the StreamRequestDelegate upon completion.
- virtual void RequestStream(const HttpRequestInfo* info,
- SSLConfig* ssl_config,
- ProxyInfo* proxy_info,
- StreamRequestDelegate* delegate,
- const BoundNetLog& net_log,
- const scoped_refptr<HttpNetworkSession>& session,
- scoped_refptr<StreamRequestJob>* stream) = 0;
+ // |info|, |ssl_config|, and |proxy_info| must been kept alive until
+ // |delegate| is called.
+ virtual StreamRequest* RequestStream(const HttpRequestInfo* info,
+ SSLConfig* ssl_config,
+ ProxyInfo* proxy_info,
+ HttpNetworkSession* session,
+ StreamRequest::Delegate* delegate,
+ const BoundNetLog& net_log) = 0;
// TLS Intolerant Server API
virtual void AddTLSIntolerantServer(const GURL& url) = 0;