Remove Request from Map in OnNewSpdySessionReady().

SpdySessionPool::OnNewSpdySessionReady() calls
HttpStreamFactoryImpl::Request::Complete() repeatedly while there are
any Requests in the map.  This relies on
HttpStreamRequest::Delegate::OnStreamReady() destroying the Request,
which in its destructor calls
HttpStreamFactoryImpl::JobController::OnRequestComplete(), which calls
HttpStreamFactoryImpl::JobController::CancelJobs(), which calls
HttpStreamFactoryImpl::JobController::RemoveRequestFromSpdySessionRequestMap(),
which calls SpdySessionPool::RemoveRequestFromSpdySessionRequestMap().
However, if HttpStreamRequest::Delegate::OnStreamReady() does not
destroy the Request, that results in DCHECK(!completed_) triggering in
HttpStreamFactoryImpl::Request::Complete() in a debug build, and an
infinite loop in production build.

This CL fixes this by explicitly calling
RemoveRequestFromSpdySessionRequestMap() in
SpdySessionPool::OnNewSpdySessionReady().

This revives https://crrev.com/2784143003, which was abandoned to do all
the necessary cleanup first.  Kudos to xunjieli@ for paving the way.

BUG=706974

Review-Url: https://codereview.chromium.org/2930323002
Cr-Commit-Position: refs/heads/master@{#478704}

net/http/http_stream_factory_impl_unittest.cc

@@ -2101,6 +2101,67 @@ TEST_F(HttpStreamFactoryTest, NewSpdySessionCloseIdleH2Sockets) {
   EXPECT_EQ(1, GetSpdySessionCount(session.get()));
 }
 
+// Regression test for https://crbug.com/706974.
+TEST_F(HttpStreamFactoryTest, TwoSpdyConnects) {
+  SpdySessionDependencies session_deps(ProxyService::CreateDirect());
+
+  SSLSocketDataProvider ssl_socket_data0(ASYNC, OK);
+  ssl_socket_data0.next_proto = kProtoHTTP2;
+  session_deps.socket_factory->AddSSLSocketDataProvider(&ssl_socket_data0);
+
+  MockRead reads0[] = {MockRead(SYNCHRONOUS, ERR_IO_PENDING)};
+  SequencedSocketData data0(reads0, arraysize(reads0), nullptr, 0);
+  data0.set_connect_data(MockConnect(ASYNC, OK));
+  session_deps.socket_factory->AddSocketDataProvider(&data0);
+
+  SSLSocketDataProvider ssl_socket_data1(ASYNC, OK);
+  ssl_socket_data1.next_proto = kProtoHTTP2;
+  session_deps.socket_factory->AddSSLSocketDataProvider(&ssl_socket_data1);
+
+  SequencedSocketData data1(nullptr, 0, nullptr, 0);
+  data1.set_connect_data(MockConnect(ASYNC, OK));
+  session_deps.socket_factory->AddSocketDataProvider(&data1);
+
+  std::unique_ptr<HttpNetworkSession> session =
+      SpdySessionDependencies::SpdyCreateSession(&session_deps);
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("https://www.google.com");
+  request_info.load_flags = 0;
+  SSLConfig ssl_config;
+
+  // Request two streams at once and make sure they use the same connection.
+  StreamRequestWaiter waiter1;
+  std::unique_ptr<HttpStreamRequest> request1 =
+      session->http_stream_factory()->RequestStream(
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter1,
+          /* enable_ip_based_pooling = */ true,
+          /* enable_alternative_services = */ true, NetLogWithSource());
+
+  StreamRequestWaiter waiter2;
+  std::unique_ptr<HttpStreamRequest> request2 =
+      session->http_stream_factory()->RequestStream(
+          request_info, DEFAULT_PRIORITY, ssl_config, ssl_config, &waiter2,
+          /* enable_ip_based_pooling = */ true,
+          /* enable_alternative_services = */ 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()));
+  EXPECT_TRUE(data0.AllReadDataConsumed());
+  EXPECT_TRUE(data1.AllReadDataConsumed());
+}
+
 TEST_F(HttpStreamFactoryTest, RequestBidirectionalStreamImpl) {
   SpdySessionDependencies session_deps(ProxyService::CreateDirect());
 

net/spdy/chromium/spdy_session_pool.cc

@@ -421,6 +421,7 @@ void SpdySessionPool::OnNewSpdySessionReady(
       return;
     HttpStreamFactoryImpl::Request* request = *iter->second.begin();
     request->Complete(was_alpn_negotiated, negotiated_protocol, using_spdy);
+    RemoveRequestFromSpdySessionRequestMap(request);
     if (request->stream_type() == HttpStreamRequest::BIDIRECTIONAL_STREAM) {
       request->OnBidirectionalStreamImplReady(
           used_ssl_config, used_proxy_info,