[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 <yurys@chromium.org>
Reviewed-by: Maksim Sadym <sadym@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1487924}
diff --git a/AUTHORS b/AUTHORS
index 44d0bd6..2625532 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -1400,6 +1400,7 @@
Simon Arlott <simon.arlott@gmail.com>
Simon Cadman <simon@cadman.uk>
Simon Jackson <simon.jackson@sonocent.com>
+Simon Knott <info@simonknott.de>
Simon La Macchia <smacchia@amazon.com>
Siva Kumar Gunturi <siva.gunturi@samsung.com>
Slava Aseev <nullptrnine@gmail.com>
diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
index b95ac954..aa4e2dd 100644
--- a/content/browser/devtools/protocol/page_handler.cc
+++ b/content/browser/devtools/protocol/page_handler.cc
@@ -815,7 +815,8 @@
// abort to DevTools anyway.
if (!request->IsNavigationStarted()) {
callback->sendSuccess(frame_id, std::nullopt,
- net::ErrorToString(net::ERR_ABORTED));
+ net::ErrorToString(net::ERR_ABORTED),
+ request->IsDownload());
return;
}
std::optional<std::string> opt_error;
@@ -825,7 +826,8 @@
request->IsSameDocument()
? std::optional<std::string>()
: request->devtools_navigation_token().ToString();
- callback->sendSuccess(frame_id, std::move(loader_id), std::move(opt_error));
+ callback->sendSuccess(frame_id, std::move(loader_id), std::move(opt_error),
+ request->IsDownload());
}
} // namespace
@@ -938,7 +940,7 @@
return;
if (!navigation_handle) {
callback->sendSuccess(out_frame_id, std::nullopt,
- net::ErrorToString(net::ERR_ABORTED));
+ net::ErrorToString(net::ERR_ABORTED), std::nullopt);
return;
}
auto* navigation_request =
diff --git a/third_party/blink/public/devtools_protocol/browser_protocol.pdl b/third_party/blink/public/devtools_protocol/browser_protocol.pdl
index b36951d5..00c4263f 100644
--- a/third_party/blink/public/devtools_protocol/browser_protocol.pdl
+++ b/third_party/blink/public/devtools_protocol/browser_protocol.pdl
@@ -9452,6 +9452,8 @@
optional Network.LoaderId loaderId
# User friendly error message, present if and only if navigation has failed.
optional string errorText
+ # Whether the navigation resulted in a download.
+ experimental optional boolean isDownload
# Navigates current page to the given history entry.
command navigateToHistoryEntry
diff --git a/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-204-expected.txt b/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-204-expected.txt
index 80c99ba..de1012d 100644
--- a/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-204-expected.txt
+++ b/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-204-expected.txt
@@ -4,6 +4,7 @@
result : {
errorText : net::ERR_ABORTED
frameId : <string>
+ isDownload : false
loaderId : <string>
}
sessionId : <string>
diff --git a/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment-expected.txt b/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment-expected.txt
new file mode 100644
index 0000000..6a010824
--- /dev/null
+++ b/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment-expected.txt
@@ -0,0 +1,12 @@
+Tests that Page.navigate returns isDownload when server responds with Content-Disposition: attachment.
+{
+ id : <number>
+ result : {
+ errorText : net::ERR_ABORTED
+ frameId : <string>
+ isDownload : true
+ loaderId : <string>
+ }
+ sessionId : <string>
+}
+
diff --git a/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment.js b/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment.js
new file mode 100644
index 0000000..7024511
--- /dev/null
+++ b/third_party/blink/web_tests/http/tests/inspector-protocol/page/navigate-attachment.js
@@ -0,0 +1,9 @@
+(async function(/** @type {import('test_runner').TestRunner} */ testRunner) {
+ var {page, session, dp} = await testRunner.startBlank(
+ `Tests that Page.navigate returns isDownload when server responds with Content-Disposition: attachment.`);
+
+ await dp.Page.enable();
+ const response = await dp.Page.navigate({ url: testRunner.url('./resources/httpAttachment.php')});
+ testRunner.log(response);
+ testRunner.completeTest();
+})
diff --git a/third_party/blink/web_tests/http/tests/inspector-protocol/page/resources/httpAttachment.php b/third_party/blink/web_tests/http/tests/inspector-protocol/page/resources/httpAttachment.php
new file mode 100644
index 0000000..7634d08
--- /dev/null
+++ b/third_party/blink/web_tests/http/tests/inspector-protocol/page/resources/httpAttachment.php
@@ -0,0 +1,5 @@
+<?php
+ob_start();
+header('Content-Disposition: attachment; filename="downloaded.txt"');
+?>
+foo bar baz
diff --git a/third_party/blink/web_tests/inspector-protocol/page/pageNavigateToFragment-expected.txt b/third_party/blink/web_tests/inspector-protocol/page/pageNavigateToFragment-expected.txt
index 138fe2a..f197826 100644
--- a/third_party/blink/web_tests/inspector-protocol/page/pageNavigateToFragment-expected.txt
+++ b/third_party/blink/web_tests/inspector-protocol/page/pageNavigateToFragment-expected.txt
@@ -3,6 +3,7 @@
id : <number>
result : {
frameId : <string>
+ isDownload : false
loaderId : <string>
}
sessionId : <string>
@@ -11,6 +12,7 @@
id : <number>
result : {
frameId : <string>
+ isDownload : false
}
sessionId : <string>
}
