Prevent making real DNS lookups by chrome tests.

- by default a test which makes external DNS lookup directly or indirectly will fail

- added a quite simple way to allow a test to make external queries

- added a way to make external queries fail (for tests which don't need them to succeed but it's hard to not make the query)

- made neccessary adjustments to existing tests so that they still pass

http://crbug.com/9109

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

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

9 files changed, 104 insertions, 18 deletions

diff --git a/chrome/browser/extensions/extension_view_unittest.cc b/chrome/browser/extensions/extension_view_unittest.cc
index bc17d4c..3eb614f 100755
--- a/chrome/browser/extensions/extension_view_unittest.cc
+++ b/chrome/browser/extensions/extension_view_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/message_loop.h"
+#include "base/ref_counted.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/profile.h"
@@ -13,6 +14,7 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
+#include "net/base/host_resolver_unittest.h"
 
 namespace {
 
@@ -99,6 +101,14 @@ class ExtensionLoadedObserver : public NotificationObserver {
 
 class ExtensionViewTest : public InProcessBrowserTest {
  public:
+  ExtensionViewTest() {
+    host_mapper_ = new net::RuleBasedHostMapper();
+    // TODO(aa): Don't make a real dns lookup here or simulate a failing lookup.
+    // But if it's really needed then remove the TODO.
+    host_mapper_->AllowDirectLookup("*.google.com");
+    scoped_host_mapper_.Init(host_mapper_.get());
+  }
+
   virtual void SetUp() {
     // Initialize the error reporter here, otherwise BrowserMain will create it
     // with the wrong MessageLoop.
@@ -109,6 +119,10 @@ class ExtensionViewTest : public InProcessBrowserTest {
 
     InProcessBrowserTest::SetUp();
   }
+
+ private:
+  scoped_refptr<net::RuleBasedHostMapper> host_mapper_;
+  net::ScopedHostMapper scoped_host_mapper_;
 };
 
 // Tests that ExtensionView starts an extension process and runs the script
diff --git a/chrome/browser/net/dns_master_unittest.cc b/chrome/browser/net/dns_master_unittest.cc
index b57a2f0..a997b18 100644
--- a/chrome/browser/net/dns_master_unittest.cc
+++ b/chrome/browser/net/dns_master_unittest.cc
@@ -82,9 +82,10 @@ class DnsMasterTest : public testing::Test {
     MessageLoop::current()->Run();
   }
 
+  scoped_refptr<net::RuleBasedHostMapper> mapper_;
+
  private:
   MessageLoop loop;
-  scoped_refptr<net::RuleBasedHostMapper> mapper_;
   net::ScopedHostMapper scoped_mapper_;
 };
 
@@ -121,6 +122,8 @@ TimeDelta BlockingDnsLookup(const std::string& hostname) {
 // First test to be sure the OS is caching lookups, which is the whole premise
 // of DNS prefetching.
 TEST_F(DnsMasterTest, OsCachesLookupsTest) {
+  mapper_->AllowDirectLookup("*.google.com");
+
   const Time start = Time::Now();
   int all_lookups = 0;
   int lookups_with_improvement = 0;
@@ -273,14 +276,16 @@ TEST_F(DnsMasterTest, SingleLookupTest) {
 }
 
 TEST_F(DnsMasterTest, ConcurrentLookupTest) {
+  mapper_->AddSimulatedFailure("*.notfound");
+
   DnsMaster testing_master;
 
   std::string goog("www.google.com"),
     goog2("gmail.google.com.com"),
     goog3("mail.google.com"),
     goog4("gmail.com");
-  std::string bad1(GetNonexistantDomain()),
-    bad2(GetNonexistantDomain());
+  std::string bad1("bad1.notfound"),
+    bad2("bad2.notfound");
 
   NameList names;
   names.insert(names.end(), goog);
@@ -324,11 +329,13 @@ TEST_F(DnsMasterTest, ConcurrentLookupTest) {
 }
 
 TEST_F(DnsMasterTest, MassiveConcurrentLookupTest) {
+  mapper_->AddSimulatedFailure("*.notfound");
+
   DnsMaster testing_master;
 
   NameList names;
   for (int i = 0; i < 100; i++)
-    names.push_back(GetNonexistantDomain());
+    names.push_back("host" + IntToString(i) + ".notfound");
 
   // Try to flood the master with many concurrent requests.
   for (int i = 0; i < 10; i++)
diff --git a/chrome/browser/views/find_bar_win_unittest.cc b/chrome/browser/views/find_bar_win_unittest.cc
index 4ef345c..f2d2f74 100644
--- a/chrome/browser/views/find_bar_win_unittest.cc
+++ b/chrome/browser/views/find_bar_win_unittest.cc
@@ -13,6 +13,7 @@
 #include "chrome/common/notification_service.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
+#include "net/base/host_resolver_unittest.h"
 
 const std::wstring kFramePage = L"files/find_in_page/frames.html";
 const std::wstring kFrameData = L"files/find_in_page/framedata_general.html";
@@ -79,7 +80,13 @@ typedef enum FindInPageCase { IGNORE_CASE = 0, CASE_SENSITIVE = 1 };
 
 class FindInPageControllerTest : public InProcessBrowserTest {
  public:
-  FindInPageControllerTest() {}
+  FindInPageControllerTest() {
+    host_mapper_ = new net::RuleBasedHostMapper();
+    // Avoid making external DNS lookups. In this test we don't need this
+    // to succeed.
+    host_mapper_->AddSimulatedFailure("*.google.com");
+    scoped_host_mapper_.Init(host_mapper_.get());
+  }
 
  protected:
   int FindInPage(const std::wstring& search_string,
@@ -99,6 +106,10 @@ class FindInPageControllerTest : public InProcessBrowserTest {
     }
     return 0;
   }
+
+ private:
+  scoped_refptr<net::RuleBasedHostMapper> host_mapper_;
+  net::ScopedHostMapper scoped_host_mapper_;
 };
 
 // This test loads a page with frames and starts FindInPage requests
diff --git a/chrome/test/unit/chrome_test_suite.h b/chrome/test/unit/chrome_test_suite.h
index 8dcdbf6..1d7947f 100644
--- a/chrome/test/unit/chrome_test_suite.h
+++ b/chrome/test/unit/chrome_test_suite.h
@@ -13,6 +13,7 @@
 #include "base/mac_util.h"
 #endif
 #include "base/path_service.h"
+#include "base/ref_counted.h"
 #include "base/scoped_nsautorelease_pool.h"
 #include "base/test_suite.h"
 #include "chrome/app/scoped_ole_initializer.h"
@@ -21,6 +22,30 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/resource_bundle.h"
 #include "chrome/test/testing_browser_process.h"
+#include "net/base/host_resolver_unittest.h"
+
+// In many cases it may be not obvious that a test makes a real DNS lookup.
+// We generally don't want to rely on external DNS servers for our tests,
+// so this mapper catches external queries.
+class WarningHostMapper : public net::HostMapper {
+ public:
+  virtual std::string Map(const std::string& host) {
+    const char* kLocalHostNames[] = {"localhost", "127.0.0.1"};
+    bool local = false;
+    for (size_t i = 0; i < arraysize(kLocalHostNames); i++)
+      if (host == kLocalHostNames[i]) {
+        local = true;
+        break;
+      }
+
+    // Make the test fail so it's harder to ignore.
+    // If you really need to make real DNS query, use net::RuleBasedHostMapper
+    // and its AllowDirectLookup method.
+    EXPECT_TRUE(local) << "Making external DNS lookup of " << host;
+
+    return MapUsingPrevious(host);
+  }
+};
 
 class ChromeTestSuite : public TestSuite {
 public:
@@ -34,6 +59,9 @@ protected:
 
     TestSuite::Initialize();
 
+    host_mapper_ = new WarningHostMapper();
+    scoped_host_mapper_.Init(host_mapper_.get());
+
     chrome::RegisterPathProvider();
     g_browser_process = new TestingBrowserProcess;
 
@@ -90,6 +118,8 @@ protected:
 
   StatsTable* stats_table_;
   ScopedOleInitializer ole_initializer_;
+  scoped_refptr<WarningHostMapper> host_mapper_;
+  net::ScopedHostMapper scoped_host_mapper_;
 };
 
 #endif // CHROME_TEST_UNIT_CHROME_TEST_SUITE_H_
diff --git a/net/base/host_resolver.cc b/net/base/host_resolver.cc
index d5d97e7..a8580fd 100644
--- a/net/base/host_resolver.cc
+++ b/net/base/host_resolver.cc
@@ -52,13 +52,16 @@ static int HostResolverProc(
 
 static int ResolveAddrInfo(HostMapper* mapper, const std::string& host,
                            const std::string& port, struct addrinfo** out) {
-  int rv;
   if (mapper) {
-    rv = HostResolverProc(mapper->Map(host), port, out);
+    std::string mapped_host = mapper->Map(host);
+
+    if (mapped_host.empty())
+      return ERR_NAME_NOT_RESOLVED;
+
+    return HostResolverProc(mapped_host, port, out);
   } else {
-    rv = HostResolverProc(host, port, out);
+    return HostResolverProc(host, port, out);
   }
-  return rv;
 }
 
 //-----------------------------------------------------------------------------
diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h
index 6e61eb5..c730f76 100644
--- a/net/base/host_resolver.h
+++ b/net/base/host_resolver.h
@@ -59,6 +59,9 @@ class HostResolver {
 class HostMapper : public base::RefCountedThreadSafe<HostMapper> {
  public:
   virtual ~HostMapper() {}
+
+  // Returns possibly altered hostname, or empty string to simulate
+  // a failed lookup.
   virtual std::string Map(const std::string& host) = 0;
 
  protected:
diff --git a/net/base/host_resolver_unittest.h b/net/base/host_resolver_unittest.h
index e7a9c1a..f4c5854 100644
--- a/net/base/host_resolver_unittest.h
+++ b/net/base/host_resolver_unittest.h
@@ -51,8 +51,18 @@ class RuleBasedHostMapper : public HostMapper {
     rules_.push_back(Rule(host_pattern, replacement, latency));
   }
 
- private:
-  std::string Map(const std::string& host) {
+  // Make sure that |host| will not be re-mapped or even processed by underlying
+  // host mappers. It can also be a pattern.
+  void AllowDirectLookup(const char* host) {
+    rules_.push_back(Rule(host, "", true));
+  }
+
+  // Simulate a lookup failure for |host| (it also can be a pattern).
+  void AddSimulatedFailure(const char* host) {
+    AddRule(host, "");
+  }
+
+  virtual std::string Map(const std::string& host) {
     RuleList::iterator r;
     for (r = rules_.begin(); r != rules_.end(); ++r) {
       if (MatchPattern(host, r->host_pattern)) {
@@ -60,27 +70,36 @@ class RuleBasedHostMapper : public HostMapper {
           PlatformThread::Sleep(r->latency);
           r->latency = 1;  // Simulate cache warmup.
         }
-        return r->replacement;
+        return r->direct ? host : r->replacement;
       }
     }
+
     return MapUsingPrevious(host);
   }
 
+ private:
   struct Rule {
     std::string host_pattern;
     std::string replacement;
     int latency;  // in milliseconds
+    bool direct;  // if true, don't mangle hostname and ignore replacement
     Rule(const char* h, const char* r)
         : host_pattern(h),
           replacement(r),
-          latency(0) {}
+          latency(0),
+          direct(false) {}
     Rule(const char* h, const char* r, const int l)
         : host_pattern(h),
           replacement(r),
-          latency(l) {}
+          latency(l),
+          direct(false) {}
+    Rule(const char* h, const char* r, const bool d)
+        : host_pattern(h),
+          replacement(r),
+          latency(0),
+          direct(d) {}
   };
   typedef std::list<Rule> RuleList;
-
   RuleList rules_;
 };
 
@@ -104,7 +123,7 @@ class WaitingHostMapper : public HostMapper {
   }
 
  private:
-  std::string Map(const std::string& host) {
+  virtual std::string Map(const std::string& host) {
     event_.Wait();
     return MapUsingPrevious(host);
   }
diff --git a/net/base/ssl_client_socket_unittest.cc b/net/base/ssl_client_socket_unittest.cc
index 2148dfb..7b551fe 100644
--- a/net/base/ssl_client_socket_unittest.cc
+++ b/net/base/ssl_client_socket_unittest.cc
@@ -5,7 +5,6 @@
 #include "net/base/address_list.h"
 #include "net/base/client_socket_factory.h"
 #include "net/base/host_resolver.h"
-#include "net/base/host_resolver_unittest.h"
 #include "net/base/net_errors.h"
 #include "net/base/ssl_client_socket.h"
 #include "net/base/ssl_config_service.h"
diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc
index 283872d..33891b4 100644
--- a/net/http/http_network_layer_unittest.cc
+++ b/net/http/http_network_layer_unittest.cc
@@ -17,7 +17,7 @@ class HttpNetworkLayerTest : public PlatformTest {
         scoped_host_mapper_(host_mapper_.get()) {
     // TODO(darin): kill this exception once we have a way to test out the
     // HttpNetworkLayer class using loopback connections.
-    host_mapper_->AddRule("www.google.com", "www.google.com");
+    host_mapper_->AllowDirectLookup("www.google.com");
   }
 
  private: