diff options
| -rw-r--r-- | net/dns/dns_config_service.cc | 18 | ||||
| -rw-r--r-- | net/dns/dns_config_service.h | 15 | ||||
| -rw-r--r-- | net/dns/dns_config_service_posix.cc | 33 | ||||
| -rw-r--r-- | net/dns/dns_config_service_posix.h | 11 | ||||
| -rw-r--r-- | net/dns/dns_config_service_unittest.cc | 12 | ||||
| -rw-r--r-- | net/dns/dns_config_service_win.cc | 293 | ||||
| -rw-r--r-- | net/dns/dns_config_service_win.h | 48 | ||||
| -rw-r--r-- | net/dns/dns_config_service_win_unittest.cc | 54 | ||||
| -rw-r--r-- | net/dns/serial_worker.cc | 99 | ||||
| -rw-r--r-- | net/dns/serial_worker.h | 102 | ||||
| -rw-r--r-- | net/dns/serial_worker_unittest.cc | 170 | ||||
| -rw-r--r-- | net/dns/watching_file_reader.cc | 119 | ||||
| -rw-r--r-- | net/dns/watching_file_reader.h | 71 | ||||
| -rw-r--r-- | net/dns/watching_file_reader_unittest.cc | 152 | ||||
| -rw-r--r-- | net/net.gyp | 6 |
15 files changed, 899 insertions, 304 deletions
diff --git a/net/dns/dns_config_service.cc b/net/dns/dns_config_service.cc index ed3f1d2..dcf58bd 100644 --- a/net/dns/dns_config_service.cc +++ b/net/dns/dns_config_service.cc @@ -55,9 +55,9 @@ void DnsConfigService::RemoveObserver(Observer* observer) { void DnsConfigService::OnConfigRead(const DnsConfig& config) { DCHECK(CalledOnValidThread()); if (!config.EqualsIgnoreHosts(dns_config_)) { - DnsConfig copy = config; - copy.hosts.swap(dns_config_.hosts); - dns_config_ = copy; + 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_)); @@ -76,8 +76,9 @@ void DnsConfigService::OnHostsRead(const DnsHosts& hosts) { } } -DnsHostsReader::DnsHostsReader(DnsConfigService* service) - : service_(service), +DnsHostsReader::DnsHostsReader(const FilePath& path, DnsConfigService* service) + : path_(path), + service_(service), success_(false) { DCHECK(service); } @@ -91,17 +92,18 @@ static bool ReadFile(const FilePath& path, int64 size, std::string* str) { return file_util::ReadFileToString(path, str); } -void DnsHostsReader::DoRead() { +void DnsHostsReader::DoWork() { success_ = false; std::string contents; const int64 kMaxHostsSize = 1 << 16; - if (ReadFile(get_path(), kMaxHostsSize, &contents)) { + if (ReadFile(path_, kMaxHostsSize, &contents)) { success_ = true; ParseHosts(contents, &dns_hosts_); } } -void DnsHostsReader::OnReadFinished() { +void DnsHostsReader::OnWorkFinished() { + DCHECK(!IsCancelled()); if (success_) service_->OnHostsRead(dns_hosts_); } diff --git a/net/dns/dns_config_service.h b/net/dns/dns_config_service.h index 6ceeaa6..655ea78 100644 --- a/net/dns/dns_config_service.h +++ b/net/dns/dns_config_service.h @@ -10,13 +10,15 @@ #include <string> #include <vector> +#include "base/file_path.h" #include "base/gtest_prod_util.h" #include "base/observer_list.h" +#include "base/threading/non_thread_safe.h" #include "base/time.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/watching_file_reader.h" +#include "net/dns/serial_worker.h" namespace net { @@ -110,16 +112,19 @@ class NET_EXPORT_PRIVATE DnsConfigService // A WatchingFileReader that reads a HOSTS file and notifies // DnsConfigService::OnHostsRead(). -class DnsHostsReader : public WatchingFileReader { +// Client should call Cancel() when |service| is going away. +class NET_EXPORT_PRIVATE DnsHostsReader + : NON_EXPORTED_BASE(public SerialWorker) { public: - explicit DnsHostsReader(DnsConfigService* service); + DnsHostsReader(const FilePath& path, DnsConfigService* service); - virtual void DoRead() OVERRIDE; - virtual void OnReadFinished() OVERRIDE; + virtual void DoWork() OVERRIDE; + virtual void OnWorkFinished() OVERRIDE; private: virtual ~DnsHostsReader(); + FilePath path_; DnsConfigService* service_; // Written in DoRead, read in OnReadFinished, no locking necessary. DnsHosts dns_hosts_; diff --git a/net/dns/dns_config_service_posix.cc b/net/dns/dns_config_service_posix.cc index b05b164..0224c39 100644 --- a/net/dns/dns_config_service_posix.cc +++ b/net/dns/dns_config_service_posix.cc @@ -12,6 +12,8 @@ #include "base/memory/scoped_ptr.h" #include "net/base/ip_endpoint.h" #include "net/base/net_util.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" @@ -19,15 +21,15 @@ namespace net { -// A WatcherDelegate that uses ResolverLib to initialize res_state and converts +// A SerialWorker that uses ResolverLib to initialize res_state and converts // it to DnsConfig. -class DnsConfigServicePosix::DnsConfigReader : public WatchingFileReader { +class DnsConfigServicePosix::ConfigReader : public SerialWorker { public: - explicit DnsConfigReader(DnsConfigServicePosix* service) + explicit ConfigReader(DnsConfigServicePosix* service) : service_(service), success_(false) {} - void DoRead() OVERRIDE { + void DoWork() OVERRIDE { struct __res_state res; if ((res_ninit(&res) == 0) && (res.options & RES_INIT)) { success_ = ConvertResToConfig(res, &dns_config_); @@ -42,34 +44,33 @@ class DnsConfigServicePosix::DnsConfigReader : public WatchingFileReader { #endif } - void OnReadFinished() OVERRIDE { + void OnWorkFinished() OVERRIDE { + DCHECK(!IsCancelled()); if (success_) service_->OnConfigRead(dns_config_); } private: - virtual ~DnsConfigReader() {} + virtual ~ConfigReader() {} DnsConfigServicePosix* service_; - // Written in DoRead, read in OnReadFinished, no locking necessary. + // Written in DoWork, read in OnWorkFinished, no locking necessary. DnsConfig dns_config_; bool success_; }; DnsConfigServicePosix::DnsConfigServicePosix() - : config_reader_(new DnsConfigReader(this)), - hosts_reader_(new DnsHostsReader(this)) {} + : config_watcher_(new WatchingFileReader()), + hosts_watcher_(new WatchingFileReader()) {} -DnsConfigServicePosix::~DnsConfigServicePosix() { - DCHECK(CalledOnValidThread()); - config_reader_->Cancel(); - hosts_reader_->Cancel(); -} +DnsConfigServicePosix::~DnsConfigServicePosix() {} void DnsConfigServicePosix::Watch() { DCHECK(CalledOnValidThread()); - config_reader_->StartWatch(FilePath(FILE_PATH_LITERAL(_PATH_RESCONF))); - hosts_reader_->StartWatch(FilePath(FILE_PATH_LITERAL("/etc/hosts"))); + 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)); } // static diff --git a/net/dns/dns_config_service_posix.h b/net/dns/dns_config_service_posix.h index 5349adf..91e03f4 100644 --- a/net/dns/dns_config_service_posix.h +++ b/net/dns/dns_config_service_posix.h @@ -9,25 +9,26 @@ #include <resolv.h> #include "base/compiler_specific.h" -#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" #include "net/base/net_export.h" #include "net/dns/dns_config_service.h" namespace net { +class WatchingFileReader; + class NET_EXPORT_PRIVATE DnsConfigServicePosix : NON_EXPORTED_BASE(public DnsConfigService) { public: - class DnsConfigReader; - DnsConfigServicePosix(); virtual ~DnsConfigServicePosix(); virtual void Watch() OVERRIDE; private: - scoped_refptr<WatchingFileReader> config_reader_; - scoped_refptr<WatchingFileReader> hosts_reader_; + class ConfigReader; + scoped_ptr<WatchingFileReader> config_watcher_; + scoped_ptr<WatchingFileReader> hosts_watcher_; DISALLOW_COPY_AND_ASSIGN(DnsConfigServicePosix); }; diff --git a/net/dns/dns_config_service_unittest.cc b/net/dns/dns_config_service_unittest.cc index 0a86dca..9692f33 100644 --- a/net/dns/dns_config_service_unittest.cc +++ b/net/dns/dns_config_service_unittest.cc @@ -4,9 +4,10 @@ #include "net/dns/dns_config_service.h" +#include "base/bind.h" #include "base/compiler_specific.h" +#include "base/memory/scoped_ptr.h" #include "base/message_loop.h" -#include "base/task.h" #include "base/test/test_timeouts.h" #include "testing/gtest/include/gtest/gtest.h" @@ -81,18 +82,17 @@ TEST_F(DnsConfigServiceTest, NotifyOnChange) { EXPECT_TRUE(last_config_.Equals(complete_config)); } -#if defined(OS_POSIX) -// TODO(szym): enable OS_WIN once ready +#if defined(OS_POSIX) || defined(OS_WIN) // This is really an integration test. TEST_F(DnsConfigServiceTest, GetSystemConfig) { scoped_ptr<DnsConfigService> service(DnsConfigService::CreateSystemService()); // Quit the loop after timeout unless cancelled const int64 kTimeout = TestTimeouts::action_timeout_ms(); - ScopedRunnableMethodFactory<DnsConfigServiceTest> factory_(this); + base::WeakPtrFactory<DnsConfigServiceTest> factory_(this); MessageLoop::current()->PostDelayedTask( FROM_HERE, - factory_.NewRunnableMethod(&DnsConfigServiceTest::Timeout), + base::Bind(&DnsConfigServiceTest::Timeout, factory_.GetWeakPtr()), kTimeout); service->AddObserver(this); @@ -102,7 +102,7 @@ TEST_F(DnsConfigServiceTest, GetSystemConfig) { ASSERT_TRUE(last_config_.IsValid()) << "Did not receive DnsConfig in " << kTimeout << "ms"; } -#endif // OS_POSIX +#endif // OS_POSIX || OS_WIN } // namespace net diff --git a/net/dns/dns_config_service_win.cc b/net/dns/dns_config_service_win.cc new file mode 100644 index 0000000..cd58bb8 --- /dev/null +++ b/net/dns/dns_config_service_win.cc @@ -0,0 +1,293 @@ +// 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/dns_config_service_win.h" + +#include <iphlpapi.h> +#include <windows.h> + +#include <string> + +#include "base/compiler_specific.h" +#include "base/file_path.h" +#include "base/memory/scoped_ptr.h" +#include "base/string_split.h" +#include "base/string_util.h" +#include "base/threading/thread_restrictions.h" +#include "base/utf_string_conversions.h" +#include "base/win/object_watcher.h" +#include "base/win/registry.h" +#include "googleurl/src/url_canon.h" +#include "net/base/net_util.h" +#include "net/dns/serial_worker.h" +#include "net/dns/watching_file_reader.h" + +namespace net { + +// Converts a string16 domain name to ASCII, possibly using punycode. +// Returns true if the conversion succeeds. +bool ParseDomainASCII(const string16& widestr, std::string* domain) { + DCHECK(domain); + // Check if already ASCII. + if (IsStringASCII(widestr)) { + *domain = UTF16ToASCII(widestr); + return true; + } + + // 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)) { + 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 + // copy. Since ASCII is a subset of UTF8 the following is equivalent). + bool success = UTF16ToUTF8(punycode.data(), + punycode.length(), + domain); + DCHECK(success); + DCHECK(IsStringASCII(*domain)); + return success; +} + +bool ParseSearchList(const string16& value, std::vector<std::string>* out) { + DCHECK(out); + if (value.empty()) + return false; + + // If the list includes an empty hostname (",," or ", ,"), it is terminated. + // Although nslookup and network connection property tab ignore such + // fragments ("a,b,,c" becomes ["a", "b", "c"]), our reference is getaddrinfo + // (which sees ["a", "b"]). WMI queries also return a matching search list. + std::vector<string16> woutput; + base::SplitString(value, ',', &woutput); + std::vector<std::string> output; + for (size_t i = 0; i < woutput.size(); ++i) { + // Convert non-ascii to punycode, although getaddrinfo does not properly + // handle such suffixes. + const string16& t = woutput[i]; + std::string parsed; + if (t.empty() || !ParseDomainASCII(t, &parsed)) + break; + output.push_back(parsed); + } + if (output.empty()) + return false; + out->swap(output); + return true; +} + +// Watches registry for changes and reads config from registry and IP helper. +// Reading and opening of reg keys is always performed on WorkerPool. Setting +// up watches requires IO loop. +class DnsConfigServiceWin::ConfigReader : public SerialWorker { + public: + // Watches a single registry key for changes. + class RegistryWatcher : public base::win::ObjectWatcher::Delegate { + public: + explicit RegistryWatcher(ConfigReader* reader) : reader_(reader) {} + + bool StartWatch(const wchar_t* key) { + DCHECK(!key_.IsWatching()); + if (key_.Open(HKEY_LOCAL_MACHINE, key, + KEY_NOTIFY | KEY_QUERY_VALUE) != ERROR_SUCCESS) + return false; + if (key_.StartWatching() != ERROR_SUCCESS) + return false; + if (!watcher_.StartWatching(key_.watch_event(), this)) + return false; + return true; + } + + void Cancel() { + reader_ = NULL; + key_.StopWatching(); + watcher_.StopWatching(); + } + + void OnObjectSignaled(HANDLE object) OVERRIDE { + if (reader_) + reader_->WorkNow(); + // TODO(szym): handle errors + key_.StartWatching(); + watcher_.StartWatching(key_.watch_event(), this); + } + + bool Read(const wchar_t* name, string16* value) const { + return key_.ReadValue(name, value) == ERROR_SUCCESS; + } + + private: + ConfigReader* reader_; + base::win::RegKey key_; + base::win::ObjectWatcher watcher_; + }; + + explicit ConfigReader(DnsConfigServiceWin* service) + : policy_watcher_(this), + tcpip_watcher_(this), + service_(service) {} + + bool StartWatch() { + DCHECK(loop()->BelongsToCurrentThread()); + + // This is done only once per lifetime so open the keys on this thread. + base::ThreadRestrictions::ScopedAllowIO allow_io; + if (!tcpip_watcher_.StartWatch( + L"SYSTEM\\CurrentControlSet\\Services\\Tcpip\\Parameters")) + return false; + + // DNS suffix search list can be installed via group policy which sets + // this registry key. If the key is missing, the policy does not apply, + // and the DNS client uses DHCP and manual settings. + // If a policy is installed, DnsConfigService will need to be restarted. + // BUG=99509 + policy_watcher_.StartWatch( + L"Software\\Policies\\Microsoft\\Windows NT\\DNSClient"); + WorkNow(); + return true; + } + + void Cancel() { + DCHECK(loop()->BelongsToCurrentThread()); + SerialWorker::Cancel(); + policy_watcher_.Cancel(); + tcpip_watcher_.Cancel(); + } + + // Delay between calls to WorkerPool::PostTask + static const int kWorkerPoolRetryDelayMs = 100; + + private: + friend class RegistryWatcher; + virtual ~ConfigReader() {} + + // Uses GetAdapterAddresses to get effective DNS server order and default + // DNS suffix. + bool ReadIpHelper(DnsConfig* config) { + base::ThreadRestrictions::AssertIOAllowed(); + + ULONG flags = GAA_FLAG_SKIP_ANYCAST | GAA_FLAG_SKIP_UNICAST | + GAA_FLAG_SKIP_MULTICAST | GAA_FLAG_SKIP_FRIENDLY_NAME; + IP_ADAPTER_ADDRESSES buf; + IP_ADAPTER_ADDRESSES* adapters = &buf; + ULONG len = sizeof(buf); + scoped_array<char> extra_buf; + + UINT rv = GetAdaptersAddresses(AF_UNSPEC, flags, NULL, adapters, &len); + if (rv == ERROR_BUFFER_OVERFLOW) { + extra_buf.reset(new char[len]); + adapters = reinterpret_cast<IP_ADAPTER_ADDRESSES*>(extra_buf.get()); + rv = GetAdaptersAddresses(AF_UNSPEC, flags, NULL, adapters, &len); + } + if (rv != NO_ERROR) + return false; + + config->search.clear(); + config->nameservers.clear(); + + IP_ADAPTER_ADDRESSES* adapter; + for (adapter = adapters; adapter != NULL; adapter = adapter->Next) { + if (adapter->OperStatus != IfOperStatusUp) + continue; + if (adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK) + continue; + + // IP_ADAPTER_ADDRESSES in Vista+ has a search list at FirstDnsSuffix, + // but it's only the per-interface search list. First initialize with + // DnsSuffix, then override from registry. + std::string dns_suffix; + if (ParseDomainASCII(adapter->DnsSuffix, &dns_suffix)) { + config->search.push_back(dns_suffix); + } + IP_ADAPTER_DNS_SERVER_ADDRESS* address; + for (address = adapter->FirstDnsServerAddress; address != NULL; + address = address->Next) { + IPEndPoint ipe; + if (ipe.FromSockAddr(address->Address.lpSockaddr, + address->Address.iSockaddrLength)) { + config->nameservers.push_back(ipe); + } else { + return false; + } + } + } + return true; + } + + // Read and parse SearchList from registry. Only accept the value if all + // elements of the search list are non-empty and can be converted to UTF-8. + bool ReadSearchList(const RegistryWatcher& watcher, DnsConfig* config) { + string16 value; + if (!watcher.Read(L"SearchList", &value)) + return false; + return ParseSearchList(value, &(config->search)); + } + + void DoWork() OVERRIDE { + // Should be called on WorkerPool. + dns_config_ = DnsConfig(); + if (!ReadIpHelper(&dns_config_)) + return; // no point reading the rest + + // check global registry overrides + if (!ReadSearchList(policy_watcher_, &dns_config_)) + ReadSearchList(tcpip_watcher_, &dns_config_); + + // TODO(szym): add support for DNS suffix devolution BUG=99510 + } + + void OnWorkFinished() OVERRIDE { + DCHECK(loop()->BelongsToCurrentThread()); + DCHECK(!IsCancelled()); + if (dns_config_.IsValid()) { + service_->OnConfigRead(dns_config_); + } else { + LOG(WARNING) << "Failed to read name servers from registry"; + } + } + + // Written in DoRead(), read in OnReadFinished(). No locking required. + DnsConfig dns_config_; + + RegistryWatcher policy_watcher_; + RegistryWatcher tcpip_watcher_; + + DnsConfigServiceWin* service_; +}; + +DnsConfigServiceWin::DnsConfigServiceWin() + : config_reader_(new ConfigReader(this)), + hosts_watcher_(new WatchingFileReader()) {} + +DnsConfigServiceWin::~DnsConfigServiceWin() { + DCHECK(CalledOnValidThread()); + config_reader_->Cancel(); +} + +void DnsConfigServiceWin::Watch() { + DCHECK(CalledOnValidThread()); + bool started = config_reader_->StartWatch(); + // TODO(szym): handle possible failure + DCHECK(started); + + 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)); +} + +// static +DnsConfigService* DnsConfigService::CreateSystemService() { + return new DnsConfigServiceWin(); +} + +} // namespace net + diff --git a/net/dns/dns_config_service_win.h b/net/dns/dns_config_service_win.h new file mode 100644 index 0000000..97dcf6c --- /dev/null +++ b/net/dns/dns_config_service_win.h @@ -0,0 +1,48 @@ +// 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_DNS_CONFIG_SERVICE_WIN_H_ +#define NET_DNS_DNS_CONFIG_SERVICE_WIN_H_ +#pragma once + +#include <string> +#include <vector> + +#include "base/memory/ref_counted.h" +#include "base/memory/scoped_ptr.h" +#include "base/string16.h" +#include "net/base/net_export.h" +#include "net/dns/dns_config_service.h" + +namespace net { + +class WatchingFileReader; + +class NET_EXPORT_PRIVATE DnsConfigServiceWin + : NON_EXPORTED_BASE(public DnsConfigService) { + public: + DnsConfigServiceWin(); + virtual ~DnsConfigServiceWin(); + + virtual void Watch() OVERRIDE; + + private: + class ConfigReader; + scoped_refptr<ConfigReader> config_reader_; + scoped_ptr<WatchingFileReader> hosts_watcher_; + + DISALLOW_COPY_AND_ASSIGN(DnsConfigServiceWin); +}; + +// Parses |value| as search list (comma-delimited list of domain names) from +// a registry key and stores it in |out|. Returns true on success. +// Empty entries (e.g., "chromium.org,,org") terminate the list. +// Non-ascii hostnames are converted to punycode. +bool NET_EXPORT_PRIVATE ParseSearchList(const string16& value, + std::vector<std::string>* out); + +} // 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 new file mode 100644 index 0000000..6397c32 --- /dev/null +++ b/net/dns/dns_config_service_win_unittest.cc @@ -0,0 +1,54 @@ +// 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/dns_config_service_win.h" + +#include "testing/gtest/include/gtest/gtest.h" + +namespace net { + +namespace { + +TEST(DnsConfigServiceWinTest, ParseDomain) { + const struct TestCase { + const wchar_t* input; + const char* output[4]; // NULL-terminated, empty if expected false + } cases[] = { + { L"chromium.org", { "chromium.org", NULL } }, + { L"chromium.org,org", { "chromium.org", "org", NULL } }, + // Empty suffixes terminate the list + { L"crbug.com,com,,org", { "crbug.com", "com", NULL } }, + // IDN are converted to punycode + { L"\u017c\xf3\u0142ta.pi\u0119\u015b\u0107.pl,pl", + { "xn--ta-4ja03asj.xn--pi-wla5e0q.pl", "pl", NULL } }, + // Empty search list is invalid + { L"", { NULL } }, + { L",,", { NULL } }, + { NULL, { NULL } }, + }; + + std::vector<std::string> actual_output, expected_output; + for (const TestCase* t = cases; t->input; ++t) { + actual_output.clear(); + actual_output.push_back("UNSET"); + expected_output.clear(); + for (const char* const* output = t->output; *output; ++output) { + expected_output.push_back(*output); + } + bool result = ParseSearchList(t->input, &actual_output); + if (!expected_output.empty()) { + EXPECT_TRUE(result); + EXPECT_EQ(expected_output, actual_output); + } else { + EXPECT_FALSE(result) << "Unexpected parse success on " << t->input; + expected_output.push_back("UNSET"); + EXPECT_EQ(expected_output, actual_output); + } + } +} + +} // namespace + +} // namespace net + diff --git a/net/dns/serial_worker.cc b/net/dns/serial_worker.cc new file mode 100644 index 0000000..d2e42c0 --- /dev/null +++ b/net/dns/serial_worker.cc @@ -0,0 +1,99 @@ +// 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/serial_worker.h" + +#include "base/bind.h" +#include "base/location.h" +#include "base/message_loop_proxy.h" +#include "base/threading/worker_pool.h" + +namespace net { + +SerialWorker::SerialWorker() + : message_loop_(base::MessageLoopProxy::current()), + state_(IDLE) {} + +SerialWorker::~SerialWorker() {} + +void SerialWorker::WorkNow() { + DCHECK(message_loop_->BelongsToCurrentThread()); + switch (state_) { + case IDLE: + if (!base::WorkerPool::PostTask(FROM_HERE, base::Bind( + &SerialWorker::DoWorkJob, this), false)) { +#if defined(OS_POSIX) + // See worker_pool_posix.cc. + NOTREACHED() << "WorkerPool::PostTask is not expected to fail on posix"; +#else + LOG(WARNING) << "Failed to WorkerPool::PostTask, will retry later"; + message_loop_->PostDelayedTask( + FROM_HERE, + base::Bind(&SerialWorker::RetryWork, this), + kWorkerPoolRetryDelayMs); + state_ = WAITING; + return; +#endif + } + state_ = WORKING; + return; + case WORKING: + // Remember to re-read after |DoRead| finishes. + state_ = PENDING; + return; + case CANCELLED: + case PENDING: + case WAITING: + return; + default: + NOTREACHED() << "Unexpected state " << state_; + } +} + +void SerialWorker::Cancel() { + DCHECK(message_loop_->BelongsToCurrentThread()); + state_ = CANCELLED; +} + +void SerialWorker::DoWorkJob() { + this->DoWork(); + // If this fails, the loop is gone, so there is no point retrying. + message_loop_->PostTask(FROM_HERE, base::Bind( + &SerialWorker::OnWorkJobFinished, this)); +} + +void SerialWorker::OnWorkJobFinished() { + DCHECK(message_loop_->BelongsToCurrentThread()); + switch (state_) { + case CANCELLED: + return; + case WORKING: + state_ = IDLE; + this->OnWorkFinished(); + return; + case PENDING: + state_ = IDLE; + WorkNow(); + return; + default: + NOTREACHED() << "Unexpected state " << state_; + } +} + +void SerialWorker::RetryWork() { + DCHECK(message_loop_->BelongsToCurrentThread()); + switch (state_) { + case CANCELLED: + return; + case WAITING: + state_ = IDLE; + WorkNow(); + return; + default: + NOTREACHED() << "Unexpected state " << state_; + } +} + +} // namespace net + diff --git a/net/dns/serial_worker.h b/net/dns/serial_worker.h new file mode 100644 index 0000000..4d6f8fc --- /dev/null +++ b/net/dns/serial_worker.h @@ -0,0 +1,102 @@ +// 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_SERIAL_WORKER_H_ +#define NET_DNS_SERIAL_WORKER_H_ +#pragma once + +#include <string> + +#include "base/compiler_specific.h" +#include "base/memory/ref_counted.h" +#include "net/base/net_export.h" + +// Forward declaration +namespace base { +class MessageLoopProxy; +} + +namespace net { + +// SerialWorker executes a job on WorkerPool serially -- **once at a time**. +// On |WorkNow|, a call to |DoWork| is scheduled on the WorkerPool. Once it +// completes, |OnWorkFinished| is called on the origin thread. +// If |WorkNow| is called (1 or more times) while |DoWork| is already under way, +// |DoWork| will be called once: after current |DoWork| completes, before a +// call to |OnWorkFinished|. +// +// This behavior is designed for updating a result after some trigger, for +// example reading a file once FilePathWatcher indicates it changed. +// +// Derived classes should store results of work done in |DoWork| in dedicated +// fields and read them in |OnWorkFinished| which is executed on the origin +// thread. This avoids the need to template this class. +// +// This implementation avoids locking by using the |state_| member to ensure +// that |DoWork| and |OnWorkFinished| cannot execute in parallel. +// +// TODO(szym): update to WorkerPool::PostTaskAndReply once available. +class NET_EXPORT_PRIVATE SerialWorker + : NON_EXPORTED_BASE(public base::RefCountedThreadSafe<SerialWorker>) { + public: + SerialWorker(); + + // Unless already scheduled, post |DoWork| to WorkerPool. + // Made virtual to allow mocking. + virtual void WorkNow(); + + // Stop scheduling read jobs. + void Cancel(); + + bool IsCancelled() const { return state_ == CANCELLED; } + + // Delay between calls to WorkerPool::PostTask + static const int kWorkerPoolRetryDelayMs = 100; + + protected: + friend class base::RefCountedThreadSafe<SerialWorker>; + // protected to allow sub-classing, but prevent deleting + virtual ~SerialWorker(); + + // Executed on WorkerPool, at most once at a time. + virtual void DoWork() = 0; + + // Executed on origin thread after |DoRead| completes. + virtual void OnWorkFinished() = 0; + + base::MessageLoopProxy* loop() { + return message_loop_; + } + + private: + enum State { + CANCELLED = -1, + IDLE = 0, + WORKING, // |DoWorkJob| posted on WorkerPool, until |OnWorkJobFinished| + PENDING, // |WorkNow| while WORKING, must re-do work + WAITING, // WorkerPool is busy, |RetryWork| is posted + }; + + // Called on the worker thread, executes |DoWork| and notifies the origin + // thread. + void DoWorkJob(); + + // Called on the the origin thread after |DoWork| completes. + void OnWorkJobFinished(); + + // Posted to message loop in case WorkerPool is busy. (state == WAITING) + void RetryWork(); + + // Message loop for the thread of origin. + scoped_refptr<base::MessageLoopProxy> message_loop_; + + State state_; + + DISALLOW_COPY_AND_ASSIGN(SerialWorker); +}; + +} // namespace net + +#endif // NET_DNS_SERIAL_WORKER_H_ + diff --git a/net/dns/serial_worker_unittest.cc b/net/dns/serial_worker_unittest.cc new file mode 100644 index 0000000..4062c2a --- /dev/null +++ b/net/dns/serial_worker_unittest.cc @@ -0,0 +1,170 @@ +// 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/serial_worker.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 SerialWorkerTest : public testing::Test { + public: + // The class under test + class TestSerialWorker : public SerialWorker { + public: + explicit TestSerialWorker(SerialWorkerTest* t) + : test_(t) {} + virtual void DoWork() OVERRIDE { + ASSERT_TRUE(test_); + test_->OnWork(); + } + virtual void OnWorkFinished() OVERRIDE { + ASSERT_TRUE(test_); + test_->OnWorkFinished(); + } + private: + virtual ~TestSerialWorker() {} + SerialWorkerTest* test_; + }; + + // Mocks + + void OnWork() { + { // Check that OnWork is executed serially. + base::AutoLock lock(work_lock_); + EXPECT_FALSE(work_running_) << "DoRead is not called serially!"; + work_running_ = true; + } + BreakNow("OnWork"); + work_allowed_.Wait(); + // Calling from WorkerPool, but protected by work_allowed_/work_called_. + output_value_ = input_value_; + + { // This lock might be destroyed after work_called_ is signalled. + base::AutoLock lock(work_lock_); + work_running_ = false; + } + work_called_.Signal(); + } + + void OnWorkFinished() { + EXPECT_TRUE(message_loop_ == MessageLoop::current()); + EXPECT_EQ(output_value_, input_value_); + BreakNow("OnWorkFinished"); + } + + protected: + friend class BreakTask; + class BreakTask : public Task { + public: + BreakTask(SerialWorkerTest* test, std::string breakpoint) + : test_(test), breakpoint_(breakpoint) {} + virtual ~BreakTask() {} + virtual void Run() OVERRIDE { + test_->breakpoint_ = breakpoint_; + MessageLoop::current()->QuitNow(); + } + private: + SerialWorkerTest* test_; + std::string breakpoint_; + }; + + void BreakNow(std::string b) { + message_loop_->PostTask(FROM_HERE, new BreakTask(this, b)); + } + + void RunUntilBreak(std::string b) { + message_loop_->Run(); + ASSERT_EQ(breakpoint_, b); + } + + SerialWorkerTest() + : input_value_(0), + output_value_(-1), + work_allowed_(false, false), + work_called_(false, false), + work_running_(false) { + } + + // Helpers for tests. + + // Lets OnWork run and waits for it to complete. Can only return if OnWork is + // executed on a concurrent thread. + void WaitForWork() { + RunUntilBreak("OnWork"); + work_allowed_.Signal(); + work_called_.Wait(); + } + + // test::Test methods + virtual void SetUp() OVERRIDE { + message_loop_ = MessageLoop::current(); + worker_ = new TestSerialWorker(this); + } + + virtual void TearDown() OVERRIDE { + // Cancel the worker to catch if it makes a late DoWork call. + worker_->Cancel(); + // Check if OnWork is stalled. + EXPECT_FALSE(work_running_) << "OnWork should be done by TearDown"; + // Release it for cleanliness. + if (work_running_) { + WaitForWork(); + } + } + + // Input value read on WorkerPool. + int input_value_; + // Output value written on WorkerPool. + int output_value_; + + // read is called on WorkerPool so we need to synchronize with it. + base::WaitableEvent work_allowed_; + base::WaitableEvent work_called_; + + // Protected by read_lock_. Used to verify that read calls are serialized. + bool work_running_; + base::Lock work_lock_; + + // Loop for this thread. + MessageLoop* message_loop_; + + // WatcherDelegate under test. + scoped_refptr<TestSerialWorker> worker_; + + std::string breakpoint_; +}; + +TEST_F(SerialWorkerTest, ExecuteAndSerializeReads) { + for (int i = 0; i < 3; ++i) { + ++input_value_; + worker_->WorkNow(); + WaitForWork(); + RunUntilBreak("OnWorkFinished"); + + message_loop_->AssertIdle(); + } + + // Schedule two calls. OnWork checks if it is called serially. + ++input_value_; + worker_->WorkNow(); + // read is blocked, so this will have to induce re-work + worker_->WorkNow(); + WaitForWork(); + WaitForWork(); + RunUntilBreak("OnWorkFinished"); + + // No more tasks should remain. + message_loop_->AssertIdle(); +} + +} // namespace + +} // namespace net + diff --git a/net/dns/watching_file_reader.cc b/net/dns/watching_file_reader.cc index 2a550bd..869da37 100644 --- a/net/dns/watching_file_reader.cc +++ b/net/dns/watching_file_reader.cc @@ -6,7 +6,7 @@ #include "base/bind.h" #include "base/location.h" -#include "base/message_loop_proxy.h" +#include "base/message_loop.h" #include "base/threading/worker_pool.h" namespace net { @@ -28,58 +28,69 @@ FilePathWatcherFactory::CreateFilePathWatcher() { 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() - : message_loop_(base::MessageLoopProxy::current()), - factory_(new FilePathWatcherFactory()), - reading_(false), - read_pending_(false), - cancelled_(false) {} + : watcher_factory_(new FilePathWatcherFactory()) {} -void WatchingFileReader::StartWatch(const FilePath& path) { - DCHECK(message_loop_->BelongsToCurrentThread()); +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::Cancel() { - DCHECK(message_loop_->BelongsToCurrentThread()); - cancelled_ = true; - // Let go of the watcher to break the reference cycle. - watcher_.reset(); - // Destroy the non-thread-safe factory now, since dtor is non-thread-safe. - factory_.reset(); -} - void WatchingFileReader::OnFilePathChanged(const FilePath& path) { - DCHECK(message_loop_->BelongsToCurrentThread()); - ReadNow(); + DCHECK(CalledOnValidThread()); + reader_->WorkNow(); } void WatchingFileReader::OnFilePathError(const FilePath& path) { - DCHECK(message_loop_->BelongsToCurrentThread()); + DCHECK(CalledOnValidThread()); RestartWatch(); } -WatchingFileReader::~WatchingFileReader() { - DCHECK(cancelled_); -} - void WatchingFileReader::RescheduleWatch() { - DCHECK(message_loop_->BelongsToCurrentThread()); - message_loop_->PostDelayedTask( + DCHECK(CalledOnValidThread()); + MessageLoop::current()->PostDelayedTask( FROM_HERE, - base::Bind(&WatchingFileReader::RestartWatch, this), + base::Bind(&WatchingFileReader::RestartWatch, AsWeakPtr()), kWatchRetryDelayMs); } void WatchingFileReader::RestartWatch() { - if (cancelled_) + DCHECK(CalledOnValidThread()); + if (reader_->IsCancelled()) return; - watcher_.reset(factory_->CreateFilePathWatcher()); - if (watcher_->Watch(path_, this)) { - ReadNow(); + watcher_.reset(watcher_factory_->CreateFilePathWatcher()); + if (watcher_->Watch(path_, watcher_delegate_)) { + reader_->WorkNow(); } else { LOG(WARNING) << "Watch on " << path_.LossyDisplayName() << @@ -88,50 +99,4 @@ void WatchingFileReader::RestartWatch() { } } -void WatchingFileReader::ReadNow() { - if (cancelled_) - return; - if (reading_) { - // Remember to re-read after DoRead posts results. - read_pending_ = true; - } else { - if (!base::WorkerPool::PostTask(FROM_HERE, base::Bind( - &WatchingFileReader::DoReadJob, this), false)) { -#if defined(OS_POSIX) - // See worker_pool_posix.cc. - NOTREACHED() << "WorkerPool::PostTask is not expected to fail on posix"; -#else - LOG(WARNING) << "Failed to WorkerPool::PostTask, will retry later"; - message_loop_->PostDelayedTask( - FROM_HERE, - base::Bind(&WatchingFileReader::ReadNow, this), - kWorkerPoolRetryDelayMs); - return; -#endif - } - reading_ = true; - read_pending_ = false; - } -} - -void WatchingFileReader::DoReadJob() { - this->DoRead(); - // If this fails, the loop is gone, so there is no point retrying. - message_loop_->PostTask(FROM_HERE, base::Bind( - &WatchingFileReader::OnReadJobFinished, this)); -} - -void WatchingFileReader::OnReadJobFinished() { - DCHECK(message_loop_->BelongsToCurrentThread()); - if (cancelled_) - return; - reading_ = false; - if (read_pending_) { - // Discard this result and re-read. - ReadNow(); - return; - } - this->OnReadFinished(); -} - } // namespace net diff --git a/net/dns/watching_file_reader.h b/net/dns/watching_file_reader.h index 15237ea..e93004e 100644 --- a/net/dns/watching_file_reader.h +++ b/net/dns/watching_file_reader.h @@ -11,8 +11,10 @@ #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" // Forward declaration namespace base { @@ -27,7 +29,6 @@ class NET_EXPORT_PRIVATE FilePathWatcherShim public: FilePathWatcherShim(); virtual ~FilePathWatcherShim(); - virtual bool Watch( const FilePath& path, base::files::FilePathWatcher::Delegate* delegate) WARN_UNUSED_RESULT; @@ -46,37 +47,22 @@ class NET_EXPORT_PRIVATE FilePathWatcherFactory DISALLOW_COPY_AND_ASSIGN(FilePathWatcherFactory); }; -// A FilePathWatcher::Delegate that restarts Watch on failures and executes -// DoRead on WorkerPool (one at a time) in response to FilePathChanged. -// Derived classes should store results of the work done in DoRead in dedicated -// fields and read them in OnReadFinished. -// -// This implementation avoids locking by using the reading_ flag to ensure that -// DoRead and OnReadFinished cannot execute in parallel. -// -// There's no need to call Cancel() on destruction. On the contrary, a ref-cycle -// between the WatchingFileReader and a FilePathWatcher will prevent destruction -// until Cancel() is called. -// -// TODO(szym): update to WorkerPool::PostTaskAndReply once available. +// 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::files::FilePathWatcher::Delegate) { + : 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); - - // Unless already scheduled, post DoRead to WorkerPool. - void ReadNow(); - - // Do not perform any further work, except perhaps a latent DoRead(). - // Always call it once done with the delegate to ensure destruction. - void Cancel(); + virtual void StartWatch(const FilePath& path, SerialWorker* reader); void set_watcher_factory(FilePathWatcherFactory* factory) { DCHECK(!watcher_.get()); - factory_.reset(factory); + watcher_factory_.reset(factory); } FilePath get_path() const { @@ -86,43 +72,20 @@ class NET_EXPORT_PRIVATE WatchingFileReader // Delay between calls to FilePathWatcher::Watch. static const int kWatchRetryDelayMs = 100; - // Delay between calls to WorkerPool::PostTask - static const int kWorkerPoolRetryDelayMs = 100; - - protected: - // protected to allow sub-classing, but prevent deleting - virtual ~WatchingFileReader(); - - // Executed on WorkerPool, at most once at a time. - virtual void DoRead() = 0; - // Executed on origin thread after DoRead() completes. - virtual void OnReadFinished() = 0; - private: - // FilePathWatcher::Delegate interface - virtual void OnFilePathChanged(const FilePath& path) OVERRIDE; - virtual void OnFilePathError(const FilePath& path) OVERRIDE; + class WatcherDelegate; + // FilePathWatcher::Delegate interface exported via WatcherDelegate + virtual void OnFilePathChanged(const FilePath& path); + virtual void OnFilePathError(const FilePath& path); void RescheduleWatch(); void RestartWatch(); - // Called on the worker thread, executes DoRead and notifies the origin - // thread. - void DoReadJob(); - - // Called on the the origin thread after DoRead completes. - void OnReadJobFinished(); - - // Message loop for the thread on which watcher is used (of TYPE_IO). - scoped_refptr<base::MessageLoopProxy> message_loop_; FilePath path_; - scoped_ptr<FilePathWatcherFactory> factory_; + scoped_ptr<FilePathWatcherFactory> watcher_factory_; scoped_ptr<FilePathWatcherShim> watcher_; - // True after DoRead before OnResultsAvailable. - bool reading_; - // True after OnFilePathChanged fires while |reading_| is true. - bool read_pending_; - bool cancelled_; + scoped_refptr<WatcherDelegate> watcher_delegate_; + scoped_refptr<SerialWorker> reader_; DISALLOW_COPY_AND_ASSIGN(WatchingFileReader); }; diff --git a/net/dns/watching_file_reader_unittest.cc b/net/dns/watching_file_reader_unittest.cc index 24021f0..510398c 100644 --- a/net/dns/watching_file_reader_unittest.cc +++ b/net/dns/watching_file_reader_unittest.cc @@ -16,34 +16,21 @@ namespace { class WatchingFileReaderTest : public testing::Test { public: - // The class under test - class TestWatchingFileReader : public WatchingFileReader { + // Mocks + + class MockWorker : public SerialWorker { public: - explicit TestWatchingFileReader(WatchingFileReaderTest* t) - : test_(t) {} - virtual void DoRead() OVERRIDE { - ASSERT_TRUE(test_); - test_->OnRead(); - } - virtual void OnReadFinished() OVERRIDE { - ASSERT_TRUE(test_); - test_->OnReadFinished(); - } - void TestCancel() { - // Could execute concurrently with DoRead() - test_ = NULL; + explicit MockWorker(WatchingFileReaderTest* t) : test_(t) {} + virtual void WorkNow() OVERRIDE { + test_->OnWork(); } + virtual void DoWork() OVERRIDE {} + virtual void OnWorkFinished() OVERRIDE {} private: - virtual ~TestWatchingFileReader() {} + virtual ~MockWorker() {} WatchingFileReaderTest* test_; - base::Lock lock_; }; - // Mocks - - // WatcherDelegate owns the FilePathWatcherFactory it gets, so use simple - // proxies to call the test fixture. - class MockFilePathWatcherShim : public FilePathWatcherShim { public: @@ -112,28 +99,9 @@ class WatchingFileReaderTest : public testing::Test { return !fail_on_watch_; } - void OnRead() { - { // Check that OnRead is executed serially. - base::AutoLock lock(read_lock_); - EXPECT_FALSE(read_running_) << "DoRead is not called serially!"; - read_running_ = true; - } - BreakNow("OnRead"); - read_allowed_.Wait(); - // Calling from WorkerPool, but protected by read_allowed_/read_called_. - output_value_ = input_value_; - - { // This lock might be destroyed after read_called_ is signalled. - base::AutoLock lock(read_lock_); - read_running_ = false; - } - read_called_.Signal(); - } - - void OnReadFinished() { + void OnWork() { EXPECT_TRUE(message_loop_ == MessageLoop::current()); - EXPECT_EQ(output_value_, input_value_); - BreakNow("OnReadFinished"); + BreakNow("OnWork"); } protected: @@ -164,122 +132,40 @@ class WatchingFileReaderTest : public testing::Test { WatchingFileReaderTest() : watcher_shim_(NULL), fail_on_watch_(false), - input_value_(0), - output_value_(-1), - read_allowed_(false, false), - read_called_(false, false), - read_running_(false) { - } - - // Helpers for tests. - - // Lets OnRead run and waits for it to complete. Can only return if OnRead is - // executed on a concurrent thread. - void WaitForRead() { - RunUntilBreak("OnRead"); - read_allowed_.Signal(); - read_called_.Wait(); - } - - // test::Test methods - virtual void SetUp() OVERRIDE { - message_loop_ = MessageLoop::current(); - watcher_ = new TestWatchingFileReader(this); + message_loop_(MessageLoop::current()), + watcher_(new WatchingFileReader()) { watcher_->set_watcher_factory(new MockFilePathWatcherFactory(this)); } - virtual void TearDown() OVERRIDE { - // Cancel the watcher to catch if it makes a late DoRead call. - watcher_->TestCancel(); - watcher_->Cancel(); - // Check if OnRead is stalled. - EXPECT_FALSE(read_running_) << "OnRead should be done by TearDown"; - // Release it for cleanliness. - if (read_running_) { - WaitForRead(); - } - } - MockFilePathWatcherShim* watcher_shim_; - bool fail_on_watch_; - - // Input value read on WorkerPool. - int input_value_; - // Output value written on WorkerPool. - int output_value_; - - // read is called on WorkerPool so we need to synchronize with it. - base::WaitableEvent read_allowed_; - base::WaitableEvent read_called_; - - // Protected by read_lock_. Used to verify that read calls are serialized. - bool read_running_; - base::Lock read_lock_; - - // Loop for this thread. MessageLoop* message_loop_; - - // WatcherDelegate under test. - scoped_refptr<TestWatchingFileReader> watcher_; + scoped_ptr<WatchingFileReader> watcher_; std::string breakpoint_; }; TEST_F(WatchingFileReaderTest, FilePathWatcherFailures) { fail_on_watch_ = true; - watcher_->StartWatch(FilePath(FILE_PATH_LITERAL("some_file_name"))); + 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. - WaitForRead(); - RunUntilBreak("OnReadFinished"); + RunUntilBreak("OnWork"); ASSERT_TRUE(watcher_shim_); watcher_shim_->PathError(); RunUntilBreak("OnWatch"); - WaitForRead(); - RunUntilBreak("OnReadFinished"); - - message_loop_->AssertIdle(); -} - -TEST_F(WatchingFileReaderTest, ExecuteAndSerializeReads) { - watcher_->StartWatch(FilePath(FILE_PATH_LITERAL("some_file_name"))); - RunUntilBreak("OnWatch"); - WaitForRead(); - RunUntilBreak("OnReadFinished"); - - message_loop_->AssertIdle(); - - ++input_value_; - ASSERT_TRUE(watcher_shim_); - watcher_shim_->PathChanged(); - WaitForRead(); - RunUntilBreak("OnReadFinished"); - - message_loop_->AssertIdle(); - - ++input_value_; - ASSERT_TRUE(watcher_shim_); - watcher_shim_->PathChanged(); - WaitForRead(); - RunUntilBreak("OnReadFinished"); + RunUntilBreak("OnWork"); message_loop_->AssertIdle(); - // Schedule two calls. OnRead checks if it is called serially. - ++input_value_; ASSERT_TRUE(watcher_shim_); watcher_shim_->PathChanged(); - // read is blocked, so this will have to induce read_pending - watcher_shim_->PathChanged(); - WaitForRead(); - WaitForRead(); - RunUntilBreak("OnReadFinished"); + RunUntilBreak("OnWork"); - // No more tasks should remain. message_loop_->AssertIdle(); } diff --git a/net/net.gyp b/net/net.gyp index 34f1965..9021035 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -312,6 +312,8 @@ 'dns/dns_config_service.h', 'dns/dns_config_service_posix.cc', 'dns/dns_config_service_posix.h', + 'dns/dns_config_service_win.cc', + 'dns/dns_config_service_win.h', 'dns/dns_hosts.cc', 'dns/dns_hosts.h', 'dns/dns_query.cc', @@ -320,6 +322,8 @@ 'dns/dns_response.h', 'dns/dns_transaction.cc', 'dns/dns_transaction.h', + 'dns/serial_worker.cc', + 'dns/serial_worker.h', 'dns/watching_file_reader.cc', 'dns/watching_file_reader.h', 'ftp/ftp_auth_cache.cc', @@ -982,10 +986,12 @@ 'dns/async_host_resolver_unittest.cc', 'dns/dns_config_service_posix_unittest.cc', 'dns/dns_config_service_unittest.cc', + 'dns/dns_config_service_win_unittest.cc', 'dns/dns_hosts_unittest.cc', 'dns/dns_query_unittest.cc', 'dns/dns_response_unittest.cc', 'dns/dns_transaction_unittest.cc', + 'dns/serial_worker_unittest.cc', 'dns/watching_file_reader_unittest.cc', 'ftp/ftp_auth_cache_unittest.cc', 'ftp/ftp_ctrl_response_buffer_unittest.cc', |