Implement QUIC-based net::BidirectionalStream

net/quic/quic_stream_factory.cc

Index: net/quic/quic_stream_factory.cc
diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc
index 039915bf735bc6a3a15bd77e0d0be64dae7e7d47..472c4607659d2531edbf9309b4b75f65c26a7219 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -61,6 +61,11 @@
 #include "base/cpu.h"
 #endif
 
+#if BUILDFLAG(ENABLE_BIDIRECTIONAL_STREAM)
+#include "net/http/bidirectional_stream_job.h"
+#include "net/quic/bidirectional_stream_quic_impl.h"
+#endif
+
 using std::min;
 using NetworkHandle = net::NetworkChangeNotifier::NetworkHandle;
 
@@ -498,7 +503,7 @@ int QuicStreamFactory::Job::DoConnectComplete(int rv) {
 }
 
 QuicStreamRequest::QuicStreamRequest(QuicStreamFactory* factory)
-    : factory_(factory) {}
+    : factory_(factory), for_bidirectional_(false) {}
 
 QuicStreamRequest::~QuicStreamRequest() {
   if (factory_ && !callback_.is_null())
@@ -511,12 +516,18 @@ int QuicStreamRequest::Request(const HostPortPair& host_port_pair,
                                const GURL& url,
                                base::StringPiece method,
                                const BoundNetLog& net_log,
-                               const CompletionCallback& callback) {
+                               const CompletionCallback& callback,
+                               bool for_bidirectional) {
   DCHECK(!stream_);
+#if BUILDFLAG(ENABLE_BIDIRECTIONAL_STREAM)
+  DCHECK(!bidirectional_stream_job_);
+#endif
   DCHECK(callback_.is_null());
   DCHECK(factory_);
   origin_host_ = url.host();
   privacy_mode_ = privacy_mode;
+  for_bidirectional_ = for_bidirectional;
+
   int rv = factory_->Create(host_port_pair, privacy_mode, cert_verify_flags,
                             url, method, net_log, this);
   if (rv == ERR_IO_PENDING) {
@@ -526,14 +537,29 @@ int QuicStreamRequest::Request(const HostPortPair& host_port_pair,
   } else {
     factory_ = nullptr;
   }
-  if (rv == OK)
+  if (rv == OK) {
+#if BUILDFLAG(ENABLE_BIDIRECTIONAL_STREAM)
+    DCHECK(stream_ || (for_bidirectional_ && bidirectional_stream_job_));

[Ryan Hamilton, 2016/03/03 04:14:57]

This would be true if both stream_ and bidirectional_stream_job_ were both set.
Is that desirable?

[xunjieli, 2016/03/03 16:54:19]

Done.

+#else
     DCHECK(stream_);
+#endif
+  }
   return rv;
 }
 
-void QuicStreamRequest::set_stream(scoped_ptr<QuicHttpStream> stream) {
-  DCHECK(stream);
-  stream_ = std::move(stream);
+void QuicStreamRequest::InitializeStreamFromSession(
+    QuicChromiumClientSession* session) {
+  DCHECK(session);
+  if (for_bidirectional_) {
+#if BUILDFLAG(ENABLE_BIDIRECTIONAL_STREAM)
+    bidirectional_stream_job_.reset(
+        new BidirectionalStreamQuicImpl(session->GetWeakPtr()));
+#else
+    DCHECK(false);
+#endif
+  } else {
+    stream_.reset(new QuicHttpStream(session->GetWeakPtr()));
+  }

[Ryan Hamilton, 2016/03/03 04:14:58]

How about instead of creating the QuicHttpStream or Bidirectional stream here,
and then storing it in one of two member, how about change ReleaseStream() to
create this object on the fly?

[xunjieli, 2016/03/03 16:54:19]

Done. Great idea! But this would change the semantics of ReleaseStream(), which
now sometimes can return nullptr if session is closed before ReleaseStream() is
invoked. Will that be okay? See quic_stream_factory_test.cc for an example.

[Ryan Hamilton, 2016/03/07 20:12:37]

Good point. I think you'll need to change
HttpStreamFactoryImpl::Job::DoInitConnectionComplete to detect this case. Looks
like it should be pretty simple, though.

[xunjieli, 2016/03/08 15:39:31]

Done.

 }
 
 void QuicStreamRequest::OnRequestComplete(int rv) {
@@ -550,9 +576,20 @@ base::TimeDelta QuicStreamRequest::GetTimeDelayForWaitingJob() const {
 
 scoped_ptr<QuicHttpStream> QuicStreamRequest::ReleaseStream() {
   DCHECK(stream_);
+  DCHECK(!for_bidirectional_);
   return std::move(stream_);
 }
 
+#if BUILDFLAG(ENABLE_BIDIRECTIONAL_STREAM)
+scoped_ptr<BidirectionalStreamJob>
+QuicStreamRequest::ReleaseBidirectionalStreamJob() {
+  DCHECK(bidirectional_stream_job_);
+  DCHECK(for_bidirectional_);
+
+  return std::move(bidirectional_stream_job_);
+}
+#endif
+
 QuicStreamFactory::QuicStreamFactory(
     HostResolver* host_resolver,
     ClientSocketFactory* client_socket_factory,
@@ -778,7 +815,7 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
         static_cast<QuicChromiumClientSession*>(promised->session());
     DCHECK(session);
     if (session->server_id().privacy_mode() == privacy_mode) {
-      request->set_stream(CreateFromSession(session));
+      request->InitializeStreamFromSession(session);
       ++num_push_streams_created_;
       return OK;
     }
@@ -796,7 +833,7 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
       QuicChromiumClientSession* session = it->second;
       if (!session->CanPool(url.host(), privacy_mode))
         return ERR_ALTERNATIVE_CERT_NOT_VALID_FOR_ORIGIN;
-      request->set_stream(CreateFromSession(session));
+      request->InitializeStreamFromSession(session);
       return OK;
     }
   }
@@ -852,7 +889,7 @@ int QuicStreamFactory::Create(const HostPortPair& host_port_pair,
     QuicChromiumClientSession* session = it->second;
     if (!session->CanPool(url.host(), privacy_mode))
       return ERR_ALTERNATIVE_CERT_NOT_VALID_FOR_ORIGIN;
-    request->set_stream(CreateFromSession(session));
+    request->InitializeStreamFromSession(session);
   }
   return rv;
 }
@@ -930,7 +967,7 @@ void QuicStreamFactory::OnJobComplete(Job* job, int rv) {
         request->OnRequestComplete(ERR_ALTERNATIVE_CERT_NOT_VALID_FOR_ORIGIN);
         continue;
       }
-      request->set_stream(CreateFromSession(session));
+      request->InitializeStreamFromSession(session);
       ++request_it;
     }
   }