Respect QuicAllowed policy for new streams

chrome/browser/profiles/profile_io_data.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 "chrome/browser/profiles/profile_io_data.h"
 
 #include <stddef.h>
 
 #include <string>
 #include <utility>
@@ skipping 28 matching lines @@
 #include "chrome/browser/media/media_device_id_salt.h"
 #include "chrome/browser/net/chrome_http_user_agent_settings.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/net/chrome_url_request_context_getter.h"
 #include "chrome/browser/net/proxy_service_factory.h"
 #include "chrome/browser/net/resource_prefetch_predictor_observer.h"
 #include "chrome/browser/policy/cloud/policy_header_service_factory.h"
 #include "chrome/browser/policy/policy_helpers.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor.h"
 #include "chrome/browser/predictors/resource_prefetch_predictor_factory.h"
+#include "chrome/browser/profiles/net_http_session_params_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ssl/chrome_expect_ct_reporter.h"
 #include "chrome/browser/ui/search/new_tab_page_interceptor_service.h"
 #include "chrome/browser/ui/search/new_tab_page_interceptor_service_factory.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
@@ skipping 404 matching lines @@
   profile_params_ = std::move(params);
 
   ChromeNetworkDelegate::InitializePrefsOnUIThread(
       &enable_referrers_,
       &enable_do_not_track_,
       &force_google_safesearch_,
       &force_youtube_restrict_,
       &allowed_domains_for_apps_,
       pref_service);
 
+  DCHECK(!net_http_session_params_observer_);
+  NetHttpSessionParamsObserver::UpdateNetParamsCallback
+      update_net_params_callback = base::Bind(
+          &ProfileIOData::UpdateNetParamsOnIOThread, base::Unretained(this));
+  net_http_session_params_observer_ =
+      base::MakeUnique<NetHttpSessionParamsObserver>(
+          pref_service, update_net_params_callback);

[mmenke, 2017/01/09 16:46:04]

I believe this is a bug.  We haven't even created the URLRequestContext by this
point, so this will call post a task to call ApplySettingsOnIOThread(), which
calls  ProfileIOData::UpdateNetParamsOnIOThread, which notices that there's no
URLRequestContext, so it does nothing.  Then we when we actually create the
URLRequestContext, it doesn't pick up on the configuration, and if it's enabled
in IOThread::GetNetworkSessionParams, we're out of luck.

In contravention of what I said over email earlier this morning, I think we're
going to need browser tests for this code, if there's a way to set prefs or
group policies before profile creation.  Too many races, too easy to get wrong.

[pmarko, 2017/01/09 19:53:12]

This was very misleading, sorry about that. I've tried to make this more clear
now. As we're only disabling QUIC now, and disabling it also disables it for
IOThread, both ways work:

(1) URLRequestContext already constructed: QUIC disabled through callback
(2) URLRequestContext not constructed yet: QUIC disabled in IOThread's params_,
which will be used for URLRequestContext construction -> QUIC disabled there.

You're absolutely right that this would not work for re-enabling QUIc, and as
we've discussed, I've made it obvious by naming now that we are not supporting
re-enabling.

+
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner =
       BrowserThread::GetTaskRunnerForThread(BrowserThread::IO);
 
   chrome_http_user_agent_settings_.reset(
       new ChromeHttpUserAgentSettings(pref_service));
 
   // These members are used only for sign in, which is not enabled
   // in incognito mode.  So no need to initialize them.
   if (!IsOffTheRecord()) {
     google_services_user_account_id_.Init(
@@ skipping 436 matching lines @@
 #elif defined(OS_ANDROID)
   // Android does not use the ClientCertStore infrastructure. On Android client
   // cert matching is done by the OS as part of the call to show the cert
   // selection dialog.
   return nullptr;
 #else
 #error Unknown platform.
 #endif
 }
 
+void ProfileIOData::UpdateNetParamsOnIOThread(
+    net::HttpNetworkSession::ParamsUpdate params_update) {
+  if (!main_request_context_storage_ ||
+      !main_request_context_storage_->http_network_session())
+    return;
+
+  main_request_context_storage_->http_network_session()->UpdateParams(
+      params_update);
+}
+
 void ProfileIOData::set_data_reduction_proxy_io_data(
     std::unique_ptr<data_reduction_proxy::DataReductionProxyIOData>
         data_reduction_proxy_io_data) const {
   data_reduction_proxy_io_data_ = std::move(data_reduction_proxy_io_data);
 }
 
 ProfileIOData::ResourceContext::ResourceContext(ProfileIOData* io_data)
     : io_data_(io_data),
       host_resolver_(NULL),
       request_context_(NULL) {
@@ skipping 290 matching lines @@
   if (media_device_id_salt_.get())
     media_device_id_salt_->ShutdownOnUIThread();
   session_startup_pref_.Destroy();
   if (url_blacklist_manager_)
     url_blacklist_manager_->ShutdownOnUIThread();
   if (ct_policy_manager_)
     ct_policy_manager_->Shutdown();
   if (chrome_http_user_agent_settings_)
     chrome_http_user_agent_settings_->CleanupOnUIThread();
   incognito_availibility_pref_.Destroy();
+  net_http_session_params_observer_.reset();
 
   if (!context_getters->empty()) {
     if (BrowserThread::IsMessageLoopValid(BrowserThread::IO)) {
       BrowserThread::PostTask(
           BrowserThread::IO, FROM_HERE,
           base::Bind(&NotifyContextGettersOfShutdownOnIO,
               base::Passed(&context_getters)));
     }
   }
 
@@ skipping 59 matching lines @@
 void ProfileIOData::SetCookieSettingsForTesting(
     content_settings::CookieSettings* cookie_settings) {
   DCHECK(!cookie_settings_.get());
   cookie_settings_ = cookie_settings;
 }
 
 policy::URLBlacklist::URLBlacklistState ProfileIOData::GetURLBlacklistState(
     const GURL& url) const {
   return url_blacklist_manager_->GetURLBlacklistState(url);
 }