Network Service: Fix some LoginPromptBrowserTest by adding |do_not_prompt_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}diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index aad1d45..2baf6b5 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -1001,11 +1001,6 @@
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
index e998e132..c7d09c3 100644
--- a/content/renderer/loader/web_url_loader_impl.cc
+++ b/content/renderer/loader/web_url_loader_impl.cc
@@ -680,6 +680,11 @@
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
index 0725125..b6fcfd7 100644
--- a/services/network/url_loader.cc
+++ b/services/network/url_loader.cc
@@ -285,6 +285,7 @@
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 @@
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
index 895b482..34f2751 100644
--- a/services/network/url_loader.h
+++ b/services/network/url_loader.h
@@ -129,6 +129,7 @@
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
index 433ac6f..6b9bf634 100644
--- a/services/network/url_loader_unittest.cc
+++ b/services/network/url_loader_unittest.cc
@@ -1799,4 +1799,45 @@
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
index 7bd581f..c0811cc 100644
--- a/services/test/BUILD.gn
+++ b/services/test/BUILD.gn
@@ -56,6 +56,7 @@
"//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
new file mode 100644
index 0000000..8b63948
--- /dev/null
+++ 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
index 344af64..f61ea26 100644
--- 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/third_party/WebKit/LayoutTests/flag-specific/enable-features=NetworkService/http/tests/inspector-protocol/network/xhr-interception-auth-fail-expected.txt b/third_party/WebKit/LayoutTests/flag-specific/enable-features=NetworkService/http/tests/inspector-protocol/network/xhr-interception-auth-fail-expected.txt
new file mode 100644
index 0000000..75ab74a
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/flag-specific/enable-features=NetworkService/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
+