Update BrowserTestNonsecureURLRequest set up code

This change updates the BrowserTestNonsecureURLRequest set up code to
use a URLLoaderInterceptor to handle URL requests. Using
URLLoaderInterceptor enables the test to work properly when using
Network Service code.

Bug: 778860
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I5898b166967bd8d54608b78444c0aa7c99429df5
Reviewed-on: https://chromium-review.googlesource.com/1150826
Commit-Queue: Ovidio Henriquez <odejesush@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579811}

chrome/browser/ssl/security_state_tab_helper_browsertest.cc

@@ -11,6 +11,7 @@
 #include "base/run_loop.h"
 #include "base/strings/string_split.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_command_line.h"
 #include "build/build_config.h"
@@ -56,6 +57,7 @@
 #include "content/public/common/referrer.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/url_loader_interceptor.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/cert/cert_verify_result.h"
@@ -72,7 +74,6 @@
 #include "net/test/embedded_test_server/request_handler_util.h"
 #include "net/test/test_data_directory.h"
 #include "net/test/url_request/url_request_failed_job.h"
-#include "net/test/url_request/url_request_mock_http_job.h"
 #include "net/url_request/url_request_filter.h"
 #include "net/url_request/url_request_test_util.h"
 #include "third_party/boringssl/src/include/openssl/ssl.h"
@@ -2480,77 +2481,11 @@ IN_PROC_BROWSER_TEST_P(DidChangeVisibleSecurityStateTest,
 // After AddNonsecureUrlHandler() is called, requests to this hostname
 // will use obsolete TLS settings.
 const char kMockNonsecureHostname[] = "example-nonsecure.test";
+const char kResponseFilePath[] = "chrome/test/data/title1.html";
 const int kObsoleteTLSVersion = net::SSL_CONNECTION_VERSION_TLS1_1;
 // ECDHE_RSA + AES_128_CBC with HMAC-SHA1
 const uint16_t kObsoleteCipherSuite = 0xc013;
 
-// A URLRequestMockHTTPJob that mocks a TLS connection with the obsolete
-// TLS settings specified in kObsoleteTLSVersion and
-// kObsoleteCipherSuite.
-class URLRequestObsoleteTLSJob : public net::URLRequestMockHTTPJob {
- public:
-  URLRequestObsoleteTLSJob(net::URLRequest* request,
-                           net::NetworkDelegate* network_delegate,
-                           const base::FilePath& file_path,
-                           scoped_refptr<net::X509Certificate> cert)
-      : net::URLRequestMockHTTPJob(request, network_delegate, file_path),
-        cert_(std::move(cert)) {}
-
-  void GetResponseInfo(net::HttpResponseInfo* info) override {
-    net::URLRequestMockHTTPJob::GetResponseInfo(info);
-    net::SSLConnectionStatusSetVersion(kObsoleteTLSVersion,
-                                       &info->ssl_info.connection_status);
-    net::SSLConnectionStatusSetCipherSuite(kObsoleteCipherSuite,
-                                           &info->ssl_info.connection_status);
-    info->ssl_info.cert = cert_;
-  }
-
- protected:
-  ~URLRequestObsoleteTLSJob() override {}
-
- private:
-  const scoped_refptr<net::X509Certificate> cert_;
-
-  DISALLOW_COPY_AND_ASSIGN(URLRequestObsoleteTLSJob);
-};
-
-// A URLRequestInterceptor that handles requests with
-// URLRequestObsoleteTLSJob jobs.
-class URLRequestNonsecureInterceptor : public net::URLRequestInterceptor {
- public:
-  URLRequestNonsecureInterceptor(const base::FilePath& base_path,
-                                 scoped_refptr<net::X509Certificate> cert)
-      : base_path_(base_path), cert_(std::move(cert)) {}
-
-  ~URLRequestNonsecureInterceptor() override {}
-
-  // net::URLRequestInterceptor:
-  net::URLRequestJob* MaybeInterceptRequest(
-      net::URLRequest* request,
-      net::NetworkDelegate* network_delegate) const override {
-    return new URLRequestObsoleteTLSJob(request, network_delegate, base_path_,
-                                        cert_);
-  }
-
- private:
-  const base::FilePath base_path_;
-  const scoped_refptr<net::X509Certificate> cert_;
-
-  DISALLOW_COPY_AND_ASSIGN(URLRequestNonsecureInterceptor);
-};
-
-// Installs a handler to serve HTTPS requests to
-// |kMockNonsecureHostname| with connections that have obsolete TLS
-// settings.
-void AddNonsecureUrlHandler(const base::FilePath& base_path,
-                            scoped_refptr<net::X509Certificate> cert) {
-  net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
-  filter->AddHostnameInterceptor(
-      "https", kMockNonsecureHostname,
-      std::unique_ptr<net::URLRequestInterceptor>(
-          new URLRequestNonsecureInterceptor(base_path, cert)));
-}
-
 class BrowserTestNonsecureURLRequest : public InProcessBrowserTest {
  public:
   BrowserTestNonsecureURLRequest() : InProcessBrowserTest(), cert_(nullptr) {}
@@ -2562,16 +2497,41 @@ class BrowserTestNonsecureURLRequest : public InProcessBrowserTest {
   }
 
   void SetUpOnMainThread() override {
-    base::FilePath serve_file;
-    base::PathService::Get(chrome::DIR_TEST_DATA, &serve_file);
-    serve_file = serve_file.Append(FILE_PATH_LITERAL("title1.html"));
-    content::BrowserThread::PostTask(
-        content::BrowserThread::IO, FROM_HERE,
-        base::BindOnce(&AddNonsecureUrlHandler, serve_file, cert_));
+    // Create URLLoaderInterceptor to mock a TLS connection with obsolete TLS
+    // settings specified in kObsoleteTLSVersion and kObsoleteCipherSuite.
+    url_interceptor_ = std::make_unique<content::URLLoaderInterceptor>(
+        base::BindLambdaForTesting(
+            [&](content::URLLoaderInterceptor::RequestParams* params) {
+              // Ignore non-test URLs.
+              if (params->url_request.url.host() != kMockNonsecureHostname) {
+                return false;
+              }
+
+              // Set SSLInfo to reflect an obsolete connection.
+              base::Optional<net::SSLInfo> ssl_info;
+              if (params->url_request.url.SchemeIsCryptographic()) {
+                ssl_info = net::SSLInfo();
+                net::SSLConnectionStatusSetVersion(
+                    kObsoleteTLSVersion, &ssl_info->connection_status);
+                net::SSLConnectionStatusSetCipherSuite(
+                    kObsoleteCipherSuite, &ssl_info->connection_status);
+                ssl_info->cert = cert_;
+              }
+
+              // Write the response.
+              content::URLLoaderInterceptor::WriteResponse(
+                  kResponseFilePath, params->client.get(), nullptr,
+                  std::move(ssl_info));
+
+              return true;
+            }));
   }
 
+  void TearDownOnMainThread() override { url_interceptor_.reset(); }
+
  private:
   scoped_refptr<net::X509Certificate> cert_;
+  std::unique_ptr<content::URLLoaderInterceptor> url_interceptor_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserTestNonsecureURLRequest);
 };

content/public/test/url_loader_interceptor.cc

@@ -266,13 +266,15 @@ URLLoaderInterceptor::~URLLoaderInterceptor() {
 void URLLoaderInterceptor::WriteResponse(
     const std::string& headers,
     const std::string& body,
-    network::mojom::URLLoaderClient* client) {
+    network::mojom::URLLoaderClient* client,
+    base::Optional<net::SSLInfo> ssl_info) {
   net::HttpResponseInfo info;
   info.headers = new net::HttpResponseHeaders(
       net::HttpUtil::AssembleRawHeaders(headers.c_str(), headers.length()));
   network::ResourceResponseHead response;
   response.headers = info.headers;
   response.headers->GetMimeType(&response.mime_type);
+  response.ssl_info = std::move(ssl_info);
   client->OnReceiveResponse(response);
 
   uint32_t bytes_written = body.size();
@@ -291,7 +293,8 @@ void URLLoaderInterceptor::WriteResponse(
 void URLLoaderInterceptor::WriteResponse(
     const std::string& relative_path,
     network::mojom::URLLoaderClient* client,
-    const std::string* headers) {
+    const std::string* headers,
+    base::Optional<net::SSLInfo> ssl_info) {
   base::ScopedAllowBlockingForTesting allow_io;
   std::string headers_str;
   if (headers) {
@@ -308,7 +311,8 @@ void URLLoaderInterceptor::WriteResponse(
                     "\n\n";
     }
   }
-  WriteResponse(headers_str, ReadFile(relative_path), client);
+  WriteResponse(headers_str, ReadFile(relative_path), client,
+                std::move(ssl_info));
 }
 
 void URLLoaderInterceptor::CreateURLLoaderFactoryForSubresources(

content/public/test/url_loader_interceptor.h

@@ -73,10 +73,12 @@ class URLLoaderInterceptor {
   ~URLLoaderInterceptor();
 
   // Helper methods for use when intercepting.
-  // Writes the given response body and header to |client|.
-  static void WriteResponse(const std::string& headers,
-                            const std::string& body,
-                            network::mojom::URLLoaderClient* client);
+  // Writes the given response body, header, and SSL Info to |client|.
+  static void WriteResponse(
+      const std::string& headers,
+      const std::string& body,
+      network::mojom::URLLoaderClient* client,
+      base::Optional<net::SSLInfo> ssl_info = base::nullopt);
 
   // Reads the given path, relative to the root source directory, and writes it
   // to |client|. For headers:
@@ -85,9 +87,12 @@ class URLLoaderInterceptor {
   //      found, its contents will be used
   //   3) otherwise a simple 200 response will be used, with a Content-Type
   //      guessed from the file extension
-  static void WriteResponse(const std::string& relative_path,
-                            network::mojom::URLLoaderClient* client,
-                            const std::string* headers = nullptr);
+  // For SSL info, if |ssl_info| is specified, then it is added to the response.
+  static void WriteResponse(
+      const std::string& relative_path,
+      network::mojom::URLLoaderClient* client,
+      const std::string* headers = nullptr,
+      base::Optional<net::SSLInfo> ssl_info = base::nullopt);
 
  private:
   class BrowserProcessWrapper;

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -194,9 +194,6 @@
 -ResourceLoadingHintsBrowserTest.ResourceLoadingHintsHttp
 -ResourceLoadingHintsBrowserTest.ResourceLoadingHintsHttpsNoWhitelisted
 
-# crbug.com/778860 SecurityStyleExplanations::info_explanations is empty.
--BrowserTestNonsecureURLRequest.DidChangeVisibleSecurityStateObserverObsoleteTLSSettings
-
 # These rely on proxy configuration and PAC execution being configured on the
 # legacy in-process URLRequestContext. They should be removed or updated to
 # use Mojo APIs instead.