Add HttpStreamFactory Job orphaning semantics.

net/http/http_stream_factory_impl_request.cc

Index: net/http/http_stream_factory_impl_request.cc
diff --git a/net/http/http_stream_factory_impl_request.cc b/net/http/http_stream_factory_impl_request.cc
index b6e6b9f6544eed9093ec4bd83559d22ae7c552ec..b9e0cee74664f4652863ccadf3ab358aec6f0252 100644
--- a/net/http/http_stream_factory_impl_request.cc
+++ b/net/http/http_stream_factory_impl_request.cc
@@ -7,6 +7,8 @@
 #include "base/logging.h"
 #include "base/stl_util-inl.h"
 #include "net/http/http_stream_factory_impl_job.h"
+#include "net/spdy/spdy_http_stream.h"
+#include "net/spdy/spdy_session.h"
 
 namespace net {
 
@@ -18,7 +20,6 @@ HttpStreamFactoryImpl::Request::Request(const GURL& url,
       factory_(factory),
       delegate_(delegate),
       net_log_(net_log),
-      job_(NULL),
       completed_(false),
       was_alternate_protocol_available_(false),
       was_npn_negotiated_(false),
@@ -30,12 +31,17 @@ HttpStreamFactoryImpl::Request::Request(const GURL& url,
 }
 
 HttpStreamFactoryImpl::Request::~Request() {
+  if (bound_job_.get())
+    DCHECK(jobs_.empty());
+  else
+    DCHECK(!jobs_.empty());
+
   net_log_.EndEvent(NetLog::TYPE_HTTP_STREAM_REQUEST, NULL);
 
-  factory_->request_map_.erase(job_);
+  for (std::set<Job*>::iterator it = jobs_.begin(); it != jobs_.end(); ++it)
+    factory_->request_map_.erase(*it);
 
-  // TODO(willchan): Remove this when we decouple requests and jobs.
-  delete job_;
+  STLDeleteElements(&jobs_);
 
   RemoveRequestFromSpdySessionRequestMap();
 }
@@ -50,10 +56,10 @@ void HttpStreamFactoryImpl::Request::SetSpdySessionKey(
   request_set.insert(this);
 }
 
-void HttpStreamFactoryImpl::Request::BindJob(HttpStreamFactoryImpl::Job* job) {
+void HttpStreamFactoryImpl::Request::AttachJob(Job* job) {
   DCHECK(job);
-  DCHECK(!job_);
-  job_ = job;
+  jobs_.insert(job);
+  factory_->request_map_[job] = this;
 }
 
 void HttpStreamFactoryImpl::Request::Complete(
@@ -73,49 +79,96 @@ void HttpStreamFactoryImpl::Request::Complete(
 }
 
 void HttpStreamFactoryImpl::Request::OnStreamReady(
+    Job* job,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     HttpStream* stream) {
   DCHECK(stream);
   DCHECK(completed_);
+
+  // |job| should only be NULL if we're being serviced by a late bound
+  // SpdySession (one that was not created by a job in our |jobs_| set).
+  if (!job) {
+    DCHECK(!bound_job_.get());
+    DCHECK(!jobs_.empty());
+    // NOTE(willchan): We do *NOT* call OrphanJobs() here. The reason is because
+    // we *WANT* to cancel the unnecessary Jobs from other requests if another
+    // Job completes first.
+    // TODO(mbelshe): Revisit this when we implement ip connection pooling of

[willchan no longer on Chromium, 2011/03/01 00:22:42]

Please chime in on this.

+    // SpdySessions. Do we want to orphan the jobs for a different hostname so
+    // they complete? Or do we want to prevent connecting a new SpdySession if
+    // we've already got one available for a different hostname where the ip
+    // address matches up?
+  } else if (!bound_job_.get()) {
+    // We may have other jobs in |jobs_|. For example, if we start multiple jobs
+    // for Alternate-Protocol.
+    OrphanJobsExcept(job);
+  } else {
+    DCHECK(jobs_.empty());
+  }
   delegate_->OnStreamReady(used_ssl_config, used_proxy_info, stream);
 }
 
 void HttpStreamFactoryImpl::Request::OnStreamFailed(
+    Job* job,
     int status,
     const SSLConfig& used_ssl_config) {
   DCHECK_NE(OK, status);
+  if (!bound_job_.get())
+    OrphanJobsExcept(job);
+  else
+    DCHECK(jobs_.empty());
   delegate_->OnStreamFailed(status, used_ssl_config);
 }
 
 void HttpStreamFactoryImpl::Request::OnCertificateError(
+    Job* job,
     int status,
     const SSLConfig& used_ssl_config,
     const SSLInfo& ssl_info) {
   DCHECK_NE(OK, status);
+  if (!bound_job_.get())
+    OrphanJobsExcept(job);
+  else
+    DCHECK(jobs_.empty());
   delegate_->OnCertificateError(status, used_ssl_config, ssl_info);
 }
 
 void HttpStreamFactoryImpl::Request::OnNeedsProxyAuth(
+    Job* job,
     const HttpResponseInfo& proxy_response,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     HttpAuthController* auth_controller) {
+  if (!bound_job_.get())
+    OrphanJobsExcept(job);
+  else
+    DCHECK(jobs_.empty());
   delegate_->OnNeedsProxyAuth(
       proxy_response, used_ssl_config, used_proxy_info, auth_controller);
 }
 
 void HttpStreamFactoryImpl::Request::OnNeedsClientAuth(
+    Job* job,
     const SSLConfig& used_ssl_config,
     SSLCertRequestInfo* cert_info) {
+  if (!bound_job_.get())
+    OrphanJobsExcept(job);
+  else
+    DCHECK(jobs_.empty());
   delegate_->OnNeedsClientAuth(used_ssl_config, cert_info);
 }
 
 void HttpStreamFactoryImpl::Request::OnHttpsProxyTunnelResponse(
+    Job *job,
     const HttpResponseInfo& response_info,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     HttpStream* stream) {
+  if (!bound_job_.get())
+    OrphanJobsExcept(job);
+  else
+    DCHECK(jobs_.empty());
   delegate_->OnHttpsProxyTunnelResponse(
       response_info, used_ssl_config, used_proxy_info, stream);
 }
@@ -123,15 +176,17 @@ void HttpStreamFactoryImpl::Request::OnHttpsProxyTunnelResponse(
 int HttpStreamFactoryImpl::Request::RestartTunnelWithProxyAuth(
     const string16& username,
     const string16& password) {
-  // We're restarting the job, so ditch the old key. Note that we could actually
-  // keep it around and eliminate the DCHECK in set_spdy_session_key() that
-  // |spdy_session_key_| is NULL, but I prefer to keep the assertion.
-  RemoveRequestFromSpdySessionRequestMap();
-  return job_->RestartTunnelWithProxyAuth(username, password);
+  DCHECK(bound_job_.get());
+  return bound_job_->RestartTunnelWithProxyAuth(username, password);
 }
 
 LoadState HttpStreamFactoryImpl::Request::GetLoadState() const {
-  return factory_->GetLoadState(*this);
+  if (bound_job_.get())
+    return bound_job_->GetLoadState();
+  DCHECK(!jobs_.empty());
+
+  // Just pick the first one.
+  return (*jobs_.begin())->GetLoadState();
 }
 
 bool HttpStreamFactoryImpl::Request::was_alternate_protocol_available() const {
@@ -165,4 +220,68 @@ HttpStreamFactoryImpl::Request::RemoveRequestFromSpdySessionRequestMap() {
   }
 }
 
+void HttpStreamFactoryImpl::Request::OnSpdySessionReady(
+    Job* job,
+    scoped_refptr<SpdySession> spdy_session,
+    bool direct) {
+  DCHECK(job);
+  DCHECK(job->using_spdy());
+
+  // The first case is the usual case.
+  if (!bound_job_.get()) {
+    OrphanJobsExcept(job);
+  } else { // This is the case for HTTPS proxy tunneling.
+    DCHECK_EQ(bound_job_.get(), job);
+    DCHECK(jobs_.empty());
+  }
+
+  // Cache these values in case the job gets deleted.
+  const SSLConfig used_ssl_config = job->ssl_config();
+  const ProxyInfo used_proxy_info = job->proxy_info();
+  const bool was_alternate_protocol_available =
+      job->was_alternate_protocol_available();
+  const bool was_npn_negotiated = job->was_npn_negotiated();
+  const bool using_spdy = job->using_spdy();
+  const NetLog::Source source = job->net_log().source();
+
+  Complete(was_alternate_protocol_available,
+           was_npn_negotiated,
+           using_spdy,
+           source);
+
+  // Cache this so we can still use it if the request is deleted.
+  HttpStreamFactoryImpl* factory = factory_;
+
+  bool use_relative_url = direct || url().SchemeIs("https");
+  delegate_->OnStreamReady(
+      job->ssl_config(),
+      job->proxy_info(),
+      new SpdyHttpStream(spdy_session, use_relative_url));
+  // |this| may be deleted after this point.
+  factory->OnSpdySessionReady(
+      spdy_session, direct, used_ssl_config, used_proxy_info,
+      was_alternate_protocol_available, was_npn_negotiated, using_spdy, source);
+}
+
+void HttpStreamFactoryImpl::Request::OrphanJobsExcept(Job* job) {
+  DCHECK(job);
+  DCHECK(!bound_job_.get());
+  DCHECK(ContainsKey(jobs_, job));
+  bound_job_.reset(job);
+  jobs_.erase(job);
+  factory_->request_map_.erase(job);
+
+  OrphanJobs();
+}
+
+void HttpStreamFactoryImpl::Request::OrphanJobs() {
+  RemoveRequestFromSpdySessionRequestMap();
+
+  std::set<Job*> tmp;
+  tmp.swap(jobs_);
+
+  for (std::set<Job*>::iterator it = tmp.begin(); it != tmp.end(); ++it)
+    factory_->OrphanJob(*it, this);
+}
+
 }  // namespace net