Remove custom HostResolver mock in http tests

Added functionality to MockCachingHostResolver to handle automatic
invalidation of cache entries after X resolves from cache.

As this was the last override of MockHostResolverBase, convert the
constructor to private with friends to force everything to deal with
MockHostResolver or MockCachingHostResolver as that seems to be the
intention of MockHostResolverBase.

Bug: 922699
Change-Id: I245e9ee32683f67cfcc7b1ad9a406cfbd223f56e
Reviewed-on: https://chromium-review.googlesource.com/c/1478354
Auto-Submit: Eric Orth <ericorth@chromium.org>
Reviewed-by: Asanka Herath <asanka@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633870}

net/dns/mock_host_resolver.cc

@@ -482,10 +482,12 @@ void MockHostResolverBase::TriggerMdnsListeners(
 }
 
 // start id from 1 to distinguish from NULL RequestHandle
-MockHostResolverBase::MockHostResolverBase(bool use_caching)
+MockHostResolverBase::MockHostResolverBase(bool use_caching,
+                                           int cache_invalidation_num)
     : last_request_priority_(DEFAULT_PRIORITY),
       synchronous_mode_(false),
       ondemand_mode_(false),
+      initial_cache_invalidation_num_(cache_invalidation_num),
       next_request_id_(1),
       num_resolve_(0),
       num_resolve_from_cache_(0),
@@ -497,9 +499,10 @@ MockHostResolverBase::MockHostResolverBase(bool use_caching)
   rules_map_[HostResolverSource::MULTICAST_DNS] =
       CreateCatchAllHostResolverProc();
 
-  if (use_caching) {
+  if (use_caching)
     cache_.reset(new HostCache(kMaxCacheEntries));
-  }
+  else
+    DCHECK_GE(0, cache_invalidation_num);
 }
 
 int MockHostResolverBase::Resolve(RequestImpl* request) {
@@ -602,6 +605,18 @@ int MockHostResolverBase::ResolveFromIPLiteralOrCache(
             AddressList::CopyWithPort(entry->addresses().value(), host.port());
         *out_stale_info = std::move(stale_info);
       }
+
+      auto cache_invalidation_iterator = cache_invalidation_nums_.find(key);
+      if (cache_invalidation_iterator != cache_invalidation_nums_.end()) {
+        DCHECK_LE(1, cache_invalidation_iterator->second);
+        cache_invalidation_iterator->second--;
+        if (cache_invalidation_iterator->second == 0) {
+          HostCache::Entry new_entry(*entry);
+          cache_->Set(key, new_entry, tick_clock_->NowTicks(),
+                      base::TimeDelta());
+          cache_invalidation_nums_.erase(cache_invalidation_iterator);
+        }
+      }
     }
   }
   return rv;
@@ -624,8 +639,11 @@ int MockHostResolverBase::ResolveProc(const HostPortPair& host,
                        flags, source);
     // Storing a failure with TTL 0 so that it overwrites previous value.
     base::TimeDelta ttl;
-    if (rv == OK)
+    if (rv == OK) {
       ttl = base::TimeDelta::FromSeconds(kCacheEntryTTLSeconds);
+      if (initial_cache_invalidation_num_ > 0)
+        cache_invalidation_nums_[key] = initial_cache_invalidation_num_;
+    }
     cache_->Set(key,
                 HostCache::Entry(rv, addr, HostCache::Entry::SOURCE_UNKNOWN),
                 tick_clock_->NowTicks(), ttl);

net/dns/mock_host_resolver.h

@@ -216,12 +216,17 @@ class MockHostResolverBase
     tick_clock_ = tick_clock;
   }
 
- protected:
-  explicit MockHostResolverBase(bool use_caching);
-
  private:
+  friend class MockHostResolver;
+  friend class MockCachingHostResolver;
+
   typedef std::map<size_t, RequestImpl*> RequestMap;
 
+  // If > 0, |cache_invalidation_num| is the number of times a cached entry can
+  // be read before it invalidates itself. Useful to force cache expiration
+  // scenarios.
+  explicit MockHostResolverBase(bool use_caching, int cache_invalidation_num);
+
   // Handle resolution for |request|. Expected to be called only the RequestImpl
   // object itself.
   int Resolve(RequestImpl* request);
@@ -253,6 +258,9 @@ class MockHostResolverBase
       rules_map_;
   std::unique_ptr<HostCache> cache_;
 
+  const int initial_cache_invalidation_num_;
+  std::map<HostCache::Key, int> cache_invalidation_nums_;
+
   // Maintain non-owning pointers to outstanding requests and listeners to allow
   // completing/notifying them. The objects are owned by callers, and should be
   // removed from |this| on destruction by calling DetachRequest() or
@@ -274,7 +282,9 @@ class MockHostResolverBase
 
 class MockHostResolver : public MockHostResolverBase {
  public:
-  MockHostResolver() : MockHostResolverBase(false /*use_caching*/) {}
+  MockHostResolver()
+      : MockHostResolverBase(false /*use_caching*/,
+                             0 /* cache_invalidation_num */) {}
   ~MockHostResolver() override {}
 };
 
@@ -285,7 +295,11 @@ class MockHostResolver : public MockHostResolverBase {
 // operation mode in case that is what you needed from the caching version).
 class MockCachingHostResolver : public MockHostResolverBase {
  public:
-  MockCachingHostResolver() : MockHostResolverBase(true /*use_caching*/) {}
+  // If > 0, |cache_invalidation_num| is the number of times a cached entry can
+  // be read before it invalidates itself. Useful to force cache expiration
+  // scenarios.
+  explicit MockCachingHostResolver(int cache_invalidation_num = 0)
+      : MockHostResolverBase(true /*use_caching*/, cache_invalidation_num) {}
   ~MockCachingHostResolver() override {}
 };
 

net/http/http_network_transaction_unittest.cc

@@ -51,7 +51,6 @@
 #include "net/base/upload_file_element_reader.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/mock_cert_verifier.h"
-#include "net/dns/host_cache.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/http/http_auth_challenge_tokenizer.h"
 #include "net/http/http_auth_handler_digest.h"
@@ -15744,30 +15743,11 @@ TEST_F(HttpNetworkTransactionTest, ReturnHTTP421OnRetry) {
   EXPECT_EQ("hello!", response_data);
 }
 
-class OneTimeCachingHostResolver : public MockHostResolverBase {
- public:
-  explicit OneTimeCachingHostResolver(const HostPortPair& host_port)
-      : MockHostResolverBase(/* use_caching = */ true), host_port_(host_port) {}
-  ~OneTimeCachingHostResolver() override = default;
-
-  int ResolveFromCache(const RequestInfo& info,
-                       AddressList* addresses,
-                       const NetLogWithSource& net_log) override {
-    int rv = MockHostResolverBase::ResolveFromCache(info, addresses, net_log);
-    if (rv == OK && info.host_port_pair().Equals(host_port_))
-      GetHostCache()->clear();
-    return rv;
-  }
-
- private:
-  const HostPortPair host_port_;
-};
-
 TEST_F(HttpNetworkTransactionTest,
        UseIPConnectionPoolingWithHostCacheExpiration) {
-  // Set up a special HttpNetworkSession with a OneTimeCachingHostResolver.
-  session_deps_.host_resolver = std::make_unique<OneTimeCachingHostResolver>(
-      HostPortPair("mail.example.com", 443));
+  // Set up HostResolver to invalidate cached entries after 1 cached resolve.
+  session_deps_.host_resolver =
+      std::make_unique<MockCachingHostResolver>(1 /* cache_invalidation_num */);
   std::unique_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
 
   AddSSLSocketData();