diff options
author | szym@chromium.org <szym@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-16 17:13:11 +0000 |
---|---|---|
committer | szym@chromium.org <szym@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-03-16 17:13:11 +0000 |
commit | b4481b2296ce6035e52db00c8011cf20c165d7b6 (patch) | |
tree | 6a5b751c14fdc713b4ae76e407ac29c82d9f1df5 /net/dns | |
parent | e1c39ae523ad5cce2f6d8d9e0dd177c2d5358676 (diff) | |
download | chromium_src-b4481b2296ce6035e52db00c8011cf20c165d7b6.zip chromium_src-b4481b2296ce6035e52db00c8011cf20c165d7b6.tar.gz chromium_src-b4481b2296ce6035e52db00c8011cf20c165d7b6.tar.bz2 |
[net/dns] Refactoring of DnsConfigService.
- Replaces Observer with Callback.
- The Callback is run within 100ms of any config change, not only on
successful read. It is also run on errors in watches (registry and
files). This allows HostResolverImpl to withdraw from using DnsTask
until DnsConfig is read successfully.
- Moves DnsHostsReader to dns_hosts.{h,cc}).
- Handles missing HOSTS files.
- Respects network adapter binding order in DnsConfigServiceWin.
- Adds NetLog source for DnsConfig-related events.
BUG=112856, 115460, 115494
TEST=./net_unittests --gtest_filter=DnsConfig*
Review URL: http://codereview.chromium.org/9597029
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@127190 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/dns')
-rw-r--r-- | net/dns/dns_config_service.cc | 111 | ||||
-rw-r--r-- | net/dns/dns_config_service.h | 83 | ||||
-rw-r--r-- | net/dns/dns_config_service_posix.cc | 103 | ||||
-rw-r--r-- | net/dns/dns_config_service_posix.h | 24 | ||||
-rw-r--r-- | net/dns/dns_config_service_posix_unittest.cc | 6 | ||||
-rw-r--r-- | net/dns/dns_config_service_unittest.cc | 200 | ||||
-rw-r--r-- | net/dns/dns_config_service_win.cc | 147 | ||||
-rw-r--r-- | net/dns/dns_config_service_win.h | 20 | ||||
-rw-r--r-- | net/dns/dns_config_service_win_unittest.cc | 31 | ||||
-rw-r--r-- | net/dns/dns_hosts.cc | 46 | ||||
-rw-r--r-- | net/dns/dns_hosts.h | 31 | ||||
-rw-r--r-- | net/dns/dns_test_util.cc | 4 | ||||
-rw-r--r-- | net/dns/dns_test_util.h | 2 | ||||
-rw-r--r-- | net/dns/file_path_watcher_wrapper.cc | 99 | ||||
-rw-r--r-- | net/dns/file_path_watcher_wrapper.h | 68 | ||||
-rw-r--r-- | net/dns/file_path_watcher_wrapper_unittest.cc | 142 | ||||
-rw-r--r-- | net/dns/serial_worker.h | 4 | ||||
-rw-r--r-- | net/dns/watching_file_reader.cc | 102 | ||||
-rw-r--r-- | net/dns/watching_file_reader.h | 90 | ||||
-rw-r--r-- | net/dns/watching_file_reader_unittest.cc | 168 |
20 files changed, 875 insertions, 606 deletions
diff --git a/net/dns/dns_config_service.cc b/net/dns/dns_config_service.cc index d64b6735..be48d2a 100644 --- a/net/dns/dns_config_service.cc +++ b/net/dns/dns_config_service.cc @@ -4,7 +4,7 @@ #include "net/dns/dns_config_service.h" -#include "base/file_util.h" +#include "base/logging.h" #include "net/base/ip_endpoint.h" namespace net { @@ -20,6 +20,10 @@ DnsConfig::DnsConfig() DnsConfig::~DnsConfig() {} +bool DnsConfig::Equals(const DnsConfig& d) const { + return EqualsIgnoreHosts(d) && (hosts == d.hosts); +} + bool DnsConfig::EqualsIgnoreHosts(const DnsConfig& d) const { return (nameservers == d.nameservers) && (search == d.search) && @@ -31,86 +35,93 @@ bool DnsConfig::EqualsIgnoreHosts(const DnsConfig& d) const { (edns0 == d.edns0); } -bool DnsConfig::Equals(const DnsConfig& d) const { - return EqualsIgnoreHosts(d) && (hosts == d.hosts); +void DnsConfig::CopyIgnoreHosts(const DnsConfig& d) { + nameservers = d.nameservers; + search = d.search; + append_to_multi_label_name = d.append_to_multi_label_name; + ndots = d.ndots; + timeout = d.timeout; + attempts = d.attempts; + rotate = d.rotate; + edns0 = d.edns0; } + DnsConfigService::DnsConfigService() : have_config_(false), - have_hosts_(false) {} + have_hosts_(false), + need_update_(false) {} DnsConfigService::~DnsConfigService() {} -void DnsConfigService::AddObserver(Observer* observer) { +void DnsConfigService::InvalidateConfig() { DCHECK(CalledOnValidThread()); - observers_.AddObserver(observer); - if (have_config_ && have_hosts_) { - observer->OnConfigChanged(dns_config_); - } + if (!have_config_) + return; + have_config_ = false; + StartTimer(); } -void DnsConfigService::RemoveObserver(Observer* observer) { +void DnsConfigService::InvalidateHosts() { DCHECK(CalledOnValidThread()); - observers_.RemoveObserver(observer); + if (!have_hosts_) + return; + have_hosts_ = false; + StartTimer(); } void DnsConfigService::OnConfigRead(const DnsConfig& config) { DCHECK(CalledOnValidThread()); + DCHECK(config.IsValid()); + if (!config.EqualsIgnoreHosts(dns_config_)) { - DnsHosts current_hosts = dns_config_.hosts; - dns_config_ = config; - dns_config_.hosts.swap(current_hosts); - have_config_ = true; - if (have_hosts_) { - FOR_EACH_OBSERVER(Observer, observers_, OnConfigChanged(dns_config_)); - } + dns_config_.CopyIgnoreHosts(config); + need_update_ = true; } + + have_config_ = true; + if (have_hosts_) + OnCompleteConfig(); } void DnsConfigService::OnHostsRead(const DnsHosts& hosts) { DCHECK(CalledOnValidThread()); - if (hosts != dns_config_.hosts || !have_hosts_) { + + if (hosts != dns_config_.hosts) { dns_config_.hosts = hosts; - have_hosts_ = true; - if (have_config_) { - FOR_EACH_OBSERVER(Observer, observers_, OnConfigChanged(dns_config_)); - } + need_update_ = true; } -} -DnsHostsReader::DnsHostsReader(const FilePath& path, DnsConfigService* service) - : path_(path), - service_(service), - success_(false) { - DCHECK(service); + have_hosts_ = true; + if (have_config_) + OnCompleteConfig(); } -// Reads the contents of the file at |path| into |str| if the total length is -// less than |size|. -static bool ReadFile(const FilePath& path, int64 size, std::string* str) { - int64 sz; - if (!file_util::GetFileSize(path, &sz) || sz > size) - return false; - return file_util::ReadFileToString(path, str); +void DnsConfigService::StartTimer() { + DCHECK(CalledOnValidThread()); + timer_.Stop(); + const base::TimeDelta kTimeout = base::TimeDelta::FromMilliseconds(100); + timer_.Start(FROM_HERE, + kTimeout, + this, + &DnsConfigService::OnTimeout); } -void DnsHostsReader::DoWork() { - success_ = false; - std::string contents; - const int64 kMaxHostsSize = 1 << 16; - if (ReadFile(path_, kMaxHostsSize, &contents)) { - success_ = true; - ParseHosts(contents, &dns_hosts_); - } +void DnsConfigService::OnTimeout() { + DCHECK(CalledOnValidThread()); + // Indicate that even if there is no change in On*Read, we will need to + // update the receiver when the config becomes complete. + need_update_ = true; + callback_.Run(DnsConfig()); } -void DnsHostsReader::OnWorkFinished() { - DCHECK(!IsCancelled()); - if (success_) - service_->OnHostsRead(dns_hosts_); +void DnsConfigService::OnCompleteConfig() { + timer_.Stop(); + if (need_update_) { + need_update_ = false; + callback_.Run(dns_config_); + } } -DnsHostsReader::~DnsHostsReader() {} - } // namespace net diff --git a/net/dns/dns_config_service.h b/net/dns/dns_config_service.h index 51e26b3..cdbdb40 100644 --- a/net/dns/dns_config_service.h +++ b/net/dns/dns_config_service.h @@ -10,16 +10,14 @@ #include <string> #include <vector> -#include "base/file_path.h" #include "base/gtest_prod_util.h" #include "base/memory/scoped_ptr.h" -#include "base/observer_list.h" #include "base/threading/non_thread_safe.h" #include "base/time.h" +#include "base/timer.h" #include "net/base/ip_endpoint.h" // win requires size of IPEndPoint #include "net/base/net_export.h" #include "net/dns/dns_hosts.h" -#include "net/dns/serial_worker.h" namespace net { @@ -32,6 +30,8 @@ struct NET_EXPORT_PRIVATE DnsConfig { bool EqualsIgnoreHosts(const DnsConfig& d) const; + void CopyIgnoreHosts(const DnsConfig& src); + bool IsValid() const { return !nameservers.empty(); } @@ -64,20 +64,13 @@ struct NET_EXPORT_PRIVATE DnsConfig { // Service for watching when the system DNS settings have changed. -// Depending on the platform, watches files in /etc/ or win registry. -// This class also serves as the do-nothing mock implementation. +// Depending on the platform, watches files in /etc/ or Windows registry. class NET_EXPORT_PRIVATE DnsConfigService : NON_EXPORTED_BASE(public base::NonThreadSafe) { public: // Callback interface for the client. The observer is called on the same // thread as Watch(). Observer must outlive the service. - class Observer { - public: - virtual ~Observer() {} - - // Called only when |dns_config| is different from the last call. - virtual void OnConfigChanged(const DnsConfig& dns_config) = 0; - }; + typedef base::Callback<void(const DnsConfig& config)> CallbackType; // Creates the platform-specific DnsConfigService. static scoped_ptr<DnsConfigService> CreateSystemService(); @@ -88,54 +81,50 @@ class NET_EXPORT_PRIVATE DnsConfigService // Immediately starts watching system configuration for changes and attempts // to read the configuration. For some platform implementations, the current // thread must have an IO loop (for base::files::FilePathWatcher). - virtual void Watch() {} - - // If a config is available, |observer| will immediately be called with - // OnConfigChanged. - void AddObserver(Observer* observer); - void RemoveObserver(Observer* observer); + virtual void Watch(const CallbackType& callback) = 0; protected: - FRIEND_TEST_ALL_PREFIXES(DnsConfigServiceTest, NotifyOnChange); friend class DnsHostsReader; - // To be called with new config. |config|.hosts is ignored. - virtual void OnConfigRead(const DnsConfig& config); - // To be called with new hosts. Rest of the config is assumed unchanged. - virtual void OnHostsRead(const DnsHosts& hosts); - DnsConfig dns_config_; - // True after first OnConfigRead and OnHostsRead; indicate complete config. - bool have_config_; - bool have_hosts_; + // Called when the current config (except hosts) has changed. + void InvalidateConfig(); + // Called when the current hosts have changed. + void InvalidateHosts(); + + // Called with new config. |config|.hosts is ignored. + void OnConfigRead(const DnsConfig& config); + // Called with new hosts. Rest of the config is assumed unchanged. + void OnHostsRead(const DnsHosts& hosts); + + void set_callback(const CallbackType& callback) { + callback_ = callback; + } - ObserverList<Observer> observers_; private: - DISALLOW_COPY_AND_ASSIGN(DnsConfigService); -}; + void StartTimer(); + // Called when the timer expires. + void OnTimeout(); + // Called when the config becomes complete. + void OnCompleteConfig(); -// A WatchingFileReader that reads a HOSTS file and notifies -// DnsConfigService::OnHostsRead(). -// Client should call Cancel() when |service| is going away. -class NET_EXPORT_PRIVATE DnsHostsReader - : NON_EXPORTED_BASE(public SerialWorker) { - public: - DnsHostsReader(const FilePath& path, DnsConfigService* service); + CallbackType callback_; - virtual void DoWork() OVERRIDE; - virtual void OnWorkFinished() OVERRIDE; + DnsConfig dns_config_; - private: - virtual ~DnsHostsReader(); + // True after On*Read, before Invalidate*. Tell if the config is complete. + bool have_config_; + bool have_hosts_; + // True if receiver needs to be updated when the config becomes complete. + bool need_update_; - FilePath path_; - DnsConfigService* service_; - // Written in DoRead, read in OnReadFinished, no locking necessary. - DnsHosts dns_hosts_; - bool success_; + // Started in Invalidate*, cleared in On*Read. + base::OneShotTimer<DnsConfigService> timer_; - DISALLOW_COPY_AND_ASSIGN(DnsHostsReader); + private: + DISALLOW_COPY_AND_ASSIGN(DnsConfigService); }; + } // namespace net #endif // NET_DNS_DNS_CONFIG_SERVICE_H_ diff --git a/net/dns/dns_config_service_posix.cc b/net/dns/dns_config_service_posix.cc index 681405f..b3977fc 100644 --- a/net/dns/dns_config_service_posix.cc +++ b/net/dns/dns_config_service_posix.cc @@ -6,14 +6,15 @@ #include <string> -#include "base/compiler_specific.h" +#include "base/basictypes.h" +#include "base/bind.h" #include "base/file_path.h" #include "base/file_util.h" #include "base/memory/scoped_ptr.h" #include "net/base/ip_endpoint.h" #include "net/base/net_util.h" +#include "net/dns/file_path_watcher_wrapper.h" #include "net/dns/serial_worker.h" -#include "net/dns/watching_file_reader.h" #ifndef _PATH_RESCONF // Normally defined in <resolv.h> #define _PATH_RESCONF "/etc/resolv.conf" @@ -21,13 +22,19 @@ namespace net { -// A SerialWorker that uses ResolverLib to initialize res_state and converts +namespace { + +const FilePath::CharType* kFilePathConfig = FILE_PATH_LITERAL(_PATH_RESCONF); +const FilePath::CharType* kFilePathHosts = FILE_PATH_LITERAL("/etc/hosts"); + +// A SerialWorker that uses libresolv to initialize res_state and converts // it to DnsConfig. -class DnsConfigServicePosix::ConfigReader : public SerialWorker { +class ConfigReader : public SerialWorker { public: - explicit ConfigReader(DnsConfigServicePosix* service) - : service_(service), - success_(false) {} + typedef base::Callback<void(const DnsConfig& config)> CallbackType; + explicit ConfigReader(const CallbackType& callback) + : callback_(callback), + success_(false) {} void DoWork() OVERRIDE { success_ = false; @@ -38,13 +45,13 @@ class DnsConfigServicePosix::ConfigReader : public SerialWorker { // res_init behaves the same way. memset(&_res, 0, sizeof(_res)); if ((res_init() == 0) && (_res.options & RES_INIT)) { - success_ = ConvertResStateToDnsConfig(_res, &dns_config_); + success_ = internal::ConvertResStateToDnsConfig(_res, &dns_config_); } #else // all other OS_POSIX struct __res_state res; memset(&res, 0, sizeof(res)); if ((res_ninit(&res) == 0) && (res.options & RES_INIT)) { - success_ = ConvertResStateToDnsConfig(res, &dns_config_); + success_ = internal::ConvertResStateToDnsConfig(res, &dns_config_); } // Prefer res_ndestroy where available. #if defined(OS_MACOSX) @@ -59,35 +66,80 @@ class DnsConfigServicePosix::ConfigReader : public SerialWorker { void OnWorkFinished() OVERRIDE { DCHECK(!IsCancelled()); if (success_) - service_->OnConfigRead(dns_config_); + callback_.Run(dns_config_); } private: virtual ~ConfigReader() {} - DnsConfigServicePosix* service_; + CallbackType callback_; // Written in DoWork, read in OnWorkFinished, no locking necessary. DnsConfig dns_config_; bool success_; }; +} // namespace + +namespace internal { + DnsConfigServicePosix::DnsConfigServicePosix() - : config_watcher_(new WatchingFileReader()), - hosts_watcher_(new WatchingFileReader()) {} + : config_watcher_(new FilePathWatcherWrapper()), + hosts_watcher_(new FilePathWatcherWrapper()) { + config_reader_ = new ConfigReader( + base::Bind(&DnsConfigServicePosix::OnConfigRead, + base::Unretained(this))); + hosts_reader_ = new DnsHostsReader( + FilePath(kFilePathHosts), + base::Bind(&DnsConfigServicePosix::OnHostsRead, + base::Unretained(this))); +} -DnsConfigServicePosix::~DnsConfigServicePosix() {} +DnsConfigServicePosix::~DnsConfigServicePosix() { + config_reader_->Cancel(); + hosts_reader_->Cancel(); +} -void DnsConfigServicePosix::Watch() { +void DnsConfigServicePosix::Watch(const CallbackType& callback) { DCHECK(CalledOnValidThread()); - config_watcher_->StartWatch(FilePath(FILE_PATH_LITERAL(_PATH_RESCONF)), - new ConfigReader(this)); - FilePath hosts_path(FILE_PATH_LITERAL("/etc/hosts")); - hosts_watcher_->StartWatch(hosts_path, new DnsHostsReader(hosts_path, this)); + DCHECK(!callback.is_null()); + set_callback(callback); + + // Even if watchers fail, we keep the other one as it provides useful signals. + if (config_watcher_->Watch( + FilePath(kFilePathConfig), + base::Bind(&DnsConfigServicePosix::OnConfigChanged, + base::Unretained(this)))) { + OnConfigChanged(true); + } else { + OnConfigChanged(false); + } + + if (hosts_watcher_->Watch( + FilePath(kFilePathHosts), + base::Bind(&DnsConfigServicePosix::OnHostsChanged, + base::Unretained(this)))) { + OnHostsChanged(true); + } else { + OnHostsChanged(false); + } } -// static -scoped_ptr<DnsConfigService> DnsConfigService::CreateSystemService() { - return scoped_ptr<DnsConfigService>(new DnsConfigServicePosix()); +void DnsConfigServicePosix::OnConfigChanged(bool watch_succeeded) { + InvalidateConfig(); + // We don't trust a config that we cannot watch in the future. + // TODO(szym): re-start watcher if that makes sense. http://crbug.com/116139 + if (watch_succeeded) + config_reader_->WorkNow(); + else + LOG(ERROR) << "Failed to watch DNS config"; +} + +void DnsConfigServicePosix::OnHostsChanged(bool watch_succeeded) { + InvalidateHosts(); + if (watch_succeeded) + hosts_reader_->WorkNow(); + else + LOG(ERROR) << "Failed to watch DNS hosts"; } #if !defined(OS_ANDROID) @@ -146,4 +198,11 @@ bool ConvertResStateToDnsConfig(const struct __res_state& res, } #endif // !defined(OS_ANDROID) +} // namespace internal + +// static +scoped_ptr<DnsConfigService> DnsConfigService::CreateSystemService() { + return scoped_ptr<DnsConfigService>(new internal::DnsConfigServicePosix()); +} + } // namespace net diff --git a/net/dns/dns_config_service_posix.h b/net/dns/dns_config_service_posix.h index fa9599e..e266d06 100644 --- a/net/dns/dns_config_service_posix.h +++ b/net/dns/dns_config_service_posix.h @@ -17,28 +17,38 @@ namespace net { -class WatchingFileReader; +class FilePathWatcherWrapper; + +// Use DnsConfigService::CreateSystemService to use it outside of tests. +namespace internal { class NET_EXPORT_PRIVATE DnsConfigServicePosix - : NON_EXPORTED_BASE(public DnsConfigService) { + : NON_EXPORTED_BASE(public DnsConfigService) { public: DnsConfigServicePosix(); virtual ~DnsConfigServicePosix(); - virtual void Watch() OVERRIDE; + virtual void Watch(const CallbackType& callback) OVERRIDE; private: - class ConfigReader; - scoped_ptr<WatchingFileReader> config_watcher_; - scoped_ptr<WatchingFileReader> hosts_watcher_; + void OnConfigChanged(bool watch_succeeded); + void OnHostsChanged(bool watch_succeeded); + + scoped_ptr<FilePathWatcherWrapper> config_watcher_; + scoped_ptr<FilePathWatcherWrapper> hosts_watcher_; + + scoped_refptr<SerialWorker> config_reader_; + scoped_refptr<SerialWorker> hosts_reader_; DISALLOW_COPY_AND_ASSIGN(DnsConfigServicePosix); }; -// Fills in |dns_config| from |res|. Exposed for tests. +// Fills in |dns_config| from |res|. bool NET_EXPORT_PRIVATE ConvertResStateToDnsConfig( const struct __res_state& res, DnsConfig* dns_config); +} // namespace internal + } // namespace net #endif // NET_DNS_DNS_CONFIG_SERVICE_POSIX_H_ diff --git a/net/dns/dns_config_service_posix_unittest.cc b/net/dns/dns_config_service_posix_unittest.cc index f1ff015..e4ae0d8 100644 --- a/net/dns/dns_config_service_posix_unittest.cc +++ b/net/dns/dns_config_service_posix_unittest.cc @@ -10,7 +10,6 @@ #include "testing/gtest/include/gtest/gtest.h" namespace net { - namespace { void CompareConfig(const struct __res_state &res, const DnsConfig& config) { @@ -108,15 +107,13 @@ void CloseResState(res_state res) { #endif } -} // namespace - TEST(DnsConfigServicePosixTest, ConvertResStateToDnsConfig) { struct __res_state res[2]; DnsConfig config[2]; for (unsigned i = 0; i < 2; ++i) { EXPECT_FALSE(config[i].IsValid()); InitializeResState(&res[i], i); - ASSERT_TRUE(ConvertResStateToDnsConfig(res[i], &config[i])); + ASSERT_TRUE(internal::ConvertResStateToDnsConfig(res[i], &config[i])); } for (unsigned i = 0; i < 2; ++i) { EXPECT_TRUE(config[i].IsValid()); @@ -128,6 +125,7 @@ TEST(DnsConfigServicePosixTest, ConvertResStateToDnsConfig) { EXPECT_FALSE(config[1].Equals(config[0])); } +} // namespace } // namespace net diff --git a/net/dns/dns_config_service_unittest.cc b/net/dns/dns_config_service_unittest.cc index ee76a9a..9d27cb7 100644 --- a/net/dns/dns_config_service_unittest.cc +++ b/net/dns/dns_config_service_unittest.cc @@ -4,8 +4,9 @@ #include "net/dns/dns_config_service.h" +#include "base/basictypes.h" #include "base/bind.h" -#include "base/compiler_specific.h" +#include "base/cancelable_callback.h" #include "base/memory/scoped_ptr.h" #include "base/message_loop.h" #include "base/test/test_timeouts.h" @@ -15,92 +16,189 @@ namespace net { namespace { -class DnsConfigServiceTest : public testing::Test, - public DnsConfigService::Observer { +class DnsConfigServiceTest : public testing::Test { public: - void OnConfigChanged(const DnsConfig& config) OVERRIDE { + void OnConfigChanged(const DnsConfig& config) { EXPECT_FALSE(config.Equals(last_config_)) << "Config must be different from last call."; last_config_ = config; if (quit_on_config_) MessageLoop::current()->Quit(); } - void Timeout() const { - MessageLoop::current()->Quit(); - } protected: - // test::Test methods + class TestDnsConfigService : public DnsConfigService { + public: + virtual void Watch(const CallbackType& callback) OVERRIDE { + set_callback(callback); + } + + // Expose the protected methods to this test suite. + void InvalidateConfig() { + DnsConfigService::InvalidateConfig(); + } + + void InvalidateHosts() { + DnsConfigService::InvalidateHosts(); + } + + void OnConfigRead(const DnsConfig& config) { + DnsConfigService::OnConfigRead(config); + } + + void OnHostsRead(const DnsHosts& hosts) { + DnsConfigService::OnHostsRead(hosts); + } + }; + + void WaitForConfig(base::TimeDelta timeout) { + base::CancelableClosure closure(MessageLoop::QuitClosure()); + MessageLoop::current()->PostDelayedTask(FROM_HERE, + closure.callback(), + timeout); + quit_on_config_ = true; + MessageLoop::current()->Run(); + quit_on_config_ = false; + closure.Cancel(); + } + + // Generate a config using the given seed.. + DnsConfig MakeConfig(unsigned seed) { + DnsConfig config; + IPAddressNumber ip; + EXPECT_TRUE(ParseIPLiteralToNumber("1.2.3.4", &ip)); + config.nameservers.push_back(IPEndPoint(ip, seed & 0xFFFF)); + EXPECT_TRUE(config.IsValid()); + return config; + } + + // Generate hosts using the given seed. + DnsHosts MakeHosts(unsigned seed) { + DnsHosts hosts; + std::string hosts_content = "127.0.0.1 localhost"; + hosts_content.append(seed, '1'); + ParseHosts(hosts_content, &hosts); + EXPECT_FALSE(hosts.empty()); + return hosts; + } + virtual void SetUp() OVERRIDE { - service_.reset(new DnsConfigService()); quit_on_config_ = false; + + service_.reset(new TestDnsConfigService()); + service_->Watch(base::Bind(&DnsConfigServiceTest::OnConfigChanged, + base::Unretained(this))); + EXPECT_FALSE(last_config_.IsValid()); } DnsConfig last_config_; bool quit_on_config_; // Service under test. - scoped_ptr<DnsConfigService> service_; + scoped_ptr<TestDnsConfigService> service_; }; } // namespace -TEST_F(DnsConfigServiceTest, NotifyOnChange) { - service_->AddObserver(this); - service_->Watch(); +TEST_F(DnsConfigServiceTest, FirstConfig) { + DnsConfig config = MakeConfig(1); + + service_->OnConfigRead(config); + // No hosts yet, so no config. + EXPECT_FALSE(last_config_.IsValid()); + + service_->OnHostsRead(config.hosts); + // Empty hosts is acceptable. + EXPECT_TRUE(last_config_.IsValid()); + EXPECT_TRUE(last_config_.Equals(config)); +} + +TEST_F(DnsConfigServiceTest, Timeout) { + DnsConfig config = MakeConfig(1); + config.hosts = MakeHosts(1); - DnsConfig config; - IPAddressNumber ip; - ASSERT_TRUE(ParseIPLiteralToNumber("1.2.3.4", &ip)); - config.nameservers.push_back(IPEndPoint(ip, 53)); - ASSERT_TRUE(config.IsValid()); service_->OnConfigRead(config); + service_->OnHostsRead(config.hosts); + EXPECT_TRUE(last_config_.IsValid()); + EXPECT_TRUE(last_config_.Equals(config)); - EXPECT_FALSE(last_config_.Equals(config)) << - "Unexpected notification with incomplete config."; + service_->InvalidateConfig(); + WaitForConfig(TestTimeouts::action_timeout()); + EXPECT_FALSE(last_config_.IsValid()); - DnsHosts hosts; - ParseHosts("127.0.0.1 localhost", &hosts); - ASSERT_FALSE(hosts.empty()); - service_->OnHostsRead(hosts); + service_->OnConfigRead(config); + EXPECT_TRUE(last_config_.Equals(config)); + + service_->InvalidateHosts(); + WaitForConfig(TestTimeouts::action_timeout()); + EXPECT_FALSE(last_config_.IsValid()); - EXPECT_TRUE(last_config_.hosts == hosts); - EXPECT_TRUE(last_config_.EqualsIgnoreHosts(config)); + service_->OnHostsRead(config.hosts); + EXPECT_TRUE(last_config_.Equals(config)); +} - DnsConfig complete_config = config; - complete_config.hosts = hosts; - EXPECT_TRUE(last_config_.Equals(complete_config)); +TEST_F(DnsConfigServiceTest, SameConfig) { + DnsConfig config = MakeConfig(1); + config.hosts = MakeHosts(1); - // Check if it ignores no-change in config or hosts. service_->OnConfigRead(config); - service_->OnHostsRead(hosts); + service_->OnHostsRead(config.hosts); + EXPECT_TRUE(last_config_.Equals(config)); - // Check if it provides config and hosts after AddObserver; - service_->RemoveObserver(this); - last_config_ = DnsConfig(); - service_->AddObserver(this); - EXPECT_TRUE(last_config_.Equals(complete_config)); + // OnConfigChanged will catch if there was no change. + service_->OnConfigRead(config); + EXPECT_TRUE(last_config_.Equals(config)); + + service_->OnHostsRead(config.hosts); + EXPECT_TRUE(last_config_.Equals(config)); +} + +TEST_F(DnsConfigServiceTest, DifferentConfig) { + DnsConfig config1 = MakeConfig(1); + DnsConfig config2 = MakeConfig(2); + DnsConfig config3 = MakeConfig(1); + config1.hosts = MakeHosts(1); + config2.hosts = MakeHosts(1); + config3.hosts = MakeHosts(2); + ASSERT_TRUE(config1.EqualsIgnoreHosts(config3)); + ASSERT_FALSE(config1.Equals(config2)); + ASSERT_FALSE(config1.Equals(config3)); + ASSERT_FALSE(config2.Equals(config3)); + + service_->OnConfigRead(config1); + service_->OnHostsRead(config1.hosts); + EXPECT_TRUE(last_config_.Equals(config1)); + + // It doesn't matter for this tests, but increases coverage. + service_->InvalidateConfig(); + service_->InvalidateHosts(); + + service_->OnConfigRead(config2); + service_->OnHostsRead(config2.hosts); + EXPECT_TRUE(last_config_.Equals(config2)); + + service_->OnConfigRead(config3); + service_->OnHostsRead(config3.hosts); + EXPECT_TRUE(last_config_.Equals(config3)); } #if defined(OS_POSIX) || defined(OS_WIN) -// This is really an integration test. -TEST_F(DnsConfigServiceTest, GetSystemConfig) { +// TODO(szym): This is really an integration test and can time out if HOSTS is +// huge. http://crbug.com/107810 +TEST_F(DnsConfigServiceTest, FLAKY_GetSystemConfig) { + service_.reset(); scoped_ptr<DnsConfigService> service(DnsConfigService::CreateSystemService()); - // Quit the loop after timeout unless cancelled - const base::TimeDelta kTimeout = TestTimeouts::action_timeout(); - base::WeakPtrFactory<DnsConfigServiceTest> factory_(this); - MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&DnsConfigServiceTest::Timeout, factory_.GetWeakPtr()), - kTimeout); - - service->AddObserver(this); - service->Watch(); - quit_on_config_ = true; - MessageLoop::current()->Run(); + service->Watch(base::Bind(&DnsConfigServiceTest::OnConfigChanged, + base::Unretained(this))); + base::TimeDelta kTimeout = TestTimeouts::action_max_timeout(); + WaitForConfig(kTimeout); ASSERT_TRUE(last_config_.IsValid()) << "Did not receive DnsConfig in " << - kTimeout.InMilliseconds() << "ms"; + kTimeout.InSecondsF() << "s"; + + // Restart watch to confirm it's allowed. + service->Watch(base::Bind(&DnsConfigServiceTest::OnConfigChanged, + base::Unretained(this))); } #endif // OS_POSIX || OS_WIN diff --git a/net/dns/dns_config_service_win.cc b/net/dns/dns_config_service_win.cc index 14bb773..047b92d 100644 --- a/net/dns/dns_config_service_win.cc +++ b/net/dns/dns_config_service_win.cc @@ -11,9 +11,11 @@ #include "base/callback.h" #include "base/compiler_specific.h" #include "base/file_path.h" +#include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/string_split.h" #include "base/string_util.h" +#include "base/synchronization/lock.h" #include "base/threading/thread_restrictions.h" #include "base/utf_string_conversions.h" #include "base/win/object_watcher.h" @@ -22,26 +24,27 @@ #include "googleurl/src/url_canon.h" #include "net/base/net_util.h" #include "net/dns/dns_protocol.h" +#include "net/dns/file_path_watcher_wrapper.h" #include "net/dns/serial_worker.h" -#include "net/dns/watching_file_reader.h" #pragma comment(lib, "iphlpapi.lib") namespace net { +namespace internal { + namespace { -// Watches a single registry key for changes. This class is thread-safe with -// the exception of StartWatch which must be called once and only once. +// Watches a single registry key for changes. Thread-safe. class RegistryWatcher : public base::win::ObjectWatcher::Delegate { public: + typedef base::Callback<void(bool succeeded)> CallbackType; RegistryWatcher() {} - bool StartWatch(const wchar_t* key, const base::Closure& callback) { + bool Watch(const wchar_t* key, const CallbackType& callback) { + base::AutoLock lock(lock_); DCHECK(!callback.is_null()); - DCHECK(callback_.is_null()); - // TODO(szym): This will need to change to address http://crbug.com/99509 - DCHECK(!key_.Valid()); + CancelLocked(); if (key_.Open(HKEY_LOCAL_MACHINE, key, KEY_NOTIFY | KEY_QUERY_VALUE) != ERROR_SUCCESS) return false; @@ -54,22 +57,21 @@ class RegistryWatcher : public base::win::ObjectWatcher::Delegate { } void Cancel() { - callback_.Reset(); - key_.StopWatching(); - watcher_.StopWatching(); + base::AutoLock lock(lock_); + CancelLocked(); } virtual void OnObjectSignaled(HANDLE object) OVERRIDE { + base::AutoLock lock(lock_); + bool succeeded = (key_.StartWatching() == ERROR_SUCCESS) && + watcher_.StartWatching(key_.watch_event(), this); if (!callback_.is_null()) - callback_.Run(); - // TODO(szym): handle errors - bool success = key_.StartWatching() && - watcher_.StartWatching(key_.watch_event(), this); - DCHECK(success); + callback_.Run(succeeded); } bool ReadString(const wchar_t* name, DnsSystemSettings::RegString* out) const { + base::AutoLock lock(lock_); out->set = false; if (!key_.Valid()) { // Assume that if the |key_| is invalid then the key is missing. @@ -85,6 +87,7 @@ class RegistryWatcher : public base::win::ObjectWatcher::Delegate { bool ReadDword(const wchar_t* name, DnsSystemSettings::RegDword* out) const { + base::AutoLock lock(lock_); out->set = false; if (!key_.Valid()) { // Assume that if the |key_| is invalid then the key is missing. @@ -99,7 +102,17 @@ class RegistryWatcher : public base::win::ObjectWatcher::Delegate { } private: - base::Closure callback_; + void CancelLocked() { + lock_.AssertAcquired(); + callback_.Reset(); + if (key_.Valid()) { + watcher_.StopWatching(); + key_.StopWatching(); + } + } + + mutable base::Lock lock_; + CallbackType callback_; base::win::RegKey key_; base::win::ObjectWatcher watcher_; @@ -123,9 +136,8 @@ bool ParseDomainASCII(const string16& widestr, std::string* domain) { // Otherwise try to convert it from IDN to punycode. const int kInitialBufferSize = 256; url_canon::RawCanonOutputT<char16, kInitialBufferSize> punycode; - if (!url_canon::IDNToASCII(widestr.data(), widestr.length(), &punycode)) { + if (!url_canon::IDNToASCII(widestr.data(), widestr.length(), &punycode)) return false; - } // |punycode_output| should now be ASCII; convert it to a std::string. // (We could use UTF16ToASCII() instead, but that requires an extra string @@ -169,9 +181,10 @@ bool ConvertSettingsToDnsConfig(const DnsSystemSettings& settings, // Use GetAdapterAddresses to get effective DNS server order and // connection-specific DNS suffix. Ignore disconnected and loopback adapters. - // TODO(szym): Also ignore VM adapters. http://crbug.com/112856 + // The order of adapters is the network binding order, so stick to the + // first good adapter. for (const IP_ADAPTER_ADDRESSES* adapter = settings.addresses.get(); - adapter != NULL; + adapter != NULL && config->nameservers.empty(); adapter = adapter->Next) { if (adapter->OperStatus != IfOperStatusUp) continue; @@ -200,9 +213,8 @@ bool ConvertSettingsToDnsConfig(const DnsSystemSettings& settings, // obtained via DHCP (regkey: Tcpip\Parameters\Interfaces\{XXX}\DhcpDomain) // or specified by the user (regkey: Tcpip\Parameters\Domain). std::string dns_suffix; - if (ParseDomainASCII(adapter->DnsSuffix, &dns_suffix)) { + if (ParseDomainASCII(adapter->DnsSuffix, &dns_suffix)) config->search.push_back(dns_suffix); - } } if (config->nameservers.empty()) @@ -304,31 +316,36 @@ class DnsConfigServiceWin::ConfigReader : public SerialWorker { : service_(service), success_(false) {} - bool StartWatch() { + bool Watch() { DCHECK(loop()->BelongsToCurrentThread()); - base::Closure callback = base::Bind(&SerialWorker::WorkNow, - base::Unretained(this)); + RegistryWatcher::CallbackType callback = + base::Bind(&ConfigReader::OnChange, base::Unretained(this)); + + // The Tcpip key must be present. + if (!tcpip_watcher_.Watch( + L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters", + callback)) + return false; + + // Watch for IPv6 nameservers. + tcpip6_watcher_.Watch( + L"SYSTEM\\CurrentControlSet\\Services\\Tcpip6\\Parameters", + callback); // DNS suffix search list and devolution can be configured via group // policy which sets this registry key. If the key is missing, the policy // does not apply, and the DNS client uses Tcpip and Dnscache settings. // If a policy is installed, DnsConfigService will need to be restarted. // BUG=99509 - policy_watcher_.StartWatch( - L"SOFTWARE\\Policies\\Microsoft\\Windows NT\\DNSClient", - callback); - // The Dnscache key might also be missing. - dnscache_watcher_.StartWatch( + dnscache_watcher_.Watch( L"SYSTEM\\CurrentControlSet\\Services\\Dnscache\\Parameters", callback); - // The Tcpip key must be present. - if (!tcpip_watcher_.StartWatch( - L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters", - callback)) - return false; + policy_watcher_.Watch( + L"SOFTWARE\\Policies\\Microsoft\\Windows NT\\DNSClient", + callback); WorkNow(); return true; @@ -339,18 +356,27 @@ class DnsConfigServiceWin::ConfigReader : public SerialWorker { SerialWorker::Cancel(); policy_watcher_.Cancel(); dnscache_watcher_.Cancel(); + tcpip6_watcher_.Cancel(); tcpip_watcher_.Cancel(); } - // Delay between calls to WorkerPool::PostTask - static const int kWorkerPoolRetryDelayMs = 100; - private: - friend class RegistryWatcher; virtual ~ConfigReader() { DCHECK(IsCancelled()); } + void OnChange(bool succeeded) { + DCHECK(loop()->BelongsToCurrentThread()); + if (!IsCancelled()) + service_->InvalidateConfig(); + // We don't trust a config that we cannot watch in the future. + // TODO(szym): re-start watcher if that makes sense. http://crbug.com/116139 + if (succeeded) + WorkNow(); + else + LOG(ERROR) << "Failed to watch DNS config"; + } + bool ReadIpHelper(DnsSystemSettings* settings) { base::ThreadRestrictions::AssertIOAllowed(); @@ -429,43 +455,68 @@ class DnsConfigServiceWin::ConfigReader : public SerialWorker { DnsConfig dns_config_; bool success_; - RegistryWatcher policy_watcher_; - RegistryWatcher dnscache_watcher_; RegistryWatcher tcpip_watcher_; + RegistryWatcher tcpip6_watcher_; + RegistryWatcher dnscache_watcher_; + RegistryWatcher policy_watcher_; }; DnsConfigServiceWin::DnsConfigServiceWin() : config_reader_(new ConfigReader(this)), - hosts_watcher_(new WatchingFileReader()) {} + hosts_watcher_(new FilePathWatcherWrapper()) {} DnsConfigServiceWin::~DnsConfigServiceWin() { DCHECK(CalledOnValidThread()); config_reader_->Cancel(); + if (hosts_reader_) + hosts_reader_->Cancel(); } -void DnsConfigServiceWin::Watch() { +void DnsConfigServiceWin::Watch(const CallbackType& callback) { DCHECK(CalledOnValidThread()); + DCHECK(!callback.is_null()); + set_callback(callback); // This is done only once per lifetime so open the keys and file watcher // handles on this thread. // TODO(szym): Should/can this be avoided? http://crbug.com/114223 base::ThreadRestrictions::ScopedAllowIO allow_io; - bool started = config_reader_->StartWatch(); - // TODO(szym): handle possible failure - DCHECK(started); + if (!config_reader_->Watch()) { + LOG(ERROR) << "Failed to start watching DNS config"; + InvalidateConfig(); + } TCHAR buffer[MAX_PATH]; UINT rc = GetSystemDirectory(buffer, MAX_PATH); DCHECK(0 < rc && rc < MAX_PATH); FilePath hosts_path = FilePath(buffer). Append(FILE_PATH_LITERAL("drivers\\etc\\hosts")); - hosts_watcher_->StartWatch(hosts_path, new DnsHostsReader(hosts_path, this)); + hosts_reader_ = new DnsHostsReader( + hosts_path, + base::Bind(&DnsConfigServiceWin::OnHostsRead, base::Unretained(this))); + if (hosts_watcher_->Watch(hosts_path, + base::Bind(&DnsConfigServiceWin::OnHostsChanged, + base::Unretained(this)))) { + OnHostsChanged(true); + } else { + OnHostsChanged(false); + } +} + +void DnsConfigServiceWin::OnHostsChanged(bool succeeded) { + InvalidateHosts(); + if (succeeded) + hosts_reader_->WorkNow(); + else + LOG(ERROR) << "Failed to watch DNS hosts"; } +} // namespace internal + // static scoped_ptr<DnsConfigService> DnsConfigService::CreateSystemService() { - return scoped_ptr<DnsConfigService>(new DnsConfigServiceWin()); + return scoped_ptr<DnsConfigService>(new internal::DnsConfigServiceWin()); } } // namespace net diff --git a/net/dns/dns_config_service_win.h b/net/dns/dns_config_service_win.h index d60ae04..3159f77 100644 --- a/net/dns/dns_config_service_win.h +++ b/net/dns/dns_config_service_win.h @@ -33,20 +33,27 @@ namespace net { -class WatchingFileReader; +class FilePathWatcherWrapper; + +// Use DnsConfigService::CreateSystemService to use it outside of tests. +namespace internal { class NET_EXPORT_PRIVATE DnsConfigServiceWin - : NON_EXPORTED_BASE(public DnsConfigService) { + : NON_EXPORTED_BASE(public DnsConfigService) { public: DnsConfigServiceWin(); virtual ~DnsConfigServiceWin(); - virtual void Watch() OVERRIDE; + virtual void Watch(const CallbackType& callback) OVERRIDE; private: class ConfigReader; + + void OnHostsChanged(bool succeeded); + scoped_refptr<ConfigReader> config_reader_; - scoped_ptr<WatchingFileReader> hosts_watcher_; + scoped_ptr<FilePathWatcherWrapper> hosts_watcher_; + scoped_refptr<SerialWorker> hosts_reader_; DISALLOW_COPY_AND_ASSIGN(DnsConfigServiceWin); }; @@ -73,7 +80,8 @@ struct NET_EXPORT_PRIVATE DnsSystemSettings { RegDword level; }; - // Filled in by GetAdapterAddresses. + // Filled in by GetAdapterAddresses. Note that the alternative + // GetNetworkParams does not include IPv6 addresses. scoped_ptr_malloc<IP_ADAPTER_ADDRESSES> addresses; // SOFTWARE\Policies\Microsoft\Windows NT\DNSClient\SearchList @@ -105,6 +113,8 @@ bool NET_EXPORT_PRIVATE ParseSearchList(const string16& value, bool NET_EXPORT_PRIVATE ConvertSettingsToDnsConfig( const DnsSystemSettings& settings, DnsConfig* dns_config); +} // namespace internal + } // namespace net #endif // NET_DNS_DNS_CONFIG_SERVICE_WIN_H_ diff --git a/net/dns/dns_config_service_win_unittest.cc b/net/dns/dns_config_service_win_unittest.cc index 269bb55..fa5da54 100644 --- a/net/dns/dns_config_service_win_unittest.cc +++ b/net/dns/dns_config_service_win_unittest.cc @@ -4,6 +4,7 @@ #include "net/dns/dns_config_service_win.h" +#include "base/logging.h" #include "base/win/windows_version.h" #include "net/dns/dns_protocol.h" #include "testing/gtest/include/gtest/gtest.h" @@ -38,7 +39,7 @@ TEST(DnsConfigServiceWinTest, ParseSearchList) { for (const char* const* output = t.output; *output; ++output) { expected_output.push_back(*output); } - bool result = ParseSearchList(t.input, &actual_output); + bool result = internal::ParseSearchList(t.input, &actual_output); if (!expected_output.empty()) { EXPECT_TRUE(result); EXPECT_EQ(expected_output, actual_output); @@ -143,6 +144,19 @@ TEST(DnsConfigServiceWinTest, ConvertAdapterAddresses) { "chromium.org", { 1024, 24 }, }, + { // Use the preferred adapter (first in binding order). + { + { IF_TYPE_SOFTWARE_LOOPBACK, IfOperStatusUp, L"funnyloop", + { "2.0.0.2" } }, + { IF_TYPE_FASTETHER, IfOperStatusUp, L"example.com", + { "1.0.0.1" } }, + { IF_TYPE_USB, IfOperStatusUp, L"chromium.org", + { "10.0.0.10", "2001:FFFF::1111" } }, + { 0 }, + }, + { "1.0.0.1" }, + "example.com", + }, { // No usable nameservers. { { IF_TYPE_SOFTWARE_LOOPBACK, IfOperStatusUp, L"localhost", @@ -157,7 +171,7 @@ TEST(DnsConfigServiceWinTest, ConvertAdapterAddresses) { for (size_t i = 0; i < arraysize(cases); ++i) { const TestCase& t = cases[i]; - DnsSystemSettings settings = { + internal::DnsSystemSettings settings = { CreateAdapterAddresses(t.input_adapters), // Default settings for the rest. }; @@ -172,7 +186,7 @@ TEST(DnsConfigServiceWinTest, ConvertAdapterAddresses) { } DnsConfig config; - bool result = ConvertSettingsToDnsConfig(settings, &config); + bool result = internal::ConvertSettingsToDnsConfig(settings, &config); bool expected_result = !expected_nameservers.empty(); ASSERT_EQ(expected_result, result); EXPECT_EQ(expected_nameservers, config.nameservers); @@ -190,7 +204,7 @@ TEST(DnsConfigServiceWinTest, ConvertSuffixSearch) { }; const struct TestCase { - DnsSystemSettings input_settings; + internal::DnsSystemSettings input_settings; std::string expected_search[5]; } cases[] = { { // Policy SearchList override. @@ -320,7 +334,8 @@ TEST(DnsConfigServiceWinTest, ConvertSuffixSearch) { for (size_t i = 0; i < arraysize(cases); ++i) { const TestCase& t = cases[i]; DnsConfig config; - ASSERT_TRUE(ConvertSettingsToDnsConfig(t.input_settings, &config)); + ASSERT_TRUE(internal::ConvertSettingsToDnsConfig(t.input_settings, + &config)); std::vector<std::string> expected_search; for (size_t j = 0; !t.expected_search[j].empty(); ++j) { expected_search.push_back(t.expected_search[j]); @@ -339,7 +354,7 @@ TEST(DnsConfigServiceWinTest, AppendToMultiLabelName) { bool default_value = (base::win::GetVersion() < base::win::VERSION_VISTA); const struct TestCase { - DnsSystemSettings::RegDword input; + internal::DnsSystemSettings::RegDword input; bool expected_output; } cases[] = { { { true, 0 }, false }, @@ -349,7 +364,7 @@ TEST(DnsConfigServiceWinTest, AppendToMultiLabelName) { for (size_t i = 0; i < arraysize(cases); ++i) { const TestCase& t = cases[i]; - DnsSystemSettings settings = { + internal::DnsSystemSettings settings = { CreateAdapterAddresses(infos), { false }, { false }, { false }, { { false }, { false } }, @@ -358,7 +373,7 @@ TEST(DnsConfigServiceWinTest, AppendToMultiLabelName) { t.input, }; DnsConfig config; - ASSERT_TRUE(ConvertSettingsToDnsConfig(settings, &config)); + ASSERT_TRUE(internal::ConvertSettingsToDnsConfig(settings, &config)); EXPECT_EQ(config.append_to_multi_label_name, t.expected_output); } } diff --git a/net/dns/dns_hosts.cc b/net/dns/dns_hosts.cc index 1342808..bd46912 100644 --- a/net/dns/dns_hosts.cc +++ b/net/dns/dns_hosts.cc @@ -1,9 +1,10 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. #include "net/dns/dns_hosts.h" +#include "base/file_util.h" #include "base/logging.h" #include "base/string_tokenizer.h" @@ -43,5 +44,48 @@ void ParseHosts(const std::string& contents, DnsHosts* dns_hosts) { } } +DnsHostsReader::DnsHostsReader(const FilePath& path, + const CallbackType& callback) + : path_(path), + callback_(callback), + success_(false) { + DCHECK(!callback.is_null()); +} + +// Reads the contents of the file at |path| into |str| if the total length is +// less than |max_size|. +static bool ReadFile(const FilePath& path, int64 max_size, std::string* str) { + int64 size; + if (!file_util::GetFileSize(path, &size) || size > max_size) + return false; + return file_util::ReadFileToString(path, str); +} + +void DnsHostsReader::DoWork() { + success_ = false; + dns_hosts_.clear(); + + // Missing file indicates empty HOSTS. + if (!file_util::PathExists(path_)) { + success_ = true; + return; + } + + std::string contents; + const int64 kMaxHostsSize = 1 << 16; + if (ReadFile(path_, kMaxHostsSize, &contents)) { + success_ = true; + ParseHosts(contents, &dns_hosts_); + } +} + +void DnsHostsReader::OnWorkFinished() { + DCHECK(!IsCancelled()); + if (success_) + callback_.Run(dns_hosts_); +} + +DnsHostsReader::~DnsHostsReader() {} + } // namespace net diff --git a/net/dns/dns_hosts.h b/net/dns/dns_hosts.h index fda7733..91c35ba 100644 --- a/net/dns/dns_hosts.h +++ b/net/dns/dns_hosts.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -11,9 +11,12 @@ #include <utility> #include <vector> +#include "base/callback.h" +#include "base/file_path.h" #include "net/base/address_family.h" #include "net/base/net_export.h" #include "net/base/net_util.h" // can't forward-declare IPAddressNumber +#include "net/dns/serial_worker.h" namespace net { @@ -35,6 +38,32 @@ typedef std::map<DnsHostsKey, IPAddressNumber> DnsHosts; void NET_EXPORT_PRIVATE ParseHosts(const std::string& contents, DnsHosts* dns_hosts); +// A SerialWorker that reads a HOSTS file and runs Callback. +// Call WorkNow() to indicate file needs to be re-read. +// Call Cancel() to disable the callback. +class NET_EXPORT_PRIVATE DnsHostsReader + : NON_EXPORTED_BASE(public SerialWorker) { + public: + typedef base::Callback<void(const DnsHosts& hosts)> CallbackType; + + DnsHostsReader(const FilePath& path, const CallbackType& callback); + + private: + virtual ~DnsHostsReader(); + + virtual void DoWork() OVERRIDE; + virtual void OnWorkFinished() OVERRIDE; + + FilePath path_; + CallbackType callback_; + // Written in DoWork, read in OnWorkFinished, no locking necessary. + DnsHosts dns_hosts_; + bool success_; + + DISALLOW_COPY_AND_ASSIGN(DnsHostsReader); +}; + + } // namespace net #endif // NET_DNS_DNS_HOSTS_H_ diff --git a/net/dns/dns_test_util.cc b/net/dns/dns_test_util.cc index fa1bed1..a3c26cf 100644 --- a/net/dns/dns_test_util.cc +++ b/net/dns/dns_test_util.cc @@ -162,5 +162,9 @@ scoped_ptr<DnsClient> CreateMockDnsClient(const DnsConfig& config) { return scoped_ptr<DnsClient>(new MockDnsClient(config)); } +void MockDnsConfigService::Watch(const CallbackType& callback) { + set_callback(callback); +} + } // namespace net diff --git a/net/dns/dns_test_util.h b/net/dns/dns_test_util.h index e247be0..019dba8 100644 --- a/net/dns/dns_test_util.h +++ b/net/dns/dns_test_util.h @@ -166,6 +166,8 @@ class MockDnsConfigService : public DnsConfigService { public: virtual ~MockDnsConfigService() {} + virtual void Watch(const CallbackType& callback) OVERRIDE; + // Expose the protected methods for tests. void ChangeConfig(const DnsConfig& config) { DnsConfigService::OnConfigRead(config); diff --git a/net/dns/file_path_watcher_wrapper.cc b/net/dns/file_path_watcher_wrapper.cc new file mode 100644 index 0000000..909c03c --- /dev/null +++ b/net/dns/file_path_watcher_wrapper.cc @@ -0,0 +1,99 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/dns/file_path_watcher_wrapper.h" + +#include "base/callback.h" +#include "base/files/file_path_watcher.h" +#include "base/logging.h" + +namespace net { + +// A FilePathWatcher::Delegate that forwards calls to FilePathWatcherCallback. +class FilePathWatcherWrapper::WatcherDelegate + : public base::files::FilePathWatcher::Delegate { + public: + explicit WatcherDelegate(FilePathWatcherWrapper* watcher) + : watcher_(watcher) {} + + void Cancel() { + watcher_ = NULL; + } + + void OnFilePathChanged(const FilePath& path) OVERRIDE { + if (watcher_) + watcher_->OnFilePathChanged(); + } + + void OnFilePathError(const FilePath& path) OVERRIDE { + if (watcher_) + watcher_->OnFilePathError(); + } + + private: + virtual ~WatcherDelegate() {} + FilePathWatcherWrapper* watcher_; +}; + +FilePathWatcherWrapper::FilePathWatcherWrapper() { + // No need to restrict the thread until the state is created in Watch. + DetachFromThread(); +} + +FilePathWatcherWrapper::~FilePathWatcherWrapper() { + Cancel(); +} + +void FilePathWatcherWrapper::Cancel() { + DCHECK(CalledOnValidThread()); + if (delegate_) { + delegate_->Cancel(); + delegate_.release(); + } + watcher_.reset(NULL); + // All state is gone so it's okay to detach from thread. + DetachFromThread(); +} + +bool FilePathWatcherWrapper::Watch( + const FilePath& path, + const base::Callback<void(bool succeeded)>& callback) { + DCHECK(CalledOnValidThread()); + if (delegate_) + delegate_->Cancel(); + callback_ = callback; + watcher_ = this->CreateFilePathWatcher(); + delegate_ = new WatcherDelegate(this); + if (watcher_->Watch(path, delegate_.get())) + return true; + + Cancel(); + return false; +} + +bool FilePathWatcherWrapper::IsWatching() const { + DCHECK(CalledOnValidThread()); + return watcher_.get() != NULL; +} + +scoped_ptr<base::files::FilePathWatcher> + FilePathWatcherWrapper::CreateFilePathWatcher() { + return scoped_ptr<base::files::FilePathWatcher>( + new base::files::FilePathWatcher()); +} + +void FilePathWatcherWrapper::OnFilePathChanged() { + DCHECK(CalledOnValidThread()); + callback_.Run(true); +} + +void FilePathWatcherWrapper::OnFilePathError() { + DCHECK(CalledOnValidThread()); + base::Callback<void(bool succeeded)> callback = callback_; + Cancel(); + callback.Run(false); +} + +} // namespace net + diff --git a/net/dns/file_path_watcher_wrapper.h b/net/dns/file_path_watcher_wrapper.h new file mode 100644 index 0000000..172bbaf65 --- /dev/null +++ b/net/dns/file_path_watcher_wrapper.h @@ -0,0 +1,68 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef NET_FILE_PATH_WATCHER_CALLBACK_H_ +#define NET_FILE_PATH_WATCHER_WRAPPER_H_ +#pragma once + +#include <string> + +#include "base/basictypes.h" +#include "base/callback.h" +#include "base/file_path.h" +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/threading/non_thread_safe.h" +#include "net/base/net_export.h" + +namespace base { +namespace files { +class FilePathWatcher; +} +} + +namespace net { + +// Convenience wrapper which holds a FilePathWatcher and a +// FilePathWatcher::Delegate which forwards its calls to a Callback. +class NET_EXPORT FilePathWatcherWrapper + : NON_EXPORTED_BASE(public base::NonThreadSafe) { + public: + FilePathWatcherWrapper(); + // When deleted, automatically cancels the FilePathWatcher. + virtual ~FilePathWatcherWrapper(); + + // Starts watching the file at |path|. Returns true if Watch succeeds. + // If so, the delegate will call callback.Run(true) from OnFilePathChanged and + // callback.Run(false) from OnFilePathError. After failure the watch is + // cancelled and will have to be restarted. If called again, the previous + // watch is cancelled. + bool Watch(const FilePath& path, + const base::Callback<void(bool succeeded)>& callback); + + bool IsWatching() const; + + // Cancels the current watch. + void Cancel(); + + protected: + // Creates a FilePathWatcher. Override to provide a different implementation. + virtual scoped_ptr<base::files::FilePathWatcher> CreateFilePathWatcher(); + + private: + class WatcherDelegate; + + void OnFilePathChanged(); + void OnFilePathError(); + + base::Callback<void(bool succeeded)> callback_; + scoped_ptr<base::files::FilePathWatcher> watcher_; + scoped_refptr<WatcherDelegate> delegate_; + DISALLOW_COPY_AND_ASSIGN(FilePathWatcherWrapper); +}; + +} // namespace net + +#endif // NET_FILE_PATH_WATCHER_CALLBACK_H_ + diff --git a/net/dns/file_path_watcher_wrapper_unittest.cc b/net/dns/file_path_watcher_wrapper_unittest.cc new file mode 100644 index 0000000..e925e27 --- /dev/null +++ b/net/dns/file_path_watcher_wrapper_unittest.cc @@ -0,0 +1,142 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/dns/file_path_watcher_wrapper.h" + +#include "base/bind.h" +#include "base/files/file_path_watcher.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace net { + +namespace { + +class FilePathWatcherWrapperTest : public testing::Test { + protected: + class MockFilePathWatcher : public base::files::FilePathWatcher { + public: + virtual bool Watch(const FilePath& path, Delegate* delegate) OVERRIDE { + DCHECK(!delegate_); + DCHECK(delegate); + path_ = path; + delegate_ = delegate; + return true; + } + + void OnFilePathChanged() { + delegate_->OnFilePathChanged(path_); + } + + void OnFilePathError() { + delegate_->OnFilePathError(path_); + } + + private: + FilePath path_; + scoped_refptr<Delegate> delegate_; + }; + + class FailingFilePathWatcher : public MockFilePathWatcher { + public: + virtual bool Watch(const FilePath& path, Delegate* delegate) OVERRIDE { + MockFilePathWatcher::Watch(path, delegate); + return false; + } + }; + + // Class under test. + class TestFilePathWatcherWrapper : public FilePathWatcherWrapper { + public: + TestFilePathWatcherWrapper() : fail_on_watch_(false) {} + + MockFilePathWatcher* file_path_watcher() { + return test_watcher_; + } + + void set_fail_on_watch(bool value) { + fail_on_watch_ = value; + } + + private: + virtual scoped_ptr<base::files::FilePathWatcher> + CreateFilePathWatcher() OVERRIDE { + test_watcher_ = fail_on_watch_ ? new FailingFilePathWatcher() + : new MockFilePathWatcher(); + return scoped_ptr<base::files::FilePathWatcher>(test_watcher_); + } + + MockFilePathWatcher* test_watcher_; + bool fail_on_watch_; + }; + + virtual void SetUp() OVERRIDE { + watcher_wrapper_.reset(new TestFilePathWatcherWrapper()); + got_change_ = false; + } + + bool Watch() { + return watcher_wrapper_->Watch( + FilePath(FILE_PATH_LITERAL("some_file_name")), + base::Bind(&FilePathWatcherWrapperTest::OnChange, + base::Unretained(this))); + } + + void OnChange(bool succeeded) { + got_change_ = true; + last_result_ = succeeded; + } + + scoped_ptr<TestFilePathWatcherWrapper> watcher_wrapper_; + bool got_change_; + bool last_result_; +}; + +TEST_F(FilePathWatcherWrapperTest, Basic) { + watcher_wrapper_->set_fail_on_watch(true); + EXPECT_FALSE(Watch()); + EXPECT_FALSE(watcher_wrapper_->IsWatching()); + EXPECT_FALSE(got_change_); + + watcher_wrapper_->set_fail_on_watch(false); + EXPECT_TRUE(Watch()); + EXPECT_TRUE(watcher_wrapper_->IsWatching()); + EXPECT_FALSE(got_change_); + + EXPECT_TRUE(Watch()); + EXPECT_TRUE(watcher_wrapper_->IsWatching()); + EXPECT_FALSE(got_change_); + + watcher_wrapper_->file_path_watcher()->OnFilePathChanged(); + EXPECT_TRUE(watcher_wrapper_->IsWatching()); + EXPECT_TRUE(got_change_); + EXPECT_TRUE(last_result_); + + got_change_ = false; + watcher_wrapper_->file_path_watcher()->OnFilePathError(); + EXPECT_FALSE(watcher_wrapper_->IsWatching()); + EXPECT_TRUE(got_change_); + EXPECT_FALSE(last_result_); + + got_change_ = false; + EXPECT_TRUE(Watch()); + EXPECT_TRUE(watcher_wrapper_->IsWatching()); + EXPECT_FALSE(got_change_); +} + +TEST_F(FilePathWatcherWrapperTest, Cancel) { + EXPECT_TRUE(Watch()); + EXPECT_TRUE(watcher_wrapper_->IsWatching()); + + watcher_wrapper_->Cancel(); + EXPECT_FALSE(watcher_wrapper_->IsWatching()); + EXPECT_FALSE(got_change_); + + EXPECT_TRUE(Watch()); + EXPECT_TRUE(watcher_wrapper_->IsWatching()); +} + +} // namespace + +} // namespace net + diff --git a/net/dns/serial_worker.h b/net/dns/serial_worker.h index 4d6f8fc..41ca768 100644 --- a/net/dns/serial_worker.h +++ b/net/dns/serial_worker.h @@ -1,4 +1,4 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Copyright (c) 2012 The Chromium Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. @@ -46,7 +46,7 @@ class NET_EXPORT_PRIVATE SerialWorker // Made virtual to allow mocking. virtual void WorkNow(); - // Stop scheduling read jobs. + // Stop scheduling jobs. void Cancel(); bool IsCancelled() const { return state_ == CANCELLED; } diff --git a/net/dns/watching_file_reader.cc b/net/dns/watching_file_reader.cc deleted file mode 100644 index cf4f2e9..0000000 --- a/net/dns/watching_file_reader.cc +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright (c) 2012 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/dns/watching_file_reader.h" - -#include "base/bind.h" -#include "base/location.h" -#include "base/message_loop.h" -#include "base/threading/worker_pool.h" - -namespace net { - -FilePathWatcherShim::FilePathWatcherShim() - : watcher_(new base::files::FilePathWatcher()) {} -FilePathWatcherShim::~FilePathWatcherShim() {} - -bool FilePathWatcherShim::Watch( - const FilePath& path, - base::files::FilePathWatcher::Delegate* delegate) { - DCHECK(CalledOnValidThread()); - return watcher_->Watch(path, delegate); -} - -FilePathWatcherShim* -FilePathWatcherFactory::CreateFilePathWatcher() { - DCHECK(CalledOnValidThread()); - return new FilePathWatcherShim(); -} - -// A FilePathWatcher::Delegate that forwards calls to the WatchingFileReader. -// This is separated out to keep WatchingFileReader strictly single-threaded. -class WatchingFileReader::WatcherDelegate - : public base::files::FilePathWatcher::Delegate { - public: - explicit WatcherDelegate(base::WeakPtr<WatchingFileReader> reader) - : reader_(reader) {} - void OnFilePathChanged(const FilePath& path) OVERRIDE { - if (reader_) reader_->OnFilePathChanged(path); - } - void OnFilePathError(const FilePath& path) OVERRIDE { - if (reader_) reader_->OnFilePathError(path); - } - private: - virtual ~WatcherDelegate() {} - base::WeakPtr<WatchingFileReader> reader_; -}; - -WatchingFileReader::WatchingFileReader() - : watcher_factory_(new FilePathWatcherFactory()) {} - -WatchingFileReader::~WatchingFileReader() { - if (reader_) - reader_->Cancel(); -} - -void WatchingFileReader::StartWatch(const FilePath& path, - SerialWorker* reader) { - DCHECK(CalledOnValidThread()); - DCHECK(!watcher_.get()); - DCHECK(!watcher_delegate_.get()); - DCHECK(path_.empty()); - path_ = path; - watcher_delegate_ = new WatcherDelegate(AsWeakPtr()); - reader_ = reader; - RestartWatch(); -} - -void WatchingFileReader::OnFilePathChanged(const FilePath& path) { - DCHECK(CalledOnValidThread()); - reader_->WorkNow(); -} - -void WatchingFileReader::OnFilePathError(const FilePath& path) { - DCHECK(CalledOnValidThread()); - RestartWatch(); -} - -void WatchingFileReader::RescheduleWatch() { - DCHECK(CalledOnValidThread()); - MessageLoop::current()->PostDelayedTask( - FROM_HERE, - base::Bind(&WatchingFileReader::RestartWatch, AsWeakPtr()), - base::TimeDelta::FromMilliseconds(kWatchRetryDelayMs)); -} - -void WatchingFileReader::RestartWatch() { - DCHECK(CalledOnValidThread()); - if (reader_->IsCancelled()) - return; - watcher_.reset(watcher_factory_->CreateFilePathWatcher()); - if (watcher_->Watch(path_, watcher_delegate_)) { - reader_->WorkNow(); - } else { - LOG(WARNING) << "Watch on " << - path_.LossyDisplayName() << - " failed, scheduling restart"; - RescheduleWatch(); - } -} - -} // namespace net diff --git a/net/dns/watching_file_reader.h b/net/dns/watching_file_reader.h deleted file mode 100644 index 6290dd3..0000000 --- a/net/dns/watching_file_reader.h +++ /dev/null @@ -1,90 +0,0 @@ -// 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. - -#ifndef NET_DNS_WATCHING_FILE_READER_H_ -#define NET_DNS_WATCHING_FILE_READER_H_ -#pragma once - -#include <string> - -#include "base/compiler_specific.h" -#include "base/files/file_path_watcher.h" -#include "base/memory/scoped_ptr.h" -#include "base/memory/weak_ptr.h" -#include "base/threading/non_thread_safe.h" -#include "net/base/net_export.h" -#include "net/dns/serial_worker.h" - -namespace net { - -// Allows mocking FilePathWatcher -class NET_EXPORT_PRIVATE FilePathWatcherShim - : NON_EXPORTED_BASE(public base::NonThreadSafe) { - public: - FilePathWatcherShim(); - virtual ~FilePathWatcherShim(); - virtual bool Watch( - const FilePath& path, - base::files::FilePathWatcher::Delegate* delegate) WARN_UNUSED_RESULT; - private: - scoped_ptr<base::files::FilePathWatcher> watcher_; - DISALLOW_COPY_AND_ASSIGN(FilePathWatcherShim); -}; - -class NET_EXPORT_PRIVATE FilePathWatcherFactory - : NON_EXPORTED_BASE(public base::NonThreadSafe) { - public: - FilePathWatcherFactory() {} - virtual ~FilePathWatcherFactory() {} - virtual FilePathWatcherShim* CreateFilePathWatcher(); - private: - DISALLOW_COPY_AND_ASSIGN(FilePathWatcherFactory); -}; - -// Convenience wrapper which maintains a FilePathWatcher::Delegate, restarts -// Watch on failures and calls PooledReader::ReadNow when OnFilePathChanged. -// Cancels the PooledReader upon destruction. -class NET_EXPORT_PRIVATE WatchingFileReader - : NON_EXPORTED_BASE(public base::NonThreadSafe), - NON_EXPORTED_BASE(public base::SupportsWeakPtr<WatchingFileReader>) { - public: - WatchingFileReader(); - virtual ~WatchingFileReader(); - - // Must be called at most once. Made virtual to allow mocking. - virtual void StartWatch(const FilePath& path, SerialWorker* reader); - - void set_watcher_factory(FilePathWatcherFactory* factory) { - DCHECK(!watcher_.get()); - watcher_factory_.reset(factory); - } - - FilePath get_path() const { - return path_; - } - - // Delay between calls to FilePathWatcher::Watch. - static const int kWatchRetryDelayMs = 100; - - private: - class WatcherDelegate; - // FilePathWatcher::Delegate interface exported via WatcherDelegate - virtual void OnFilePathChanged(const FilePath& path); - virtual void OnFilePathError(const FilePath& path); - - void RescheduleWatch(); - void RestartWatch(); - - FilePath path_; - scoped_ptr<FilePathWatcherFactory> watcher_factory_; - scoped_ptr<FilePathWatcherShim> watcher_; - scoped_refptr<WatcherDelegate> watcher_delegate_; - scoped_refptr<SerialWorker> reader_; - - DISALLOW_COPY_AND_ASSIGN(WatchingFileReader); -}; - -} // namespace net - -#endif // NET_DNS_WATCHING_FILE_READER_H_ diff --git a/net/dns/watching_file_reader_unittest.cc b/net/dns/watching_file_reader_unittest.cc deleted file mode 100644 index 7ff5534..0000000 --- a/net/dns/watching_file_reader_unittest.cc +++ /dev/null @@ -1,168 +0,0 @@ -// 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. - -#include "net/dns/watching_file_reader.h" - -#include "base/bind.h" -#include "base/message_loop.h" -#include "base/synchronization/lock.h" -#include "base/synchronization/waitable_event.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace net { - -namespace { - -class WatchingFileReaderTest : public testing::Test { - public: - - // Mocks - - class MockWorker : public SerialWorker { - public: - explicit MockWorker(WatchingFileReaderTest* t) : test_(t) {} - virtual void WorkNow() OVERRIDE { - test_->OnWork(); - } - virtual void DoWork() OVERRIDE {} - virtual void OnWorkFinished() OVERRIDE {} - private: - virtual ~MockWorker() {} - WatchingFileReaderTest* test_; - }; - - class MockFilePathWatcherShim - : public FilePathWatcherShim { - public: - typedef base::files::FilePathWatcher::Delegate Delegate; - - explicit MockFilePathWatcherShim(WatchingFileReaderTest* t) : test_(t) {} - virtual ~MockFilePathWatcherShim() { - test_->OnShimDestroyed(this); - } - - // Enforce one-Watch-per-lifetime as the original FilePathWatcher - virtual bool Watch(const FilePath& path, - Delegate* delegate) OVERRIDE { - EXPECT_TRUE(path_.empty()) << "Only one-Watch-per-lifetime allowed."; - EXPECT_TRUE(!delegate_.get()); - path_ = path; - delegate_ = delegate; - return test_->OnWatch(); - } - - void PathChanged() { - delegate_->OnFilePathChanged(path_); - } - - void PathError() { - delegate_->OnFilePathError(path_); - } - - private: - FilePath path_; - scoped_refptr<Delegate> delegate_; - WatchingFileReaderTest* test_; - }; - - class MockFilePathWatcherFactory - : public FilePathWatcherFactory { - public: - explicit MockFilePathWatcherFactory(WatchingFileReaderTest* t) - : test(t) {} - virtual FilePathWatcherShim* - CreateFilePathWatcher() OVERRIDE { - return test->CreateFilePathWatcher(); - } - WatchingFileReaderTest* test; - }; - - // Helpers for mocks. - - FilePathWatcherShim* CreateFilePathWatcher() { - watcher_shim_ = new MockFilePathWatcherShim(this); - return watcher_shim_; - } - - void OnShimDestroyed(MockFilePathWatcherShim* destroyed_shim) { - // Precaution to avoid segfault. - if (watcher_shim_ == destroyed_shim) - watcher_shim_ = NULL; - } - - // On each event, post QuitTask to allow use of MessageLoop::Run() to - // synchronize the threads. - - bool OnWatch() { - EXPECT_TRUE(message_loop_ == MessageLoop::current()); - BreakNow("OnWatch"); - return !fail_on_watch_; - } - - void OnWork() { - EXPECT_TRUE(message_loop_ == MessageLoop::current()); - BreakNow("OnWork"); - } - - protected: - void BreakCallback(std::string breakpoint) { - breakpoint_ = breakpoint; - MessageLoop::current()->QuitNow(); - } - - void BreakNow(std::string b) { - message_loop_->PostTask(FROM_HERE, - base::Bind(&WatchingFileReaderTest::BreakCallback, - base::Unretained(this), b)); - } - - void RunUntilBreak(std::string b) { - message_loop_->Run(); - ASSERT_EQ(breakpoint_, b); - } - - WatchingFileReaderTest() - : watcher_shim_(NULL), - fail_on_watch_(false), - message_loop_(MessageLoop::current()), - watcher_(new WatchingFileReader()) { - watcher_->set_watcher_factory(new MockFilePathWatcherFactory(this)); - } - - MockFilePathWatcherShim* watcher_shim_; - bool fail_on_watch_; - MessageLoop* message_loop_; - scoped_ptr<WatchingFileReader> watcher_; - - std::string breakpoint_; -}; - -TEST_F(WatchingFileReaderTest, FilePathWatcherFailures) { - fail_on_watch_ = true; - watcher_->StartWatch(FilePath(FILE_PATH_LITERAL("some_file_name")), - new MockWorker(this)); - RunUntilBreak("OnWatch"); - - fail_on_watch_ = false; - RunUntilBreak("OnWatch"); // Due to backoff this will take 100ms. - RunUntilBreak("OnWork"); - - ASSERT_TRUE(watcher_shim_); - watcher_shim_->PathError(); - RunUntilBreak("OnWatch"); - RunUntilBreak("OnWork"); - - message_loop_->AssertIdle(); - - ASSERT_TRUE(watcher_shim_); - watcher_shim_->PathChanged(); - RunUntilBreak("OnWork"); - - message_loop_->AssertIdle(); -} - -} // namespace - -} // namespace net - |