Add an option to disable net::BidirectionalStreamQuicImpl

This CL adds an option to disable
net::BidirectionalStreamQuicImpl in net::HttpNetworkSession.
When the flag is set to true, alternative service job will
not be negotiated.

BUG=584338

Review URL: https://codereview.chromium.org/1796253002

Cr-Commit-Position: refs/heads/master@{#382084}

8 files changed, 168 insertions, 5 deletions

diff --git a/components/cronet/url_request_context_config.cc b/components/cronet/url_request_context_config.cc
index 329d9a0..708a570 100644
--- a/components/cronet/url_request_context_config.cc
+++ b/components/cronet/url_request_context_config.cc
@@ -44,6 +44,8 @@ const char kQuicMigrateSessionsOnNetworkChange[] =
 const char kQuicPreferAes[] = "prefer_aes";
 const char kQuicUserAgentId[] = "user_agent_id";
 const char kQuicMigrateSessionsEarly[] = "migrate_sessions_early";
+const char kQuicDisableBidirectionalStreams[] =
+    "quic_disable_bidirectional_streams";
 
 // AsyncDNS experiment dictionary name.
 const char kAsyncDnsFieldTrialName[] = "AsyncDNS";
@@ -168,6 +170,13 @@ void ParseAndSetExperimentalOptions(
       context_builder->set_quic_migrate_sessions_early(
           quic_migrate_sessions_early);
     }
+
+    bool quic_disable_bidirectional_streams = false;
+    if (quic_args->GetBoolean(kQuicDisableBidirectionalStreams,
+                              &quic_disable_bidirectional_streams)) {
+      context_builder->set_quic_disable_bidirectional_streams(
+          quic_disable_bidirectional_streams);
+    }
   }
 
   const base::DictionaryValue* async_dns_args = nullptr;
diff --git a/net/http/http_network_session.cc b/net/http/http_network_session.cc
index b53a3c0..da649b0 100644
--- a/net/http/http_network_session.cc
+++ b/net/http/http_network_session.cc
@@ -129,6 +129,7 @@ HttpNetworkSession::Params::Params()
       quic_disable_preconnect_if_0rtt(false),
       quic_migrate_sessions_on_network_change(false),
       quic_migrate_sessions_early(false),
+      quic_disable_bidirectional_streams(false),
       proxy_delegate(NULL),
       enable_token_binding(false) {
   quic_supported_versions.push_back(QUIC_VERSION_30);
diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h
index 132195c..094eea8 100644
--- a/net/http/http_network_session.h
+++ b/net/http/http_network_session.h
@@ -180,6 +180,9 @@ class NET_EXPORT HttpNetworkSession
     // If true, active QUIC sessions experiencing poor connectivity may be
     // migrated onto a new network.
     bool quic_migrate_sessions_early;
+    // If true, bidirectional streams over QUIC will be disabled.
+    bool quic_disable_bidirectional_streams;
+
     ProxyDelegate* proxy_delegate;
     // Enable support for Token Binding.
     bool enable_token_binding;
diff --git a/net/http/http_stream_factory_impl.cc b/net/http/http_stream_factory_impl.cc
index 37ac778..a36b832 100644
--- a/net/http/http_stream_factory_impl.cc
+++ b/net/http/http_stream_factory_impl.cc
@@ -106,7 +106,7 @@ HttpStreamRequest* HttpStreamFactoryImpl::RequestStreamInternal(
   request->AttachJob(job);
 
   const AlternativeService alternative_service =
-      GetAlternativeServiceFor(request_info, delegate);
+      GetAlternativeServiceFor(request_info, delegate, stream_type);
 
   if (alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL) {
     // Never share connection with other jobs for FTP requests.
@@ -144,8 +144,8 @@ void HttpStreamFactoryImpl::PreconnectStreams(
     const SSLConfig& server_ssl_config,
     const SSLConfig& proxy_ssl_config) {
   DCHECK(!for_websockets_);
-  AlternativeService alternative_service =
-      GetAlternativeServiceFor(request_info, nullptr);
+  AlternativeService alternative_service = GetAlternativeServiceFor(
+      request_info, nullptr, HttpStreamRequest::HTTP_STREAM);
   HostPortPair server;
   if (alternative_service.protocol != UNINITIALIZED_ALTERNATE_PROTOCOL) {
     server = alternative_service.host_port_pair();
@@ -176,7 +176,8 @@ const HostMappingRules* HttpStreamFactoryImpl::GetHostMappingRules() const {
 
 AlternativeService HttpStreamFactoryImpl::GetAlternativeServiceFor(
     const HttpRequestInfo& request_info,
-    HttpStreamRequest::Delegate* delegate) {
+    HttpStreamRequest::Delegate* delegate,
+    HttpStreamRequest::StreamType stream_type) {
   GURL original_url = request_info.url;
 
   if (original_url.SchemeIs("ftp"))
@@ -243,6 +244,11 @@ AlternativeService HttpStreamFactoryImpl::GetAlternativeServiceFor(
     if (!session_->params().enable_quic)
       continue;
 
+    if (stream_type == HttpStreamRequest::BIDIRECTIONAL_STREAM &&
+        session_->params().quic_disable_bidirectional_streams) {
+      continue;
+    }
+
     if (session_->quic_stream_factory()->IsQuicDisabled(origin.port()))
       continue;
 
diff --git a/net/http/http_stream_factory_impl.h b/net/http/http_stream_factory_impl.h
index d0aa390..a509f01 100644
--- a/net/http/http_stream_factory_impl.h
+++ b/net/http/http_stream_factory_impl.h
@@ -89,7 +89,8 @@ class NET_EXPORT_PRIVATE HttpStreamFactoryImpl : public HttpStreamFactory {
 
   AlternativeService GetAlternativeServiceFor(
       const HttpRequestInfo& request_info,
-      HttpStreamRequest::Delegate* delegate);
+      HttpStreamRequest::Delegate* delegate,
+      HttpStreamRequest::StreamType stream_type);
 
   // Detaches |job| from |request|.
   void OrphanJob(Job* job, const Request* request);
diff --git a/net/http/http_stream_factory_impl_unittest.cc b/net/http/http_stream_factory_impl_unittest.cc
index c2172e2..341b1b2 100644
--- a/net/http/http_stream_factory_impl_unittest.cc
+++ b/net/http/http_stream_factory_impl_unittest.cc
@@ -1517,6 +1517,12 @@ class HttpStreamFactoryBidirectionalQuicTest
     clock_->AdvanceTime(QuicTime::Delta::FromMilliseconds(20));
   }
 
+  // Disable bidirectional stream over QUIC. This should be invoked before
+  // Initialize().
+  void DisableQuicBidirectionalStream() {
+    params_.quic_disable_bidirectional_streams = true;
+  }
+
   void Initialize() {
     params_.enable_quic = true;
     params_.http_server_properties = http_server_properties_.GetWeakPtr();
@@ -1647,6 +1653,7 @@ TEST_P(HttpStreamFactoryBidirectionalQuicTest,
 
   scoped_refptr<IOBuffer> buffer = new net::IOBuffer(1);
   EXPECT_EQ(OK, job->ReadData(buffer.get(), 1));
+  EXPECT_EQ(kProtoQUIC1SPDY3, job->GetProtocol());
   EXPECT_EQ("200", delegate.response_headers().find(":status")->second);
   EXPECT_EQ(1, GetSocketPoolGroupCount(session()->GetTransportSocketPool(
                    HttpNetworkSession::NORMAL_SOCKET_POOL)));
@@ -1659,6 +1666,133 @@ TEST_P(HttpStreamFactoryBidirectionalQuicTest,
   EXPECT_TRUE(waiter.used_proxy_info().is_direct());
 }
 
+// Tests that when QUIC is not enabled for bidirectional streaming, HTTP/2 is
+// used instead.
+TEST_P(HttpStreamFactoryBidirectionalQuicTest,
+       RequestBidirectionalStreamJobQuicNotEnabled) {
+  GURL url = GURL("https://www.example.org");
+
+  // Make the http job fail.
+  scoped_ptr<StaticSocketDataProvider> http_job_data;
+  http_job_data.reset(new StaticSocketDataProvider());
+  MockConnect failed_connect(ASYNC, ERR_CONNECTION_REFUSED);
+  http_job_data->set_connect_data(failed_connect);
+  socket_factory().AddSocketDataProvider(http_job_data.get());
+  SSLSocketDataProvider ssl_data(ASYNC, OK);
+  socket_factory().AddSSLSocketDataProvider(&ssl_data);
+
+  // Set up QUIC as alternative_service.
+  AddQuicAlternativeService();
+  DisableQuicBidirectionalStream();
+  Initialize();
+
+  // Now request a stream.
+  SSLConfig ssl_config;
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("https://www.example.org");
+  request_info.load_flags = 0;
+
+  StreamRequestWaiter waiter;
+  scoped_ptr<HttpStreamRequest> request(
+      session()->http_stream_factory()->RequestBidirectionalStreamJob(
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter,
+          BoundNetLog()));
+
+  waiter.WaitForStream();
+  EXPECT_TRUE(waiter.stream_done());
+  EXPECT_FALSE(waiter.websocket_stream());
+  ASSERT_FALSE(waiter.stream());
+  ASSERT_FALSE(waiter.bidirectional_stream_job());
+  // Since the alternative service job is not started, we will get the error
+  // from the http job.
+  ASSERT_EQ(ERR_CONNECTION_REFUSED, waiter.error_status());
+}
+
+// Tests that if Http job fails, but Quic job succeeds, we return
+// BidirectionalStreamQuicImpl.
+TEST_P(HttpStreamFactoryBidirectionalQuicTest,
+       RequestBidirectionalStreamJobHttpJobFailsQuicJobSucceeds) {
+  GURL url = GURL("https://www.example.org");
+
+  // Set up Quic data.
+  MockQuicData mock_quic_data;
+  SpdyPriority priority =
+      ConvertRequestPriorityToQuicPriority(DEFAULT_PRIORITY);
+  size_t spdy_headers_frame_length;
+  mock_quic_data.AddWrite(packet_maker().MakeRequestHeadersPacket(
+      1, test::kClientDataStreamId1, /*should_include_version=*/true,
+      /*fin=*/true, priority,
+      packet_maker().GetRequestHeaders("GET", "https", "/"),
+      &spdy_headers_frame_length));
+  size_t spdy_response_headers_frame_length;
+  mock_quic_data.AddRead(packet_maker().MakeResponseHeadersPacket(
+      1, test::kClientDataStreamId1, /*should_include_version=*/false,
+      /*fin=*/true, packet_maker().GetResponseHeaders("200"),
+      &spdy_response_headers_frame_length));
+  mock_quic_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);  // No more read data.
+  mock_quic_data.AddSocketDataToFactory(&socket_factory());
+
+  // Make the http job fail.
+  scoped_ptr<StaticSocketDataProvider> http_job_data;
+  http_job_data.reset(new StaticSocketDataProvider());
+  MockConnect failed_connect(ASYNC, ERR_CONNECTION_REFUSED);
+  http_job_data->set_connect_data(failed_connect);
+  socket_factory().AddSocketDataProvider(http_job_data.get());
+  SSLSocketDataProvider ssl_data(ASYNC, OK);
+  socket_factory().AddSSLSocketDataProvider(&ssl_data);
+
+  // Set up QUIC as alternative_service.
+  AddQuicAlternativeService();
+  Initialize();
+
+  // Now request a stream.
+  SSLConfig ssl_config;
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("https://www.example.org");
+  request_info.load_flags = 0;
+
+  StreamRequestWaiter waiter;
+  scoped_ptr<HttpStreamRequest> request(
+      session()->http_stream_factory()->RequestBidirectionalStreamJob(
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter,
+          BoundNetLog()));
+
+  waiter.WaitForStream();
+  EXPECT_TRUE(waiter.stream_done());
+  EXPECT_FALSE(waiter.websocket_stream());
+  ASSERT_FALSE(waiter.stream());
+  ASSERT_TRUE(waiter.bidirectional_stream_job());
+  BidirectionalStreamJob* job = waiter.bidirectional_stream_job();
+
+  BidirectionalStreamRequestInfo bidi_request_info;
+  bidi_request_info.method = "GET";
+  bidi_request_info.url = GURL("https://www.example.org/");
+  bidi_request_info.end_stream_on_headers = true;
+  bidi_request_info.priority = LOWEST;
+
+  TestBidirectionalDelegate delegate;
+  job->Start(&bidi_request_info, BoundNetLog(), &delegate, nullptr);
+  delegate.WaitUntilDone();
+
+  // Make sure the BidirectionalStream negotiated goes through QUIC.
+  scoped_refptr<IOBuffer> buffer = new net::IOBuffer(1);
+  EXPECT_EQ(OK, job->ReadData(buffer.get(), 1));
+  EXPECT_EQ(kProtoQUIC1SPDY3, job->GetProtocol());
+  EXPECT_EQ("200", delegate.response_headers().find(":status")->second);
+  // There is no Http2 socket pool.
+  EXPECT_EQ(0, GetSocketPoolGroupCount(session()->GetTransportSocketPool(
+                   HttpNetworkSession::NORMAL_SOCKET_POOL)));
+  EXPECT_EQ(0, GetSocketPoolGroupCount(session()->GetSSLSocketPool(
+                   HttpNetworkSession::NORMAL_SOCKET_POOL)));
+  EXPECT_EQ(0, GetSocketPoolGroupCount(session()->GetTransportSocketPool(
+                   HttpNetworkSession::WEBSOCKET_SOCKET_POOL)));
+  EXPECT_EQ(0, GetSocketPoolGroupCount(session()->GetSSLSocketPool(
+                   HttpNetworkSession::WEBSOCKET_SOCKET_POOL)));
+  EXPECT_TRUE(waiter.used_proxy_info().is_direct());
+}
+
 TEST_P(HttpStreamFactoryTest, RequestBidirectionalStreamJobFailure) {
   SpdySessionDependencies session_deps(GetParam(),
                                        ProxyService::CreateDirect());
diff --git a/net/url_request/url_request_context_builder.cc b/net/url_request/url_request_context_builder.cc
index bd59db6..2024e7a 100644
--- a/net/url_request/url_request_context_builder.cc
+++ b/net/url_request/url_request_context_builder.cc
@@ -431,6 +431,8 @@ scoped_ptr<URLRequestContext> URLRequestContextBuilder::Build() {
       http_network_session_params_.quic_prefer_aes;
   network_session_params.quic_migrate_sessions_early =
       http_network_session_params_.quic_migrate_sessions_early;
+  network_session_params.quic_disable_bidirectional_streams =
+      http_network_session_params_.quic_disable_bidirectional_streams;
   if (proxy_delegate_) {
     network_session_params.proxy_delegate = proxy_delegate_.get();
     storage->set_proxy_delegate(std::move(proxy_delegate_));
diff --git a/net/url_request/url_request_context_builder.h b/net/url_request/url_request_context_builder.h
index affcd4e..a028180 100644
--- a/net/url_request/url_request_context_builder.h
+++ b/net/url_request/url_request_context_builder.h
@@ -105,6 +105,7 @@ class NET_EXPORT URLRequestContextBuilder {
     bool quic_close_sessions_on_ip_change;
     bool quic_migrate_sessions_on_network_change;
     bool quic_migrate_sessions_early;
+    bool quic_disable_bidirectional_streams;
   };
 
   URLRequestContextBuilder();
@@ -276,6 +277,12 @@ class NET_EXPORT URLRequestContextBuilder {
         quic_migrate_sessions_early;
   }
 
+  void set_quic_disable_bidirectional_streams(
+      bool quic_disable_bidirectional_streams) {
+    http_network_session_params_.quic_disable_bidirectional_streams =
+        quic_disable_bidirectional_streams;
+  }
+
   void set_throttling_enabled(bool throttling_enabled) {
     throttling_enabled_ = throttling_enabled;
   }