Don't clear DNS cache when closing last incognito Window.

The DNS cache is now a per-NetworkContext cache, instead of a global,
so clearing the cache when closing incognito is no longer useful -
destroying the incognito profile will destroy its own DNS cache.

Bug: 1042354
Change-Id: Ib3fd2dc03cf48ca1ca1d1fd7fca025fccb2a4a2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2003212Reviewed-by: Eric Orth <ericorth@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732622}

chrome/browser/net/network_context_configuration_browsertest.cc

@@ -14,6 +14,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/guid.h"
 #include "base/location.h"
+#include "base/optional.h"
 #include "base/path_service.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
@@ -59,6 +60,7 @@
 #include "content/public/test/simple_url_loader_test_helper.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/system/data_pipe_utils.h"
+#include "net/base/address_list.h"
 #include "net/base/filename_util.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/load_flags.h"
@@ -67,6 +69,7 @@
 #include "net/cookies/cookie_options.h"
 #include "net/cookies/cookie_util.h"
 #include "net/dns/mock_host_resolver.h"
+#include "net/dns/public/resolve_error_info.h"
 #include "net/http/http_response_headers.h"
 #include "net/reporting/reporting_policy.h"
 #include "net/ssl/ssl_config.h"
@@ -85,9 +88,11 @@
 #include "services/network/public/cpp/network_connection_tracker.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/mojom/cookie_manager.mojom.h"
+#include "services/network/public/mojom/host_resolver.mojom.h"
 #include "services/network/public/mojom/network_service.mojom.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
+#include "services/network/test/test_dns_util.h"
 #include "services/network/test/test_url_loader_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -104,6 +109,9 @@
 
 namespace {
 
+constexpr char kHostname[] = "foo.test";
+constexpr char kAddress[] = "5.8.13.21";
+
 const char kCacheRandomPath[] = "/cacherandom";
 
 // Path using a ControllableHttpResponse that's part of the test fixture.
@@ -236,6 +244,8 @@ class NetworkContextConfigurationBrowserTest
     // Used in a bunch of proxy tests. Should not resolve.
     host_resolver()->AddSimulatedFailure("does.not.resolve.test");
 
+    host_resolver()->AddRule(kHostname, kAddress);
+
     controllable_http_response_ =
         std::make_unique<net::test_server::ControllableHttpResponse>(
             embedded_test_server(), kControllablePath);
@@ -1109,6 +1119,53 @@ IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, DiskCache) {
   }
 }
 
+// Make sure that NetworkContexts have separate DNS caches.
+IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest,
+                       DnsCacheIsolation) {
+  net::NetworkIsolationKey network_isolation_key =
+      net::NetworkIsolationKey::CreateTransient();
+  net::HostPortPair host_port_pair(kHostname, 0);
+
+  // Resolve |host_port_pair|, which should succeed and put it in the
+  // NetworkContext's cache.
+  network::DnsLookupResult result =
+      network::BlockingDnsLookup(network_context(), host_port_pair,
+                                 nullptr /* params */, network_isolation_key);
+  EXPECT_EQ(net::OK, result.error);
+  ASSERT_TRUE(result.resolved_addresses.has_value());
+  ASSERT_EQ(1u, result.resolved_addresses->size());
+  EXPECT_EQ(kAddress,
+            result.resolved_addresses.value()[0].ToStringWithoutPort());
+
+  // Make a cache-only request for the same hostname, for each other network
+  // context, and make sure no result is returned.
+  ForEachOtherContext(
+      base::BindLambdaForTesting([&](NetworkContextType network_context_type) {
+        network::mojom::ResolveHostParametersPtr params =
+            network::mojom::ResolveHostParameters::New();
+        // Cache only lookup.
+        params->source = net::HostResolverSource::LOCAL_ONLY;
+        network::DnsLookupResult result = network::BlockingDnsLookup(
+            GetNetworkContextForContextType(network_context_type),
+            host_port_pair, std::move(params), network_isolation_key);
+        EXPECT_EQ(net::ERR_DNS_CACHE_MISS, result.error);
+      }));
+
+  // Do a cache-only lookup using the original network context, which should
+  // return the same result it initially did.
+  network::mojom::ResolveHostParametersPtr params =
+      network::mojom::ResolveHostParameters::New();
+  // Cache only lookup.
+  params->source = net::HostResolverSource::LOCAL_ONLY;
+  result = network::BlockingDnsLookup(network_context(), host_port_pair,
+                                      std::move(params), network_isolation_key);
+  EXPECT_EQ(net::OK, result.error);
+  ASSERT_TRUE(result.resolved_addresses.has_value());
+  ASSERT_EQ(1u, result.resolved_addresses->size());
+  EXPECT_EQ(kAddress,
+            result.resolved_addresses.value()[0].ToStringWithoutPort());
+}
+
 // Visits a URL with an HSTS header, and makes sure it is respected.
 IN_PROC_BROWSER_TEST_P(NetworkContextConfigurationBrowserTest, PRE_Hsts) {
   if (IsRestartStateWithInProcessNetworkService())

chrome/browser/profiles/off_the_record_profile_impl.cc

@@ -71,7 +71,6 @@
 #include "media/mojo/services/video_decode_perf_history.h"
 #include "net/http/transport_security_state.h"
 #include "ppapi/buildflags/buildflags.h"
-#include "services/network/public/mojom/network_context.mojom.h"
 #include "storage/browser/database/database_tracker.h"
 
 #if defined(OS_ANDROID)
@@ -190,12 +189,6 @@ OffTheRecordProfileImpl::~OffTheRecordProfileImpl() {
   ChromePluginServiceFilter::GetInstance()->UnregisterProfile(this);
 #endif
 
-  // Clears any data the network stack contains that may be related to the
-  // OTR session. Must be done before DestroyBrowserContextServices, since
-  // the NetworkContext is managed by one such service.
-  GetDefaultStoragePartition(this)->GetNetworkContext()->ClearHostCache(
-      nullptr, network::mojom::NetworkContext::ClearHostCacheCallback());
-
   FullBrowserTransitionManager::Get()->OnProfileDestroyed(this);
 
   // The SimpleDependencyManager should always be passed after the