Set up HostResolverImpl to flush cache on IP address change.

BUG=http://crbug.com/26159

Review URL: http://codereview.chromium.org/545003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35956 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 93 insertions, 28 deletions

diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc
index 36975f8..5644223 100644
--- a/net/base/host_resolver_impl.cc
+++ b/net/base/host_resolver_impl.cc
@@ -15,6 +15,7 @@
 #include "net/base/host_resolver_proc.h"
 #include "net/base/load_log.h"
 #include "net/base/net_errors.h"
+#include "net/base/network_change_notifier.h"
 
 #if defined(OS_WIN)
 #include "net/base/winsock_init.h"
@@ -22,7 +23,9 @@
 
 namespace net {
 
-HostResolver* CreateSystemHostResolver() {
+namespace {
+
+HostCache* CreateDefaultCache() {
   static const size_t kMaxHostCacheEntries = 100;
 
   HostCache* cache = new HostCache(
@@ -30,7 +33,14 @@ HostResolver* CreateSystemHostResolver() {
       base::TimeDelta::FromMinutes(1),
       base::TimeDelta::FromSeconds(1));
 
-  return new HostResolverImpl(NULL, cache);
+  return cache;
+}
+
+}  // anonymous namespace
+
+HostResolver* CreateSystemHostResolver() {
+  // TODO(willchan): Pass in the NetworkChangeNotifier.
+  return new HostResolverImpl(NULL, CreateDefaultCache(), NULL);
 }
 
 static int ResolveAddrInfo(HostResolverProc* resolver_proc,
@@ -279,16 +289,21 @@ class HostResolverImpl::Job
 
 //-----------------------------------------------------------------------------
 
-HostResolverImpl::HostResolverImpl(HostResolverProc* resolver_proc,
-                                   HostCache* cache)
+HostResolverImpl::HostResolverImpl(
+    HostResolverProc* resolver_proc,
+    HostCache* cache,
+    const scoped_refptr<NetworkChangeNotifier>& network_change_notifier)
     : cache_(cache),
       next_request_id_(0),
       resolver_proc_(resolver_proc),
       default_address_family_(ADDRESS_FAMILY_UNSPECIFIED),
-      shutdown_(false) {
+      shutdown_(false),
+      network_change_notifier_(network_change_notifier) {
 #if defined(OS_WIN)
   EnsureWinsockInit();
 #endif
+  if (network_change_notifier_)
+    network_change_notifier_->AddObserver(this);
 }
 
 HostResolverImpl::~HostResolverImpl() {
@@ -300,6 +315,9 @@ HostResolverImpl::~HostResolverImpl() {
   // In case we are being deleted during the processing of a callback.
   if (cur_completing_job_)
     cur_completing_job_->Cancel();
+
+  if (network_change_notifier_)
+    network_change_notifier_->RemoveObserver(this);
 }
 
 // TODO(eroman): Don't create cache entries for hostnames which are simply IP
@@ -408,11 +426,11 @@ void HostResolverImpl::CancelRequest(RequestHandle req_handle) {
   OnCancelRequest(req->load_log(), req->id(), req->info());
 }
 
-void HostResolverImpl::AddObserver(Observer* observer) {
+void HostResolverImpl::AddObserver(HostResolver::Observer* observer) {
   observers_.push_back(observer);
 }
 
-void HostResolverImpl::RemoveObserver(Observer* observer) {
+void HostResolverImpl::RemoveObserver(HostResolver::Observer* observer) {
   ObserversList::iterator it =
       std::find(observers_.begin(), observers_.end(), observer);
 
@@ -555,4 +573,9 @@ void HostResolverImpl::OnCancelRequest(LoadLog* load_log,
   LoadLog::EndEvent(load_log, LoadLog::TYPE_HOST_RESOLVER_IMPL);
 }
 
+void HostResolverImpl::OnIPAddressChanged() {
+  if (cache_.get())
+    cache_->clear();
+}
+
 }  // namespace net
diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h
index 84acdc2..6cabcf9 100644
--- a/net/base/host_resolver_impl.h
+++ b/net/base/host_resolver_impl.h
@@ -12,6 +12,7 @@
 #include "net/base/host_cache.h"
 #include "net/base/host_resolver.h"
 #include "net/base/host_resolver_proc.h"
+#include "net/base/network_change_notifier.h"
 
 namespace net {
 
@@ -40,7 +41,8 @@ namespace net {
 // Thread safety: This class is not threadsafe, and must only be called
 // from one thread!
 //
-class HostResolverImpl : public HostResolver {
+class HostResolverImpl : public HostResolver,
+                         public NetworkChangeNotifier::Observer {
  public:
   // Creates a HostResolver that first uses the local cache |cache|, and then
   // falls back to |resolver_proc|.
@@ -52,7 +54,9 @@ class HostResolverImpl : public HostResolver {
   // thread-safe since it is run from multiple worker threads. If
   // |resolver_proc| is NULL then the default host resolver procedure is
   // used (which is SystemHostResolverProc except if overridden).
-  HostResolverImpl(HostResolverProc* resolver_proc, HostCache* cache);
+  HostResolverImpl(HostResolverProc* resolver_proc,
+                   HostCache* cache,
+                   const scoped_refptr<NetworkChangeNotifier>& notifier);
 
   // HostResolver methods:
   virtual int Resolve(const RequestInfo& info,
@@ -61,8 +65,8 @@ class HostResolverImpl : public HostResolver {
                       RequestHandle* out_req,
                       LoadLog* load_log);
   virtual void CancelRequest(RequestHandle req);
-  virtual void AddObserver(Observer* observer);
-  virtual void RemoveObserver(Observer* observer);
+  virtual void AddObserver(HostResolver::Observer* observer);
+  virtual void RemoveObserver(HostResolver::Observer* observer);
   virtual HostCache* GetHostCache();
 
   // TODO(eroman): temp hack for http://crbug.com/15513
@@ -78,7 +82,7 @@ class HostResolverImpl : public HostResolver {
   typedef std::vector<Request*> RequestsList;
   typedef HostCache::Key Key;
   typedef std::map<Key, scoped_refptr<Job> > JobMap;
-  typedef std::vector<Observer*> ObserversList;
+  typedef std::vector<HostResolver::Observer*> ObserversList;
 
   // If any completion callbacks are pending when the resolver is destroyed,
   // the host resolutions are cancelled, and the completion callbacks will not
@@ -119,6 +123,9 @@ class HostResolverImpl : public HostResolver {
                        int request_id,
                        const RequestInfo& info);
 
+  // NetworkChangeNotifier::Observer methods:
+  virtual void OnIPAddressChanged();
+
   // Cache of host resolution results.
   scoped_ptr<HostCache> cache_;
 
@@ -146,6 +153,8 @@ class HostResolverImpl : public HostResolver {
   // TODO(eroman): temp hack for http://crbug.com/15513
   bool shutdown_;
 
+  const scoped_refptr<NetworkChangeNotifier> network_change_notifier_;
+
   DISALLOW_COPY_AND_ASSIGN(HostResolverImpl);
 };
 
diff --git a/net/base/host_resolver_impl_unittest.cc b/net/base/host_resolver_impl_unittest.cc
index 09436e3..68bd8a6 100644
--- a/net/base/host_resolver_impl_unittest.cc
+++ b/net/base/host_resolver_impl_unittest.cc
@@ -13,6 +13,7 @@
 #include "net/base/completion_callback.h"
 #include "net/base/load_log_unittest.h"
 #include "net/base/mock_host_resolver.h"
+#include "net/base/mock_network_change_notifier.h"
 #include "net/base/net_errors.h"
 #include "net/base/sys_addrinfo.h"
 #include "net/base/test_completion_callback.h"
@@ -185,7 +186,7 @@ TEST_F(HostResolverImplTest, SynchronousLookup) {
   resolver_proc->AddRule("just.testing", "192.168.1.42");
 
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
 
   HostResolver::RequestInfo info("just.testing", kPortnum);
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
@@ -217,7 +218,7 @@ TEST_F(HostResolverImplTest, AsynchronousLookup) {
   resolver_proc->AddRule("just.testing", "192.168.1.42");
 
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
 
   HostResolver::RequestInfo info("just.testing", kPortnum);
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
@@ -254,7 +255,7 @@ TEST_F(HostResolverImplTest, CanceledAsynchronousLookup) {
   scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded));
   {
     scoped_refptr<HostResolver> host_resolver(
-        new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+        new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
     AddressList adrlist;
     const int kPortnum = 80;
 
@@ -289,7 +290,7 @@ TEST_F(HostResolverImplTest, NumericIPv4Address) {
   resolver_proc->AllowDirectLookup("*");
 
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
   AddressList adrlist;
   const int kPortnum = 5555;
   HostResolver::RequestInfo info("127.1.2.3", kPortnum);
@@ -314,7 +315,7 @@ TEST_F(HostResolverImplTest, NumericIPv6Address) {
   // Resolve a plain IPv6 address.  Don't worry about [brackets], because
   // the caller should have removed them.
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
   AddressList adrlist;
   const int kPortnum = 5555;
   HostResolver::RequestInfo info("2001:db8::1", kPortnum);
@@ -349,7 +350,7 @@ TEST_F(HostResolverImplTest, EmptyHost) {
   resolver_proc->AllowDirectLookup("*");
 
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
   AddressList adrlist;
   const int kPortnum = 5555;
   HostResolver::RequestInfo info("", kPortnum);
@@ -410,7 +411,7 @@ TEST_F(HostResolverImplTest, DeDupeRequests) {
       new CapturingHostResolverProc(NULL);
 
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
 
   // The class will receive callbacks for when each resolve completes. It
   // checks that the right things happened.
@@ -461,7 +462,7 @@ TEST_F(HostResolverImplTest, CancelMultipleRequests) {
       new CapturingHostResolverProc(NULL);
 
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
 
   // The class will receive callbacks for when each resolve completes. It
   // checks that the right things happened.
@@ -548,7 +549,7 @@ TEST_F(HostResolverImplTest, CancelWithinCallback) {
       new CapturingHostResolverProc(NULL);
 
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, CreateDefaultCache()));
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL));
 
   // The class will receive callbacks for when each resolve completes. It
   // checks that the right things happened.
@@ -609,7 +610,7 @@ TEST_F(HostResolverImplTest, DeleteWithinCallback) {
   // checks that the right things happened. Note that the verifier holds the
   // only reference to |host_resolver|, so it can delete it within callback.
   HostResolver* host_resolver =
-      new HostResolverImpl(resolver_proc, CreateDefaultCache());
+      new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL);
   DeleteWithinCallbackVerifier verifier(host_resolver);
 
   // Start 4 requests, duplicating hosts "a". Since the resolver_proc is
@@ -662,7 +663,7 @@ TEST_F(HostResolverImplTest, StartWithinCallback) {
 
   // Turn off caching for this host resolver.
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(resolver_proc, NULL));
+      new HostResolverImpl(resolver_proc, NULL, NULL));
 
   // The class will receive callbacks for when each resolve completes. It
   // checks that the right things happened.
@@ -727,7 +728,7 @@ class BypassCacheVerifier : public ResolveRequest::Delegate {
 
 TEST_F(HostResolverImplTest, BypassCache) {
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(NULL, CreateDefaultCache()));
+      new HostResolverImpl(NULL, CreateDefaultCache(), NULL));
 
   // The class will receive callbacks for when each resolve completes. It
   // checks that the right things happened.
@@ -811,7 +812,7 @@ class CapturingObserver : public HostResolver::Observer {
 // synchronous.
 TEST_F(HostResolverImplTest, Observers) {
   scoped_refptr<HostResolver> host_resolver(
-      new HostResolverImpl(NULL, CreateDefaultCache()));
+      new HostResolverImpl(NULL, CreateDefaultCache(), NULL));
 
   CapturingObserver observer;
 
@@ -897,7 +898,7 @@ TEST_F(HostResolverImplTest, CancellationObserver) {
   {
     // Create a host resolver and attach an observer.
     scoped_refptr<HostResolver> host_resolver(
-        new HostResolverImpl(NULL, CreateDefaultCache()));
+        new HostResolverImpl(NULL, CreateDefaultCache(), NULL));
     host_resolver->AddObserver(&observer);
 
     TestCompletionCallback callback;
@@ -960,5 +961,37 @@ TEST_F(HostResolverImplTest, CancellationObserver) {
               CapturingObserver::StartOrCancelEntry(1, info));
 }
 
+// Test that IP address changes flush the cache.
+TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) {
+  scoped_refptr<MockNetworkChangeNotifier> mock_network_change_notifier(
+      new MockNetworkChangeNotifier);
+  scoped_refptr<HostResolver> host_resolver(
+      new HostResolverImpl(NULL, CreateDefaultCache(),
+                           mock_network_change_notifier));
+
+  AddressList addrlist;
+
+  // Resolve "host1".
+  HostResolver::RequestInfo info1("host1", 70);
+  TestCompletionCallback callback;
+  int rv = host_resolver->Resolve(info1, &addrlist, &callback, NULL, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  // Resolve "host1" again -- this time it will be served from cache, but it
+  // should still notify of completion.
+  rv = host_resolver->Resolve(info1, &addrlist, &callback, NULL, NULL);
+  ASSERT_EQ(OK, rv);  // Should complete synchronously.
+
+  // Flush cache by triggering an IP address change.
+  mock_network_change_notifier->NotifyIPAddressChange();
+
+  // Resolve "host1" again -- this time it won't be served from cache, so it
+  // will complete asynchronously.
+  rv = host_resolver->Resolve(info1, &addrlist, &callback, NULL, NULL);
+  ASSERT_EQ(ERR_IO_PENDING, rv);  // Should complete asynchronously.
+  EXPECT_EQ(OK, callback.WaitForResult());
+}
+
 }  // namespace
 }  // namespace net
diff --git a/net/base/mock_host_resolver.cc b/net/base/mock_host_resolver.cc
index cf38490..0bac1ff 100644
--- a/net/base/mock_host_resolver.cc
+++ b/net/base/mock_host_resolver.cc
@@ -102,7 +102,7 @@ void MockHostResolverBase::Reset(HostResolverProc* interceptor) {
         base::TimeDelta::FromSeconds(0));
   }
 
-  impl_ = new HostResolverImpl(proc, cache);
+  impl_ = new HostResolverImpl(proc, cache, NULL);
 }
 
 //-----------------------------------------------------------------------------
diff --git a/net/tools/hresolv/hresolv.cc b/net/tools/hresolv/hresolv.cc
index ed63330..6581bdc 100644
--- a/net/tools/hresolv/hresolv.cc
+++ b/net/tools/hresolv/hresolv.cc
@@ -446,7 +446,7 @@ int main(int argc, char** argv) {
       base::TimeDelta::FromSeconds(0));
 
   scoped_refptr<net::HostResolver> host_resolver(
-      new net::HostResolverImpl(NULL, cache));
+      new net::HostResolverImpl(NULL, cache, NULL));
   ResolverInvoker invoker(host_resolver.get());
   invoker.ResolveAll(hosts_and_times, options.async);