Fix a potential infinite loop in HttpStreamFactoryImpl when a new SpdySession is established

net/http/http_stream_factory_impl_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_stream_factory_impl.h"
 
 #include <stdint.h>
 
 #include <memory>
 #include <string>
@@ skipping 161 matching lines @@
     if (waiting_for_preconnect_)
       base::MessageLoop::current()->QuitWhenIdle();
   }
 
   bool preconnect_done_;
   bool waiting_for_preconnect_;
 };
 
 class StreamRequestWaiter : public HttpStreamRequest::Delegate {
  public:
-  StreamRequestWaiter()
-      : waiting_for_stream_(false), stream_done_(false), error_status_(OK) {}
+  StreamRequestWaiter() : stream_done_(false), error_status_(OK) {}
 
   // HttpStreamRequest::Delegate
 
   void OnStreamReady(const SSLConfig& used_ssl_config,
                      const ProxyInfo& used_proxy_info,
                      HttpStream* stream) override {
     stream_done_ = true;
-    if (waiting_for_stream_)
-      loop_.Quit();
+    loop_.Quit();
     stream_.reset(stream);
     used_ssl_config_ = used_ssl_config;
     used_proxy_info_ = used_proxy_info;
   }
 
   void OnWebSocketHandshakeStreamReady(
       const SSLConfig& used_ssl_config,
       const ProxyInfo& used_proxy_info,
       WebSocketHandshakeStreamBase* stream) override {
     stream_done_ = true;
-    if (waiting_for_stream_)
-      loop_.Quit();
+    loop_.Quit();
     websocket_stream_.reset(stream);
     used_ssl_config_ = used_ssl_config;
     used_proxy_info_ = used_proxy_info;
   }
 
   void OnBidirectionalStreamImplReady(
       const SSLConfig& used_ssl_config,
       const ProxyInfo& used_proxy_info,
       BidirectionalStreamImpl* stream) override {
     stream_done_ = true;
-    if (waiting_for_stream_)
-      loop_.Quit();
+    loop_.Quit();
     bidirectional_stream_impl_.reset(stream);
     used_ssl_config_ = used_ssl_config;
     used_proxy_info_ = used_proxy_info;
   }
 
   void OnStreamFailed(int status, const SSLConfig& used_ssl_config) override {
     stream_done_ = true;
-    if (waiting_for_stream_)
-      loop_.Quit();
+    loop_.Quit();
     used_ssl_config_ = used_ssl_config;
     error_status_ = status;
   }
 
   void OnCertificateError(int status,
                           const SSLConfig& used_ssl_config,
                           const SSLInfo& ssl_info) override {}
 
   void OnNeedsProxyAuth(const HttpResponseInfo& proxy_response,
                         const SSLConfig& used_ssl_config,
                         const ProxyInfo& used_proxy_info,
                         HttpAuthController* auth_controller) override {}
 
   void OnNeedsClientAuth(const SSLConfig& used_ssl_config,
                          SSLCertRequestInfo* cert_info) override {}
 
   void OnHttpsProxyTunnelResponse(const HttpResponseInfo& response_info,
                                   const SSLConfig& used_ssl_config,
                                   const ProxyInfo& used_proxy_info,
                                   HttpStream* stream) override {}
 
   void OnQuicBroken() override {}
 
-  void WaitForStream() {
-    while (!stream_done_) {
-      waiting_for_stream_ = true;
-      loop_.Run();
-      waiting_for_stream_ = false;
-    }
-  }
+  void WaitForStream() { loop_.Run(); }
 
   const SSLConfig& used_ssl_config() const {
     return used_ssl_config_;
   }
 
   const ProxyInfo& used_proxy_info() const {
     return used_proxy_info_;
   }
 
   HttpStream* stream() {
     return stream_.get();
   }
 
   MockWebSocketHandshakeStream* websocket_stream() {
     return static_cast<MockWebSocketHandshakeStream*>(websocket_stream_.get());
   }
 
   BidirectionalStreamImpl* bidirectional_stream_impl() {
     return bidirectional_stream_impl_.get();
   }
 
   bool stream_done() const { return stream_done_; }
   int error_status() const { return error_status_; }
 
  private:
-  bool waiting_for_stream_;
   bool stream_done_;
   base::RunLoop loop_;
   std::unique_ptr<HttpStream> stream_;
   std::unique_ptr<WebSocketHandshakeStreamBase> websocket_stream_;
   std::unique_ptr<BidirectionalStreamImpl> bidirectional_stream_impl_;
   SSLConfig used_ssl_config_;
   ProxyInfo used_proxy_info_;
   int error_status_;
 
   DISALLOW_COPY_AND_ASSIGN(StreamRequestWaiter);
@@ skipping 667 matching lines @@
       socket_data_direct_second_request.set_connect_data(
           MockConnect(ASYNC, OK));
       socket_factory.AddSocketDataProvider(&socket_data_direct_second_request);
 
       // Now request a stream. It should succeed using the DIRECT.
       HttpRequestInfo request_info;
       request_info.method = "GET";
       request_info.url = GURL("http://www.google.com");
 
       SSLConfig ssl_config;
-      StreamRequestWaiter waiter;
 
       EXPECT_EQ(set_alternative_proxy_server,
                 test_proxy_delegate.alternative_proxy_server().is_quic());
 
       // Start two requests. The first request should consume data from
       // |socket_data_proxy_main_job|,
       // |socket_data_proxy_alternate_job| and
       // |socket_data_direct_first_request|. The second request should consume
       // data from |socket_data_direct_second_request|.
       for (size_t i = 0; i < 2; ++i) {
+        StreamRequestWaiter waiter;
         std::unique_ptr<HttpStreamRequest> request(
             session->http_stream_factory()->RequestStream(
                 request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter,
                 /* enable_ip_based_pooling = */ true, NetLogWithSource()));
         waiter.WaitForStream();
 
         // The proxy that failed should now be known to the proxy_service as
         // bad.
         const ProxyRetryInfoMap retry_info =
             session->proxy_service()->proxy_retry_info();
@@ skipping 75 matching lines @@
     socket_factory.AddSocketDataProvider(&socket_data_https_second);
     socket_factory.AddSSLSocketDataProvider(&ssl_data);
 
     // Now request a stream. It should succeed using the second proxy in the
     // list.
     HttpRequestInfo request_info;
     request_info.method = "GET";
     request_info.url = GURL("http://www.google.com");
 
     SSLConfig ssl_config;
-    StreamRequestWaiter waiter;
 
     EXPECT_TRUE(test_proxy_delegate.alternative_proxy_server().is_quic());
 
     // Start two requests. The first request should consume data from
     // |socket_data_proxy_main_job| and |socket_data_https_first|.
     // The second request should consume data from |socket_data_https_second|.
     for (size_t i = 0; i < 2; ++i) {
+      StreamRequestWaiter waiter;
       std::unique_ptr<HttpStreamRequest> request(
           session->http_stream_factory()->RequestStream(
               request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter,
               /* enable_ip_based_pooling = */ true, NetLogWithSource()));
       waiter.WaitForStream();
 
       // The proxy that failed should now be known to the proxy_service as
       // bad.
       const ProxyRetryInfoMap retry_info =
           session->proxy_service()->proxy_retry_info();
@@ skipping 398 matching lines @@
 
   EXPECT_EQ(GetSocketPoolGroupCount(ssl_pool), 0);
 
   HttpRequestInfo request_info;
   request_info.method = "GET";
   request_info.url = GURL("https://www.google.com");
   request_info.load_flags = 0;
   request_info.privacy_mode = PRIVACY_MODE_DISABLED;
 
   SSLConfig ssl_config;
-  StreamRequestWaiter waiter;
-
+  StreamRequestWaiter waiter1;
   std::unique_ptr<HttpStreamRequest> request1(
       session->http_stream_factory()->RequestStream(
-          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter,
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter1,
           /* enable_ip_based_pooling = */ true, NetLogWithSource()));
-  waiter.WaitForStream();
+  waiter1.WaitForStream();
 
   EXPECT_EQ(GetSocketPoolGroupCount(ssl_pool), 1);
 
+  StreamRequestWaiter waiter2;
   std::unique_ptr<HttpStreamRequest> request2(
       session->http_stream_factory()->RequestStream(
-          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter,
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter2,
           /* enable_ip_based_pooling = */ true, NetLogWithSource()));
-  waiter.WaitForStream();
+  waiter2.WaitForStream();
 
   EXPECT_EQ(GetSocketPoolGroupCount(ssl_pool), 1);
 
+  StreamRequestWaiter waiter3;
   request_info.privacy_mode = PRIVACY_MODE_ENABLED;
   std::unique_ptr<HttpStreamRequest> request3(
       session->http_stream_factory()->RequestStream(
-          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter,
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter3,
           /* enable_ip_based_pooling = */ true, NetLogWithSource()));
-  waiter.WaitForStream();
+  waiter3.WaitForStream();
 
   EXPECT_EQ(GetSocketPoolGroupCount(ssl_pool), 2);
 }
 
 TEST_F(HttpStreamFactoryTest, GetLoadState) {
   SpdySessionDependencies session_deps(ProxyService::CreateDirect());
 
   // Force asynchronous host resolutions, so that the LoadState will be
   // resolving the host.
   session_deps.host_resolver->set_synchronous_mode(false);
@@ skipping 601 matching lines @@
   ASSERT_NE(nullptr, waiter1.stream());
   ASSERT_NE(nullptr, waiter2.stream());
   ASSERT_NE(waiter1.stream(), waiter2.stream());
 
   // Establishing the SpdySession will close idle H2 sockets.
   EXPECT_EQ(0, session->GetSSLSocketPool(HttpNetworkSession::NORMAL_SOCKET_POOL)
                    ->IdleSocketCount());
   EXPECT_EQ(1, GetSpdySessionCount(session.get()));
 }
 
+// Regression test for crbug.com/706974.
+TEST_F(HttpStreamFactoryTest, TwoSpdyConnects) {
+  SpdySessionDependencies session_deps(ProxyService::CreateDirect());
+
+  MockRead reads[] = {MockRead(SYNCHRONOUS, ERR_IO_PENDING)};
+  std::vector<std::unique_ptr<SequencedSocketData>> providers;
+  SSLSocketDataProvider ssl_socket_data(ASYNC, OK);
+  ssl_socket_data.next_proto = kProtoHTTP2;
+  for (int i = 0; i < 2; i++) {
+    auto provider = base::MakeUnique<SequencedSocketData>(
+        reads, arraysize(reads), nullptr, 0);
+    provider->set_connect_data(MockConnect(ASYNC, OK));
+    session_deps.socket_factory->AddSocketDataProvider(provider.get());
+    providers.push_back(std::move(provider));
+    session_deps.socket_factory->AddSSLSocketDataProvider(&ssl_socket_data);
+  }
+
+  std::unique_ptr<HttpNetworkSession> session(
+      SpdySessionDependencies::SpdyCreateSession(&session_deps));
+
+  HostPortPair host_port_pair("www.google.com", 443);
+
+  // Request two streams at once and make sure they use the same connection.
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("https://www.google.com");
+  request_info.load_flags = 0;
+
+  SSLConfig ssl_config;
+  StreamRequestWaiter waiter1;
+  StreamRequestWaiter waiter2;
+  std::unique_ptr<HttpStreamRequest> request1(
+      session->http_stream_factory()->RequestStream(
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter1,
+          /* enable_ip_based_pooling = */ true, NetLogWithSource()));
+  std::unique_ptr<HttpStreamRequest> request2(
+      session->http_stream_factory()->RequestStream(
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter2,
+          /* enable_ip_based_pooling = */ true, NetLogWithSource()));
+  waiter1.WaitForStream();
+  waiter2.WaitForStream();
+  EXPECT_TRUE(waiter1.stream_done());
+  EXPECT_TRUE(waiter2.stream_done());
+  ASSERT_NE(nullptr, waiter1.stream());
+  ASSERT_NE(nullptr, waiter2.stream());
+  ASSERT_NE(waiter1.stream(), waiter2.stream());
+
+  // Establishing the SpdySession will close the extra H2 socket.
+  EXPECT_EQ(0, session->GetSSLSocketPool(HttpNetworkSession::NORMAL_SOCKET_POOL)
+                   ->IdleSocketCount());
+  EXPECT_EQ(1, GetSpdySessionCount(session.get()));
+}
+
 TEST_F(HttpStreamFactoryTest, RequestBidirectionalStreamImpl) {
   SpdySessionDependencies session_deps(ProxyService::CreateDirect());
 
   MockRead mock_read(ASYNC, OK);
   SequencedSocketData socket_data(&mock_read, 1, nullptr, 0);
   socket_data.set_connect_data(MockConnect(ASYNC, OK));
   session_deps.socket_factory->AddSocketDataProvider(&socket_data);
 
   SSLSocketDataProvider ssl_socket_data(ASYNC, OK);
   ssl_socket_data.next_proto = kProtoHTTP2;
@@ skipping 577 matching lines @@
       session->GetTransportSocketPool(
           HttpNetworkSession::WEBSOCKET_SOCKET_POOL)));
   EXPECT_EQ(1, GetSocketPoolGroupCount(
       session->GetSSLSocketPool(HttpNetworkSession::WEBSOCKET_SOCKET_POOL)));
   EXPECT_TRUE(waiter.used_proxy_info().is_direct());
 }
 
 }  // namespace
 
 }  // namespace net