Hook up DisableQuic logic when the network service is enabled.
This CL also refactors how it works in the case when the
network service is disabled to go through the IOThread's
NetworkService.
Bug: 715695
Change-Id: I3df5126207df21065a42309b8e786f74dabae6cf
Reviewed-on: https://chromium-review.googlesource.com/594670
Reviewed-by: Bernhard Bauer <[email protected]>
Reviewed-by: Julian Pastarmov <[email protected]>
Reviewed-by: Ryan Hamilton <[email protected]>
Reviewed-by: Nasko Oskov <[email protected]>
Commit-Queue: Matt Menke <[email protected]>
Cr-Commit-Position: refs/heads/master@{#491716}diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 96c7b72..a3a1a92b 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -1111,8 +1111,6 @@
"profiles/incognito_helpers.h",
"profiles/incognito_mode_policy_handler.cc",
"profiles/incognito_mode_policy_handler.h",
- "profiles/net_http_session_params_observer.cc",
- "profiles/net_http_session_params_observer.h",
"profiles/off_the_record_profile_impl.cc",
"profiles/off_the_record_profile_impl.h",
"profiles/off_the_record_profile_io_data.cc",
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 245b1afb..190ec43 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -305,7 +305,7 @@
extension_event_router_forwarder_(extension_event_router_forwarder),
#endif
globals_(nullptr),
- is_quic_allowed_by_policy_(true),
+ is_quic_allowed_on_init_(true),
http_09_on_non_default_ports_enabled_(false),
weak_factory_(this) {
scoped_refptr<base::SingleThreadTaskRunner> io_thread_proxy =
@@ -381,16 +381,11 @@
local_state);
pac_https_url_stripping_enabled_.MoveToThread(io_thread_proxy);
- const base::Value* value = policy_service->GetPolicies(
- policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME,
- std::string())).GetValue(policy::key::kQuicAllowed);
- if (value)
- value->GetAsBoolean(&is_quic_allowed_by_policy_);
-
- value = policy_service
- ->GetPolicies(policy::PolicyNamespace(
- policy::POLICY_DOMAIN_CHROME, std::string()))
- .GetValue(policy::key::kHttp09OnNonDefaultPortsEnabled);
+ const base::Value* value =
+ policy_service
+ ->GetPolicies(policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME,
+ std::string()))
+ .GetValue(policy::key::kHttp09OnNonDefaultPortsEnabled);
if (value)
value->GetAsBoolean(&http_09_on_non_default_ports_enabled_);
@@ -400,7 +395,8 @@
BrowserThread::SetIOThreadDelegate(this);
SystemNetworkContextManager::SetUp(&network_context_request_,
- &network_context_params_);
+ &network_context_params_,
+ &is_quic_allowed_on_init_);
}
IOThread::~IOThread() {
@@ -557,8 +553,7 @@
net::CheckSupportAndMaybeEnableTCPFastOpen(always_enable_tfo_if_supported);
ConfigureParamsFromFieldTrialsAndCommandLine(
- command_line, is_quic_allowed_by_policy_,
- http_09_on_non_default_ports_enabled_, &session_params_);
+ command_line, http_09_on_non_default_ports_enabled_, &session_params_);
#if defined(OS_MACOSX)
// Start observing Keychain events. This needs to be done on the UI thread,
@@ -726,12 +721,8 @@
}
void IOThread::DisableQuic() {
- session_params_.enable_quic = false;
-
- if (globals_->system_request_context)
- globals_->system_request_context->http_transaction_factory()
- ->GetSession()
- ->DisableQuic();
+ DCHECK_CURRENTLY_ON(BrowserThread::IO);
+ globals_->network_service->DisableQuic();
}
net::SSLConfigService* IOThread::GetSSLConfigService() {
@@ -869,6 +860,9 @@
builder->DisableHttpCache();
globals_->network_service = content::NetworkService::Create();
+ if (!is_quic_allowed_on_init_)
+ globals_->network_service->DisableQuic();
+
globals_->system_network_context =
globals_->network_service->CreateNetworkContextWithBuilder(
std::move(network_context_request_),
@@ -887,7 +881,6 @@
// static
void IOThread::ConfigureParamsFromFieldTrialsAndCommandLine(
const base::CommandLine& command_line,
- bool is_quic_allowed_by_policy,
bool http_09_on_non_default_ports_enabled,
net::HttpNetworkSession::Params* params) {
std::string quic_user_agent_id = chrome::GetChannelString();
@@ -899,7 +892,8 @@
quic_user_agent_id.append(content::BuildOSCpuInfo());
network_session_configurator::ParseCommandLineAndFieldTrials(
- command_line, !is_quic_allowed_by_policy, quic_user_agent_id, params);
+ command_line, false /* is_quic_force_disabled */, quic_user_agent_id,
+ params);
if (command_line.HasSwitch(switches::kIgnoreUrlFetcherCertRequests))
net::URLFetcher::SetIgnoreCertificateRequests(true);
diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h
index 3451dcc6..0f15e4a 100644
--- a/chrome/browser/io_thread.h
+++ b/chrome/browser/io_thread.h
@@ -202,10 +202,10 @@
const net::HttpNetworkSession::Params& NetworkSessionParams() const;
- // Dynamically disables QUIC for HttpNetworkSessions owned by io_thread, and
- // to HttpNetworkSession::Params which are used for the creation of new
- // HttpNetworkSessions. Not that re-enabling Quic dynamically is not
- // supported for simplicity and requires a browser restart.
+ // Dynamically disables QUIC for all NetworkContexts using the IOThread's
+ // NetworkService. Re-enabling Quic dynamically is not supported for
+ // simplicity and requires a browser restart. May only be called on the IO
+ // thread.
void DisableQuic();
// Returns the callback for updating data use prefs.
@@ -273,7 +273,6 @@
// configure |params|.
static void ConfigureParamsFromFieldTrialsAndCommandLine(
const base::CommandLine& command_line,
- bool is_quic_allowed_by_policy,
bool http_09_on_non_default_ports_enabled,
net::HttpNetworkSession::Params* params);
@@ -354,8 +353,8 @@
scoped_refptr<net::URLRequestContextGetter>
system_url_request_context_getter_;
- // True if QUIC is allowed by policy.
- bool is_quic_allowed_by_policy_;
+ // True if QUIC is initially enabled.
+ bool is_quic_allowed_on_init_;
// True if HTTP/0.9 is allowed on non-default ports by policy.
bool http_09_on_non_default_ports_enabled_;
diff --git a/chrome/browser/io_thread_unittest.cc b/chrome/browser/io_thread_unittest.cc
index 842f9c2..f91d263 100644
--- a/chrome/browser/io_thread_unittest.cc
+++ b/chrome/browser/io_thread_unittest.cc
@@ -61,10 +61,9 @@
}
static void ConfigureParamsFromFieldTrialsAndCommandLine(
const base::CommandLine& command_line,
- bool is_quic_allowed_by_policy,
net::HttpNetworkSession::Params* params) {
- IOThread::ConfigureParamsFromFieldTrialsAndCommandLine(
- command_line, is_quic_allowed_by_policy, false, params);
+ IOThread::ConfigureParamsFromFieldTrialsAndCommandLine(command_line, false,
+ params);
}
};
@@ -330,17 +329,15 @@
: public ::testing::Test {
public:
ConfigureParamsFromFieldTrialsAndCommandLineTest()
- : command_line_(base::CommandLine::NO_PROGRAM),
- is_quic_allowed_by_policy_(true) {}
+ : command_line_(base::CommandLine::NO_PROGRAM) {}
protected:
void ConfigureParamsFromFieldTrialsAndCommandLine() {
- IOThreadPeer::ConfigureParamsFromFieldTrialsAndCommandLine(
- command_line_, is_quic_allowed_by_policy_, ¶ms_);
+ IOThreadPeer::ConfigureParamsFromFieldTrialsAndCommandLine(command_line_,
+ ¶ms_);
}
base::CommandLine command_line_;
- bool is_quic_allowed_by_policy_;
net::HttpNetworkSession::Params params_;
};
@@ -453,16 +450,6 @@
base::ContainsKey(params_.origins_to_force_quic_on, net::HostPortPair()));
}
-TEST_F(ConfigureParamsFromFieldTrialsAndCommandLineTest,
- QuicDisallowedByPolicy) {
- command_line_.AppendSwitch("enable-quic");
- is_quic_allowed_by_policy_ = false;
-
- ConfigureParamsFromFieldTrialsAndCommandLine();
-
- EXPECT_FALSE(params_.enable_quic);
-}
-
TEST_F(ConfigureParamsFromFieldTrialsAndCommandLineTest, QuicMaxPacketLength) {
command_line_.AppendSwitch("enable-quic");
command_line_.AppendSwitchASCII("quic-max-packet-length", "1450");
diff --git a/chrome/browser/net/profile_network_context_service.cc b/chrome/browser/net/profile_network_context_service.cc
index 0b942d87..7daa9d8 100644
--- a/chrome/browser/net/profile_network_context_service.cc
+++ b/chrome/browser/net/profile_network_context_service.cc
@@ -4,10 +4,14 @@
#include "chrome/browser/net/profile_network_context_service.h"
+#include "base/bind.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "build/build_config.h"
+#include "chrome/browser/net/system_network_context_manager.h"
#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "components/pref_registry/pref_registry_syncable.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
@@ -39,7 +43,16 @@
} // namespace
ProfileNetworkContextService::ProfileNetworkContextService(Profile* profile)
- : profile_(profile) {}
+ : profile_(profile) {
+ quic_allowed_.Init(
+ prefs::kQuicAllowed, profile->GetPrefs(),
+ base::Bind(&ProfileNetworkContextService::DisableQuicIfNotAllowed,
+ base::Unretained(this)));
+ // The system context must be initialized before any other network contexts.
+ // TODO(mmenke): Figure out a way to enforce this.
+ SystemNetworkContextManager::Context();
+ DisableQuicIfNotAllowed();
+}
ProfileNetworkContextService::~ProfileNetworkContextService() {}
@@ -78,3 +91,19 @@
MakeRequest(&network_context), CreateMainNetworkContextParams());
return network_context;
}
+
+void ProfileNetworkContextService::RegisterProfilePrefs(
+ user_prefs::PrefRegistrySyncable* registry) {
+ registry->RegisterBooleanPref(prefs::kQuicAllowed, true);
+}
+
+void ProfileNetworkContextService::DisableQuicIfNotAllowed() {
+ if (!quic_allowed_.IsManaged())
+ return;
+
+ // If QUIC is allowed, do nothing (re-enabling QUIC is not supported).
+ if (quic_allowed_.GetValue())
+ return;
+
+ SystemNetworkContextManager::DisableQuic();
+}
diff --git a/chrome/browser/net/profile_network_context_service.h b/chrome/browser/net/profile_network_context_service.h
index fd4c343..91352939 100644
--- a/chrome/browser/net/profile_network_context_service.h
+++ b/chrome/browser/net/profile_network_context_service.h
@@ -7,10 +7,15 @@
#include "base/macros.h"
#include "components/keyed_service/core/keyed_service.h"
+#include "components/prefs/pref_member.h"
#include "content/public/common/network_service.mojom.h"
class Profile;
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
// KeyedService that initializes and provides access to the NetworkContexts for
// a Profile. This will eventually replace ProfileIOData.
class ProfileNetworkContextService : public KeyedService {
@@ -54,7 +59,12 @@
// be called once for a profile, from the ChromeContentBrowserClient.
content::mojom::NetworkContextPtr CreateMainNetworkContext();
+ static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
private:
+ // Checks |quic_allowed_|, and disables QUIC if needed.
+ void DisableQuicIfNotAllowed();
+
Profile* const profile_;
// This is a NetworkContext that wraps ProfileIOData's main URLRequestContext.
@@ -62,6 +72,8 @@
// by Context() when the network service is disabled.
content::mojom::NetworkContextPtr profile_io_data_main_network_context_;
+ BooleanPrefMember quic_allowed_;
+
DISALLOW_COPY_AND_ASSIGN(ProfileNetworkContextService);
};
diff --git a/chrome/browser/net/system_network_context_manager.cc b/chrome/browser/net/system_network_context_manager.cc
index 3ecc596..a8469d82 100644
--- a/chrome/browser/net/system_network_context_manager.cc
+++ b/chrome/browser/net/system_network_context_manager.cc
@@ -6,7 +6,14 @@
#include "base/feature_list.h"
#include "base/logging.h"
+#include "base/values.h"
#include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/io_thread.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "components/policy/core/common/policy_namespace.h"
+#include "components/policy/core/common/policy_service.h"
+#include "components/policy/policy_constants.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/content_features.h"
@@ -31,6 +38,22 @@
return network_context_params;
}
+// Called on IOThread to disable QUIC for HttpNetworkSessions not using the
+// network service. Note that re-enabling QUIC dynamically is not supported for
+// simpliciy and requires a browser restart.
+void DisableQuicOnIOThread(
+ IOThread* io_thread,
+ safe_browsing::SafeBrowsingService* safe_browsing_service) {
+ DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+
+ // Disable QUIC for HttpNetworkSessions using IOThread's NetworkService.
+ io_thread->DisableQuic();
+
+ // Safebrowsing isn't yet using the IOThread's NetworkService, so must be
+ // handled separately.
+ safe_browsing_service->DisableQuicOnIOThread();
+}
+
} // namespace
base::LazyInstance<SystemNetworkContextManager>::Leaky
@@ -42,11 +65,14 @@
void SystemNetworkContextManager::SetUp(
content::mojom::NetworkContextRequest* network_context_request,
- content::mojom::NetworkContextParamsPtr* network_context_params) {
- DCHECK(!GetInstance()->io_thread_network_context_);
+ content::mojom::NetworkContextParamsPtr* network_context_params,
+ bool* is_quic_allowed) {
+ SystemNetworkContextManager* manager = GetInstance();
+ DCHECK(!manager->io_thread_network_context_);
*network_context_request =
- mojo::MakeRequest(&GetInstance()->io_thread_network_context_);
+ mojo::MakeRequest(&manager->io_thread_network_context_);
*network_context_params = CreateNetworkContextParams();
+ *is_quic_allowed = manager->is_quic_allowed_;
}
SystemNetworkContextManager* SystemNetworkContextManager::GetInstance() {
@@ -54,7 +80,15 @@
return g_system_network_context_manager.Pointer();
}
-SystemNetworkContextManager::SystemNetworkContextManager() {}
+SystemNetworkContextManager::SystemNetworkContextManager() {
+ const base::Value* value =
+ g_browser_process->policy_service()
+ ->GetPolicies(policy::PolicyNamespace(policy::POLICY_DOMAIN_CHROME,
+ std::string()))
+ .GetValue(policy::key::kQuicAllowed);
+ if (value)
+ value->GetAsBoolean(&is_quic_allowed_);
+}
SystemNetworkContextManager::~SystemNetworkContextManager() {}
@@ -66,9 +100,37 @@
}
if (!network_service_network_context_) {
- content::GetNetworkService()->CreateNetworkContext(
+ content::mojom::NetworkService* network_service =
+ content::GetNetworkService();
+ if (!is_quic_allowed_)
+ network_service->DisableQuic();
+ network_service->CreateNetworkContext(
MakeRequest(&network_service_network_context_),
CreateNetworkContextParams());
}
return network_service_network_context_.get();
}
+
+void SystemNetworkContextManager::DisableQuic() {
+ SystemNetworkContextManager* manager = GetInstance();
+ if (!manager->is_quic_allowed_)
+ return;
+
+ manager->is_quic_allowed_ = false;
+
+ // Disabling QUIC for a profile disables QUIC globally. As a side effect, new
+ // Profiles will also have QUIC disabled (because both IOThread's
+ // NetworkService and the network service, if enabled will disable QUIC).
+
+ if (base::FeatureList::IsEnabled(features::kNetworkService))
+ content::GetNetworkService()->DisableQuic();
+
+ IOThread* io_thread = g_browser_process->io_thread();
+ safe_browsing::SafeBrowsingService* safe_browsing_service =
+ g_browser_process->safe_browsing_service();
+
+ content::BrowserThread::PostTask(
+ content::BrowserThread::IO, FROM_HERE,
+ base::BindOnce(&DisableQuicOnIOThread, io_thread,
+ base::Unretained(safe_browsing_service)));
+}
diff --git a/chrome/browser/net/system_network_context_manager.h b/chrome/browser/net/system_network_context_manager.h
index b3003c2..a9e03bf 100644
--- a/chrome/browser/net/system_network_context_manager.h
+++ b/chrome/browser/net/system_network_context_manager.h
@@ -39,13 +39,22 @@
// NetworkContext will not be used by the SystemNetworkContextManager.
//
// Must be called before the system NetworkContext is first used.
+ //
+ // |is_quic_allowed| is set to true if policy allows QUIC to be enabled.
static void SetUp(
content::mojom::NetworkContextRequest* network_context_request,
- content::mojom::NetworkContextParamsPtr* network_context_params);
+ content::mojom::NetworkContextParamsPtr* network_context_params,
+ bool* is_quic_allowed);
- // Returns the System NetworkContext. May only be called after SetUp().
+ // Returns the System NetworkContext. May only be called after SetUp(). Does
+ // any initialization of the NetworkService that may be needed when first
+ // called.
static content::mojom::NetworkContext* Context();
+ // Permanently disables QUIC, both for NetworkContexts using the IOThread's
+ // NetworkService, and for those using the network service (if enabled).
+ static void DisableQuic();
+
private:
static SystemNetworkContextManager* GetInstance();
@@ -66,6 +75,8 @@
// network service is disabled.
content::mojom::NetworkContextPtr io_thread_network_context_;
+ bool is_quic_allowed_ = true;
+
DISALLOW_COPY_AND_ASSIGN(SystemNetworkContextManager);
};
diff --git a/chrome/browser/policy/policy_network_browsertest.cc b/chrome/browser/policy/policy_network_browsertest.cc
index 0dc25a0..701502b 100644
--- a/chrome/browser/policy/policy_network_browsertest.cc
+++ b/chrome/browser/policy/policy_network_browsertest.cc
@@ -403,7 +403,7 @@
EXPECT_FALSE(IsQuicEnabled(system_request_context()));
EXPECT_FALSE(IsQuicEnabled(safe_browsing_service_request_context()));
EXPECT_FALSE(IsQuicEnabled(profile_1()->GetRequestContext()));
- EXPECT_TRUE(IsQuicEnabled(profile_2()->GetRequestContext()));
+ EXPECT_FALSE(IsQuicEnabled(profile_2()->GetRequestContext()));
// Disable QUIC in second profile
SetQuicAllowedPolicy(policy_for_profile_2(), false);
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index f16d3af..41d9382 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -34,6 +34,7 @@
#include "chrome/browser/net/nqe/ui_network_quality_estimator_service.h"
#include "chrome/browser/net/prediction_options.h"
#include "chrome/browser/net/predictor.h"
+#include "chrome/browser/net/profile_network_context_service.h"
#include "chrome/browser/notifications/extension_welcome_notification.h"
#include "chrome/browser/notifications/notifier_state_tracker.h"
#include "chrome/browser/pepper_flash_settings_manager.h"
@@ -43,7 +44,6 @@
#include "chrome/browser/prefs/origin_trial_prefs.h"
#include "chrome/browser/prefs/session_startup_pref.h"
#include "chrome/browser/profiles/chrome_version_service.h"
-#include "chrome/browser/profiles/net_http_session_params_observer.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_impl.h"
#include "chrome/browser/profiles/profile_info_cache.h"
@@ -472,7 +472,6 @@
MediaDeviceIDSalt::RegisterProfilePrefs(registry);
MediaStreamDevicesController::RegisterProfilePrefs(registry);
NavigationCorrectionTabObserver::RegisterProfilePrefs(registry);
- NetHttpSessionParamsObserver::RegisterProfilePrefs(registry);
NotifierStateTracker::RegisterProfilePrefs(registry);
ntp_snippets::BreakingNewsGCMAppHandler::RegisterProfilePrefs(registry);
ntp_snippets::ContentSuggestionsService::RegisterProfilePrefs(registry);
@@ -491,6 +490,7 @@
PrefsTabHelper::RegisterProfilePrefs(registry);
Profile::RegisterProfilePrefs(registry);
ProfileImpl::RegisterProfilePrefs(registry);
+ ProfileNetworkContextService::RegisterProfilePrefs(registry);
ProtocolHandlerRegistry::RegisterProfilePrefs(registry);
PushMessagingAppIdentifier::RegisterProfilePrefs(registry);
RegisterBrowserUserPrefs(registry);
diff --git a/chrome/browser/profiles/net_http_session_params_observer.cc b/chrome/browser/profiles/net_http_session_params_observer.cc
deleted file mode 100644
index cc14716..0000000
--- a/chrome/browser/profiles/net_http_session_params_observer.cc
+++ /dev/null
@@ -1,96 +0,0 @@
-// Copyright 2017 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/net_http_session_params_observer.h"
-
-#include "base/bind.h"
-#include "base/command_line.h"
-#include "base/logging.h"
-#include "chrome/browser/browser_process.h"
-#include "chrome/browser/io_thread.h"
-#include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/safe_browsing/safe_browsing_service.h"
-#include "chrome/common/chrome_switches.h"
-#include "chrome/common/pref_names.h"
-#include "components/pref_registry/pref_registry_syncable.h"
-#include "components/prefs/pref_service.h"
-#include "content/public/browser/browser_thread.h"
-
-using content::BrowserThread;
-
-namespace {
-
-// Called on IOThread to disable QUIC for globally-owned HttpNetworkSessions
-// and for the profile (thrpugh |disable_quic_callback|). Note that re-enabling
-// QUIC dynamically is not supported for simpliciy and requires a browser
-// restart.
-void DisableQuicOnIOThread(
- NetHttpSessionParamsObserver::DisableQuicCallback disable_quic_callback,
- IOThread* io_thread,
- safe_browsing::SafeBrowsingService* safe_browsing_service) {
- DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
- // Disable QUIC for globally-owned objects.
- io_thread->DisableQuic();
- safe_browsing_service->DisableQuicOnIOThread();
-
- // Call profile's disable QUIC callback.
- disable_quic_callback.Run();
-}
-
-} // namespace
-
-NetHttpSessionParamsObserver::NetHttpSessionParamsObserver(
- PrefService* prefs,
- DisableQuicCallback disable_quic_callback)
- : disable_quic_callback_(disable_quic_callback) {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
- DCHECK(prefs);
-
- base::Closure prefs_callback = base::Bind(
- &NetHttpSessionParamsObserver::ApplySettings, base::Unretained(this));
- quic_allowed_.Init(prefs::kQuicAllowed, prefs, prefs_callback);
-
- // Apply the initial settings, in case QUIC is disallowed by policy on profile
- // initialization time. Note that even if this may not affect the profile
- // which is being initialized directly (as its URLRequestContext will not be
- // constructed yet), it still applies to globally-owned HttpNetworkSessions
- // and to IOThread's HttpNetworkSession Params, which will in turn be used as
- // a base for the new profile's URLRequestContext.
- ApplySettings();
-}
-
-NetHttpSessionParamsObserver::~NetHttpSessionParamsObserver() {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
-}
-
-void NetHttpSessionParamsObserver::ApplySettings() {
- DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
- bool quic_allowed_for_profile = true;
- if (quic_allowed_.IsManaged())
- quic_allowed_for_profile = *quic_allowed_;
-
- // We only support disabling QUIC.
- if (quic_allowed_for_profile)
- return;
-
- // Disabling QUIC for a profile also disables QUIC for globally-owned
- // HttpNetworkSessions. As a side effect, new Profiles will also have QUIC
- // disabled (because they inherit IOThread's HttpNetworkSession Params).
- IOThread* io_thread = g_browser_process->io_thread();
- safe_browsing::SafeBrowsingService* safe_browsing_service =
- g_browser_process->safe_browsing_service();
-
- BrowserThread::PostTask(
- BrowserThread::IO, FROM_HERE,
- base::BindOnce(&DisableQuicOnIOThread, disable_quic_callback_, io_thread,
- base::Unretained(safe_browsing_service)));
-}
-
-// static
-void NetHttpSessionParamsObserver::RegisterProfilePrefs(
- user_prefs::PrefRegistrySyncable* registry) {
- registry->RegisterBooleanPref(prefs::kQuicAllowed, true);
-}
diff --git a/chrome/browser/profiles/net_http_session_params_observer.h b/chrome/browser/profiles/net_http_session_params_observer.h
deleted file mode 100644
index 4004c74..0000000
--- a/chrome/browser/profiles/net_http_session_params_observer.h
+++ /dev/null
@@ -1,51 +0,0 @@
-// Copyright 2017 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.
-
-#ifndef CHROME_BROWSER_PROFILES_NET_HTTP_SESSION_PARAMS_OBSERVER_H_
-#define CHROME_BROWSER_PROFILES_NET_HTTP_SESSION_PARAMS_OBSERVER_H_
-
-#include "base/callback_forward.h"
-#include "base/macros.h"
-#include "components/prefs/pref_member.h"
-
-class PrefService;
-
-namespace user_prefs {
-class PrefRegistrySyncable;
-}
-
-// Monitors network-related preferences for changes and applies them to
-// globally owned HttpNetworkSessions.
-// Profile-owned HttpNetworkSessions are taken care of by a
-// UpdateNetParamsCallback passed to the constructor.
-// The supplied PrefService must outlive this NetHttpSessionParamsObserver.
-// Must be used only on the UI thread.
-class NetHttpSessionParamsObserver {
- public:
- // Type of callback which will be called when QUIC is disabled. This
- // callback will be called on the IO thread.
- typedef base::Callback<void()> DisableQuicCallback;
-
- // |prefs| must be non-NULL and |*prefs| must outlive this.
- // |update_net_params_callback| will be invoked on the IO thread when an
- // observed network parmeter is changed.
- NetHttpSessionParamsObserver(PrefService* prefs,
- DisableQuicCallback disable_quic_callback);
- ~NetHttpSessionParamsObserver();
-
- // Register user preferences which NetHttpSessionParamsObserver uses into
- // |registry|.
- static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
-
- private:
- // Called on UI thread when an observed net pref changes
- void ApplySettings();
-
- BooleanPrefMember quic_allowed_;
- DisableQuicCallback disable_quic_callback_;
-
- DISALLOW_COPY_AND_ASSIGN(NetHttpSessionParamsObserver);
-};
-
-#endif // CHROME_BROWSER_PROFILES_NET_HTTP_SESSION_PARAMS_OBSERVER_H_
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc
index a4101643..1714a92 100644
--- a/chrome/browser/profiles/profile_io_data.cc
+++ b/chrome/browser/profiles/profile_io_data.cc
@@ -46,7 +46,6 @@
#include "chrome/browser/policy/policy_helpers.h"
#include "chrome/browser/predictors/loading_predictor.h"
#include "chrome/browser/predictors/loading_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"
@@ -483,13 +482,6 @@
&allowed_domains_for_apps_,
pref_service);
- DCHECK(!net_http_session_params_observer_);
- NetHttpSessionParamsObserver::DisableQuicCallback disable_quic_callback =
- base::Bind(&ProfileIOData::DisableQuicOnIOThread, base::Unretained(this));
- net_http_session_params_observer_ =
- base::MakeUnique<NetHttpSessionParamsObserver>(pref_service,
- disable_quic_callback);
-
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner =
BrowserThread::GetTaskRunnerForThread(BrowserThread::IO);
@@ -972,19 +964,6 @@
#endif
}
-void ProfileIOData::DisableQuicOnIOThread() {
- // If the URLRequestContext has not been created yet, it will not be updated
- // here. Instead, it will inherit its QUIC enablement from IOThread on
- // construction, which is fine, as NetHttpSessionParamsObserver also disables
- // QUIC there.
- if (!main_request_context_)
- return;
-
- main_request_context_->http_transaction_factory()
- ->GetSession()
- ->DisableQuic();
-}
-
void ProfileIOData::set_data_reduction_proxy_io_data(
std::unique_ptr<data_reduction_proxy::DataReductionProxyIOData>
data_reduction_proxy_io_data) const {
@@ -1387,7 +1366,6 @@
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)) {
diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h
index 635956e..1fdf2008 100644
--- a/chrome/browser/profiles/profile_io_data.h
+++ b/chrome/browser/profiles/profile_io_data.h
@@ -44,7 +44,6 @@
class ChromeURLRequestContextGetter;
class ChromeExpectCTReporter;
class HostContentSettingsMap;
-class NetHttpSessionParamsObserver;
class ProtocolHandlerRegistry;
namespace chromeos {
@@ -252,9 +251,6 @@
// Get platform ClientCertStore. May return nullptr.
std::unique_ptr<net::ClientCertStore> CreateClientCertStore();
- // Called on IO thread thread to disable QUIC.
- void DisableQuicOnIOThread();
-
protected:
// A URLRequestContext for media that owns its HTTP factory, to ensure
// it is deleted.
@@ -563,11 +559,6 @@
BooleanPrefMember enable_metrics_;
- // Observes profile's preference for changes to prefs which affect
- // HttpNetworkSession params.
- std::unique_ptr<NetHttpSessionParamsObserver>
- net_http_session_params_observer_;
-
// Pointed to by NetworkDelegate.
mutable std::unique_ptr<policy::URLBlacklistManager> url_blacklist_manager_;
mutable std::unique_ptr<policy::PolicyHeaderIOHelper> policy_header_helper_;
diff --git a/content/network/network_context.cc b/content/network/network_context.cc
index b739602..e5b727f 100644
--- a/content/network/network_context.cc
+++ b/content/network/network_context.cc
@@ -18,6 +18,7 @@
#include "net/dns/host_resolver.h"
#include "net/dns/mapped_host_resolver.h"
#include "net/http/http_network_session.h"
+#include "net/http/http_transaction_factory.h"
#include "net/proxy/proxy_config.h"
#include "net/proxy/proxy_config_service_fixed.h"
#include "net/url_request/url_request_context.h"
@@ -29,7 +30,8 @@
void ApplyContextParamsToBuilder(
net::URLRequestContextBuilder* builder,
- mojom::NetworkContextParams* network_context_params) {
+ mojom::NetworkContextParams* network_context_params,
+ NetworkServiceImpl* network_service) {
builder->set_data_enabled(network_context_params->enable_data_url_support);
#if !BUILDFLAG(DISABLE_FILE_SUPPORT)
builder->set_file_enabled(network_context_params->enable_file_url_support);
@@ -41,16 +43,22 @@
#else // BUILDFLAG(DISABLE_FTP_SUPPORT)
DCHECK(!network_context_params->enable_ftp_url_support);
#endif
+
+ if (network_service && network_service->quic_disabled())
+ builder->SetQuicEnabled(false);
}
std::unique_ptr<net::URLRequestContext> MakeURLRequestContext(
- mojom::NetworkContextParams* network_context_params) {
+ mojom::NetworkContextParams* network_context_params,
+ NetworkServiceImpl* network_service) {
net::URLRequestContextBuilder builder;
net::HttpNetworkSession::Params params;
const base::CommandLine* command_line =
base::CommandLine::ForCurrentProcess();
if (command_line->HasSwitch(switches::kIgnoreCertificateErrors))
params.ignore_certificate_errors = true;
+ if (command_line->HasSwitch(switches::kEnableQuic))
+ params.enable_quic = true;
if (command_line->HasSwitch(switches::kTestingFixedHttpPort)) {
int value;
@@ -98,7 +106,8 @@
builder.set_proxy_service(net::ProxyService::CreateDirect());
}
- ApplyContextParamsToBuilder(&builder, network_context_params);
+ ApplyContextParamsToBuilder(&builder, network_context_params,
+ network_service);
return builder.Build();
}
@@ -109,7 +118,8 @@
mojom::NetworkContextRequest request,
mojom::NetworkContextParamsPtr params)
: network_service_(network_service),
- url_request_context_(MakeURLRequestContext(params.get())),
+ url_request_context_(
+ MakeURLRequestContext(params.get(), network_service)),
params_(std::move(params)),
binding_(this, std::move(request)) {
network_service_->RegisterNetworkContext(this);
@@ -121,13 +131,15 @@
// constructors. Can only share them once consumer code is ready for its
// corresponding options to be overwritten.
NetworkContext::NetworkContext(
+ NetworkServiceImpl* network_service,
mojom::NetworkContextRequest request,
mojom::NetworkContextParamsPtr params,
std::unique_ptr<net::URLRequestContextBuilder> builder)
- : network_service_(nullptr),
+ : network_service_(network_service),
params_(std::move(params)),
binding_(this, std::move(request)) {
- ApplyContextParamsToBuilder(builder.get(), params_.get());
+ network_service_->RegisterNetworkContext(this);
+ ApplyContextParamsToBuilder(builder.get(), params_.get(), network_service);
url_request_context_ = builder->Build();
}
@@ -171,6 +183,10 @@
StartCacheURLLoader(url, url_request_context_.get(), std::move(client));
}
+void NetworkContext::DisableQuic() {
+ url_request_context_->http_transaction_factory()->GetSession()->DisableQuic();
+}
+
void NetworkContext::Cleanup() {
// The NetworkService is going away, so have to destroy the
// net::URLRequestContext held by this NetworkContext.
@@ -181,7 +197,7 @@
: network_service_(nullptr),
params_(mojom::NetworkContextParams::New()),
binding_(this) {
- url_request_context_ = MakeURLRequestContext(params_.get());
+ url_request_context_ = MakeURLRequestContext(params_.get(), network_service_);
}
void NetworkContext::OnConnectionError() {
diff --git a/content/network/network_context.h b/content/network/network_context.h
index 6ceef3c..2c5d524 100644
--- a/content/network/network_context.h
+++ b/content/network/network_context.h
@@ -47,7 +47,8 @@
// Temporary constructor that allows creating an in-process NetworkContext
// with a pre-populated URLRequestContextBuilder.
- NetworkContext(mojom::NetworkContextRequest request,
+ NetworkContext(NetworkServiceImpl* network_service,
+ mojom::NetworkContextRequest request,
mojom::NetworkContextParamsPtr params,
std::unique_ptr<net::URLRequestContextBuilder> builder);
@@ -74,6 +75,9 @@
// destroy NetworkContext's URLRequestContext.
void Cleanup();
+ // Disables use of QUIC by the NetworkContext.
+ void DisableQuic();
+
private:
NetworkContext();
diff --git a/content/network/network_context_unittest.cc b/content/network/network_context_unittest.cc
index 3f32e28..2d6c0f9 100644
--- a/content/network/network_context_unittest.cc
+++ b/content/network/network_context_unittest.cc
@@ -4,13 +4,17 @@
#include <memory>
+#include "base/command_line.h"
#include "base/test/scoped_task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
+#include "components/network_session_configurator/common/network_switches.h"
#include "content/network/network_context.h"
#include "content/network/network_service_impl.h"
#include "content/public/common/network_service.mojom.h"
#include "mojo/public/cpp/bindings/interface_request.h"
+#include "net/http/http_network_session.h"
+#include "net/http/http_transaction_factory.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_job_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -35,6 +39,10 @@
std::move(context_params));
}
+ mojom::NetworkService* network_service() const {
+ return network_service_.get();
+ }
+
private:
base::test::ScopedTaskEnvironment scoped_task_environment_;
std::unique_ptr<NetworkServiceImpl> network_service_;
@@ -45,6 +53,47 @@
mojom::NetworkContextPtr network_context_ptr_;
};
+TEST_F(NetworkContextTest, DisableQuic) {
+ base::CommandLine::ForCurrentProcess()->AppendSwitch(switches::kEnableQuic);
+
+ std::unique_ptr<NetworkContext> network_context =
+ CreateContextWithParams(mojom::NetworkContextParams::New());
+ // By default, QUIC should be enabled for new NetworkContexts when the command
+ // line indicates it should be.
+ EXPECT_TRUE(network_context->url_request_context()
+ ->http_transaction_factory()
+ ->GetSession()
+ ->params()
+ .enable_quic);
+
+ // Disabling QUIC should disable it on existing NetworkContexts.
+ network_service()->DisableQuic();
+ EXPECT_FALSE(network_context->url_request_context()
+ ->http_transaction_factory()
+ ->GetSession()
+ ->params()
+ .enable_quic);
+
+ // Disabling QUIC should disable it new NetworkContexts.
+ std::unique_ptr<NetworkContext> network_context2 =
+ CreateContextWithParams(mojom::NetworkContextParams::New());
+ EXPECT_FALSE(network_context2->url_request_context()
+ ->http_transaction_factory()
+ ->GetSession()
+ ->params()
+ .enable_quic);
+
+ // Disabling QUIC again should be harmless.
+ network_service()->DisableQuic();
+ std::unique_ptr<NetworkContext> network_context3 =
+ CreateContextWithParams(mojom::NetworkContextParams::New());
+ EXPECT_FALSE(network_context3->url_request_context()
+ ->http_transaction_factory()
+ ->GetSession()
+ ->params()
+ .enable_quic);
+}
+
TEST_F(NetworkContextTest, DisableDataUrlSupport) {
mojom::NetworkContextParamsPtr context_params =
mojom::NetworkContextParams::New();
diff --git a/content/network/network_service_impl.cc b/content/network/network_service_impl.cc
index 9b4b9b7c..01890e7 100644
--- a/content/network/network_service_impl.cc
+++ b/content/network/network_service_impl.cc
@@ -87,8 +87,8 @@
std::unique_ptr<net::URLRequestContextBuilder> builder,
net::URLRequestContext** url_request_context) {
std::unique_ptr<NetworkContext> network_context =
- base::MakeUnique<NetworkContext>(std::move(request), std::move(params),
- std::move(builder));
+ base::MakeUnique<NetworkContext>(this, std::move(request),
+ std::move(params), std::move(builder));
*url_request_context = network_context->url_request_context();
return network_context;
}
@@ -118,6 +118,14 @@
new NetworkContext(this, std::move(request), std::move(params));
}
+void NetworkServiceImpl::DisableQuic() {
+ quic_disabled_ = true;
+
+ for (auto* network_context : network_contexts_) {
+ network_context->DisableQuic();
+ }
+}
+
void NetworkServiceImpl::OnBindInterface(
const service_manager::BindSourceInfo& source_info,
const std::string& interface_name,
diff --git a/content/network/network_service_impl.h b/content/network/network_service_impl.h
index d2397f3..7ec0ef5 100644
--- a/content/network/network_service_impl.h
+++ b/content/network/network_service_impl.h
@@ -48,10 +48,15 @@
// mojom::NetworkService implementation:
void CreateNetworkContext(mojom::NetworkContextRequest request,
mojom::NetworkContextParamsPtr params) override;
+ void DisableQuic() override;
+
+ bool quic_disabled() const { return quic_disabled_; }
private:
class MojoNetLog;
+ friend class NetworkService;
+
// service_manager::Service implementation.
void OnBindInterface(const service_manager::BindSourceInfo& source_info,
const std::string& interface_name,
@@ -71,6 +76,8 @@
// destroyed first.
std::set<NetworkContext*> network_contexts_;
+ bool quic_disabled_ = false;
+
DISALLOW_COPY_AND_ASSIGN(NetworkServiceImpl);
};
diff --git a/content/public/common/network_service.mojom b/content/public/common/network_service.mojom
index dad388fd..3c004330 100644
--- a/content/public/common/network_service.mojom
+++ b/content/public/common/network_service.mojom
@@ -50,4 +50,8 @@
// Creates a new network context with the given parameters.
CreateNetworkContext(NetworkContext& context,
NetworkContextParams params);
+
+ // Disables QUIC for the NetworkService. Affects all existing NetworkContexts,
+ // and all new ones that are created. Once called, QUIC cannot be re-enabled.
+ DisableQuic();
};
diff --git a/content/public/network/network_service.h b/content/public/network/network_service.h
index a4bb4de..e2578535 100644
--- a/content/public/network/network_service.h
+++ b/content/public/network/network_service.h
@@ -20,7 +20,6 @@
// Allows an in-process NetworkService to be set up.
class CONTENT_EXPORT NetworkService : public mojom::NetworkService {
public:
- // TODO(mmenke): Take in a NetworkServiceRequest.
static std::unique_ptr<NetworkService> Create();
// Can be used to seed a NetworkContext with a consumer-configured
diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index ab0ac6e..c3e425f 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -1527,6 +1527,7 @@
"//cc/ipc",
"//components/leveldb/public/cpp",
"//components/metrics/proto",
+ "//components/network_session_configurator/common",
"//components/offline_pages/features:features",
"//components/rappor:test_support",
"//components/ukm:test_support",
diff --git a/net/url_request/url_request_context_builder.cc b/net/url_request/url_request_context_builder.cc
index d939129f..fb5deb56 100644
--- a/net/url_request/url_request_context_builder.cc
+++ b/net/url_request/url_request_context_builder.cc
@@ -275,6 +275,10 @@
http_network_session_params_.enable_quic = quic_enabled;
}
+void URLRequestContextBuilder::SetQuicEnabled(bool quic_enabled) {
+ http_network_session_params_.enable_quic = quic_enabled;
+}
+
void URLRequestContextBuilder::set_ct_verifier(
std::unique_ptr<CTVerifier> ct_verifier) {
ct_verifier_ = std::move(ct_verifier);
diff --git a/net/url_request/url_request_context_builder.h b/net/url_request/url_request_context_builder.h
index 8f1b0d5..1a1b1b1 100644
--- a/net/url_request/url_request_context_builder.h
+++ b/net/url_request/url_request_context_builder.h
@@ -292,6 +292,9 @@
void SetSpdyAndQuicEnabled(bool spdy_enabled,
bool quic_enabled);
+ // TODO(mmenke): Remove this.
+ void SetQuicEnabled(bool quic_enabled);
+
void set_throttling_enabled(bool throttling_enabled) {
throttling_enabled_ = throttling_enabled;
}
