Add source support to mock resolver.

In order to avoid incompatible changes to RulesBasedHostResolverProc,
which is used all over the place (see crbug.com/878838), created
separate internal rules procs to handle those requests specifying
different sources.

Updated tests for the one piece of code currently specifying SYSTEM
source (via flag through the old API).

Bug: 846423
Change-Id: I1b92384d831579fe119817afc3e0cb92cfdf4565
Reviewed-on: https://chromium-review.googlesource.com/1195547
Commit-Queue: Eric Orth <ericorth@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587746}

net/dns/mock_host_resolver.cc

@@ -63,23 +63,10 @@ class MockHostResolverBase::RequestImpl
   RequestImpl(const HostPortPair& request_host,
               const base::Optional<ResolveHostParameters>& optional_parameters,
               base::WeakPtr<MockHostResolverBase> resolver)
-      : RequestImpl(request_host,
-                    optional_parameters,
-                    0 /* host_resolver_flags */,
-                    true /* allow_cached_response */,
-                    resolver) {}
-
-  RequestImpl(const HostPortPair& request_host,
-              const base::Optional<ResolveHostParameters>& optional_parameters,
-              HostResolverFlags host_resolver_flags,
-              bool allow_cached_response,
-              base::WeakPtr<MockHostResolverBase> resolver)
       : request_host_(request_host),
-        allow_cached_response_(allow_cached_response),
         parameters_(optional_parameters ? optional_parameters.value()
                                         : ResolveHostParameters()),
-        host_resolver_flags_(host_resolver_flags |
-                             ParametersToHostResolverFlags(parameters_)),
+        host_resolver_flags_(ParametersToHostResolverFlags(parameters_)),
         id_(0),
         resolver_(resolver),
         complete_(false) {}
@@ -143,8 +130,6 @@ class MockHostResolverBase::RequestImpl
 
   const HostPortPair& request_host() const { return request_host_; }
 
-  bool allow_cached_response() const { return allow_cached_response_; }
-
   const ResolveHostParameters& parameters() const { return parameters_; }
 
   int host_resolver_flags() const { return host_resolver_flags_; }
@@ -162,7 +147,6 @@ class MockHostResolverBase::RequestImpl
 
  private:
   const HostPortPair request_host_;
-  bool allow_cached_response_;
   const ResolveHostParameters parameters_;
   int host_resolver_flags_;
 
@@ -259,7 +243,7 @@ int MockHostResolverBase::Resolve(const RequestInfo& info,
 
   auto request = std::make_unique<RequestImpl>(
       info.host_port_pair(), RequestInfoToResolveHostParameters(info, priority),
-      info.host_resolver_flags(), info.allow_cached_response(), AsWeakPtr());
+      AsWeakPtr());
   auto wrapped_request =
       std::make_unique<LegacyRequestImpl>(std::move(request));
 
@@ -284,7 +268,7 @@ int MockHostResolverBase::ResolveFromCache(const RequestInfo& info,
   next_request_id_++;
   int rv = ResolveFromIPLiteralOrCache(
       info.host_port_pair(), info.address_family(), info.host_resolver_flags(),
-      info.allow_cached_response(), addresses);
+      HostResolverSource::ANY, info.allow_cached_response(), addresses);
   return rv;
 }
 
@@ -298,7 +282,7 @@ int MockHostResolverBase::ResolveStaleFromCache(
   next_request_id_++;
   int rv = ResolveFromIPLiteralOrCache(
       info.host_port_pair(), info.address_family(), info.host_resolver_flags(),
-      info.allow_cached_response(), addresses);
+      HostResolverSource::ANY, info.allow_cached_response(), addresses);
   return rv;
 }
 
@@ -340,7 +324,9 @@ MockHostResolverBase::MockHostResolverBase(bool use_caching)
       next_request_id_(1),
       num_resolve_(0),
       num_resolve_from_cache_(0) {
-  rules_ = CreateCatchAllHostResolverProc();
+  rules_map_[HostResolverSource::ANY] = CreateCatchAllHostResolverProc();
+  rules_map_[HostResolverSource::SYSTEM] = CreateCatchAllHostResolverProc();
+  rules_map_[HostResolverSource::DNS] = CreateCatchAllHostResolverProc();
 
   if (use_caching) {
     cache_.reset(new HostCache(kMaxCacheEntries));
@@ -356,8 +342,8 @@ int MockHostResolverBase::Resolve(RequestImpl* request) {
   int rv = ResolveFromIPLiteralOrCache(
       request->request_host(),
       DnsQueryTypeToAddressFamily(request->parameters().dns_query_type),
-      request->host_resolver_flags(), request->allow_cached_response(),
-      &addresses);
+      request->host_resolver_flags(), request->parameters().source,
+      request->parameters().allow_cached_response, &addresses);
   if (rv == OK && !request->parameters().is_speculative)
     request->set_address_results(addresses);
   if (rv != ERR_DNS_CACHE_MISS)
@@ -372,7 +358,8 @@ int MockHostResolverBase::Resolve(RequestImpl* request) {
     int rv = ResolveProc(
         request->request_host(),
         DnsQueryTypeToAddressFamily(request->parameters().dns_query_type),
-        request->host_resolver_flags(), &addresses);
+        request->host_resolver_flags(), request->parameters().source,
+        &addresses);
     if (rv == OK && !request->parameters().is_speculative)
       request->set_address_results(addresses);
     return rv;
@@ -396,6 +383,7 @@ int MockHostResolverBase::ResolveFromIPLiteralOrCache(
     const HostPortPair& host,
     AddressFamily requested_address_family,
     HostResolverFlags flags,
+    HostResolverSource source,
     bool allow_cache,
     AddressList* addresses,
     HostCache::EntryStaleness* stale_info) {
@@ -414,7 +402,7 @@ int MockHostResolverBase::ResolveFromIPLiteralOrCache(
   }
   int rv = ERR_DNS_CACHE_MISS;
   if (cache_.get() && allow_cache) {
-    HostCache::Key key(host.host(), requested_address_family, flags);
+    HostCache::Key key(host.host(), requested_address_family, flags, source);
     const HostCache::Entry* entry;
     if (stale_info)
       entry = cache_->LookupStale(key, base::TimeTicks::Now(), stale_info);
@@ -432,12 +420,15 @@ int MockHostResolverBase::ResolveFromIPLiteralOrCache(
 int MockHostResolverBase::ResolveProc(const HostPortPair& host,
                                       AddressFamily requested_address_family,
                                       HostResolverFlags flags,
+                                      HostResolverSource source,
                                       AddressList* addresses) {
+  DCHECK(rules_map_.find(source) != rules_map_.end());
+
   AddressList addr;
-  int rv = rules_->Resolve(host.host(), requested_address_family, flags, &addr,
-                           nullptr);
+  int rv = rules_map_[source]->Resolve(host.host(), requested_address_family,
+                                       flags, &addr, nullptr);
   if (cache_.get()) {
-    HostCache::Key key(host.host(), requested_address_family, flags);
+    HostCache::Key key(host.host(), requested_address_family, flags, source);
     // Storing a failure with TTL 0 so that it overwrites previous value.
     base::TimeDelta ttl;
     if (rv == OK)
@@ -460,10 +451,10 @@ void MockHostResolverBase::ResolveNow(size_t id) {
   requests_.erase(it);
 
   AddressList addresses;
-  int error =
-      ResolveProc(req->request_host(),
-                  DnsQueryTypeToAddressFamily(req->parameters().dns_query_type),
-                  req->host_resolver_flags(), &addresses);
+  int error = ResolveProc(
+      req->request_host(),
+      DnsQueryTypeToAddressFamily(req->parameters().dns_query_type),
+      req->host_resolver_flags(), req->parameters().source, &addresses);
   if (error == OK && !req->parameters().is_speculative)
     req->set_address_results(addresses);
   req->OnAsyncCompleted(id, error);

net/dns/mock_host_resolver.h

@@ -20,6 +20,7 @@
 #include "net/base/completion_once_callback.h"
 #include "net/dns/host_resolver.h"
 #include "net/dns/host_resolver_proc.h"
+#include "net/dns/host_resolver_source.h"
 
 namespace net {
 
@@ -56,6 +57,14 @@ int ParseAddressList(const std::string& host_list,
 //
 // By default, MockHostResolvers include a single rule that maps all hosts to
 // 127.0.0.1.
+//
+// Separate rules are used for separate HostResolverSource (eg
+// HostResolverSource::SYSTEM for requests that should only be resolved using
+// the system resolver).  Use rules_map() to access the separate rules if tests
+// involve requests specifying sources:
+//
+//    host_resolver->rules_map()[HostResolverSource::DNS]->AddRule("foo.com",
+//                                                                 "1.2.3.4");
 
 // Base class shared by MockHostResolver and MockCachingHostResolver.
 class MockHostResolverBase
@@ -68,8 +77,16 @@ class MockHostResolverBase
  public:
   ~MockHostResolverBase() override;
 
-  RuleBasedHostResolverProc* rules() { return rules_.get(); }
-  void set_rules(RuleBasedHostResolverProc* rules) { rules_ = rules; }
+  RuleBasedHostResolverProc* rules() {
+    return rules_map_[HostResolverSource::ANY].get();
+  }
+  void set_rules(RuleBasedHostResolverProc* rules) {
+    rules_map_[HostResolverSource::ANY] = rules;
+  }
+  std::map<HostResolverSource, scoped_refptr<RuleBasedHostResolverProc>>
+  rules_map() {
+    return rules_map_;
+  }
 
   // Controls whether resolutions complete synchronously or asynchronously.
   void set_synchronous_mode(bool is_synchronous) {
@@ -152,6 +169,7 @@ class MockHostResolverBase
       const HostPortPair& host,
       AddressFamily requested_address_family,
       HostResolverFlags flags,
+      HostResolverSource source,
       bool allow_cache,
       AddressList* addresses,
       HostCache::EntryStaleness* stale_info = nullptr);
@@ -159,6 +177,7 @@ class MockHostResolverBase
   int ResolveProc(const HostPortPair& host,
                   AddressFamily requested_address_family,
                   HostResolverFlags flags,
+                  HostResolverSource source,
                   AddressList* addresses);
   // Resolve request stored in |requests_|. Pass rv to callback.
   void ResolveNow(size_t id);
@@ -166,7 +185,8 @@ class MockHostResolverBase
   RequestPriority last_request_priority_;
   bool synchronous_mode_;
   bool ondemand_mode_;
-  scoped_refptr<RuleBasedHostResolverProc> rules_;
+  std::map<HostResolverSource, scoped_refptr<RuleBasedHostResolverProc>>
+      rules_map_;
   std::unique_ptr<HostCache> cache_;
   RequestMap requests_;
   size_t next_request_id_;

net/proxy_resolution/pac_file_decider_unittest.cc

@@ -370,7 +370,7 @@ class PacFileDeciderQuickCheckTest : public TestWithScopedTaskEnvironment {
 // Fails if a synchronous DNS lookup success for wpad causes QuickCheck to fail.
 TEST_F(PacFileDeciderQuickCheckTest, SyncSuccess) {
   resolver_.set_synchronous_mode(true);
-  resolver_.rules()->AddRule("wpad", "1.2.3.4");
+  resolver_.rules_map()[HostResolverSource::SYSTEM]->AddRule("wpad", "1.2.3.4");
 
   EXPECT_THAT(StartDecider(), IsOk());
   EXPECT_EQ(rule_.text(), decider_->script_data()->utf16());
@@ -383,7 +383,7 @@ TEST_F(PacFileDeciderQuickCheckTest, SyncSuccess) {
 // fail.
 TEST_F(PacFileDeciderQuickCheckTest, AsyncSuccess) {
   resolver_.set_ondemand_mode(true);
-  resolver_.rules()->AddRule("wpad", "1.2.3.4");
+  resolver_.rules_map()[HostResolverSource::SYSTEM]->AddRule("wpad", "1.2.3.4");
 
   EXPECT_THAT(StartDecider(), IsError(ERR_IO_PENDING));
   ASSERT_TRUE(resolver_.has_pending_requests());
@@ -399,7 +399,8 @@ TEST_F(PacFileDeciderQuickCheckTest, AsyncSuccess) {
 // PacFileDecider to yield a PAC URL.
 TEST_F(PacFileDeciderQuickCheckTest, AsyncFail) {
   resolver_.set_ondemand_mode(true);
-  resolver_.rules()->AddSimulatedFailure("wpad");
+  resolver_.rules_map()[HostResolverSource::SYSTEM]->AddSimulatedFailure(
+      "wpad");
   EXPECT_THAT(StartDecider(), IsError(ERR_IO_PENDING));
   ASSERT_TRUE(resolver_.has_pending_requests());
   resolver_.ResolveAllPending();
@@ -439,7 +440,8 @@ TEST_F(PacFileDeciderQuickCheckTest, QuickCheckInhibitsDhcp) {
 TEST_F(PacFileDeciderQuickCheckTest, QuickCheckDisabled) {
   const char* kPac = "function FindProxyForURL(u,h) { return \"DIRECT\"; }";
   resolver_.set_synchronous_mode(true);
-  resolver_.rules()->AddSimulatedFailure("wpad");
+  resolver_.rules_map()[HostResolverSource::SYSTEM]->AddSimulatedFailure(
+      "wpad");
   MockPacFileFetcher fetcher;
   decider_.reset(new PacFileDecider(&fetcher, &dhcp_fetcher_, NULL));
   EXPECT_THAT(StartDecider(), IsError(ERR_IO_PENDING));
@@ -451,8 +453,10 @@ TEST_F(PacFileDeciderQuickCheckTest, ExplicitPacUrl) {
   const char* kCustomUrl = "http://custom/proxy.pac";
   config_.set_pac_url(GURL(kCustomUrl));
   Rules::Rule rule = rules_.AddSuccessRule(kCustomUrl);
-  resolver_.rules()->AddSimulatedFailure("wpad");
-  resolver_.rules()->AddRule("custom", "1.2.3.4");
+  resolver_.rules_map()[HostResolverSource::SYSTEM]->AddSimulatedFailure(
+      "wpad");
+  resolver_.rules_map()[HostResolverSource::SYSTEM]->AddRule("custom",
+                                                             "1.2.3.4");
   EXPECT_THAT(StartDecider(), IsError(ERR_IO_PENDING));
   callback_.WaitForResult();
   EXPECT_TRUE(decider_->effective_config().value().has_pac_url());