Add synchronous-mode operation to MockHostResolver; this helps clarify some tests, which were using caching to get synchronous resolutions.TEST=existingBUG=NONE (addresses a TODO however).

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

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

6 files changed, 86 insertions, 47 deletions

diff --git a/chrome/browser/net/dns_master_unittest.cc b/chrome/browser/net/dns_master_unittest.cc
index c3746bb..321348d 100644
--- a/chrome/browser/net/dns_master_unittest.cc
+++ b/chrome/browser/net/dns_master_unittest.cc
@@ -132,8 +132,6 @@ TimeDelta BlockingDnsLookup(net::HostResolver* resolver,
 // First test to be sure the OS is caching lookups, which is the whole premise
 // of DNS prefetching.
 TEST_F(DnsMasterTest, OsCachesLookupsTest) {
-  // Make sure caching is disabled in the mock host resolver.
-  host_resolver_->Reset(NULL, 0, 0);
   host_resolver_->rules()->AllowDirectLookup("*.google.com");
 
   const Time start = Time::Now();
@@ -243,7 +241,7 @@ TEST_F(DnsMasterTest, BenefitLookupTest) {
 TEST_F(DnsMasterTest, ShutdownWhenResolutionIsPendingTest) {
   scoped_refptr<net::WaitingHostResolverProc> resolver_proc =
       new net::WaitingHostResolverProc(NULL);
-  host_resolver_->Reset(resolver_proc, 0, 0);
+  host_resolver_->Reset(resolver_proc);
 
   scoped_refptr<DnsMaster> testing_master = new DnsMaster(host_resolver_,
       MessageLoop::current(), default_max_queueing_delay_,
diff --git a/net/base/host_resolver_proc.h b/net/base/host_resolver_proc.h
index 2690987..66978f7 100644
--- a/net/base/host_resolver_proc.h
+++ b/net/base/host_resolver_proc.h
@@ -36,7 +36,7 @@ class HostResolverProc : public base::RefCountedThreadSafe<HostResolverProc> {
 
  private:
   friend class HostResolverImpl;
-  friend class MockHostResolver;
+  friend class MockHostResolverBase;
   friend class ScopedDefaultHostResolverProc;
 
   // Sets the previous procedure in the chain.
diff --git a/net/base/mock_host_resolver.cc b/net/base/mock_host_resolver.cc
index 58ad552..508ef38 100644
--- a/net/base/mock_host_resolver.cc
+++ b/net/base/mock_host_resolver.cc
@@ -11,36 +11,41 @@
 
 namespace net {
 
-MockHostResolver::MockHostResolver() {
-  Reset(NULL, 0, 0);
-}
-
-int MockHostResolver::Resolve(const RequestInfo& info,
-                              AddressList* addresses,
-                              CompletionCallback* callback,
-                              RequestHandle* out_req) {
+MockHostResolverBase::MockHostResolverBase(bool use_caching)
+    : use_caching_(use_caching) {
+  Reset(NULL);
+}
+
+int MockHostResolverBase::Resolve(const RequestInfo& info,
+                                  AddressList* addresses,
+                                  CompletionCallback* callback,
+                                  RequestHandle* out_req) {
+  if (synchronous_mode_) {
+    callback = NULL;
+    out_req = NULL;
+  }
   return impl_->Resolve(info, addresses, callback, out_req);
 }
 
-void MockHostResolver::CancelRequest(RequestHandle req) {
+void MockHostResolverBase::CancelRequest(RequestHandle req) {
   impl_->CancelRequest(req);
 }
 
-void MockHostResolver::AddObserver(Observer* observer) {
+void MockHostResolverBase::AddObserver(Observer* observer) {
   impl_->AddObserver(observer);
 }
 
-void MockHostResolver::RemoveObserver(Observer* observer) {
+void MockHostResolverBase::RemoveObserver(Observer* observer) {
   impl_->RemoveObserver(observer);
 }
 
-void MockHostResolver::Shutdown() {
+void MockHostResolverBase::Shutdown() {
   impl_->Shutdown();
 }
 
-void MockHostResolver::Reset(HostResolverProc* interceptor,
-                             int max_cache_entries,
-                             int max_cache_age_ms) {
+void MockHostResolverBase::Reset(HostResolverProc* interceptor) {
+  synchronous_mode_ = false;
+
   // At the root of the chain, map everything to localhost.
   scoped_refptr<RuleBasedHostResolverProc> catchall =
       new RuleBasedHostResolverProc(NULL);
@@ -57,6 +62,9 @@ void MockHostResolver::Reset(HostResolverProc* interceptor,
     proc = interceptor;
   }
 
+  int max_cache_entries = use_caching_ ? 100 : 0;
+  int max_cache_age_ms = use_caching_ ? 60000 : 0;
+
   impl_ = new HostResolverImpl(proc, max_cache_entries, max_cache_age_ms);
 }
 
diff --git a/net/base/mock_host_resolver.h b/net/base/mock_host_resolver.h
index ee83105..1d29684 100644
--- a/net/base/mock_host_resolver.h
+++ b/net/base/mock_host_resolver.h
@@ -34,14 +34,11 @@ class RuleBasedHostResolverProc;
 //
 // Replacement doesn't have to be string representing an IP address. It can
 // re-map one hostname to another as well.
-class MockHostResolver : public HostResolver {
- public:
-  // Creates a MockHostResolver that does NOT cache entries
-  // (the HostResolverProc will be called for every lookup). If you need
-  // caching behavior, call Reset() with non-zero cache size.
-  MockHostResolver();
 
-  virtual ~MockHostResolver() {}
+// Base class shared by MockHostResolver and MockCachingHostResolver.
+class MockHostResolverBase : public HostResolver {
+ public:
+  virtual ~MockHostResolverBase() {}
 
   // HostResolver methods:
   virtual int Resolve(const RequestInfo& info, AddressList* addresses,
@@ -54,14 +51,37 @@ class MockHostResolver : public HostResolver {
 
   RuleBasedHostResolverProc* rules() { return rules_; }
 
+  // Controls whether resolutions complete synchronously or asynchronously.
+  void set_synchronous_mode(bool is_synchronous) {
+    synchronous_mode_ = is_synchronous;
+  }
+
   // Resets the mock.
-  void Reset(HostResolverProc* interceptor,
-             int max_cache_entries,
-             int max_cache_age_ms);
+  void Reset(HostResolverProc* interceptor);
+
+ protected:
+  MockHostResolverBase(bool use_caching);
 
  private:
   scoped_refptr<HostResolverImpl> impl_;
   scoped_refptr<RuleBasedHostResolverProc> rules_;
+  bool synchronous_mode_;
+  bool use_caching_;
+};
+
+class MockHostResolver : public MockHostResolverBase {
+ public:
+  MockHostResolver() : MockHostResolverBase(false /*use_caching*/) {}
+};
+
+// Same as MockHostResolver, except internally it uses a host-cache.
+//
+// Note that tests are advised to use MockHostResolver instead, since it is
+// more predictable. (MockHostResolver also can be put into synchronous
+// operation mode in case that is what you needed from the caching version).
+class MockCachingHostResolver : public MockHostResolverBase {
+ public:
+  MockCachingHostResolver() : MockHostResolverBase(true /*use_caching*/) {}
 };
 
 // RuleBasedHostResolverProc applies a set of rules to map a host string to
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index c1510a1..44a06e7 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -42,7 +42,7 @@ class SessionDependencies {
   explicit SessionDependencies(ProxyService* proxy_service)
       : host_resolver(new MockHostResolver), proxy_service(proxy_service) {}
 
-  scoped_refptr<MockHostResolver> host_resolver;
+  scoped_refptr<MockHostResolverBase> host_resolver;
   scoped_ptr<ProxyService> proxy_service;
   MockClientSocketFactory socket_factory;
 };
@@ -3457,8 +3457,8 @@ TEST_F(HttpNetworkTransactionTest, ResolveMadeWithReferrer) {
 TEST_F(HttpNetworkTransactionTest, BypassHostCacheOnRefresh) {
   SessionDependencies session_deps;
 
-  // Enable caching in the mock host resolver (it is off by default).
-  session_deps.host_resolver->Reset(NULL, 100, 60000);
+  // Select a host resolver that does caching.
+  session_deps.host_resolver = new MockCachingHostResolver;
 
   scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(
       CreateSession(&session_deps), &session_deps.socket_factory));
diff --git a/net/socket/tcp_client_socket_pool_unittest.cc b/net/socket/tcp_client_socket_pool_unittest.cc
index b5cce2c..d81137d 100644
--- a/net/socket/tcp_client_socket_pool_unittest.cc
+++ b/net/socket/tcp_client_socket_pool_unittest.cc
@@ -228,9 +228,6 @@ class TCPClientSocketPoolTest : public testing::Test {
         pool_(new TCPClientSocketPool(kMaxSocketsPerGroup,
                                       host_resolver_,
                                       &client_socket_factory_)) {
-    // We enable caching on the mock host-resolver (it is off by default),
-    // because some of the tests in this file expect it.
-    host_resolver_->Reset(NULL, 100, 60000);
   }
 
   virtual void SetUp() {
@@ -281,8 +278,9 @@ TEST_F(TCPClientSocketPoolTest, InitConnectionFailure) {
   EXPECT_EQ(ERR_IO_PENDING,
             req.handle.Init("a", info, 5, &req));
   EXPECT_EQ(ERR_CONNECTION_FAILED, req.WaitForResult());
-  // HostCache caches it, so MockFailingClientSocket will cause Init() to
-  // synchronously fail.
+
+  // Make the host resolutions complete synchronously this time.
+  host_resolver_->set_synchronous_mode(true);
   EXPECT_EQ(ERR_CONNECTION_FAILED,
             req.handle.Init("a", info, 5, &req));
 }
@@ -301,7 +299,10 @@ TEST_F(TCPClientSocketPoolTest, PendingRequests) {
   EXPECT_EQ(ERR_IO_PENDING, rv);
   EXPECT_EQ(OK, reqs[0]->WaitForResult());
 
-  // Rest of them finish synchronously, since they're in the HostCache.
+  // Make all subsequent host resolutions complete synchronously.
+  host_resolver_->set_synchronous_mode(true);
+
+  // Rest of them finish synchronously.
   for (int i = 1; i < kMaxSocketsPerGroup; ++i) {
     rv = reqs[i]->handle.Init("a", info, 5, reqs[i].get());
     EXPECT_EQ(OK, rv);
@@ -362,7 +363,10 @@ TEST_F(TCPClientSocketPoolTest, PendingRequests_NoKeepAlive) {
   EXPECT_EQ(ERR_IO_PENDING, rv);
   EXPECT_EQ(OK, reqs[0]->WaitForResult());
 
-  // Rest of them finish synchronously, since they're in the HostCache.
+  // Make all subsequent host resolutions complete synchronously.
+  host_resolver_->set_synchronous_mode(true);
+
+  // Rest of them finish synchronously.
   for (int i = 1; i < kMaxSocketsPerGroup; ++i) {
     rv = reqs[i]->handle.Init("a", info, 5, reqs[i].get());
     EXPECT_EQ(OK, rv);
@@ -444,13 +448,15 @@ TEST_F(TCPClientSocketPoolTest, ConnectCancelConnect) {
   TestCompletionCallback callback2;
   EXPECT_EQ(ERR_IO_PENDING, handle.Init("a", info, 5, &callback2));
 
+  host_resolver_->set_synchronous_mode(true);
   // At this point, handle has two ConnectingSockets out for it.  Due to the
-  // host cache, the host resolution for both will return in the same loop of
-  // the MessageLoop.  The client socket is a pending socket, so the Connect()
-  // will asynchronously complete on the next loop of the MessageLoop.  That
-  // means that the first ConnectingSocket will enter OnIOComplete, and then the
-  // second one will.  If the first one is not cancelled, it will advance the
-  // load state, and then the second one will crash.
+  // setting the mock resolver into synchronous mode, the host resolution for
+  // both will return in the same loop of the MessageLoop.  The client socket
+  // is a pending socket, so the Connect() will asynchronously complete on the
+  // next loop of the MessageLoop.  That means that the first
+  // ConnectingSocket will enter OnIOComplete, and then the second one will.
+  // If the first one is not cancelled, it will advance the load state, and
+  // then the second one will crash.
 
   EXPECT_EQ(OK, callback2.WaitForResult());
   EXPECT_FALSE(callback.have_result());
@@ -472,7 +478,10 @@ TEST_F(TCPClientSocketPoolTest, CancelRequest) {
   EXPECT_EQ(ERR_IO_PENDING, rv);
   EXPECT_EQ(OK, reqs[0]->WaitForResult());
 
-  // Rest of them finish synchronously, since they're in the HostCache.
+  // Make all subsequent host resolutions complete synchronously.
+  host_resolver_->set_synchronous_mode(true);
+
+  // Rest of them finish synchronously.
   for (int i = 1; i < kMaxSocketsPerGroup; ++i) {
     rv = reqs[i]->handle.Init("a", info, 5, reqs[i].get());
     EXPECT_EQ(OK, rv);
@@ -529,7 +538,7 @@ TEST_F(TCPClientSocketPoolTest, CancelRequest) {
 
 class RequestSocketCallback : public CallbackRunner< Tuple1<int> > {
  public:
-  RequestSocketCallback(ClientSocketHandle* handle)
+  explicit RequestSocketCallback(ClientSocketHandle* handle)
       : handle_(handle),
         within_callback_(false) {}
 
@@ -563,6 +572,10 @@ TEST_F(TCPClientSocketPoolTest, RequestTwice) {
       "a", HostResolver::RequestInfo("www.google.com", 80), 0, &callback);
   ASSERT_EQ(ERR_IO_PENDING, rv);
 
+  // The callback is going to request "www.google.com". We want it to complete
+  // synchronously this time.
+  host_resolver_->set_synchronous_mode(true);
+
   EXPECT_EQ(OK, callback.WaitForResult());
 
   handle.Reset();