Network Service: Fix some LoginPromptBrowserTest by adding |do_not_pr…

…ompt_for_login_| flag for URLLoader This CL fixes some LoginPromptBrowserTest by adding a |do_not_prompt_for_login_| flag for URLLoader. It is similar to what ResourceLoader::OnAuthRequired() does. Bug: 783990 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I86d72fcf64429a3d78231ce55fc1bb07359c7caa Reviewed-on: https://chromium-review.googlesource.com/1015742 Commit-Queue: Jun Cai <juncai@chromium.org> Reviewed-by: Matt Menke <mmenke@chromium.org> Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#555653}
Denger-Network · May 3, 2018 · 1d62846 · 1d62846
1 parent 45c8104
commit 1d62846
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 11 deletions.
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -1001,11 +1001,6 @@ void ResourceDispatcherHostImpl::ContinuePendingBeginRequest(
       report_raw_headers = false;
   }
 
-  if (request_data.resource_type == RESOURCE_TYPE_PREFETCH ||
-      request_data.resource_type == RESOURCE_TYPE_FAVICON) {
-    do_not_prompt_for_login = true;
-  }
-
   if (DoNotPromptForLogin(static_cast<ResourceType>(request_data.resource_type),
                           request_data.url, request_data.site_for_cookies)) {
     // Prevent third-party image content from prompting for login, as this

diff --git a/content/renderer/loader/web_url_loader_impl.cc b/content/renderer/loader/web_url_loader_impl.cc
@@ -680,6 +680,11 @@ void WebURLLoaderImpl::Context::Start(const WebURLRequest& request,
                                                  network::kDefaultAcceptHeader);
   }
 
+  if (resource_request->resource_type == RESOURCE_TYPE_PREFETCH ||
+      resource_request->resource_type == RESOURCE_TYPE_FAVICON) {
+    resource_request->do_not_prompt_for_login = true;
+  }
+
   resource_request->load_flags = GetLoadFlagsForWebURLRequest(request);
 
   // |plugin_child_id| only needs to be non-zero if the request originates

diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
@@ -285,6 +285,7 @@ URLLoader::URLLoader(
       render_frame_id_(request.render_frame_id),
       request_id_(request_id),
       keepalive_(request.keepalive),
+      do_not_prompt_for_login_(request.do_not_prompt_for_login),
       binding_(this, std::move(url_loader_request)),
       auth_challenge_responder_binding_(this),
       url_loader_client_(std::move(url_loader_client)),
@@ -476,6 +477,11 @@ void URLLoader::OnAuthRequired(net::URLRequest* unused,
     return;
   }
 
+  if (do_not_prompt_for_login_) {
+    OnAuthCredentials(base::nullopt);
+    return;
+  }
+
   network::mojom::AuthChallengeResponderPtr auth_challenge_responder;
   auto request = mojo::MakeRequest(&auth_challenge_responder);
   DCHECK(!auth_challenge_responder_binding_.is_bound());

diff --git a/services/network/url_loader.h b/services/network/url_loader.h
@@ -129,6 +129,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) URLLoader
   uint32_t render_frame_id_;
   uint32_t request_id_;
   const bool keepalive_;
+  const bool do_not_prompt_for_login_;
   std::unique_ptr<net::URLRequest> url_request_;
   mojo::Binding<mojom::URLLoader> binding_;
   mojo::Binding<mojom::AuthChallengeResponder>

diff --git a/services/network/url_loader_unittest.cc b/services/network/url_loader_unittest.cc
@@ -1799,4 +1799,45 @@ TEST_F(URLLoaderTest, TwoChallenges) {
   ASSERT_FALSE(url_loader);
 }
 
+TEST_F(URLLoaderTest, NoAuthRequiredForFavicon) {
+  constexpr char kFaviconTestPage[] = "/has_favicon.html";
+
+  TestAuthNetworkServiceClient network_service_client;
+  network_service_client.set_credentials_response(
+      TestAuthNetworkServiceClient::CredentialsResponse::CORRECT_CREDENTIALS);
+
+  ResourceRequest request =
+      CreateResourceRequest("GET", test_server()->GetURL(kFaviconTestPage));
+  base::RunLoop delete_run_loop;
+  mojom::URLLoaderPtr loader;
+  std::unique_ptr<URLLoader> url_loader = std::make_unique<URLLoader>(
+      context(), &network_service_client,
+      DeleteLoaderCallback(&delete_run_loop, &url_loader),
+      mojo::MakeRequest(&loader), 0, request, false,
+      client()->CreateInterfacePtr(), TRAFFIC_ANNOTATION_FOR_TESTS, kProcessId,
+      0 /* request_id */, resource_scheduler_client(), nullptr,
+      nullptr /* network_usage_accumulator */);
+  base::RunLoop().RunUntilIdle();
+
+  ASSERT_TRUE(url_loader);
+
+  client()->RunUntilResponseBodyArrived();
+  EXPECT_TRUE(client()->has_received_response());
+  EXPECT_FALSE(client()->has_received_completion());
+
+  // Spin the message loop until the delete callback is invoked, and then delete
+  // the URLLoader.
+  delete_run_loop.Run();
+
+  client()->RunUntilComplete();
+  EXPECT_TRUE(client()->has_received_completion());
+  scoped_refptr<net::HttpResponseHeaders> headers =
+      client()->response_head().headers;
+  ASSERT_TRUE(headers);
+  EXPECT_EQ(200, headers->response_code());
+  // No auth required for favicon.
+  EXPECT_EQ(0, network_service_client.on_auth_required_call_counter());
+  ASSERT_FALSE(url_loader);
+}
+
 }  // namespace network
diff --git a/services/test/BUILD.gn b/services/test/BUILD.gn
@@ -56,6 +56,7 @@ bundle_data("tests_bundle_data") {
     "//services/test/data/content-sniffer-test4.html",
     "//services/test/data/content-sniffer-test4.html.mock-http-headers",
     "//services/test/data/empty.html",
+    "//services/test/data/has_favicon.html",
     "//services/test/data/hello.html",
     "//services/test/data/hello.html.mock-http-headers",
     "//services/test/data/nocache.html",

diff --git a/services/test/data/has_favicon.html b/services/test/data/has_favicon.html
@@ -0,0 +1,8 @@
+<html>
+  <head>
+    <link rel="icon" type="image/gif" href="/auth-basic/favicon.gif" />
+  </head>
+  <body>
+    <h1>Has favicon</h1>
+  </body>
+</html>
diff --git a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
@@ -129,12 +129,6 @@
 # crbug.com/778860 SecurityStyleExplanations::info_explanations is empty.
 -BrowserTestNonsecureURLRequest.DidChangeVisibleSecurityStateObserverObsoleteTLSSettings
 
-# http://crbug.com/783990
-# Add support for http auth.
--DownloadExtensionTest.DownloadExtensionTest_Download_AuthBasic_Fail
--LoginPromptBrowserTest.NoLoginPromptForFavicon
--LoginPromptBrowserTest.NoLoginPromptForXHRWithBadCredentials
-
 # 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.

diff --git a/...workService/http/tests/inspector-protocol/network/xhr-interception-auth-fail-expected.txt b/...workService/http/tests/inspector-protocol/network/xhr-interception-auth-fail-expected.txt
@@ -0,0 +1,15 @@
+Tests interception of an XHR request that fails due to lack of credentials.
+Test started
+Network agent enabled
+Request interception enabled
+Page agent enabled
+Runtime agent enabled
+Network.requestIntercepted ID 1 GET xhr-iframe-auth-fail.html type: Document
+allowRequest ID 1
+Network.responseReceived xhr-iframe-auth-fail.html 200 text/html
+Network.requestIntercepted ID 2 GET unauthorised.pl type: XHR
+allowRequest ID 2
+Network.responseReceived unauthorised.pl 401 text/plain
+Page.frameStoppedLoading
+xhr.status = 401
+