Introduce RequestWebSocketStream into HttpStreamFactory

net/http/http_stream_factory_impl_job.cc

Index: net/http/http_stream_factory_impl_job.cc
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index d9e7f24501c343475d710f6f46e36c65404c3c77..bc36846f60fa7a5fd912598af71b4ff93aa7237c 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -4,6 +4,8 @@
 
 #include "net/http/http_stream_factory_impl_job.h"
 
+#include <string>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
@@ -276,6 +278,7 @@ bool HttpStreamFactoryImpl::Job::CanUseExistingSpdySession() const {
 }
 
 void HttpStreamFactoryImpl::Job::OnStreamReadyCallback() {
+  DCHECK(!request_->for_websocket());
   DCHECK(stream_.get());
   DCHECK(!IsPreconnecting());
   if (IsOrphaned()) {
@@ -291,20 +294,59 @@ void HttpStreamFactoryImpl::Job::OnStreamReadyCallback() {
   // |this| may be deleted after this call.
 }
 
-void HttpStreamFactoryImpl::Job::OnSpdySessionReadyCallback() {
+void HttpStreamFactoryImpl::Job::OnSocketReadyForWebSocketCallback() {
+  DCHECK(request_->for_websocket());
+  DCHECK(connection_.get() && connection_->is_initialized());
+  DCHECK(!IsPreconnecting());

[mmenke, 2013/05/23 20:26:20]

Looks like when we're preconnected, request_ is NULL, so this should probably go
before we dereference request_.

[yhirano, 2013/05/24 14:11:16]

Done.

+  if (IsOrphaned()) {
+    stream_factory_->OnOrphanedJobComplete(this);
+  } else {
+    request_->Complete(was_npn_negotiated(),
+                       protocol_negotiated(),
+                       using_spdy(),
+                       net_log_);
+    request_->OnSocketReadyForWebSocket(this, server_ssl_config_, proxy_info_,
+                                        connection_.release());
+  }
+  // |this| may be deleted after this call.
+}
+
+void HttpStreamFactoryImpl::Job::OnSpdySessionReadyForWebSocketCallback() {
+  DCHECK(request_->for_websocket());
+  DCHECK(spdy_session_to_pass_);
+  DCHECK(!IsPreconnecting());
+  scoped_refptr<SpdySession> session = spdy_session_to_pass_;
+  spdy_session_to_pass_ = NULL;
+  if (IsOrphaned()) {
+    stream_factory_->OnOrphanedJobComplete(this);

[mmenke, 2013/05/23 20:26:20]

This is what was confusing me before - we don't call OnNewSpdySessionReady here.
 So if we're orphaned, and there's another request out for the same SpdySession,
when we're here, don't make the session available for the other request.  This
seems like a problem.

[yhirano, 2013/05/24 14:11:16]

In PS 13 (or newer), OnNewSpdySessionReady is called when a new spdy session is
generated regardless of for_websocket value.

+  } else {
+    request_->Complete(was_npn_negotiated(),
+                       protocol_negotiated(),
+                       using_spdy(),
+                       net_log_);
+    request_->OnSpdySessionReadyForWebSocket(this,
+                                             server_ssl_config_,
+                                             proxy_info_,
+                                             session);
+  }
+  // |this| may be deleted after this call.
+}
+
+void HttpStreamFactoryImpl::Job::OnNewSpdySessionReadyCallback() {
   DCHECK(!stream_.get());
   DCHECK(!IsPreconnecting());
+  DCHECK(!request_->for_websocket());
   DCHECK(using_spdy());
   DCHECK(new_spdy_session_);
   scoped_refptr<SpdySession> spdy_session = new_spdy_session_;
   new_spdy_session_ = NULL;
   if (IsOrphaned()) {
-    stream_factory_->OnSpdySessionReady(
+    stream_factory_->OnNewSpdySessionReady(
         spdy_session, spdy_session_direct_, server_ssl_config_, proxy_info_,
         was_npn_negotiated(), protocol_negotiated(), using_spdy(), net_log_);
     stream_factory_->OnOrphanedJobComplete(this);
   } else {
-    request_->OnSpdySessionReady(this, spdy_session, spdy_session_direct_);
+    request_->OnNewSpdySessionReady(this, spdy_session, spdy_session_direct_);
   }
   // |this| may be deleted after this call.
 }
@@ -365,7 +407,7 @@ void HttpStreamFactoryImpl::Job::OnHttpsProxyTunnelResponseCallback(
 void HttpStreamFactoryImpl::Job::OnPreconnectsComplete() {
   DCHECK(!request_);
   if (new_spdy_session_) {
-    stream_factory_->OnSpdySessionReady(
+    stream_factory_->OnNewSpdySessionReady(
         new_spdy_session_, spdy_session_direct_, server_ssl_config_,
         proxy_info_, was_npn_negotiated(), protocol_negotiated(), using_spdy(),
         net_log_);
@@ -441,7 +483,7 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
         MessageLoop::current()->PostTask(
             FROM_HERE,
             base::Bind(
-                &HttpStreamFactoryImpl::Job::OnNeedsProxyAuthCallback,
+                &Job::OnNeedsProxyAuthCallback,
                 ptr_factory_.GetWeakPtr(),
                 *tunnel_auth_response,
                 proxy_socket->GetAuthController()));
@@ -452,7 +494,7 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
       MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(
-              &HttpStreamFactoryImpl::Job::OnNeedsClientAuthCallback,
+              &Job::OnNeedsClientAuthCallback,
               ptr_factory_.GetWeakPtr(),
               connection_->ssl_error_response_info().cert_request_info));
       return ERR_IO_PENDING;
@@ -468,7 +510,7 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
         MessageLoop::current()->PostTask(
             FROM_HERE,
             base::Bind(
-                &HttpStreamFactoryImpl::Job::OnHttpsProxyTunnelResponseCallback,
+                &Job::OnHttpsProxyTunnelResponseCallback,
                 ptr_factory_.GetWeakPtr(),
                 *proxy_socket->GetConnectResponseInfo(),
                 proxy_socket->CreateConnectResponseStream()));
@@ -477,17 +519,33 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
 
     case OK:
       next_state_ = STATE_DONE;
-      if (new_spdy_session_) {
+      if (request_->for_websocket()) {
+        if (spdy_session_to_pass_) {
+          MessageLoop::current()->PostTask(
+              FROM_HERE,
+              base::Bind(
+                  &Job::OnSpdySessionReadyForWebSocketCallback,
+                  ptr_factory_.GetWeakPtr()));
+        } else if (connection_ && connection_->is_initialized()) {
+          MessageLoop::current()->PostTask(
+              FROM_HERE,
+              base::Bind(
+                  &Job::OnSocketReadyForWebSocketCallback,
+                  ptr_factory_.GetWeakPtr()));
+        } else {
+          return ERR_FAILED;

[mmenke, 2013/05/23 20:26:20]

We should fail asynchronously in this case, just like in the default case. 
Notice that HttpStreamFactoryImpl::Job::OnIOComplete() doesn't actually do
anything with the result, so this function always has to return ERR_IO_PENDING
(Which, admittedly, makes having a return value a little weird, but that's
beyond the scope of this CL).

[yhirano, 2013/05/24 14:11:16]

Done.

+        }
+      } else if (new_spdy_session_) {
         MessageLoop::current()->PostTask(
             FROM_HERE,
             base::Bind(
-                &HttpStreamFactoryImpl::Job::OnSpdySessionReadyCallback,
+                &Job::OnNewSpdySessionReadyCallback,
                 ptr_factory_.GetWeakPtr()));
       } else {
         MessageLoop::current()->PostTask(
             FROM_HERE,
             base::Bind(
-                &HttpStreamFactoryImpl::Job::OnStreamReadyCallback,
+                &Job::OnStreamReadyCallback,
                 ptr_factory_.GetWeakPtr()));
       }
       return ERR_IO_PENDING;
@@ -496,7 +554,7 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
       MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(
-              &HttpStreamFactoryImpl::Job::OnStreamFailedCallback,
+              &Job::OnStreamFailedCallback,
               ptr_factory_.GetWeakPtr(),
               result));
       return ERR_IO_PENDING;
@@ -854,9 +912,9 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
           SSLClientSocket::NextProtoFromString(proto);
       protocol_negotiated_ = protocol_negotiated;
       net_log_.AddEvent(
-           NetLog::TYPE_HTTP_STREAM_REQUEST_PROTO,
-           base::Bind(&NetLogHttpStreamProtoCallback,
-                      status, &proto, &server_protos));
+          NetLog::TYPE_HTTP_STREAM_REQUEST_PROTO,
+          base::Bind(&NetLogHttpStreamProtoCallback,
+                     status, &proto, &server_protos));
       if (ssl_socket->was_spdy_negotiated())
         SwitchToSpdyMode();
     }
@@ -970,10 +1028,14 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
                         request_info_.url.SchemeIs("ftp"));
     if (stream_factory_->http_pipelined_host_pool_.
             IsExistingPipelineAvailableForKey(*http_pipelining_key_.get())) {
+      DCHECK(!request_->for_websocket());
       stream_.reset(stream_factory_->http_pipelined_host_pool_.
                     CreateStreamOnExistingPipeline(
                         *http_pipelining_key_.get()));
       CHECK(stream_.get());
+    } else if (request_->for_websocket()) {
+      // Do nothing.
+      // connection_ will be passed to the delegate.

[mmenke, 2013/05/23 20:26:20]

Not creating a stream in DoCreateStream in this case seems a bit odd.

[yhirano, 2013/05/24 14:11:16]

Done.

     } else if (!using_proxy && IsRequestEligibleForPipelining()) {
       // TODO(simonjam): Support proxies.
       stream_.reset(
@@ -1028,6 +1090,10 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
       if (http_server_properties)
         http_server_properties->SetSupportsSpdy(host_port_pair, true);
       spdy_session_direct_ = direct;
+      if (request_->for_websocket()) {
+        DCHECK(!spdy_session_to_pass_);
+        spdy_session_to_pass_.swap(new_spdy_session_);
+      }
       return OK;
     }
   }
@@ -1035,6 +1101,12 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
   if (spdy_session->IsClosed())
     return ERR_CONNECTION_CLOSED;
 
+  if (request_->for_websocket()) {
+    DCHECK(!spdy_session_to_pass_);
+    spdy_session_to_pass_.swap(spdy_session);
+    return OK;
+  }
+
   // TODO(willchan): Delete this code, because eventually, the
   // HttpStreamFactoryImpl will be creating all the SpdyHttpStreams, since it
   // will know when SpdySessions become available.
@@ -1337,6 +1409,9 @@ bool HttpStreamFactoryImpl::Job::IsRequestEligibleForPipelining() {
   if (IsPreconnecting() || !request_) {
     return false;
   }
+  if (request_->for_websocket()) {
+    return false;
+  }
   if (session_->force_http_pipelining()) {
     return true;
   }