Disable some unit tests in NavigationURLLoaderTest and...

Disable some unit tests in NavigationURLLoaderTest and ResourceDispatcherHostTest and fix one test in the former when network service is enabled.

Fix NavigationURLLoaderTest.RequestFailedCertErrorFatal to work with network service. The other tests that are disabled in NavigationURLLoaderTest depend on net:: test objects that are an implementation detail of the old path, to ensure that the request is cancelled correctly when the navigation or context goes away. It's not clear how much value these tests actually provide, and this is a bit hard to test in a unit test with network service, so I'm disabling them for now.

ResourceDispatcherHostTest.ForbiddenDownload is disabled because it's a regression test for a ResourceDispatcherHost bug.

Bug: 966633
Change-Id: If979805546875ca616a1c54ff559d9f40498038a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1631768
Auto-Submit: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663768}

content/browser/loader/navigation_url_loader_unittest.cc

@@ -22,6 +22,7 @@
 #include "content/public/browser/navigation_ui_data.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/resource_dispatcher_host_delegate.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/test_browser_context.h"
 #include "content/public/test/test_browser_thread_bundle.h"
@@ -38,6 +39,8 @@
 #include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_test_job.h"
 #include "net/url_request/url_request_test_util.h"
+#include "services/network/public/cpp/features.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/origin.h"
 
@@ -176,13 +179,22 @@ TEST_F(NavigationURLLoaderTest, RequestFailedCertErrorFatal) {
   GURL url = https_server.GetURL("/");
 
   // Set HSTS for the test domain in order to make SSL errors fatal.
-  net::TransportSecurityState* transport_security_state =
-      browser_context_->GetRequestContext()
-          ->GetURLRequestContext()
-          ->transport_security_state();
   base::Time expiry = base::Time::Now() + base::TimeDelta::FromDays(1000);
   bool include_subdomains = false;
-  transport_security_state->AddHSTS(url.host(), expiry, include_subdomains);
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    auto* storage_partition =
+        BrowserContext::GetDefaultStoragePartition(browser_context_.get());
+    base::RunLoop run_loop;
+    storage_partition->GetNetworkContext()->AddHSTS(
+        url.host(), expiry, include_subdomains, run_loop.QuitClosure());
+    run_loop.Run();
+  } else {
+    net::TransportSecurityState* transport_security_state =
+        browser_context_->GetRequestContext()
+            ->GetURLRequestContext()
+            ->transport_security_state();
+    transport_security_state->AddHSTS(url.host(), expiry, include_subdomains);
+  }
 
   TestNavigationURLLoaderDelegate delegate;
   std::unique_ptr<NavigationURLLoader> loader = MakeTestLoader(url, &delegate);
@@ -202,6 +214,10 @@ TEST_F(NavigationURLLoaderTest, RequestFailedCertErrorFatal) {
 
 // Tests that the destroying the loader cancels the request.
 TEST_F(NavigationURLLoaderTest, CancelOnDestruct) {
+  // Specific to non-NetworkService path.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
   // Fake a top-level request. Choose a URL which redirects so the request can
   // be paused before the response comes in.
   TestNavigationURLLoaderDelegate delegate;
@@ -221,6 +237,10 @@ TEST_F(NavigationURLLoaderTest, CancelOnDestruct) {
 // Test that the delegate is not called if OnResponseStarted and destroying the
 // loader race.
 TEST_F(NavigationURLLoaderTest, CancelResponseRace) {
+  // Specific to non-NetworkService path.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
   TestNavigationURLLoaderDelegate delegate;
   std::unique_ptr<NavigationURLLoader> loader = MakeTestLoader(
       net::URLRequestTestJob::test_url_redirect_to_url_2(), &delegate);
@@ -242,6 +262,10 @@ TEST_F(NavigationURLLoaderTest, CancelResponseRace) {
 
 // Tests that the loader may be canceled by context.
 TEST_F(NavigationURLLoaderTest, CancelByContext) {
+  // Specific to non-NetworkService path.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
   TestNavigationURLLoaderDelegate delegate;
   std::unique_ptr<NavigationURLLoader> loader = MakeTestLoader(
       net::URLRequestTestJob::test_url_redirect_to_url_2(), &delegate);
@@ -261,6 +285,10 @@ TEST_F(NavigationURLLoaderTest, CancelByContext) {
 // Tests that the request stays alive as long as the URLLoaderClient endpoints
 // are not destructed.
 TEST_F(NavigationURLLoaderTest, OwnedByHandle) {
+  // Specific to non-NetworkService path.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
   // Fake a top-level request to a URL whose body does not load immediately.
   TestNavigationURLLoaderDelegate delegate;
   std::unique_ptr<NavigationURLLoader> loader =

content/browser/loader/resource_dispatcher_host_unittest.cc

@@ -2122,6 +2122,12 @@ TEST_P(ResourceDispatcherHostTest, MimeSniffEmpty) {
 
 // Tests for crbug.com/31266 (Non-2xx + application/octet-stream).
 TEST_P(ResourceDispatcherHostTest, ForbiddenDownload) {
+  // This is a regression test for code in ResourceDispatcherHost, but it's
+  // written in a way that uses code from network service if that feature is
+  // enabled. This will fail because not everything is setup.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
   std::string raw_headers("HTTP/1.1 403 Forbidden\n"
                           "Content-disposition: attachment; filename=blah\n"
                           "Content-type: application/octet-stream\n\n");