Throttle HttpStreamFactoryImpl::Job to HTTP/2-supported servers

net/http/http_stream_factory_impl_job.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_job.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 206 matching lines @@
       quic_request_(session_->quic_stream_factory()),
       using_existing_quic_session_(false),
       establishing_tunnel_(false),
       was_alpn_negotiated_(false),
       negotiated_protocol_(kProtoUnknown),
       num_streams_(0),
       spdy_session_direct_(
           !(proxy_info.is_https() && origin_url_.SchemeIs(url::kHttpScheme))),
       spdy_session_key_(GetSpdySessionKey()),
       stream_type_(HttpStreamRequest::BIDIRECTIONAL_STREAM),
+      resumed_init_connection_(false),
       ptr_factory_(this) {
   DCHECK(session);
   // The job can't have alternative service and alternative proxy server set at
   // the same time since alternative services are used for requests that are
   // fetched directly, while the alternative proxy server is used for requests
   // that should be fetched using proxy.
   DCHECK(alternative_service_.protocol == kProtoUnknown ||
          !alternative_proxy_server_.is_valid());
   DCHECK(!alternative_proxy_server_.is_valid() ||
          !(IsSpdyAlternative() || IsQuicAlternative()));
@@ skipping 311 matching lines @@
   RunLoop(result);
 }
 
 void HttpStreamFactoryImpl::Job::RunLoop(int result) {
   TRACE_EVENT0(kNetTracingCategory, "HttpStreamFactoryImpl::Job::RunLoop");
   result = DoLoop(result);
 
   if (result == ERR_IO_PENDING)
     return;
 
+  session_->spdy_session_pool()->ResumeAllPendingRequests(spdy_session_key_);
   if (job_type_ == PRECONNECT) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
         base::Bind(&HttpStreamFactoryImpl::Job::OnPreconnectsComplete,
                    ptr_factory_.GetWeakPtr()));
     return;
   }
 
   if (IsCertificateError(result)) {
     // Retrieve SSL information from the socket.
@@ skipping 111 matching lines @@
         DCHECK_EQ(OK, rv);
         rv = DoStart();
         break;
       case STATE_WAIT:
         DCHECK_EQ(OK, rv);
         rv = DoWait();
         break;
       case STATE_WAIT_COMPLETE:
         rv = DoWaitComplete(rv);
         break;
+      case STATE_BEFORE_INIT_CONNECTION:
+        DCHECK_EQ(OK, rv);
+        rv = DoBeforeInitConnection();
+        break;
       case STATE_INIT_CONNECTION:
         DCHECK_EQ(OK, rv);
         rv = DoInitConnection();
         break;
       case STATE_INIT_CONNECTION_COMPLETE:
         rv = DoInitConnectionComplete(rv);
         break;
       case STATE_WAITING_USER_ACTION:
         rv = DoWaitingUserAction(rv);
         break;
@@ skipping 66 matching lines @@
                       NetLog::BoolCallback("should_wait", should_wait));
   if (should_wait)
     return ERR_IO_PENDING;
 
   return OK;
 }
 
 int HttpStreamFactoryImpl::Job::DoWaitComplete(int result) {
   net_log_.EndEvent(NetLogEventType::HTTP_STREAM_JOB_WAITING);
   DCHECK_EQ(OK, result);
+  next_state_ = STATE_BEFORE_INIT_CONNECTION;
+  return OK;
+}
+
+int HttpStreamFactoryImpl::Job::DoBeforeInitConnection() {
   next_state_ = STATE_INIT_CONNECTION;
+  // Throttle connect to an HTTP/2 supported server, if there are pending
+  // requests with the same SpdySessionKey.
+  if (!session_->http_server_properties()->RequiresHTTP11(

[Bence, 2017/06/09 19:17:38]

Please invert this condition and do an early return.

[xunjieli, 2017/06/13 15:23:10]

Done.

+          spdy_session_key_.host_port_pair())) {
+    url::SchemeHostPort scheme_host_port(
+        using_ssl_ ? url::kHttpsScheme : url::kHttpScheme,
+        spdy_session_key_.host_port_pair().host(),
+        spdy_session_key_.host_port_pair().port());
+    if (session_->http_server_properties()->GetSupportsSpdy(scheme_host_port)) {

[Bence, 2017/06/09 19:17:38]

Please invert this condition and do an early return.

[xunjieli, 2017/06/13 15:23:10]

Done.

+      base::Closure callback =
+          base::Bind(&HttpStreamFactoryImpl::Job::ResumeInitConnection,
+                     ptr_factory_.GetWeakPtr());
+      if (!session_->spdy_session_pool()->StartRequest(spdy_session_key_,
+                                                       callback)) {
+        base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+            FROM_HERE, callback, base::TimeDelta::FromMilliseconds(300));

[Bence, 2017/06/09 19:17:38]

Please define a file-scope const int at the top of this file instead of
hardcoding 300 here.

[xunjieli, 2017/06/13 15:23:10]

Done.

+        return ERR_IO_PENDING;
+      }
+    }
+  }
   return OK;
 }
 
+void HttpStreamFactoryImpl::Job::ResumeInitConnection() {
+  if (resumed_init_connection_)

[Bence, 2017/06/09 19:17:38]

Under what circumstances can this be called multiple times?  In other words, why
is this member necessary at all?

[xunjieli, 2017/06/13 15:23:10]

Assuming there are 2 Jobs (A and B) issued at the same time to the same
H2-supported server. Job B is throttled because A went first, so B posts a
delayed task to resume itself. When the Job A completes, Job A posts a task to
resume B (through SpdySessionPool).
The boolean makes sure that when the delayed task run, it doesn't make the Job B
run DoLoop() again. 

One the other hand, if B already has resumed itself (and is happily doing
connection establishment), when A completes, SpdySession doesn't try to make B
do DoLoop() because B might be in the middle of DoInitConnection() or is in
STATE_DONE.

[Bence, 2017/06/13 16:04:51]

Thanks for the explanation.

+    return;
+  CHECK_EQ(next_state_, STATE_INIT_CONNECTION);
+  resumed_init_connection_ = true;
+  OnIOComplete(OK);
+}
+
 int HttpStreamFactoryImpl::Job::DoInitConnection() {
   net_log_.BeginEvent(NetLogEventType::HTTP_STREAM_JOB_INIT_CONNECTION);
   int result = DoInitConnectionImpl();
   if (result != ERR_SPDY_SESSION_ALREADY_EXISTS)
     delegate_->OnConnectionInitialized(this, result);
 
   return result;
 }
 
 int HttpStreamFactoryImpl::Job::DoInitConnectionImpl() {
@@ skipping 697 matching lines @@
     bool enable_ip_based_pooling,
     NetLog* net_log) {
   return base::MakeUnique<HttpStreamFactoryImpl::Job>(
       delegate, job_type, session, request_info, priority, proxy_info,
       server_ssl_config, proxy_ssl_config, destination, origin_url,
       AlternativeService(), alternative_proxy_server, enable_ip_based_pooling,
       net_log);
 }
 
 }  // namespace net