From 3a29593d5ef0a3d243a2417eca5510d2abfd8c13 Mon Sep 17 00:00:00 2001
From: "mnissler@chromium.org"
<mnissler@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Mon, 11 Apr 2011 10:07:52 +0000
Subject: Allow ProxyConfigService to report "no configuration set"
Introduce a ConfigAvailability enum such that ProxyConfigService is able to return configuration status at a finer granularity level. This allows to fall back to default values (potentially configured through policy) if the system service doesn't have configuration.
BUG=none
TEST=unit tests, recommended proxy policy works on CrOS.
Review URL: http://codereview.chromium.org/6597070
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81085 0039d316-1c4b-4281-b951-d872f2087c98
---
net/proxy/polling_proxy_config_service.cc | 9 +-
net/proxy/polling_proxy_config_service.h | 2 +-
net/proxy/proxy_config_service.h | 29 ++++-
net/proxy/proxy_config_service_fixed.cc | 7 +-
net/proxy/proxy_config_service_fixed.h | 4 +-
net/proxy/proxy_config_service_linux.cc | 20 ++--
net/proxy/proxy_config_service_linux.h | 6 +-
net/proxy/proxy_config_service_linux_unittest.cc | 143 ++++++++++++++++++-----
net/proxy/proxy_config_service_mac.cc | 10 +-
net/proxy/proxy_config_service_mac.h | 2 +-
net/proxy/proxy_service.cc | 59 +++++++---
net/proxy/proxy_service.h | 8 +-
net/proxy/proxy_service_unittest.cc | 22 ++--
13 files changed, 228 insertions(+), 93 deletions(-)
(limited to 'net/proxy')
diff --git a/net/proxy/polling_proxy_config_service.cc b/net/proxy/polling_proxy_config_service.cc
index d80b4f2..b347bc5 100644
--- a/net/proxy/polling_proxy_config_service.cc
+++ b/net/proxy/polling_proxy_config_service.cc
@@ -121,7 +121,9 @@ class PollingProxyConfigService::Core
// If the configuration has changed, notify the observers.
has_config_ = true;
last_config_ = config;
- FOR_EACH_OBSERVER(Observer, observers_, OnProxyConfigChanged(config));
+ FOR_EACH_OBSERVER(Observer, observers_,
+ OnProxyConfigChanged(config,
+ ProxyConfigService::CONFIG_VALID));
}
if (poll_task_queued_)
@@ -162,8 +164,9 @@ void PollingProxyConfigService::RemoveObserver(Observer* observer) {
core_->RemoveObserver(observer);
}
-bool PollingProxyConfigService::GetLatestProxyConfig(ProxyConfig* config) {
- return core_->GetLatestProxyConfig(config);
+ProxyConfigService::ConfigAvailability
+ PollingProxyConfigService::GetLatestProxyConfig(ProxyConfig* config) {
+ return core_->GetLatestProxyConfig(config) ? CONFIG_VALID : CONFIG_PENDING;
}
void PollingProxyConfigService::OnLazyPoll() {
diff --git a/net/proxy/polling_proxy_config_service.h b/net/proxy/polling_proxy_config_service.h
index 37aedd4..6cd1244 100644
--- a/net/proxy/polling_proxy_config_service.h
+++ b/net/proxy/polling_proxy_config_service.h
@@ -22,7 +22,7 @@ class PollingProxyConfigService : public ProxyConfigService {
// ProxyConfigService implementation:
virtual void AddObserver(Observer* observer);
virtual void RemoveObserver(Observer* observer);
- virtual bool GetLatestProxyConfig(ProxyConfig* config);
+ virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* config);
virtual void OnLazyPoll();
protected:
diff --git a/net/proxy/proxy_config_service.h b/net/proxy/proxy_config_service.h
index da67cd6..f194ca0 100644
--- a/net/proxy/proxy_config_service.h
+++ b/net/proxy/proxy_config_service.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -13,11 +13,27 @@ class ProxyConfig;
// Service for watching when the proxy settings have changed.
class ProxyConfigService {
public:
+ // Indicates whether proxy configuration is valid, and if not, why.
+ enum ConfigAvailability {
+ // Configuration is pending, observers will be notified later.
+ CONFIG_PENDING,
+ // Configuration is present and valid.
+ CONFIG_VALID,
+ // No configuration is set.
+ CONFIG_UNSET
+ };
+
// Observer for being notified when the proxy settings have changed.
class Observer {
public:
virtual ~Observer() {}
- virtual void OnProxyConfigChanged(const ProxyConfig& config) = 0;
+ // Notification callback that should be invoked by ProxyConfigService
+ // implementors whenever the configuration changes. |availability| indicates
+ // the new availability status and can be CONFIG_UNSET or CONFIG_VALID (in
+ // which case |config| contains the configuration). Implementors must not
+ // pass CONFIG_PENDING.
+ virtual void OnProxyConfigChanged(const ProxyConfig& config,
+ ConfigAvailability availability) = 0;
};
virtual ~ProxyConfigService() {}
@@ -27,13 +43,14 @@ class ProxyConfigService {
virtual void AddObserver(Observer* observer) = 0;
virtual void RemoveObserver(Observer* observer) = 0;
- // Gets the most recent value of the proxy configuration. Returns false if
- // it is not available yet. In the case where we returned false, it is
- // guaranteed that subscribed observers will be notified of a change at
+ // Gets the most recent availability status. If a configuration is present,
+ // the proxy configuration is written to |config| and CONFIG_VALID is
+ // returned. Returns CONFIG_PENDING if it is not available yet. In this case,
+ // it is guaranteed that subscribed observers will be notified of a change at
// some point in the future once the configuration is available.
// Note that to avoid re-entrancy problems, implementations should not
// dispatch any change notifications from within this function.
- virtual bool GetLatestProxyConfig(ProxyConfig* config) = 0;
+ virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* config) = 0;
// ProxyService will call this periodically during periods of activity.
// It can be used as a signal for polling-based implementations.
diff --git a/net/proxy/proxy_config_service_fixed.cc b/net/proxy/proxy_config_service_fixed.cc
index b23c265..3081ea5 100644
--- a/net/proxy/proxy_config_service_fixed.cc
+++ b/net/proxy/proxy_config_service_fixed.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -12,9 +12,10 @@ ProxyConfigServiceFixed::ProxyConfigServiceFixed(const ProxyConfig& pc)
ProxyConfigServiceFixed::~ProxyConfigServiceFixed() {}
-bool ProxyConfigServiceFixed::GetLatestProxyConfig(ProxyConfig* config) {
+ProxyConfigService::ConfigAvailability
+ ProxyConfigServiceFixed::GetLatestProxyConfig(ProxyConfig* config) {
*config = pc_;
- return true;
+ return CONFIG_VALID;
}
} // namespace net
diff --git a/net/proxy/proxy_config_service_fixed.h b/net/proxy/proxy_config_service_fixed.h
index b0d8f03..54baa56 100644
--- a/net/proxy/proxy_config_service_fixed.h
+++ b/net/proxy/proxy_config_service_fixed.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -21,7 +21,7 @@ class ProxyConfigServiceFixed : public ProxyConfigService {
// ProxyConfigService methods:
virtual void AddObserver(Observer* observer) {}
virtual void RemoveObserver(Observer* observer) {}
- virtual bool GetLatestProxyConfig(ProxyConfig* config);
+ virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* config);
private:
ProxyConfig pc_;
diff --git a/net/proxy/proxy_config_service_linux.cc b/net/proxy/proxy_config_service_linux.cc
index 893a8e5..8392d96 100644
--- a/net/proxy/proxy_config_service_linux.cc
+++ b/net/proxy/proxy_config_service_linux.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -22,13 +22,13 @@
#include "base/file_util.h"
#include "base/logging.h"
#include "base/message_loop.h"
+#include "base/nix/xdg_util.h"
#include "base/string_number_conversions.h"
#include "base/string_tokenizer.h"
#include "base/string_util.h"
#include "base/task.h"
#include "base/threading/thread_restrictions.h"
#include "base/timer.h"
-#include "base/nix/xdg_util.h"
#include "googleurl/src/url_canon.h"
#include "net/base/net_errors.h"
#include "net/http/http_util.h"
@@ -1166,8 +1166,9 @@ void ProxyConfigServiceLinux::Delegate::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
-bool ProxyConfigServiceLinux::Delegate::GetLatestProxyConfig(
- ProxyConfig* config) {
+ProxyConfigService::ConfigAvailability
+ ProxyConfigServiceLinux::Delegate::GetLatestProxyConfig(
+ ProxyConfig* config) {
// This is called from the IO thread.
DCHECK(!io_loop_ || MessageLoop::current() == io_loop_);
@@ -1176,12 +1177,12 @@ bool ProxyConfigServiceLinux::Delegate::GetLatestProxyConfig(
*config = cached_config_.is_valid() ?
cached_config_ : ProxyConfig::CreateDirect();
- // We return true to indicate that *config was filled in. It is always
+ // We return CONFIG_VALID to indicate that *config was filled in. It is always
// going to be available since we initialized eagerly on the UI thread.
// TODO(eroman): do lazy initialization instead, so we no longer need
// to construct ProxyConfigServiceLinux on the UI thread.
// In which case, we may return false here.
- return true;
+ return CONFIG_VALID;
}
// Depending on the GConfSettingGetter in use, this method will be called
@@ -1215,7 +1216,9 @@ void ProxyConfigServiceLinux::Delegate::SetNewProxyConfig(
DCHECK(MessageLoop::current() == io_loop_);
VLOG(1) << "Proxy configuration changed";
cached_config_ = new_config;
- FOR_EACH_OBSERVER(Observer, observers_, OnProxyConfigChanged(new_config));
+ FOR_EACH_OBSERVER(
+ Observer, observers_,
+ OnProxyConfigChanged(new_config, ProxyConfigService::CONFIG_VALID));
}
void ProxyConfigServiceLinux::Delegate::PostDestroyTask() {
@@ -1269,7 +1272,8 @@ void ProxyConfigServiceLinux::RemoveObserver(Observer* observer) {
delegate_->RemoveObserver(observer);
}
-bool ProxyConfigServiceLinux::GetLatestProxyConfig(ProxyConfig* config) {
+ProxyConfigService::ConfigAvailability
+ ProxyConfigServiceLinux::GetLatestProxyConfig(ProxyConfig* config) {
return delegate_->GetLatestProxyConfig(config);
}
diff --git a/net/proxy/proxy_config_service_linux.h b/net/proxy/proxy_config_service_linux.h
index 5435226..2f1ad0c 100644
--- a/net/proxy/proxy_config_service_linux.h
+++ b/net/proxy/proxy_config_service_linux.h
@@ -144,7 +144,8 @@ class ProxyConfigServiceLinux : public ProxyConfigService {
// Called from IO thread.
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
- bool GetLatestProxyConfig(ProxyConfig* config);
+ ProxyConfigService::ConfigAvailability GetLatestProxyConfig(
+ ProxyConfig* config);
// Posts a call to OnDestroy() to the UI thread. Called from
// ProxyConfigServiceLinux's destructor.
@@ -239,7 +240,8 @@ class ProxyConfigServiceLinux : public ProxyConfigService {
// Called from IO thread.
virtual void AddObserver(Observer* observer);
virtual void RemoveObserver(Observer* observer);
- virtual bool GetLatestProxyConfig(ProxyConfig* config);
+ virtual ProxyConfigService::ConfigAvailability GetLatestProxyConfig(
+ ProxyConfig* config);
private:
scoped_refptr<Delegate> delegate_;
diff --git a/net/proxy/proxy_config_service_linux_unittest.cc b/net/proxy/proxy_config_service_linux_unittest.cc
index 3005326d..a90142f 100644
--- a/net/proxy/proxy_config_service_linux_unittest.cc
+++ b/net/proxy/proxy_config_service_linux_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -14,9 +14,9 @@
#include "base/logging.h"
#include "base/string_util.h"
#include "base/stringprintf.h"
+#include "base/synchronization/waitable_event.h"
#include "base/task.h"
#include "base/threading/thread.h"
-#include "base/synchronization/waitable_event.h"
#include "net/proxy/proxy_config.h"
#include "net/proxy/proxy_config_service_common_unittest.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -290,7 +290,8 @@ class SynchConfigGetter {
static_cast<MessageLoopForIO*>(file_loop));
}
// Synchronously gets the proxy config.
- bool SyncGetLatestProxyConfig(net::ProxyConfig* config) {
+ net::ProxyConfigService::ConfigAvailability SyncGetLatestProxyConfig(
+ net::ProxyConfig* config) {
io_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(
this, &SynchConfigGetter::GetLatestConfigOnIOThread));
Wait();
@@ -331,7 +332,9 @@ class SynchConfigGetter {
// The config obtained by |io_thread_| and read back by the main
// thread.
net::ProxyConfig proxy_config_;
- bool get_latest_config_result_; // Return value from GetLatestProxyConfig().
+
+ // Return value from GetLatestProxyConfig().
+ net::ProxyConfigService::ConfigAvailability get_latest_config_result_;
};
DISABLE_RUNNABLE_METHOD_REFCOUNT(SynchConfigGetter);
@@ -394,7 +397,8 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
// Input.
GConfValues values;
- // Expected outputs (fields of the ProxyConfig).
+ // Expected outputs (availability and fields of ProxyConfig).
+ ProxyConfigService::ConfigAvailability availability;
bool auto_detect;
GURL pac_url;
ProxyRulesExpectation proxy_rules;
@@ -411,6 +415,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -428,6 +433,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
true, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -445,6 +451,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL("http://wpad/wpad.dat"), // pac_url
ProxyRulesExpectation::Empty(),
@@ -462,7 +469,8 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
- false, // auto_detect
+ ProxyConfigService::CONFIG_VALID,
+ false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
},
@@ -479,6 +487,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -498,6 +507,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -515,6 +525,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -536,6 +547,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -558,6 +570,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -579,6 +592,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -597,6 +611,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
google_ignores, // ignore_hosts
},
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -615,11 +630,15 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicGConfTest) {
ProxyConfig config;
gconf_getter->values = tests[i].values;
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
-
- EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
- EXPECT_EQ(tests[i].pac_url, config.pac_url());
- EXPECT_TRUE(tests[i].proxy_rules.Matches(config.proxy_rules()));
+ ProxyConfigService::ConfigAvailability availability =
+ sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(tests[i].availability, availability);
+
+ if (availability == ProxyConfigService::CONFIG_VALID) {
+ EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
+ EXPECT_EQ(tests[i].pac_url, config.pac_url());
+ EXPECT_TRUE(tests[i].proxy_rules.Matches(config.proxy_rules()));
+ }
}
}
@@ -632,7 +651,8 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
// Input.
EnvVarValues values;
- // Expected outputs (fields of the ProxyConfig).
+ // Expected outputs (availability and fields of ProxyConfig).
+ ProxyConfigService::ConfigAvailability availability;
bool auto_detect;
GURL pac_url;
ProxyRulesExpectation proxy_rules;
@@ -652,6 +672,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -672,6 +693,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
true, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -692,6 +714,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL("http://wpad/wpad.dat"), // pac_url
ProxyRulesExpectation::Empty(),
@@ -712,6 +735,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -732,6 +756,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -754,6 +779,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -776,6 +802,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -798,6 +825,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -822,6 +850,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -844,6 +873,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -866,6 +896,7 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
},
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -887,6 +918,8 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
".google.com, foo.com:99, 1.2.3.4:22, 127.0.0.1/8", // no_proxy
},
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Single(
@@ -905,11 +938,15 @@ TEST_F(ProxyConfigServiceLinuxTest, BasicEnvTest) {
ProxyConfig config;
env->values = tests[i].values;
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
-
- EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
- EXPECT_EQ(tests[i].pac_url, config.pac_url());
- EXPECT_TRUE(tests[i].proxy_rules.Matches(config.proxy_rules()));
+ ProxyConfigService::ConfigAvailability availability =
+ sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(tests[i].availability, availability);
+
+ if (availability == ProxyConfigService::CONFIG_VALID) {
+ EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
+ EXPECT_EQ(tests[i].pac_url, config.pac_url());
+ EXPECT_TRUE(tests[i].proxy_rules.Matches(config.proxy_rules()));
+ }
}
}
@@ -924,14 +961,16 @@ TEST_F(ProxyConfigServiceLinuxTest, GconfNotification) {
// Start with no proxy.
gconf_getter->values.mode = "none";
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(ProxyConfigService::CONFIG_VALID,
+ sync_config_getter.SyncGetLatestProxyConfig(&config));
EXPECT_FALSE(config.auto_detect());
// Now set to auto-detect.
gconf_getter->values.mode = "auto";
// Simulate gconf notification callback.
service->OnCheckProxyConfigSettings();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(ProxyConfigService::CONFIG_VALID,
+ sync_config_getter.SyncGetLatestProxyConfig(&config));
EXPECT_TRUE(config.auto_detect());
}
@@ -952,7 +991,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
std::string kioslaverc;
EnvVarValues env_values;
- // Expected outputs (fields of the ProxyConfig).
+ // Expected outputs (availability and fields of ProxyConfig).
+ ProxyConfigService::ConfigAvailability availability;
bool auto_detect;
GURL pac_url;
ProxyRulesExpectation proxy_rules;
@@ -965,6 +1005,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
{}, // env_values
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -978,6 +1019,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
{}, // env_values
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
true, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -992,6 +1034,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
{}, // env_values
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL("http://wpad/wpad.dat"), // pac_url
ProxyRulesExpectation::Empty(),
@@ -1006,6 +1049,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
{}, // env_values
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1024,6 +1068,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
{}, // env_values
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1042,6 +1087,7 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
{}, // env_values
// Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1059,6 +1105,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"NoProxyFor=.google.com\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1076,6 +1124,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"NoProxyFor=.google.com,.kde.org\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1093,6 +1143,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"NoProxyFor=.google.com\nReversedException=true\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerSchemeWithBypassReversed(
@@ -1110,6 +1162,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"NoProxyFor=google.com,kde.org,<local>\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1127,6 +1181,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"NoProxyFor=.google.com\nReversedException=true \n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerSchemeWithBypassReversed(
@@ -1144,6 +1200,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"httpProxy=www.google.com\n[Other Section]\nftpProxy=ftp.foo.com\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1160,6 +1218,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"[Proxy Settings]\r\nProxyType=1\r\nhttpProxy=www.google.com\r\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1176,6 +1236,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"[Proxy Settings]\r\n\nProxyType=1\n\r\nhttpProxy=www.google.com\n\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1192,6 +1254,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"[Proxy Settings]\nProxyType[$e]=1\nhttpProxy[$e]=www.google.com\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1209,6 +1273,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"httpsProxy$e]=www.foo.com\nftpProxy=ftp.foo.com\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1226,6 +1292,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
" Proxy Config Script = http:// foo\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL("http:// foo"), // pac_url
ProxyRulesExpectation::Empty(),
@@ -1239,6 +1307,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
long_line + "httpsProxy=www.foo.com\nhttpProxy=www.google.com\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1256,6 +1326,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
"httpsProxy=https_proxy\nftpProxy=ftp_proxy\nNoProxyFor=no_proxy\n",
{}, // env_values
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::Empty(),
@@ -1281,6 +1353,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
".google.com, .kde.org", // no_proxy
},
+ // Expected result.
+ ProxyConfigService::CONFIG_VALID,
false, // auto_detect
GURL(), // pac_url
ProxyRulesExpectation::PerScheme(
@@ -1307,11 +1381,15 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEConfigParser) {
file_util::WriteFile(kioslaverc_, tests[i].kioslaverc.c_str(),
tests[i].kioslaverc.length());
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
-
- EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
- EXPECT_EQ(tests[i].pac_url, config.pac_url());
- EXPECT_TRUE(tests[i].proxy_rules.Matches(config.proxy_rules()));
+ ProxyConfigService::ConfigAvailability availability =
+ sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(tests[i].availability, availability);
+
+ if (availability == ProxyConfigService::CONFIG_VALID) {
+ EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
+ EXPECT_EQ(tests[i].pac_url, config.pac_url());
+ EXPECT_TRUE(tests[i].proxy_rules.Matches(config.proxy_rules()));
+ }
}
}
@@ -1338,7 +1416,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEHomePicker) {
new ProxyConfigServiceLinux(env));
ProxyConfig config;
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(ProxyConfigService::CONFIG_VALID,
+ sync_config_getter.SyncGetLatestProxyConfig(&config));
EXPECT_TRUE(config.auto_detect());
EXPECT_EQ(GURL(), config.pac_url());
}
@@ -1357,7 +1436,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEHomePicker) {
new ProxyConfigServiceLinux(env));
ProxyConfig config;
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(ProxyConfigService::CONFIG_VALID,
+ sync_config_getter.SyncGetLatestProxyConfig(&config));
EXPECT_FALSE(config.auto_detect());
EXPECT_EQ(slaverc4_pac_url, config.pac_url());
}
@@ -1370,7 +1450,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEHomePicker) {
new ProxyConfigServiceLinux(env));
ProxyConfig config;
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(ProxyConfigService::CONFIG_VALID,
+ sync_config_getter.SyncGetLatestProxyConfig(&config));
EXPECT_TRUE(config.auto_detect());
EXPECT_EQ(GURL(), config.pac_url());
}
@@ -1384,7 +1465,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEHomePicker) {
new ProxyConfigServiceLinux(env));
ProxyConfig config;
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(ProxyConfigService::CONFIG_VALID,
+ sync_config_getter.SyncGetLatestProxyConfig(&config));
EXPECT_TRUE(config.auto_detect());
EXPECT_EQ(GURL(), config.pac_url());
}
@@ -1401,7 +1483,8 @@ TEST_F(ProxyConfigServiceLinuxTest, KDEHomePicker) {
new ProxyConfigServiceLinux(env));
ProxyConfig config;
sync_config_getter.SetupAndInitialFetch();
- sync_config_getter.SyncGetLatestProxyConfig(&config);
+ EXPECT_EQ(ProxyConfigService::CONFIG_VALID,
+ sync_config_getter.SyncGetLatestProxyConfig(&config));
EXPECT_TRUE(config.auto_detect());
EXPECT_EQ(GURL(), config.pac_url());
}
diff --git a/net/proxy/proxy_config_service_mac.cc b/net/proxy/proxy_config_service_mac.cc
index 3dacf77..ffc9c77 100644
--- a/net/proxy/proxy_config_service_mac.cc
+++ b/net/proxy/proxy_config_service_mac.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -210,7 +210,8 @@ void ProxyConfigServiceMac::RemoveObserver(Observer* observer) {
observers_.RemoveObserver(observer);
}
-bool ProxyConfigServiceMac::GetLatestProxyConfig(ProxyConfig* config) {
+net::ProxyConfigService::ConfigAvailability
+ ProxyConfigServiceMac::GetLatestProxyConfig(ProxyConfig* config) {
DCHECK_EQ(io_loop_, MessageLoop::current());
// Lazy-initialize by fetching the proxy setting from this thread.
@@ -220,7 +221,7 @@ bool ProxyConfigServiceMac::GetLatestProxyConfig(ProxyConfig* config) {
}
*config = last_config_fetched_;
- return has_fetched_config_;
+ return has_fetched_config_ ? CONFIG_VALID : CONFIG_PENDING;
}
void ProxyConfigServiceMac::SetDynamicStoreNotificationKeys(
@@ -262,7 +263,8 @@ void ProxyConfigServiceMac::OnProxyConfigChanged(
last_config_fetched_ = new_config;
// Notify all the observers.
- FOR_EACH_OBSERVER(Observer, observers_, OnProxyConfigChanged(new_config));
+ FOR_EACH_OBSERVER(Observer, observers_,
+ OnProxyConfigChanged(new_config, CONFIG_VALID));
}
} // namespace net
diff --git a/net/proxy/proxy_config_service_mac.h b/net/proxy/proxy_config_service_mac.h
index 6fb8fcd..cd5d576 100644
--- a/net/proxy/proxy_config_service_mac.h
+++ b/net/proxy/proxy_config_service_mac.h
@@ -27,7 +27,7 @@ class ProxyConfigServiceMac : public ProxyConfigService {
// ProxyConfigService implementation:
virtual void AddObserver(Observer* observer);
virtual void RemoveObserver(Observer* observer);
- virtual bool GetLatestProxyConfig(ProxyConfig* config);
+ virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* config);
private:
class Helper;
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index 47673c5..7449204 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
@@ -8,17 +8,23 @@
#include "base/compiler_specific.h"
#include "base/logging.h"
-#include "base/values.h"
#include "base/message_loop.h"
#include "base/string_util.h"
+#include "base/values.h"
#include "googleurl/src/gurl.h"
-#include "net/base/net_log.h"
#include "net/base/net_errors.h"
+#include "net/base/net_log.h"
#include "net/base/net_util.h"
#include "net/proxy/init_proxy_resolver.h"
#include "net/proxy/multi_threaded_proxy_resolver.h"
#include "net/proxy/proxy_config_service_fixed.h"
+#include "net/proxy/proxy_resolver.h"
+#include "net/proxy/proxy_resolver_js_bindings.h"
+#include "net/proxy/proxy_resolver_v8.h"
#include "net/proxy/proxy_script_fetcher.h"
+#include "net/proxy/sync_host_resolver_bridge.h"
+#include "net/url_request/url_request_context.h"
+
#if defined(OS_WIN)
#include "net/proxy/proxy_config_service_win.h"
#include "net/proxy/proxy_resolver_winhttp.h"
@@ -28,11 +34,6 @@
#elif defined(OS_LINUX) && !defined(OS_CHROMEOS)
#include "net/proxy/proxy_config_service_linux.h"
#endif
-#include "net/proxy/proxy_resolver.h"
-#include "net/proxy/proxy_resolver_js_bindings.h"
-#include "net/proxy/proxy_resolver_v8.h"
-#include "net/proxy/sync_host_resolver_bridge.h"
-#include "net/url_request/url_request_context.h"
using base::TimeDelta;
using base::TimeTicks;
@@ -89,9 +90,9 @@ class ProxyConfigServiceDirect : public ProxyConfigService {
// ProxyConfigService implementation:
virtual void AddObserver(Observer* observer) {}
virtual void RemoveObserver(Observer* observer) {}
- virtual bool GetLatestProxyConfig(ProxyConfig* config) {
+ virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* config) {
*config = ProxyConfig::CreateDirect();
- return true;
+ return CONFIG_VALID;
}
};
@@ -627,9 +628,10 @@ void ProxyService::ApplyProxyConfigIfAvailable() {
// If a configuration is not available yet, we will get called back later
// by our ProxyConfigService::Observer once it changes.
ProxyConfig config;
- bool has_config = config_service_->GetLatestProxyConfig(&config);
- if (has_config)
- OnProxyConfigChanged(config);
+ ProxyConfigService::ConfigAvailability availability =
+ config_service_->GetLatestProxyConfig(&config);
+ if (availability != ProxyConfigService::CONFIG_PENDING)
+ OnProxyConfigChanged(config, availability);
}
void ProxyService::OnInitProxyResolverComplete(int result) {
@@ -793,11 +795,11 @@ ProxyConfigService* ProxyService::CreateSystemProxyConfigService(
return new ProxyConfigServiceMac(io_loop);
#elif defined(OS_CHROMEOS)
NOTREACHED() << "ProxyConfigService for ChromeOS should be created in "
- << "chrome_url_request_context.cc::CreateProxyConfigService.";
+ << "profile_io_data.cc::CreateProxyConfigService.";
return NULL;
#elif defined(OS_LINUX)
- ProxyConfigServiceLinux* linux_config_service
- = new ProxyConfigServiceLinux();
+ ProxyConfigServiceLinux* linux_config_service =
+ new ProxyConfigServiceLinux();
// Assume we got called from the UI loop, which runs the default
// glib main loop, so the current thread is where we should be
@@ -822,11 +824,30 @@ ProxyConfigService* ProxyService::CreateSystemProxyConfigService(
#endif
}
-void ProxyService::OnProxyConfigChanged(const ProxyConfig& config) {
+void ProxyService::OnProxyConfigChanged(
+ const ProxyConfig& config,
+ ProxyConfigService::ConfigAvailability availability) {
+ // Retrieve the current proxy configuration from the ProxyConfigService.
+ // If a configuration is not available yet, we will get called back later
+ // by our ProxyConfigService::Observer once it changes.
+ ProxyConfig effective_config;
+ switch (availability) {
+ case ProxyConfigService::CONFIG_PENDING:
+ // ProxyConfigService implementors should never pass CONFIG_PENDING.
+ NOTREACHED() << "Proxy config change with CONFIG_PENDING availability!";
+ return;
+ case ProxyConfigService::CONFIG_VALID:
+ effective_config = config;
+ break;
+ case ProxyConfigService::CONFIG_UNSET:
+ effective_config = ProxyConfig::CreateDirect();
+ break;
+ }
+
// Emit the proxy settings change to the NetLog stream.
if (net_log_) {
scoped_refptr<NetLog::EventParameters> params(
- new ProxyConfigChangedNetLogParam(fetched_config_, config));
+ new ProxyConfigChangedNetLogParam(fetched_config_, effective_config));
net_log_->AddEntry(net::NetLog::TYPE_PROXY_CONFIG_CHANGED,
base::TimeTicks::Now(),
NetLog::Source(),
@@ -835,7 +856,7 @@ void ProxyService::OnProxyConfigChanged(const ProxyConfig& config) {
}
// Set the new configuration as the most recently fetched one.
- fetched_config_ = config;
+ fetched_config_ = effective_config;
fetched_config_.set_id(1); // Needed for a later DCHECK of is_valid().
InitializeUsingLastFetchedConfig();
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index 10ca883..edb588d 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -13,11 +13,11 @@
#include "base/memory/scoped_ptr.h"
#include "base/synchronization/waitable_event.h"
#include "net/base/completion_callback.h"
-#include "net/base/network_change_notifier.h"
#include "net/base/net_log.h"
+#include "net/base/network_change_notifier.h"
#include "net/proxy/proxy_config_service.h"
-#include "net/proxy/proxy_server.h"
#include "net/proxy/proxy_info.h"
+#include "net/proxy/proxy_server.h"
class GURL;
class MessageLoop;
@@ -285,7 +285,9 @@ class ProxyService : public base::RefCountedThreadSafe<ProxyService>,
virtual void OnIPAddressChanged();
// ProxyConfigService::Observer
- virtual void OnProxyConfigChanged(const ProxyConfig& config);
+ virtual void OnProxyConfigChanged(
+ const ProxyConfig& config,
+ ProxyConfigService::ConfigAvailability availability);
scoped_ptr<ProxyConfigService> config_service_;
scoped_ptr<ProxyResolver> resolver_;
diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc
index 95f8bbc..7326c67 100644
--- a/net/proxy/proxy_service_unittest.cc
+++ b/net/proxy/proxy_service_unittest.cc
@@ -11,9 +11,9 @@
#include "base/string_util.h"
#include "base/utf_string_conversions.h"
#include "googleurl/src/gurl.h"
+#include "net/base/net_errors.h"
#include "net/base/net_log.h"
#include "net/base/net_log_unittest.h"
-#include "net/base/net_errors.h"
#include "net/base/test_completion_callback.h"
#include "net/proxy/mock_proxy_resolver.h"
#include "net/proxy/proxy_config_service.h"
@@ -29,11 +29,12 @@ namespace {
class MockProxyConfigService: public ProxyConfigService {
public:
explicit MockProxyConfigService(const ProxyConfig& config)
- : has_config_(true), config_(config) {
+ : availability_(CONFIG_VALID),
+ config_(config) {
}
explicit MockProxyConfigService(const std::string& pac_url)
- : has_config_(true),
+ : availability_(CONFIG_VALID),
config_(ProxyConfig::CreateFromCustomPacURL(GURL(pac_url))) {
}
@@ -45,22 +46,21 @@ class MockProxyConfigService: public ProxyConfigService {
observers_.RemoveObserver(observer);
}
- virtual bool GetLatestProxyConfig(ProxyConfig* results) {
- if (has_config_) {
+ virtual ConfigAvailability GetLatestProxyConfig(ProxyConfig* results) {
+ if (availability_ == CONFIG_VALID)
*results = config_;
- return true;
- }
- return false;
+ return availability_;
}
void SetConfig(const ProxyConfig& config) {
- has_config_ = true;
+ availability_ = CONFIG_VALID;
config_ = config;
- FOR_EACH_OBSERVER(Observer, observers_, OnProxyConfigChanged(config));
+ FOR_EACH_OBSERVER(Observer, observers_,
+ OnProxyConfigChanged(config_, availability_));
}
private:
- bool has_config_;
+ ConfigAvailability availability_;
ProxyConfig config_;
ObserverList<Observer, true> observers_;
};
--
cgit v1.1