Skip to content

Commit 743aa3c

Browse files
Simon KnottChromium LUCI CQ
Simon Knott
authored and
Chromium LUCI CQ
committed
[CDP] Detect Page.navigate download
Before this CL, navigating to a download URL yields "net::ERR_ABORTED", and is impossible to distinguish from any other error. To determine whether the error stems from a download, one needs to wait for the `Browser.downloadWillBegin` event. But since that can be emitted *after* `Page.navigate` returns, this introduces unnecessary waiting. To prevent this waiting, this CL adds a flag that shows whether the navigation turned into a download. Change-Id: I90d81d289df6621d0cdb977ba6ed7e7d74ee475e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6696011 Commit-Queue: Yury Semikhatsky <[email protected]> Reviewed-by: Maksim Sadym <[email protected]> Reviewed-by: Andrey Kosyakov <[email protected]> Cr-Commit-Position: refs/heads/main@{#1487924}
1 parent 03cdd4b commit 743aa3c

File tree

8 files changed

+37
-3
lines changed

8 files changed

+37
-3
lines changed

‎AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,6 +1400,7 @@ Simeon Kuran <[email protected]>
14001400
Simon Arlott <[email protected]>
14011401
Simon Cadman <[email protected]>
14021402
Simon Jackson <[email protected]>
1403+
Simon Knott <[email protected]>
14031404
Simon La Macchia <[email protected]>
14041405
Siva Kumar Gunturi <[email protected]>
14051406
Slava Aseev <[email protected]>

‎content/browser/devtools/protocol/page_handler.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,8 @@ void DispatchNavigateCallback(
815815
// abort to DevTools anyway.
816816
if (!request->IsNavigationStarted()) {
817817
callback->sendSuccess(frame_id, std::nullopt,
818-
net::ErrorToString(net::ERR_ABORTED));
818+
net::ErrorToString(net::ERR_ABORTED),
819+
request->IsDownload());
819820
return;
820821
}
821822
std::optional<std::string> opt_error;
@@ -825,7 +826,8 @@ void DispatchNavigateCallback(
825826
request->IsSameDocument()
826827
? std::optional<std::string>()
827828
: request->devtools_navigation_token().ToString();
828-
callback->sendSuccess(frame_id, std::move(loader_id), std::move(opt_error));
829+
callback->sendSuccess(frame_id, std::move(loader_id), std::move(opt_error),
830+
request->IsDownload());
829831
}
830832

831833
} // namespace
@@ -938,7 +940,7 @@ void PageHandler::Navigate(const std::string& url,
938940
return;
939941
if (!navigation_handle) {
940942
callback->sendSuccess(out_frame_id, std::nullopt,
941-
net::ErrorToString(net::ERR_ABORTED));
943+
net::ErrorToString(net::ERR_ABORTED), std::nullopt);
942944
return;
943945
}
944946
auto* navigation_request =

‎third_party/blink/public/devtools_protocol/browser_protocol.pdl‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9452,6 +9452,8 @@ domain Page
94529452
optional Network.LoaderId loaderId
94539453
# User friendly error message, present if and only if navigation has failed.
94549454
optional string errorText
9455+
# Whether the navigation resulted in a download.
9456+
experimental optional boolean isDownload
94559457

94569458
# Navigates current page to the given history entry.
94579459
command navigateToHistoryEntry

‎third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-204-expected.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Tests that Page.navigate returns error when server responds with HTTP 204.
44
result : {
55
errorText : net::ERR_ABORTED
66
frameId : <string>
7+
isDownload : false
78
loaderId : <string>
89
}
910
sessionId : <string>

‎third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment-expected.txt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
Tests that Page.navigate returns isDownload when server responds with Content-Disposition: attachment.
2+
{
3+
id : <number>
4+
result : {
5+
errorText : net::ERR_ABORTED
6+
frameId : <string>
7+
isDownload : true
8+
loaderId : <string>
9+
}
10+
sessionId : <string>
11+
}
12+

‎third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
2+
var {page, session, dp} = await testRunner.startBlank(
3+
`Tests that Page.navigate returns isDownload when server responds with Content-Disposition: attachment.`);
4+
5+
await dp.Page.enable();
6+
const response = await dp.Page.navigate({ url: testRunner.url('./resources/httpAttachment.php')});
7+
testRunner.log(response);
8+
testRunner.completeTest();
9+
})

‎third_party/blink/web_tests/http/tests/inspector-protocol/page/resources/httpAttachment.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
ob_start();
3+
header('Content-Disposition: attachment; filename="downloaded.txt"');
4+
?>
5+
foo bar baz

‎third_party/blink/web_tests/inspector-protocol/page/pageNavigateToFragment-expected.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ Tests Page.navigate returns for fragment navigation.
33
id : <number>
44
result : {
55
frameId : <string>
6+
isDownload : false
67
loaderId : <string>
78
}
89
sessionId : <string>
@@ -11,6 +12,7 @@ Tests Page.navigate returns for fragment navigation.
1112
id : <number>
1213
result : {
1314
frameId : <string>
15+
isDownload : false
1416
}
1517
sessionId : <string>
1618
}

0 commit comments

Comments
 (0)