Respect QuicAllowed policy for new streams

net/http/http_network_session.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_network_session.h"
 
 #include <utility>
 
 #include "base/atomic_sequence_num.h"
 #include "base/compiler_specific.h"
@@ skipping 69 matching lines @@
       proxy_service(NULL),
       ssl_config_service(NULL),
       http_auth_handler_factory(NULL),
       net_log(NULL),
       host_mapping_rules(NULL),
       socket_performance_watcher_factory(NULL),
       ignore_certificate_errors(false),
       testing_fixed_http_port(0),
       testing_fixed_https_port(0),
       enable_tcp_fast_open_for_ssl(false),
+      dynamic_shared_params(NULL),

[mmenke, 2016/12/13 18:48:54]

nullptr is now preferred (Feel free to change the others in this list, if you
want, but not needed)

[pmarko, 2016/12/19 21:25:11]

Done.

       enable_spdy_ping_based_connection_checking(true),
       enable_http2(true),
       spdy_session_max_recv_window_size(kSpdySessionMaxRecvWindowSize),
       spdy_stream_max_recv_window_size(kSpdyStreamMaxRecvWindowSize),
       time_func(&base::TimeTicks::Now),
       enable_http2_alternative_service_with_different_host(false),
       enable_quic_alternative_service_with_different_host(true),
       enable_quic(false),
       disable_quic_on_timeout_with_open_streams(false),
       enable_quic_port_selection(true),
@@ skipping 29 matching lines @@
       proxy_delegate(NULL),
       enable_token_binding(false),
       http_09_on_non_default_ports_enabled(false) {
   quic_supported_versions.push_back(QUIC_VERSION_35);
 }
 
 HttpNetworkSession::Params::Params(const Params& other) = default;
 
 HttpNetworkSession::Params::~Params() {}
 
+bool HttpNetworkSession::Params::enable_quic_for_new_streams() const {
+  return enable_quic && (!dynamic_shared_params ||
+                         dynamic_shared_params->enable_quic_for_new_streams);

[mmenke, 2016/12/13 18:48:54]

I assume we do need both this and enable_quic, and can't just merge them.

[Bence, 2016/12/13 19:24:45]

Correct.  For example, |enable_quic| can be cleared by a command line flag or a
field trial.  In that case, QUIC should not be enabled even if the user policy
changes after HttpNetworkSession initialization.

[pmarko, 2016/12/19 21:25:11]

Reduced to only enable_quic, the logic is now outside the net layer.

+}
+
+HttpNetworkSession::DynamicSharedParams::DynamicSharedParams()
+    : enable_quic_for_new_streams(true) {}
+
+HttpNetworkSession::DynamicSharedParams::DynamicSharedParams(
+    const DynamicSharedParams& other) = default;
+
+HttpNetworkSession::DynamicSharedParams::~DynamicSharedParams() {}
+
 // TODO(mbelshe): Move the socket factories into HttpStreamFactory.
 HttpNetworkSession::HttpNetworkSession(const Params& params)
     : net_log_(params.net_log),
       http_server_properties_(params.http_server_properties),
       cert_verifier_(params.cert_verifier),
       http_auth_handler_factory_(params.http_auth_handler_factory),
       proxy_service_(params.proxy_service),
       ssl_config_service_(params.ssl_config_service),
       quic_stream_factory_(
           params.net_log,
@@ skipping 135 matching lines @@
 
 std::unique_ptr<base::Value> HttpNetworkSession::SpdySessionPoolInfoToValue()
     const {
   return spdy_session_pool_.SpdySessionPoolInfoToValue();
 }
 
 std::unique_ptr<base::Value> HttpNetworkSession::QuicInfoToValue() const {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   dict->Set("sessions", quic_stream_factory_.QuicStreamFactoryInfoToValue());
   dict->SetBoolean("quic_enabled", params_.enable_quic);
+  dict->SetBoolean("quic_enabled_for_new_streams",
+                   params_.enable_quic_for_new_streams());
   dict->SetBoolean("enable_quic_port_selection",
                    params_.enable_quic_port_selection);
   std::unique_ptr<base::ListValue> connection_options(new base::ListValue);
   for (QuicTagVector::const_iterator it =
            params_.quic_connection_options.begin();
        it != params_.quic_connection_options.end(); ++it) {
     connection_options->AppendString("'" + QuicTagToString(*it) + "'");
   }
   dict->Set("connection_options", std::move(connection_options));
 
@@ skipping 47 matching lines @@
 bool HttpNetworkSession::IsProtocolEnabled(NextProto protocol) const {
   switch (protocol) {
     case kProtoUnknown:
       NOTREACHED();
       return false;
     case kProtoHTTP11:
       return true;
     case kProtoHTTP2:
       return params_.enable_http2;
     case kProtoQUIC:
-      return params_.enable_quic;
+      return params_.enable_quic_for_new_streams();

[mmenke, 2016/12/13 18:48:54]

HttpNetworkSession::IsProtocolEnabled() -> enable_quic &&
is_enabled_for_new_streams seems very unexpected to me.  Sorry I didn't notice
this before.  Can we either name functions/values in some internally consistent
manner, or merge the quic values?

[pmarko, 2016/12/19 21:25:11]

Reduced to only enable_quic, the logic is now outside the net layer.

   }
   NOTREACHED();
   return false;
 }
 
 void HttpNetworkSession::GetAlpnProtos(NextProtoVector* alpn_protos) const {
   *alpn_protos = next_protos_;
 }
 
 void HttpNetworkSession::GetSSLConfig(const HttpRequestInfo& request,
@@ skipping 47 matching lines @@
       break;
     case base::MemoryState::SUSPENDED:
     // Note: Not supported at present. Fall through.
     case base::MemoryState::UNKNOWN:
       NOTREACHED();
       break;
   }
 }
 
 }  // namespace net