diff options
23 files changed, 508 insertions, 228 deletions
diff --git a/chrome/browser/chromeos/proxy_config_service.h b/chrome/browser/chromeos/proxy_config_service.h index cabf8bf..4ede781 100644 --- a/chrome/browser/chromeos/proxy_config_service.h +++ b/chrome/browser/chromeos/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. @@ -12,8 +12,7 @@ namespace chromeos { // Wrapper class for the RefCountedThreadSafe chromeos::ProxyConfigServiceImpl // that forwards net::ProxyConfigService interface to the actual implementation, -// instantiated in -// chrome/browser/net/chrome_url_request_context.cc::CreateProxyConfigService. +// instantiated in ProfileIOData::CreateProxyConfigService. class ProxyConfigService : public net::ProxyConfigService { public: explicit ProxyConfigService(const scoped_refptr<ProxyConfigServiceImpl>& impl) @@ -27,7 +26,7 @@ class ProxyConfigService : public net::ProxyConfigService { virtual void RemoveObserver(Observer* observer) { impl_->RemoveObserver(observer); } - virtual bool GetLatestProxyConfig(net::ProxyConfig* config) { + virtual ConfigAvailability GetLatestProxyConfig(net::ProxyConfig* config) { return impl_->IOGetProxyConfig(config); } diff --git a/chrome/browser/chromeos/proxy_config_service_impl.cc b/chrome/browser/chromeos/proxy_config_service_impl.cc index 5854bb1..4613dec 100644 --- a/chrome/browser/chromeos/proxy_config_service_impl.cc +++ b/chrome/browser/chromeos/proxy_config_service_impl.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. @@ -391,7 +391,7 @@ bool ProxyConfigServiceImpl::ProxyConfig::DecodeManualProxy( ProxyConfigServiceImpl::ProxyConfigServiceImpl() : can_post_task_(false), - has_config_(false), + config_availability_(net::ProxyConfigService::CONFIG_PENDING), persist_to_device_(true), persist_to_device_pending_(false) { // Start async fetch of proxy config from settings persisted on device. @@ -409,13 +409,13 @@ ProxyConfigServiceImpl::ProxyConfigServiceImpl() } } if (use_default) - InitConfigToDefault(false); + config_availability_ = net::ProxyConfigService::CONFIG_UNSET; can_post_task_ = true; } ProxyConfigServiceImpl::ProxyConfigServiceImpl(const ProxyConfig& init_config) : can_post_task_(true), - has_config_(true), + config_availability_(net::ProxyConfigService::CONFIG_VALID), persist_to_device_(false), persist_to_device_pending_(false) { reference_config_ = init_config; @@ -514,15 +514,14 @@ void ProxyConfigServiceImpl::RemoveObserver( observers_.RemoveObserver(observer); } -bool ProxyConfigServiceImpl::IOGetProxyConfig(net::ProxyConfig* net_config) { +net::ProxyConfigService::ConfigAvailability + ProxyConfigServiceImpl::IOGetProxyConfig(net::ProxyConfig* net_config) { // Should be called from IO thread. CheckCurrentlyOnIOThread(); - if (has_config_) { - // Simply return the last cached proxy configuration. + if (config_availability_ == net::ProxyConfigService::CONFIG_VALID) cached_config_.ToNetProxyConfig(net_config); - return true; - } - return false; + + return config_availability_; } void ProxyConfigServiceImpl::OnSettingsOpCompleted( @@ -540,36 +539,26 @@ void ProxyConfigServiceImpl::OnSettingsOpCompleted( void ProxyConfigServiceImpl::OnSettingsOpCompleted( SignedSettings::ReturnCode code, std::string value) { + retrieve_property_op_ = NULL; if (SignedSettings::SUCCESS == code) { VLOG(1) << "Retrieved proxy setting from device, value=[" << value << "]"; if (reference_config_.Deserialize(value)) { - OnUISetProxyConfig(false); + IOSetProxyConfig(reference_config_, + net::ProxyConfigService::CONFIG_VALID); + return; } else { LOG(WARNING) << "Error deserializing device's proxy setting"; - InitConfigToDefault(true); } } else { LOG(WARNING) << "Error retrieving proxy setting from device"; - InitConfigToDefault(true); } - retrieve_property_op_ = NULL; + + // Update the configuration state on the IO thread. + IOSetProxyConfig(reference_config_, net::ProxyConfigService::CONFIG_UNSET); } //------------------ ProxyConfigServiceImpl: private methods ------------------- -void ProxyConfigServiceImpl::InitConfigToDefault(bool post_to_io_thread) { - VLOG(1) << "Using default proxy config: auto-detect"; - reference_config_.mode = ProxyConfig::MODE_AUTO_DETECT; - reference_config_.automatic_proxy.source = ProxyConfig::SOURCE_OWNER; - if (post_to_io_thread && can_post_task_) { - OnUISetProxyConfig(false); - } else { - // Update the IO-accessible copy in |cached_config_| as well. - cached_config_ = reference_config_; - has_config_ = true; - } -} - void ProxyConfigServiceImpl::PersistConfigToDevice() { DCHECK(!store_property_op_); persist_to_device_pending_ = false; @@ -585,15 +574,7 @@ void ProxyConfigServiceImpl::PersistConfigToDevice() { } void ProxyConfigServiceImpl::OnUISetProxyConfig(bool persist_to_device) { - // Posts a task to IO thread with the new config, so it can update - // |cached_config_|. - Task* task = NewRunnableMethod(this, - &ProxyConfigServiceImpl::IOSetProxyConfig, reference_config_); - if (!BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, task)) { - VLOG(1) << "Couldn't post task to IO thread to set new proxy config"; - delete task; - } - + IOSetProxyConfig(reference_config_, net::ProxyConfigService::CONFIG_VALID); if (persist_to_device && CrosLibrary::Get()->EnsureLoaded()) { if (store_property_op_) { persist_to_device_pending_ = true; @@ -604,25 +585,38 @@ void ProxyConfigServiceImpl::OnUISetProxyConfig(bool persist_to_device) { } } -void ProxyConfigServiceImpl::CheckCurrentlyOnIOThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); -} - -void ProxyConfigServiceImpl::CheckCurrentlyOnUIThread() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); -} +void ProxyConfigServiceImpl::IOSetProxyConfig( + const ProxyConfig& new_config, + net::ProxyConfigService::ConfigAvailability new_availability) { + if (!BrowserThread::CurrentlyOn(BrowserThread::IO) && can_post_task_) { + // Posts a task to IO thread with the new config, so it can update + // |cached_config_|. + Task* task = NewRunnableMethod(this, + &ProxyConfigServiceImpl::IOSetProxyConfig, + new_config, + new_availability); + if (!BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, task)) + VLOG(1) << "Couldn't post task to IO thread to set new proxy config"; + return; + } -void ProxyConfigServiceImpl::IOSetProxyConfig(const ProxyConfig& new_config) { - // This is called on the IO thread (posted from UI thread). - CheckCurrentlyOnIOThread(); + // Now guaranteed to be on the correct thread. VLOG(1) << "Proxy configuration changed"; - has_config_ = true; cached_config_ = new_config; + config_availability_ = new_availability; // Notify observers of new proxy config. net::ProxyConfig net_config; cached_config_.ToNetProxyConfig(&net_config); FOR_EACH_OBSERVER(net::ProxyConfigService::Observer, observers_, - OnProxyConfigChanged(net_config)); + OnProxyConfigChanged(net_config, config_availability_)); +} + +void ProxyConfigServiceImpl::CheckCurrentlyOnIOThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); +} + +void ProxyConfigServiceImpl::CheckCurrentlyOnUIThread() { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } } // namespace chromeos diff --git a/chrome/browser/chromeos/proxy_config_service_impl.h b/chrome/browser/chromeos/proxy_config_service_impl.h index 64f198c..bd1cbb6 100644 --- a/chrome/browser/chromeos/proxy_config_service_impl.h +++ b/chrome/browser/chromeos/proxy_config_service_impl.h @@ -177,7 +177,8 @@ class ProxyConfigServiceImpl void AddObserver(net::ProxyConfigService::Observer* observer); void RemoveObserver(net::ProxyConfigService::Observer* observer); // Called from GetLatestProxyConfig. - bool IOGetProxyConfig(net::ProxyConfig* config); + net::ProxyConfigService::ConfigAvailability IOGetProxyConfig( + net::ProxyConfig* config); // Called from UI thread to retrieve proxy configuration in |config|. void UIGetProxyConfig(ProxyConfig* config); @@ -215,11 +216,6 @@ class ProxyConfigServiceImpl private: friend class base::RefCountedThreadSafe<ProxyConfigServiceImpl>; - // Init proxy to default config, i.e. AutoDetect. - // If |post_to_io_thread| is true, a task will be posted to IO thread to - // update |cached_config|. - void InitConfigToDefault(bool post_to_io_thread); - // Persists proxy config to device. void PersistConfigToDevice(); @@ -228,7 +224,9 @@ class ProxyConfigServiceImpl void OnUISetProxyConfig(bool update_to_device); // Posted from UI thread to IO thread to carry the new config information. - void IOSetProxyConfig(const ProxyConfig& new_config); + void IOSetProxyConfig( + const ProxyConfig& new_config, + net::ProxyConfigService::ConfigAvailability new_availability); // Checks that method is called on BrowserThread::IO thread. void CheckCurrentlyOnIOThread(); @@ -243,8 +241,8 @@ class ProxyConfigServiceImpl // method until the class's ref_count is at least one). bool can_post_task_; - // True if config has been fetched from device or initialized properly. - bool has_config_; + // Availability status of the configuration. + net::ProxyConfigService::ConfigAvailability config_availability_; // True if settings are to be persisted to device. bool persist_to_device_; diff --git a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc b/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc index c2ac945..fa3a54b 100644 --- a/chrome/browser/chromeos/proxy_config_service_impl_unittest.cc +++ b/chrome/browser/chromeos/proxy_config_service_impl_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. @@ -547,16 +547,23 @@ TEST_F(ProxyConfigServiceImplTest, ProxyChangedObserver) { virtual ~ProxyChangedObserver() { config_service_->RemoveObserver(this); } + net::ProxyConfigService::ConfigAvailability availability() const { + return availability_; + } const net::ProxyConfig& config() const { return config_; } private: - virtual void OnProxyConfigChanged(const net::ProxyConfig& config) { + virtual void OnProxyConfigChanged( + const net::ProxyConfig& config, + net::ProxyConfigService::ConfigAvailability availability) { config_ = config; + availability_ = availability; } scoped_refptr<ProxyConfigServiceImpl> config_service_; + net::ProxyConfigService::ConfigAvailability availability_; net::ProxyConfig config_; }; @@ -576,6 +583,7 @@ TEST_F(ProxyConfigServiceImplTest, ProxyChangedObserver) { SyncGetLatestProxyConfig(&io_config); // Observer should have gotten the same new proxy config. + EXPECT_EQ(net::ProxyConfigService::CONFIG_VALID, observer.availability()); EXPECT_TRUE(io_config.Equals(observer.config())); } diff --git a/chrome/browser/net/pref_proxy_config_service.cc b/chrome/browser/net/pref_proxy_config_service.cc index b7cfbd3..7862be9 100644 --- a/chrome/browser/net/pref_proxy_config_service.cc +++ b/chrome/browser/net/pref_proxy_config_service.cc @@ -16,7 +16,7 @@ PrefProxyConfigTracker::PrefProxyConfigTracker(PrefService* pref_service) : pref_service_(pref_service) { - valid_ = ReadPrefConfig(&pref_config_); + config_state_ = ReadPrefConfig(&pref_config_); proxy_prefs_observer_.reset( PrefSetObserver::CreateProxyPrefSetObserver(pref_service_, this)); } @@ -25,11 +25,12 @@ PrefProxyConfigTracker::~PrefProxyConfigTracker() { DCHECK(pref_service_ == NULL); } -bool PrefProxyConfigTracker::GetProxyConfig(net::ProxyConfig* config) { +PrefProxyConfigTracker::ConfigState + PrefProxyConfigTracker::GetProxyConfig(net::ProxyConfig* config) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - if (valid_) + if (config_state_ != CONFIG_UNSET) *config = pref_config_; - return valid_; + return config_state_; } void PrefProxyConfigTracker::DetachFromPrefService() { @@ -58,38 +59,54 @@ void PrefProxyConfigTracker::Observe(NotificationType type, if (type == NotificationType::PREF_CHANGED && Source<PrefService>(source).ptr() == pref_service_) { net::ProxyConfig new_config; - bool valid = ReadPrefConfig(&new_config); + ConfigState config_state = ReadPrefConfig(&new_config); BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, NewRunnableMethod(this, &PrefProxyConfigTracker::InstallProxyConfig, - new_config, valid)); + new_config, config_state)); } else { NOTREACHED() << "Unexpected notification of type " << type.value; } } -void PrefProxyConfigTracker::InstallProxyConfig(const net::ProxyConfig& config, - bool valid) { +void PrefProxyConfigTracker::InstallProxyConfig( + const net::ProxyConfig& config, + PrefProxyConfigTracker::ConfigState config_state) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - if (valid_ != valid || (valid && !pref_config_.Equals(config))) { - valid_ = valid; - if (valid_) + if (config_state_ != config_state || + (config_state_ != CONFIG_UNSET && !pref_config_.Equals(config))) { + config_state_ = config_state; + if (config_state_ != CONFIG_UNSET) pref_config_ = config; FOR_EACH_OBSERVER(Observer, observers_, OnPrefProxyConfigChanged()); } } -bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) { +PrefProxyConfigTracker::ConfigState + PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); // Clear the configuration. *config = net::ProxyConfig(); + const PrefService::Preference* pref = + pref_service_->FindPreference(prefs::kProxy); const DictionaryValue* dict = pref_service_->GetDictionary(prefs::kProxy); DCHECK(dict); ProxyConfigDictionary proxy_dict(dict); + if (PrefConfigToNetConfig(proxy_dict, config)) { + return (!pref->IsUserModifiable() || pref->HasUserSetting()) ? + CONFIG_PRESENT : CONFIG_FALLBACK; + } + + return CONFIG_UNSET; +} + +bool PrefProxyConfigTracker::PrefConfigToNetConfig( + const ProxyConfigDictionary& proxy_dict, + net::ProxyConfig* config) { ProxyPrefs::ProxyMode mode; if (!proxy_dict.GetMode(&mode)) { // Fall back to system settings if the mode preference is invalid. @@ -99,7 +116,7 @@ bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) { switch (mode) { case ProxyPrefs::MODE_SYSTEM: // Use system settings. - return false; + return true; case ProxyPrefs::MODE_DIRECT: // Ignore all the other proxy config preferences if the use of a proxy // has been explicitly disabled. @@ -171,15 +188,30 @@ void PrefProxyConfigService::RemoveObserver( observers_.RemoveObserver(observer); } -bool PrefProxyConfigService::GetLatestProxyConfig(net::ProxyConfig* config) { +net::ProxyConfigService::ConfigAvailability + PrefProxyConfigService::GetLatestProxyConfig(net::ProxyConfig* config) { RegisterObservers(); net::ProxyConfig pref_config; - if (pref_config_tracker_->GetProxyConfig(&pref_config)) { + PrefProxyConfigTracker::ConfigState state = + pref_config_tracker_->GetProxyConfig(&pref_config); + if (state == PrefProxyConfigTracker::CONFIG_PRESENT) { *config = pref_config; - return true; + return CONFIG_VALID; } - return base_service_->GetLatestProxyConfig(config); + // Ask the base service. + ConfigAvailability available = base_service_->GetLatestProxyConfig(config); + + // Base service doesn't have a configuration, fall back to prefs or default. + if (available == CONFIG_UNSET) { + if (state == PrefProxyConfigTracker::CONFIG_FALLBACK) + *config = pref_config; + else + *config = net::ProxyConfig::CreateDirect(); + return CONFIG_VALID; + } + + return available; } void PrefProxyConfigService::OnLazyPoll() { @@ -187,34 +219,38 @@ void PrefProxyConfigService::OnLazyPoll() { } void PrefProxyConfigService::OnProxyConfigChanged( - const net::ProxyConfig& config) { + const net::ProxyConfig& config, + ConfigAvailability availability) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); // Check whether there is a proxy configuration defined by preferences. In // this case that proxy configuration takes precedence and the change event // from the delegate proxy service can be disregarded. - net::ProxyConfig pref_config; - if (!pref_config_tracker_->GetProxyConfig(&pref_config)) { + net::ProxyConfig actual_config; + if (pref_config_tracker_->GetProxyConfig(&actual_config) != + PrefProxyConfigTracker::CONFIG_PRESENT) { + availability = GetLatestProxyConfig(&actual_config); FOR_EACH_OBSERVER(net::ProxyConfigService::Observer, observers_, - OnProxyConfigChanged(config)); + OnProxyConfigChanged(actual_config, availability)); } } void PrefProxyConfigService::OnPrefProxyConfigChanged() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); - // Evaluate the proxy configuration. If GetLatestProxyConfig returns false, - // we are using the system proxy service, but it doesn't have a valid - // configuration yet. Once it is ready, OnProxyConfigChanged() will be called - // and broadcast the proxy configuration. + // Evaluate the proxy configuration. If GetLatestProxyConfig returns + // CONFIG_PENDING, we are using the system proxy service, but it doesn't have + // a valid configuration yet. Once it is ready, OnProxyConfigChanged() will be + // called and broadcast the proxy configuration. // Note: If a switch between a preference proxy configuration and the system // proxy configuration occurs an unnecessary notification might get send if // the two configurations agree. This case should be rare however, so we don't // handle that case specially. net::ProxyConfig new_config; - if (GetLatestProxyConfig(&new_config)) { + ConfigAvailability availability = GetLatestProxyConfig(&new_config); + if (availability != CONFIG_PENDING) { FOR_EACH_OBSERVER(net::ProxyConfigService::Observer, observers_, - OnProxyConfigChanged(new_config)); + OnProxyConfigChanged(new_config, availability)); } } diff --git a/chrome/browser/net/pref_proxy_config_service.h b/chrome/browser/net/pref_proxy_config_service.h index c807c98..d98fa6f 100644 --- a/chrome/browser/net/pref_proxy_config_service.h +++ b/chrome/browser/net/pref_proxy_config_service.h @@ -10,6 +10,7 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" #include "base/observer_list.h" +#include "chrome/browser/prefs/proxy_config_dictionary.h" #include "content/common/notification_observer.h" #include "net/proxy/proxy_config.h" #include "net/proxy/proxy_config_service.h" @@ -32,17 +33,26 @@ class PrefProxyConfigTracker virtual void OnPrefProxyConfigChanged() = 0; }; + // Return codes for GetProxyConfig. + enum ConfigState { + // Configuration is valid and present. + CONFIG_PRESENT, + // There is a fallback configuration present. + CONFIG_FALLBACK, + // Configuration is known to be not set. + CONFIG_UNSET, + }; + explicit PrefProxyConfigTracker(PrefService* pref_service); // Observer manipulation is only valid on the IO thread. void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); - // Get the proxy configuration currently defined by preferences. Writes the - // configuration to |config| and returns true on success. |config| is not - // touched and false is returned if there is no configuration defined. This - // must be called on the IO thread. - bool GetProxyConfig(net::ProxyConfig* config); + // Get the proxy configuration currently defined by preferences. Status is + // indicated in the return value. Writes the configuration to |config| unless + // the return value is CONFIG_UNSET, in which case |config| is not touched. + ConfigState GetProxyConfig(net::ProxyConfig* config); // Notifies the tracker that the pref service passed upon construction is // about to go away. This must be called from the UI thread. @@ -58,23 +68,29 @@ class PrefProxyConfigTracker const NotificationDetails& details); // Install a new configuration. This is invoked on the IO thread to update - // the internal state after handling a pref change on the UI thread. |valid| - // indicates whether there is a preference proxy configuration defined, in - // which case this configuration is given by |config|. |config| is ignored if - // |valid| is false. - void InstallProxyConfig(const net::ProxyConfig& config, bool valid); + // the internal state after handling a pref change on the UI thread. + // |config_state| indicates the new state we're switching to, and |config| is + // the new preference-based proxy configuration if |config_state| is different + // from CONFIG_UNSET. + void InstallProxyConfig(const net::ProxyConfig& config, ConfigState state); // Creates a proxy configuration from proxy-related preferences. Configuration // is stored in |config| and the return value indicates whether the // configuration is valid. - bool ReadPrefConfig(net::ProxyConfig* config); + ConfigState ReadPrefConfig(net::ProxyConfig* config); + + // Converts a ProxyConfigDictionary to net::ProxyConfig representation. + // Returns true if the data from in the dictionary is valid, false otherwise. + static bool PrefConfigToNetConfig(const ProxyConfigDictionary& proxy_dict, + net::ProxyConfig* config); // Configuration as defined by prefs. Only to be accessed from the IO thread // (except for construction). net::ProxyConfig pref_config_; - // Whether |pref_config_| is valid. Only accessed from the IO thread. - bool valid_; + // Tracks configuration state. |pref_config_| is valid only if |config_state_| + // is not CONFIG_UNSET. + ConfigState config_state_; // List of observers, accessed exclusively from the IO thread. ObserverList<Observer, true> observers_; @@ -102,14 +118,15 @@ class PrefProxyConfigService // ProxyConfigService implementation: virtual void AddObserver(net::ProxyConfigService::Observer* observer); virtual void RemoveObserver(net::ProxyConfigService::Observer* observer); - virtual bool GetLatestProxyConfig(net::ProxyConfig* config); + virtual ConfigAvailability GetLatestProxyConfig(net::ProxyConfig* config); virtual void OnLazyPoll(); static void RegisterPrefs(PrefService* user_prefs); private: // ProxyConfigService::Observer implementation: - virtual void OnProxyConfigChanged(const net::ProxyConfig& config); + virtual void OnProxyConfigChanged(const net::ProxyConfig& config, + ConfigAvailability availability); // PrefProxyConfigTracker::Observer implementation: virtual void OnPrefProxyConfigChanged(); diff --git a/chrome/browser/net/pref_proxy_config_service_unittest.cc b/chrome/browser/net/pref_proxy_config_service_unittest.cc index 75b6f82..e191887 100644 --- a/chrome/browser/net/pref_proxy_config_service_unittest.cc +++ b/chrome/browser/net/pref_proxy_config_service_unittest.cc @@ -26,14 +26,17 @@ const char kFixedPacUrl[] = "http://chromium.org/fixed_pac_url"; // Testing proxy config service that allows us to fire notifications at will. class TestProxyConfigService : public net::ProxyConfigService { public: - explicit TestProxyConfigService(const net::ProxyConfig& config) - : config_(config) { - } + TestProxyConfigService(const net::ProxyConfig& config, + ConfigAvailability availability) + : config_(config), + availability_(availability) {} - void SetProxyConfig(const net::ProxyConfig config) { + void SetProxyConfig(const net::ProxyConfig config, + ConfigAvailability availability) { config_ = config; + availability_ = availability; FOR_EACH_OBSERVER(net::ProxyConfigService::Observer, observers_, - OnProxyConfigChanged(config_)); + OnProxyConfigChanged(config, availability)); } private: @@ -45,19 +48,23 @@ class TestProxyConfigService : public net::ProxyConfigService { observers_.RemoveObserver(observer); } - virtual bool GetLatestProxyConfig(net::ProxyConfig* config) { + virtual net::ProxyConfigService::ConfigAvailability GetLatestProxyConfig( + net::ProxyConfig* config) { *config = config_; - return true; + return availability_; } net::ProxyConfig config_; + ConfigAvailability availability_; ObserverList<net::ProxyConfigService::Observer, true> observers_; }; // A mock observer for capturing callbacks. class MockObserver : public net::ProxyConfigService::Observer { public: - MOCK_METHOD1(OnProxyConfigChanged, void(const net::ProxyConfig&)); + MOCK_METHOD2(OnProxyConfigChanged, + void(const net::ProxyConfig&, + net::ProxyConfigService::ConfigAvailability)); }; template<typename TESTBASE> @@ -71,7 +78,9 @@ class PrefProxyConfigServiceTestBase : public TESTBASE { ASSERT_TRUE(pref_service); PrefProxyConfigService::RegisterPrefs(pref_service); fixed_config_.set_pac_url(GURL(kFixedPacUrl)); - delegate_service_ = new TestProxyConfigService(fixed_config_); + delegate_service_ = + new TestProxyConfigService(fixed_config_, + net::ProxyConfigService::CONFIG_VALID); proxy_config_tracker_ = new PrefProxyConfigTracker(pref_service); proxy_config_service_.reset( new PrefProxyConfigService(proxy_config_tracker_.get(), @@ -108,7 +117,8 @@ class PrefProxyConfigServiceTest TEST_F(PrefProxyConfigServiceTest, BaseConfiguration) { net::ProxyConfig actual_config; - proxy_config_service_->GetLatestProxyConfig(&actual_config); + EXPECT_EQ(net::ProxyConfigService::CONFIG_VALID, + proxy_config_service_->GetLatestProxyConfig(&actual_config)); EXPECT_EQ(GURL(kFixedPacUrl), actual_config.pac_url()); } @@ -119,7 +129,8 @@ TEST_F(PrefProxyConfigServiceTest, DynamicPrefOverrides) { loop_.RunAllPending(); net::ProxyConfig actual_config; - proxy_config_service_->GetLatestProxyConfig(&actual_config); + EXPECT_EQ(net::ProxyConfigService::CONFIG_VALID, + proxy_config_service_->GetLatestProxyConfig(&actual_config)); EXPECT_FALSE(actual_config.auto_detect()); EXPECT_EQ(net::ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY, actual_config.proxy_rules().type); @@ -131,7 +142,8 @@ TEST_F(PrefProxyConfigServiceTest, DynamicPrefOverrides) { ProxyConfigDictionary::CreateAutoDetect()); loop_.RunAllPending(); - proxy_config_service_->GetLatestProxyConfig(&actual_config); + EXPECT_EQ(net::ProxyConfigService::CONFIG_VALID, + proxy_config_service_->GetLatestProxyConfig(&actual_config)); EXPECT_TRUE(actual_config.auto_detect()); } @@ -143,15 +155,17 @@ MATCHER_P(ProxyConfigMatches, config, "") { } TEST_F(PrefProxyConfigServiceTest, Observers) { + const net::ProxyConfigService::ConfigAvailability CONFIG_VALID = + net::ProxyConfigService::CONFIG_VALID; MockObserver observer; proxy_config_service_->AddObserver(&observer); // Firing the observers in the delegate should trigger a notification. net::ProxyConfig config2; config2.set_auto_detect(true); - EXPECT_CALL(observer, - OnProxyConfigChanged(ProxyConfigMatches(config2))).Times(1); - delegate_service_->SetProxyConfig(config2); + EXPECT_CALL(observer, OnProxyConfigChanged(ProxyConfigMatches(config2), + CONFIG_VALID)).Times(1); + delegate_service_->SetProxyConfig(config2, CONFIG_VALID); loop_.RunAllPending(); Mock::VerifyAndClearExpectations(&observer); @@ -159,8 +173,8 @@ TEST_F(PrefProxyConfigServiceTest, Observers) { net::ProxyConfig pref_config; pref_config.set_pac_url(GURL(kFixedPacUrl)); - EXPECT_CALL(observer, - OnProxyConfigChanged(ProxyConfigMatches(pref_config))).Times(1); + EXPECT_CALL(observer, OnProxyConfigChanged(ProxyConfigMatches(pref_config), + CONFIG_VALID)).Times(1); pref_service_->SetManagedPref( prefs::kProxy, ProxyConfigDictionary::CreatePacScript(kFixedPacUrl)); @@ -170,15 +184,15 @@ TEST_F(PrefProxyConfigServiceTest, Observers) { // Since there are pref overrides, delegate changes should be ignored. net::ProxyConfig config3; config3.proxy_rules().ParseFromString("http=config3:80"); - EXPECT_CALL(observer, OnProxyConfigChanged(_)).Times(0); + EXPECT_CALL(observer, OnProxyConfigChanged(_, _)).Times(0); fixed_config_.set_auto_detect(true); - delegate_service_->SetProxyConfig(config3); + delegate_service_->SetProxyConfig(config3, CONFIG_VALID); loop_.RunAllPending(); Mock::VerifyAndClearExpectations(&observer); // Clear the override should switch back to the fixed configuration. - EXPECT_CALL(observer, - OnProxyConfigChanged(ProxyConfigMatches(config3))).Times(1); + EXPECT_CALL(observer, OnProxyConfigChanged(ProxyConfigMatches(config3), + CONFIG_VALID)).Times(1); pref_service_->RemoveManagedPref(prefs::kProxy); loop_.RunAllPending(); Mock::VerifyAndClearExpectations(&observer); @@ -186,11 +200,65 @@ TEST_F(PrefProxyConfigServiceTest, Observers) { // Delegate service notifications should show up again. net::ProxyConfig config4; config4.proxy_rules().ParseFromString("socks:config4"); + EXPECT_CALL(observer, OnProxyConfigChanged(ProxyConfigMatches(config4), + CONFIG_VALID)).Times(1); + delegate_service_->SetProxyConfig(config4, CONFIG_VALID); + loop_.RunAllPending(); + Mock::VerifyAndClearExpectations(&observer); + + proxy_config_service_->RemoveObserver(&observer); +} + +TEST_F(PrefProxyConfigServiceTest, Fallback) { + const net::ProxyConfigService::ConfigAvailability CONFIG_VALID = + net::ProxyConfigService::CONFIG_VALID; + MockObserver observer; + net::ProxyConfig actual_config; + delegate_service_->SetProxyConfig(net::ProxyConfig::CreateDirect(), + net::ProxyConfigService::CONFIG_UNSET); + proxy_config_service_->AddObserver(&observer); + + // Prepare test data. + net::ProxyConfig recommended_config = net::ProxyConfig::CreateAutoDetect(); + net::ProxyConfig user_config = + net::ProxyConfig::CreateFromCustomPacURL(GURL(kFixedPacUrl)); + + // Set a recommended pref. + EXPECT_CALL(observer, + OnProxyConfigChanged(ProxyConfigMatches(recommended_config), + CONFIG_VALID)).Times(1); + pref_service_->SetRecommendedPref( + prefs::kProxy, + ProxyConfigDictionary::CreateAutoDetect()); + loop_.RunAllPending(); + Mock::VerifyAndClearExpectations(&observer); + EXPECT_EQ(CONFIG_VALID, + proxy_config_service_->GetLatestProxyConfig(&actual_config)); + EXPECT_THAT(actual_config, ProxyConfigMatches(recommended_config)); + + // Override in user prefs. + EXPECT_CALL(observer, + OnProxyConfigChanged(ProxyConfigMatches(user_config), + CONFIG_VALID)).Times(1); + pref_service_->SetManagedPref( + prefs::kProxy, + ProxyConfigDictionary::CreatePacScript(kFixedPacUrl)); + loop_.RunAllPending(); + Mock::VerifyAndClearExpectations(&observer); + EXPECT_EQ(CONFIG_VALID, + proxy_config_service_->GetLatestProxyConfig(&actual_config)); + EXPECT_THAT(actual_config, ProxyConfigMatches(user_config)); + + // Go back to recommended pref. EXPECT_CALL(observer, - OnProxyConfigChanged(ProxyConfigMatches(config4))).Times(1); - delegate_service_->SetProxyConfig(config4); + OnProxyConfigChanged(ProxyConfigMatches(recommended_config), + CONFIG_VALID)).Times(1); + pref_service_->RemoveManagedPref(prefs::kProxy); loop_.RunAllPending(); Mock::VerifyAndClearExpectations(&observer); + EXPECT_EQ(CONFIG_VALID, + proxy_config_service_->GetLatestProxyConfig(&actual_config)); + EXPECT_THAT(actual_config, ProxyConfigMatches(recommended_config)); proxy_config_service_->RemoveObserver(&observer); } @@ -257,7 +325,8 @@ class PrefProxyConfigServiceCommandLineTest TEST_P(PrefProxyConfigServiceCommandLineTest, CommandLine) { net::ProxyConfig config; - proxy_config_service_->GetLatestProxyConfig(&config); + EXPECT_EQ(net::ProxyConfigService::CONFIG_VALID, + proxy_config_service_->GetLatestProxyConfig(&config)); if (GetParam().is_null) { EXPECT_EQ(GURL(kFixedPacUrl), config.pac_url()); diff --git a/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc b/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc index dffd0cf..bb3c1d8 100644 --- a/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc +++ b/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc @@ -16,9 +16,9 @@ class MockProxyConfigService : public net::ProxyConfigService { public: virtual void AddObserver(Observer* observer) {} virtual void RemoveObserver(Observer* observer) {} - virtual bool GetLatestProxyConfig(net::ProxyConfig* results) { + virtual ConfigAvailability GetLatestProxyConfig(net::ProxyConfig* results) { *results = net::ProxyConfig::CreateFromCustomPacURL(GURL("http://pac")); - return true; + return CONFIG_VALID; } }; diff --git a/chrome/test/testing_pref_service.cc b/chrome/test/testing_pref_service.cc index 815ae1e..7d2a5c1 100644 --- a/chrome/test/testing_pref_service.cc +++ b/chrome/test/testing_pref_service.cc @@ -13,17 +13,19 @@ TestingPrefServiceBase::TestingPrefServiceBase( TestingPrefStore* managed_platform_prefs, - TestingPrefStore* user_prefs) + TestingPrefStore* user_prefs, + TestingPrefStore* recommended_platform_prefs) : PrefService(managed_platform_prefs, NULL, NULL, NULL, user_prefs, - NULL, + recommended_platform_prefs, NULL, new DefaultPrefStore()), managed_platform_prefs_(managed_platform_prefs), - user_prefs_(user_prefs) { + user_prefs_(user_prefs), + recommended_platform_prefs_(recommended_platform_prefs) { } TestingPrefServiceBase::~TestingPrefServiceBase() { @@ -53,6 +55,20 @@ void TestingPrefServiceBase::RemoveUserPref(const char* path) { RemovePref(user_prefs_, path); } +const Value* TestingPrefServiceBase::GetRecommendedPref( + const char* path) const { + return GetPref(recommended_platform_prefs_, path); +} + +void TestingPrefServiceBase::SetRecommendedPref( + const char* path, Value* value) { + SetPref(recommended_platform_prefs_, path, value); +} + +void TestingPrefServiceBase::RemoveRecommendedPref(const char* path) { + RemovePref(recommended_platform_prefs_, path); +} + const Value* TestingPrefServiceBase::GetPref(TestingPrefStore* pref_store, const char* path) const { const Value* res; @@ -72,6 +88,7 @@ void TestingPrefServiceBase::RemovePref(TestingPrefStore* pref_store, TestingPrefService::TestingPrefService() : TestingPrefServiceBase(new TestingPrefStore(), + new TestingPrefStore(), new TestingPrefStore()) { } diff --git a/chrome/test/testing_pref_service.h b/chrome/test/testing_pref_service.h index 56dfc0c..fdd2de3 100644 --- a/chrome/test/testing_pref_service.h +++ b/chrome/test/testing_pref_service.h @@ -35,10 +35,16 @@ class TestingPrefServiceBase : public PrefService { void SetUserPref(const char* path, Value* value); void RemoveUserPref(const char* path); + // Similar to the above, but for recommended policy preferences. + const Value* GetRecommendedPref(const char* path) const; + void SetRecommendedPref(const char* path, Value* value); + void RemoveRecommendedPref(const char* path); + protected: TestingPrefServiceBase( TestingPrefStore* managed_platform_prefs, - TestingPrefStore* user_prefs); + TestingPrefStore* user_prefs, + TestingPrefStore* recommended_platform_prefs); private: // Reads the value of the preference indicated by |path| from |pref_store|. @@ -54,6 +60,7 @@ class TestingPrefServiceBase : public PrefService { // Pointers to the pref stores our value store uses. scoped_refptr<TestingPrefStore> managed_platform_prefs_; scoped_refptr<TestingPrefStore> user_prefs_; + scoped_refptr<TestingPrefStore> recommended_platform_prefs_; DISALLOW_COPY_AND_ASSIGN(TestingPrefServiceBase); }; 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_; }; |