Create ProxyHostResolverCache

Serves as a replacement for the net::HostCache usage in
/services/proxy_resolver.

Nice features of this cache over net::HostCache:
* Divorced from net::HostCache to allow that class to make its own
  changes with less dependencies to worry about.
* More performant (lg(n)) eviction when full.  net::HostCache is more
  complicated and always needs to do a full linear scan of the entire
  cache.
* Stale entries always removed when found. net::HostCache needs to deal
  with a lot more complications around keeping and sometimes using
  stale entries.
* Key/entry types specialized to proxy usage instead of having a bunch
  of unused fields and requiring some conversions.
* TTL logic better encapsulated instead of making callers deal with
  passing in times and deciding TTL.
* Skips lots of net::HostCache functionality that was completely
  unnecessary for proxy script resolution usage.

Old logic used default cache size from
net::HostCache::CreateDefaultCache(), which has a max size of 100 or
1000 depending on ENABLE_BUILT_IN_DNS. But this code's caching needs
has no relation to whether or not the built-in DNS resolver is being
used, so I arbitrarily split the difference in setting the new cache's
max size to 500.

Also worth noting that the new cache is only used for positive results.
HostResolverMojo previously saved error results into its
net::HostCache, but it did so with a TTL of 0, resulting in the entry
being stale and unusable as soon as any measurable amount of time has
passed.

Fixed: 1229361
Change-Id: I02f2c37f3a3419997d65d4ab90a0df31f9a23c07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3032014
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#903641}
diff --git a/net/dns/BUILD.gn b/net/dns/BUILD.gn
index 851032d..ce3c9cf 100644
--- a/net/dns/BUILD.gn
+++ b/net/dns/BUILD.gn
@@ -207,10 +207,6 @@
     "//components/cronet/*",
     "//net/*",
     "//services/network/*",
-
-    # The proxy resolution service uses its own host cache and HostResolver Mojo
-    # wrapper.
-    "//services/proxy_resolver/*",
   ]
 
   sources = [
diff --git a/services/proxy_resolver/BUILD.gn b/services/proxy_resolver/BUILD.gn
index 0fb7da2f..3f1bdda 100644
--- a/services/proxy_resolver/BUILD.gn
+++ b/services/proxy_resolver/BUILD.gn
@@ -11,6 +11,8 @@
     "mojo_proxy_resolver_v8_tracing_bindings.h",
     "pac_js_library.h",
     "proxy_host_resolver.h",
+    "proxy_host_resolver_cache.cc",
+    "proxy_host_resolver_cache.h",
     "proxy_resolver_factory_impl.cc",
     "proxy_resolver_factory_impl.h",
     "proxy_resolver_impl.cc",
@@ -51,6 +53,7 @@
     "mock_proxy_host_resolver.cc",
     "mock_proxy_host_resolver.h",
     "mojo_proxy_resolver_v8_tracing_bindings_unittest.cc",
+    "proxy_host_resolver_cache_unittest.cc",
     "proxy_resolver_factory_impl_unittest.cc",
     "proxy_resolver_impl_unittest.cc",
     "proxy_resolver_v8_tracing_unittest.cc",
diff --git a/services/proxy_resolver/host_resolver_mojo.cc b/services/proxy_resolver/host_resolver_mojo.cc
index 018dbc9..adc87721 100644
--- a/services/proxy_resolver/host_resolver_mojo.cc
+++ b/services/proxy_resolver/host_resolver_mojo.cc
@@ -10,39 +10,23 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
-#include "base/time/time.h"
+#include "base/check.h"
+#include "base/memory/weak_ptr.h"
 #include "mojo/public/cpp/bindings/receiver.h"
-#include "net/base/address_family.h"
-#include "net/base/address_list.h"
 #include "net/base/completion_once_callback.h"
 #include "net/base/ip_address.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_isolation_key.h"
-#include "net/dns/host_resolver_source.h"
-#include "net/dns/public/dns_query_type.h"
+#include "net/proxy_resolution/proxy_resolve_dns_operation.h"
+#include "services/proxy_resolver/proxy_host_resolver_cache.h"
 
 namespace proxy_resolver {
+
 namespace {
 
-// Default TTL for successful host resolutions.
-constexpr auto kCacheEntryTTL = base::TimeDelta::FromSeconds(5);
-
-// Default TTL for unsuccessful host resolutions.
-constexpr auto kNegativeCacheEntryTTL = base::TimeDelta();
-
-net::HostCache::Key CacheKeyForRequest(
-    const std::string& hostname,
-    const net::NetworkIsolationKey& network_isolation_key,
-    net::ProxyResolveDnsOperation operation) {
-  net::DnsQueryType dns_query_type = net::DnsQueryType::UNSPECIFIED;
-  if (operation == net::ProxyResolveDnsOperation::MY_IP_ADDRESS ||
-      operation == net::ProxyResolveDnsOperation::DNS_RESOLVE) {
-    dns_query_type = net::DnsQueryType::A;
-  }
-
-  return net::HostCache::Key(
-      hostname, dns_query_type, 0 /* host_resolver_flags */,
-      net::HostResolverSource::ANY, network_isolation_key);
+bool IsExOperation(net::ProxyResolveDnsOperation operation) {
+  return operation == net::ProxyResolveDnsOperation::DNS_RESOLVE_EX ||
+         operation == net::ProxyResolveDnsOperation::MY_IP_ADDRESS_EX;
 }
 
 }  // namespace
@@ -53,7 +37,7 @@
   RequestImpl(const std::string& hostname,
               net::ProxyResolveDnsOperation operation,
               const net::NetworkIsolationKey& network_isolation_key,
-              base::WeakPtr<net::HostCache> host_cache,
+              base::WeakPtr<ProxyHostResolverCache> host_cache,
               Impl* impl)
       : hostname_(hostname),
         operation_(operation),
@@ -68,10 +52,22 @@
     DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
     DVLOG(1) << "Resolve " << hostname_;
 
-    int cached_result = ResolveFromCacheInternal();
-    if (cached_result != net::ERR_DNS_CACHE_MISS) {
+    // Get from a local cache if able. Even though the network process does its
+    // own HostResolver caching, that would still require an async mojo call.
+    // Async returns are particularly expensive here, so the local cache
+    // maximizes ability to return synchronously.
+    //
+    // TODO(ericorth@chromium.org): Consider some small refactors to allow
+    // direct non-mojo access to the fast, synchronous, and self-contained logic
+    // from net::HostResolver (e.g. IP-literal and "localhost" resolution). That
+    // could allow reducing async returns even further.
+    DCHECK(host_cache_);
+    const std::vector<net::IPAddress>* cached_result = host_cache_->LookupEntry(
+        hostname_, network_isolation_key_, IsExOperation(operation_));
+    if (cached_result) {
+      results_ = *cached_result;
       DVLOG(1) << "Resolved " << hostname_ << " from cache";
-      return cached_result;
+      return net::OK;
     }
 
     callback_ = std::move(callback);
@@ -91,48 +87,20 @@
                     const std::vector<net::IPAddress>& result) override {
     DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
-    if (error == net::OK)
+    if (error == net::OK) {
       results_ = result;
-    if (host_cache_) {
-      base::TimeDelta ttl =
-          error == net::OK ? kCacheEntryTTL : kNegativeCacheEntryTTL;
-      net::HostCache::Entry entry(
-          error, net::AddressList::CreateFromIPAddressList(result, {}),
-          net::HostCache::Entry::SOURCE_UNKNOWN, ttl);
-      host_cache_->Set(
-          CacheKeyForRequest(hostname_, network_isolation_key_, operation_),
-          entry, base::TimeTicks::Now(), ttl);
+      if (host_cache_) {
+        host_cache_->StoreEntry(hostname_, network_isolation_key_,
+                                IsExOperation(operation_), result);
+      }
     }
     receiver_.reset();
     std::move(callback_).Run(error);
   }
 
  private:
-  int ResolveFromCacheInternal() {
-    DCHECK(host_cache_);
-
-    net::HostCache::Key key =
-        CacheKeyForRequest(hostname_, network_isolation_key_, operation_);
-    const std::pair<const net::HostCache::Key, net::HostCache::Entry>*
-        cache_result = host_cache_->Lookup(key, base::TimeTicks::Now());
-    if (!cache_result)
-      return net::ERR_DNS_CACHE_MISS;
-
-    results_ = AddressListToAddresses(cache_result->second.addresses().value());
-    return cache_result->second.error();
-  }
-
   void OnDisconnect() { ReportResult(net::ERR_FAILED, {} /* result */); }
 
-  static std::vector<net::IPAddress> AddressListToAddresses(
-      net::AddressList address_list) {
-    std::vector<net::IPAddress> result;
-    for (net::IPEndPoint endpoint : address_list.endpoints()) {
-      result.push_back(endpoint.address());
-    }
-    return result;
-  }
-
   const std::string hostname_;
   const net::ProxyResolveDnsOperation operation_;
   const net::NetworkIsolationKey network_isolation_key_;
@@ -140,17 +108,14 @@
   mojo::Receiver<mojom::HostResolverRequestClient> receiver_{this};
   net::CompletionOnceCallback callback_;
 
-  base::WeakPtr<net::HostCache> host_cache_;
+  base::WeakPtr<ProxyHostResolverCache> host_cache_;
   Impl* const impl_;
   std::vector<net::IPAddress> results_;
 
   THREAD_CHECKER(thread_checker_);
 };
 
-HostResolverMojo::HostResolverMojo(Impl* impl)
-    : impl_(impl),
-      host_cache_(net::HostCache::CreateDefaultCache()),
-      host_cache_weak_factory_(host_cache_.get()) {}
+HostResolverMojo::HostResolverMojo(Impl* impl) : impl_(impl) {}
 
 HostResolverMojo::~HostResolverMojo() = default;
 
diff --git a/services/proxy_resolver/host_resolver_mojo.h b/services/proxy_resolver/host_resolver_mojo.h
index 18f835d..5f60647 100644
--- a/services/proxy_resolver/host_resolver_mojo.h
+++ b/services/proxy_resolver/host_resolver_mojo.h
@@ -11,9 +11,9 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
-#include "net/dns/host_cache.h"
 #include "net/proxy_resolution/proxy_resolve_dns_operation.h"
 #include "services/proxy_resolver/proxy_host_resolver.h"
+#include "services/proxy_resolver/proxy_host_resolver_cache.h"
 #include "services/proxy_resolver/public/mojom/proxy_resolver.mojom.h"
 
 namespace net {
@@ -52,8 +52,9 @@
 
   Impl* const impl_;
 
-  std::unique_ptr<net::HostCache> host_cache_;
-  base::WeakPtrFactory<net::HostCache> host_cache_weak_factory_;
+  ProxyHostResolverCache host_cache_;
+  base::WeakPtrFactory<ProxyHostResolverCache> host_cache_weak_factory_{
+      &host_cache_};
 
   base::ThreadChecker thread_checker_;
 
diff --git a/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings.h b/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings.h
index b3cb898..c2f8ca2 100644
--- a/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings.h
+++ b/services/proxy_resolver/mojo_proxy_resolver_v8_tracing_bindings.h
@@ -15,7 +15,6 @@
 #include "net/base/address_family.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/network_isolation_key.h"
-#include "net/dns/host_resolver.h"
 #include "net/log/net_log_with_source.h"
 #include "net/proxy_resolution/proxy_resolve_dns_operation.h"
 #include "services/proxy_resolver/host_resolver_mojo.h"
diff --git a/services/proxy_resolver/proxy_host_resolver_cache.cc b/services/proxy_resolver/proxy_host_resolver_cache.cc
new file mode 100644
index 0000000..9050717
--- /dev/null
+++ b/services/proxy_resolver/proxy_host_resolver_cache.cc
@@ -0,0 +1,120 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "services/proxy_resolver/proxy_host_resolver_cache.h"
+
+#include <list>
+#include <map>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "base/check_op.h"
+#include "base/time/time.h"
+#include "net/base/ip_address.h"
+#include "net/base/net_errors.h"
+#include "net/base/network_isolation_key.h"
+
+namespace proxy_resolver {
+
+// static
+constexpr base::TimeDelta ProxyHostResolverCache::kTtl;
+
+ProxyHostResolverCache::ProxyHostResolverCache(size_t max_entries)
+    : max_entries_(max_entries) {
+  DCHECK_GT(max_entries_, 0u);
+}
+
+ProxyHostResolverCache::~ProxyHostResolverCache() = default;
+
+void ProxyHostResolverCache::StoreEntry(
+    std::string hostname,
+    net::NetworkIsolationKey network_isolation_key,
+    bool is_ex_operation,
+    std::vector<net::IPAddress> results) {
+  Key key{std::move(hostname), std::move(network_isolation_key),
+          is_ex_operation};
+
+  // Delete any old, now-obsolete entries.
+  auto old_entry = entries_.find(key);
+  if (old_entry != entries_.end()) {
+    DCHECK(old_entry->second.expiration_list_it != expiration_list_.end());
+    expiration_list_.erase(old_entry->second.expiration_list_it);
+    entries_.erase(old_entry);
+  }
+
+  Entry entry(std::move(results),
+              /*expiration=*/base::TimeTicks::Now() + kTtl,
+              expiration_list_.end());
+  auto entry_insertion_result =
+      entries_.emplace(std::move(key), std::move(entry));
+  DCHECK(entry_insertion_result.second);
+
+  entry_insertion_result.first->second.expiration_list_it =
+      expiration_list_.insert(expiration_list_.end(),
+                              &entry_insertion_result.first->first);
+
+  DCHECK_EQ(entries_.size(), expiration_list_.size());
+
+  RemoveOldestEntry();
+}
+
+const std::vector<net::IPAddress>* ProxyHostResolverCache::LookupEntry(
+    std::string hostname,
+    net::NetworkIsolationKey network_isolation_key,
+    bool is_ex_operation) {
+  Key key{std::move(hostname), std::move(network_isolation_key),
+          is_ex_operation};
+
+  auto entry = entries_.find(key);
+  if (entry == entries_.end())
+    return nullptr;
+
+  DCHECK(entry->second.expiration_list_it != expiration_list_.end());
+  if (entry->second.expiration < base::TimeTicks::Now()) {
+    expiration_list_.erase(std::move(entry->second.expiration_list_it));
+    entries_.erase(entry);
+    return nullptr;
+  }
+
+  return &entry->second.results;
+}
+
+size_t ProxyHostResolverCache::GetSizeForTesting() const {
+  CHECK_EQ(entries_.size(), expiration_list_.size());
+  return entries_.size();
+}
+
+ProxyHostResolverCache::Entry::Entry(
+    std::vector<net::IPAddress> results,
+    base::TimeTicks expiration,
+    ExpirationList::iterator expiration_list_it)
+    : results(std::move(results)),
+      expiration(expiration),
+      expiration_list_it(std::move(expiration_list_it)) {}
+
+ProxyHostResolverCache::Entry::~Entry() = default;
+
+ProxyHostResolverCache::Entry::Entry(Entry&&) = default;
+
+ProxyHostResolverCache::Entry& ProxyHostResolverCache::Entry::operator=(
+    Entry&&) = default;
+
+void ProxyHostResolverCache::RemoveOldestEntry() {
+  if (entries_.size() <= max_entries_)
+    return;
+
+  DCHECK_GT(expiration_list_.size(), 0u);
+  size_t removed = entries_.erase(**expiration_list_.begin());
+  DCHECK_EQ(removed, 1u);
+  expiration_list_.pop_front();
+
+  DCHECK_EQ(entries_.size(), expiration_list_.size());
+
+  // Should be called at least after every individual insertion, so no more than
+  // one removal should ever be needed.
+  DCHECK_LE(entries_.size(), max_entries_);
+}
+
+}  // namespace proxy_resolver
diff --git a/services/proxy_resolver/proxy_host_resolver_cache.h b/services/proxy_resolver/proxy_host_resolver_cache.h
new file mode 100644
index 0000000..aeacf89
--- /dev/null
+++ b/services/proxy_resolver/proxy_host_resolver_cache.h
@@ -0,0 +1,88 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SERVICES_PROXY_RESOLVER_PROXY_HOST_RESOLVER_CACHE_H_
+#define SERVICES_PROXY_RESOLVER_PROXY_HOST_RESOLVER_CACHE_H_
+
+#include <list>
+#include <map>
+#include <string>
+#include <tuple>
+#include <vector>
+
+#include "base/time/time.h"
+#include "net/base/ip_address.h"
+#include "net/base/network_isolation_key.h"
+
+namespace proxy_resolver {
+
+// Simple cache for proxy host resolutions. Maintains cached entries for up to
+// `kTtl`, and evicts oldest entries when filled past capacity.
+class ProxyHostResolverCache {
+ public:
+  static constexpr auto kTtl = base::TimeDelta::FromSeconds(5);
+
+  explicit ProxyHostResolverCache(size_t max_entries = 500u);
+  ~ProxyHostResolverCache();
+  ProxyHostResolverCache(const ProxyHostResolverCache&) = delete;
+  ProxyHostResolverCache& operator=(const ProxyHostResolverCache&) = delete;
+
+  void StoreEntry(std::string hostname,
+                  net::NetworkIsolationKey network_isolation_key,
+                  bool is_ex_operation,
+                  std::vector<net::IPAddress> results);
+
+  // Returns `nullptr` if entry not found or expired. Erases from cache if
+  // expired.
+  const std::vector<net::IPAddress>* LookupEntry(
+      std::string hostname,
+      net::NetworkIsolationKey network_isolation_key,
+      bool is_ex_operation);
+
+  size_t GetSizeForTesting() const;
+
+ private:
+  struct Key {
+    bool operator<(const Key& other) const {
+      return std::tie(hostname, network_isolation_key, is_ex_operation) <
+             std::tie(other.hostname, other.network_isolation_key,
+                      other.is_ex_operation);
+    }
+
+    std::string hostname;
+    net::NetworkIsolationKey network_isolation_key;
+    bool is_ex_operation;
+  };
+
+  using ExpirationList = std::list<const Key*>;
+
+  struct Entry {
+    Entry(std::vector<net::IPAddress> results,
+          base::TimeTicks expiration,
+          ExpirationList::iterator expiration_list_it);
+    ~Entry();
+    Entry(Entry&&);
+    Entry& operator=(Entry&&);
+
+    std::vector<net::IPAddress> results;
+    base::TimeTicks expiration;
+    ExpirationList::iterator expiration_list_it;
+  };
+
+  using EntryMap = std::map<Key, Entry>;
+
+  // Removes oldest entry iff `max_entries_` exceeded.
+  void RemoveOldestEntry();
+
+  size_t max_entries_;
+  EntryMap entries_;
+
+  // List of `const Key*`s in expiration order starting from the earliest to
+  // expire.
+  ExpirationList expiration_list_;
+};
+
+}  // namespace proxy_resolver
+
+#endif  // SERVICES_PROXY_RESOLVER_PROXY_HOST_RESOLVER_CACHE_H_
diff --git a/services/proxy_resolver/proxy_host_resolver_cache_unittest.cc b/services/proxy_resolver/proxy_host_resolver_cache_unittest.cc
new file mode 100644
index 0000000..635db942d
--- /dev/null
+++ b/services/proxy_resolver/proxy_host_resolver_cache_unittest.cc
@@ -0,0 +1,193 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "services/proxy_resolver/proxy_host_resolver_cache.h"
+
+#include "base/test/task_environment.h"
+#include "net/base/ip_address.h"
+#include "net/base/network_isolation_key.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace proxy_resolver {
+namespace {
+
+class ProxyHostResolverCacheTest : public testing::Test {
+ protected:
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+  ProxyHostResolverCache cache_;
+};
+
+TEST_F(ProxyHostResolverCacheTest, SimpleNegativeLookup) {
+  ASSERT_EQ(cache_.GetSizeForTesting(), 0u);
+  EXPECT_FALSE(cache_.LookupEntry("host.test", net::NetworkIsolationKey(),
+                                  /*is_ex_operation=*/false));
+}
+
+TEST_F(ProxyHostResolverCacheTest, SimpleCachedLookup) {
+  const net::IPAddress kResult(1, 2, 3, 4);
+
+  cache_.StoreEntry("host.test", net::NetworkIsolationKey(),
+                    /*is_ex_operation=*/false, {kResult});
+
+  EXPECT_EQ(cache_.GetSizeForTesting(), 1u);
+  EXPECT_THAT(cache_.LookupEntry("host.test", net::NetworkIsolationKey(),
+                                 /*is_ex_operation=*/false),
+              testing::Pointee(testing::ElementsAre(kResult)));
+}
+
+TEST_F(ProxyHostResolverCacheTest, NoResultWithNonMatchingKeyFields) {
+  cache_.StoreEntry("host.test", net::NetworkIsolationKey(),
+                    /*is_ex_operation=*/false, {net::IPAddress(1, 2, 3, 5)});
+  ASSERT_EQ(cache_.GetSizeForTesting(), 1u);
+
+  // Non-matching hostname
+  EXPECT_FALSE(cache_.LookupEntry("host1.test", net::NetworkIsolationKey(),
+                                  /*is_ex_operation=*/false));
+
+  // Non-matching isolation key
+  EXPECT_FALSE(cache_.LookupEntry("host.test",
+                                  net::NetworkIsolationKey::CreateTransient(),
+                                  /*is_ex_operation=*/false));
+
+  // Non-matching `is_ex_operation`
+  EXPECT_FALSE(cache_.LookupEntry("host.test", net::NetworkIsolationKey(),
+                                  /*is_ex_operation=*/true));
+}
+
+TEST_F(ProxyHostResolverCacheTest, NoResultForExpiredLookup) {
+  const net::IPAddress kResult(1, 2, 3, 6);
+
+  cache_.StoreEntry("host.test", net::NetworkIsolationKey(),
+                    /*is_ex_operation=*/false, {kResult});
+
+  task_environment_.FastForwardBy(ProxyHostResolverCache::kTtl -
+                                  base::TimeDelta::FromMilliseconds(5));
+  EXPECT_EQ(cache_.GetSizeForTesting(), 1u);
+  ASSERT_THAT(cache_.LookupEntry("host.test", net::NetworkIsolationKey(),
+                                 /*is_ex_operation=*/false),
+              testing::Pointee(testing::ElementsAre(kResult)));
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(10));
+  EXPECT_FALSE(cache_.LookupEntry("host.test", net::NetworkIsolationKey(),
+                                  /*is_ex_operation=*/false));
+
+  // Expect expired entry to be deleted by lookup attempt.
+  EXPECT_EQ(cache_.GetSizeForTesting(), 0u);
+}
+
+TEST_F(ProxyHostResolverCacheTest, EvictsOldestEntriesWhenFull) {
+  ProxyHostResolverCache cache(/*max_entries=*/3u);
+
+  // Initial entry to be deleted.
+  cache.StoreEntry("to-be-deleted.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+
+  // Fill to max capacity
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
+  cache.StoreEntry("other1.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+  cache.StoreEntry("other2.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+
+  // Nothing should be evicted yet.
+  EXPECT_EQ(cache.GetSizeForTesting(), 3u);
+  EXPECT_TRUE(cache.LookupEntry("to-be-deleted.test",
+                                net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("other1.test", net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("other2.test", net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+
+  // Add another entry and expect eviction of oldest.
+  cache.StoreEntry("evictor.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+
+  EXPECT_EQ(cache.GetSizeForTesting(), 3u);
+  EXPECT_FALSE(cache.LookupEntry("to-be-deleted.test",
+                                 net::NetworkIsolationKey(),
+                                 /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("other1.test", net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("other2.test", net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("evictor.test", net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+}
+
+TEST_F(ProxyHostResolverCacheTest, UpdatesAlreadyExistingEntryWithSameKey) {
+  cache_.StoreEntry("host.test", net::NetworkIsolationKey(),
+                    /*is_ex_operation=*/false, /*results=*/{});
+  ASSERT_EQ(cache_.GetSizeForTesting(), 1u);
+
+  const net::IPAddress kResult(1, 2, 3, 7);
+  cache_.StoreEntry("host.test", net::NetworkIsolationKey(),
+                    /*is_ex_operation=*/false, {kResult});
+
+  EXPECT_EQ(cache_.GetSizeForTesting(), 1u);
+  EXPECT_THAT(cache_.LookupEntry("host.test", net::NetworkIsolationKey(),
+                                 /*is_ex_operation=*/false),
+              testing::Pointee(testing::ElementsAre(kResult)));
+}
+
+TEST_F(ProxyHostResolverCacheTest, EntryUpdateRefreshesExpiration) {
+  ProxyHostResolverCache cache(/*max_entries=*/2u);
+
+  // Insert two entries, with "to-be-refreshed.test" as the older one.
+  cache.StoreEntry("to-be-refreshed.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
+  cache.StoreEntry("to-be-evicted.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+  ASSERT_EQ(cache.GetSizeForTesting(), 2u);
+
+  // Update "to-be-refreshed.test" to refresh its expiration.
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
+  cache.StoreEntry("to-be-refreshed.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+  ASSERT_EQ(cache.GetSizeForTesting(), 2u);
+
+  // Add another entry to force an eviction.
+  cache.StoreEntry("evictor.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+
+  EXPECT_EQ(cache.GetSizeForTesting(), 2u);
+  EXPECT_FALSE(cache.LookupEntry("to-be-evicted.test",
+                                 net::NetworkIsolationKey(),
+                                 /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("to-be-refreshed.test",
+                                net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("evictor.test", net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+}
+
+TEST_F(ProxyHostResolverCacheTest, EntryCanBeEvictedAfterUpdate) {
+  ProxyHostResolverCache cache(/*max_entries=*/1u);
+
+  // Add entry and then update it.
+  cache.StoreEntry("host.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+  ASSERT_EQ(cache.GetSizeForTesting(), 1u);
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
+  cache.StoreEntry("host.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+  ASSERT_EQ(cache.GetSizeForTesting(), 1u);
+
+  // Add another entry to force an eviction.
+  task_environment_.FastForwardBy(base::TimeDelta::FromMilliseconds(5));
+  cache.StoreEntry("evictor.test", net::NetworkIsolationKey(),
+                   /*is_ex_operation=*/false, /*results=*/{});
+
+  EXPECT_EQ(cache.GetSizeForTesting(), 1u);
+  EXPECT_FALSE(cache.LookupEntry("host.test", net::NetworkIsolationKey(),
+                                 /*is_ex_operation=*/false));
+  EXPECT_TRUE(cache.LookupEntry("evictor.test", net::NetworkIsolationKey(),
+                                /*is_ex_operation=*/false));
+}
+
+}  // namespace
+}  // namespace proxy_resolver