Commit f9242676 authored by Min Qin's avatar Min Qin Committed by Commit Bot

Passing local_state to ProxyConfigMonitor

SystemNetworkContextManager already takes a PrefService in its ctor.
As a memeber variable of SystemNetworkContextManager, ProxyConfigMonitor
should do the same.
This CL also fixes an issue that ProxyServiceFactory is using
BrowserThread::UI to create the system proxy service. It is possible
that BrowserThread is not created if ServiceManager is started alone.
Switching to use base::ThreadTaskRunnerHandle::Get() instead.

BUG=866028

Change-Id: I6a33310e94a6050ca0bd22bdd716660aea790b58
Reviewed-on: https://chromium-review.googlesource.com/1255144Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Reviewed-by: default avatarJialiu Lin <jialiul@chromium.org>
Reviewed-by: default avatarMatt Menke <mmenke@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595594}
parent 5f0ad396
...@@ -21,6 +21,8 @@ ...@@ -21,6 +21,8 @@
#include "chrome/browser/extensions/api/proxy/proxy_api.h" #include "chrome/browser/extensions/api/proxy/proxy_api.h"
#endif #endif
using content::BrowserThread;
ProxyConfigMonitor::ProxyConfigMonitor(Profile* profile) { ProxyConfigMonitor::ProxyConfigMonitor(Profile* profile) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
DCHECK(profile); DCHECK(profile);
...@@ -51,12 +53,13 @@ ProxyConfigMonitor::ProxyConfigMonitor(Profile* profile) { ...@@ -51,12 +53,13 @@ ProxyConfigMonitor::ProxyConfigMonitor(Profile* profile) {
proxy_config_service_->AddObserver(this); proxy_config_service_->AddObserver(this);
} }
ProxyConfigMonitor::ProxyConfigMonitor() { ProxyConfigMonitor::ProxyConfigMonitor(PrefService* local_state) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
!BrowserThread::IsThreadInitialized(BrowserThread::UI));
pref_proxy_config_tracker_.reset( pref_proxy_config_tracker_.reset(
ProxyServiceFactory::CreatePrefProxyConfigTrackerOfLocalState( ProxyServiceFactory::CreatePrefProxyConfigTrackerOfLocalState(
g_browser_process->local_state())); local_state));
proxy_config_service_ = ProxyServiceFactory::CreateProxyConfigService( proxy_config_service_ = ProxyServiceFactory::CreateProxyConfigService(
pref_proxy_config_tracker_.get()); pref_proxy_config_tracker_.get());
...@@ -65,7 +68,8 @@ ProxyConfigMonitor::ProxyConfigMonitor() { ...@@ -65,7 +68,8 @@ ProxyConfigMonitor::ProxyConfigMonitor() {
} }
ProxyConfigMonitor::~ProxyConfigMonitor() { ProxyConfigMonitor::~ProxyConfigMonitor() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
!BrowserThread::IsThreadInitialized(BrowserThread::UI));
proxy_config_service_->RemoveObserver(this); proxy_config_service_->RemoveObserver(this);
pref_proxy_config_tracker_->DetachFromPrefService(); pref_proxy_config_tracker_->DetachFromPrefService();
} }
...@@ -100,7 +104,8 @@ void ProxyConfigMonitor::FlushForTesting() { ...@@ -100,7 +104,8 @@ void ProxyConfigMonitor::FlushForTesting() {
void ProxyConfigMonitor::OnProxyConfigChanged( void ProxyConfigMonitor::OnProxyConfigChanged(
const net::ProxyConfigWithAnnotation& config, const net::ProxyConfigWithAnnotation& config,
net::ProxyConfigService::ConfigAvailability availability) { net::ProxyConfigService::ConfigAvailability availability) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
!BrowserThread::IsThreadInitialized(BrowserThread::UI));
proxy_config_client_set_.ForAllPtrs( proxy_config_client_set_.ForAllPtrs(
[config, [config,
availability](network::mojom::ProxyConfigClient* proxy_config_client) { availability](network::mojom::ProxyConfigClient* proxy_config_client) {
...@@ -134,7 +139,8 @@ void ProxyConfigMonitor::OnPACScriptError(int32_t line_number, ...@@ -134,7 +139,8 @@ void ProxyConfigMonitor::OnPACScriptError(int32_t line_number,
void ProxyConfigMonitor::OnRequestMaybeFailedDueToProxySettings( void ProxyConfigMonitor::OnRequestMaybeFailedDueToProxySettings(
int32_t net_error) { int32_t net_error) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
!BrowserThread::IsThreadInitialized(BrowserThread::UI));
if (net_error >= 0) { if (net_error >= 0) {
// If the error is obviously wrong, don't dispatch it to extensions. If the // If the error is obviously wrong, don't dispatch it to extensions. If the
......
...@@ -22,6 +22,7 @@ class ProxyConfigWithAnnotation; ...@@ -22,6 +22,7 @@ class ProxyConfigWithAnnotation;
class Profile; class Profile;
class PrefProxyConfigTracker; class PrefProxyConfigTracker;
class PrefService;
// Tracks the ProxyConfig to use, and passes any updates to a NetworkContext's // Tracks the ProxyConfig to use, and passes any updates to a NetworkContext's
// ProxyConfigClient. This also responds to errors related to proxy settings // ProxyConfigClient. This also responds to errors related to proxy settings
...@@ -42,10 +43,9 @@ class ProxyConfigMonitor : public net::ProxyConfigService::Observer, ...@@ -42,10 +43,9 @@ class ProxyConfigMonitor : public net::ProxyConfigService::Observer,
explicit ProxyConfigMonitor(Profile* profile); explicit ProxyConfigMonitor(Profile* profile);
// Creates a ProxyConfigMonitor that gets proxy settings from the // Creates a ProxyConfigMonitor that gets proxy settings from the
// BrowserProcess's |local_state_|, for use with NetworkContexts not // |local_state|, for use with NetworkContexts not
// assocaited with a profile. Must be destroyed before the BrowserProcess's // associated with a profile. Must be destroyed before |local_state|.
// |local_state_|. explicit ProxyConfigMonitor(PrefService* local_state);
ProxyConfigMonitor();
~ProxyConfigMonitor() override; ~ProxyConfigMonitor() override;
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "chrome/browser/net/proxy_service_factory.h" #include "chrome/browser/net/proxy_service_factory.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "components/proxy_config/pref_proxy_config_tracker_impl.h" #include "components/proxy_config/pref_proxy_config_tracker_impl.h"
#include "content/public/browser/browser_task_traits.h" #include "content/public/browser/browser_task_traits.h"
...@@ -22,8 +23,10 @@ using content::BrowserThread; ...@@ -22,8 +23,10 @@ using content::BrowserThread;
std::unique_ptr<net::ProxyConfigService> std::unique_ptr<net::ProxyConfigService>
ProxyServiceFactory::CreateProxyConfigService(PrefProxyConfigTracker* tracker) { ProxyServiceFactory::CreateProxyConfigService(PrefProxyConfigTracker* tracker) {
// The linux gsettings-based proxy settings getter relies on being initialized // The linux gsettings-based proxy settings getter relies on being initialized
// from the UI thread. // from the UI thread. The system proxy config service could also get created
DCHECK_CURRENTLY_ON(BrowserThread::UI); // without full browser process by launching service manager alone.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
!BrowserThread::IsThreadInitialized(BrowserThread::UI));
std::unique_ptr<net::ProxyConfigService> base_service; std::unique_ptr<net::ProxyConfigService> base_service;
...@@ -37,7 +40,7 @@ ProxyServiceFactory::CreateProxyConfigService(PrefProxyConfigTracker* tracker) { ...@@ -37,7 +40,7 @@ ProxyServiceFactory::CreateProxyConfigService(PrefProxyConfigTracker* tracker) {
// include command line and configuration policy). // include command line and configuration policy).
base_service = net::ProxyResolutionService::CreateSystemProxyConfigService( base_service = net::ProxyResolutionService::CreateSystemProxyConfigService(
base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI})); base::ThreadTaskRunnerHandle::Get());
#endif // !defined(OS_CHROMEOS) #endif // !defined(OS_CHROMEOS)
return tracker->CreateTrackingProxyConfigService(std::move(base_service)); return tracker->CreateTrackingProxyConfigService(std::move(base_service));
......
...@@ -346,7 +346,8 @@ SystemNetworkContextManager::SystemNetworkContextManager( ...@@ -346,7 +346,8 @@ SystemNetworkContextManager::SystemNetworkContextManager(
PrefService* local_state) PrefService* local_state)
: local_state_(local_state), : local_state_(local_state),
ssl_config_service_manager_( ssl_config_service_manager_(
SSLConfigServiceManager::CreateDefaultManager(local_state_)) { SSLConfigServiceManager::CreateDefaultManager(local_state_)),
proxy_config_monitor_(local_state_) {
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
// QuicAllowed was not part of Android policy. // QuicAllowed was not part of Android policy.
const base::Value* value = const base::Value* value =
...@@ -682,7 +683,8 @@ SystemNetworkContextManager::CreateNetworkContextParams() { ...@@ -682,7 +683,8 @@ SystemNetworkContextManager::CreateNetworkContextParams() {
network_context_params->primary_network_context = true; network_context_params->primary_network_context = true;
proxy_config_monitor_.AddToNetworkContextParams(network_context_params.get()); proxy_config_monitor_.AddToNetworkContextParams(
network_context_params.get());
return network_context_params; return network_context_params;
} }
......
...@@ -544,8 +544,10 @@ network::mojom::NetworkContextParamsPtr ...@@ -544,8 +544,10 @@ network::mojom::NetworkContextParamsPtr
SafeBrowsingService::CreateNetworkContextParams() { SafeBrowsingService::CreateNetworkContextParams() {
auto params = g_browser_process->system_network_context_manager() auto params = g_browser_process->system_network_context_manager()
->CreateDefaultNetworkContextParams(); ->CreateDefaultNetworkContextParams();
if (!proxy_config_monitor_) if (!proxy_config_monitor_) {
proxy_config_monitor_ = std::make_unique<ProxyConfigMonitor>(); proxy_config_monitor_ =
std::make_unique<ProxyConfigMonitor>(g_browser_process->local_state());
}
proxy_config_monitor_->AddToNetworkContextParams(params.get()); proxy_config_monitor_->AddToNetworkContextParams(params.get());
return params; return params;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment