Commit e277ebdb authored by Andrey Kosyakov's avatar Andrey Kosyakov Committed by Commit Bot

Downloads: use navigating frame rather than top frame when intercepting navigation

This sets render process/routing id in DownloadCreateInfo to that of navigating frame instead of the top frame, so that the reported frame is consistent with navigation initiated through renderer-initiated download.

Change-Id: I2fd472703ed745f01f9ec4428c1a3075055e9d7e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225832Reviewed-by: default avatarMin Qin <qinmin@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774489}
parent ad9ed98b
...@@ -870,7 +870,7 @@ void DownloadManagerImpl::InterceptNavigation( ...@@ -870,7 +870,7 @@ void DownloadManagerImpl::InterceptNavigation(
base::OnceCallback<void(bool /* download allowed */)> base::OnceCallback<void(bool /* download allowed */)>
on_download_checks_done = base::BindOnce( on_download_checks_done = base::BindOnce(
&DownloadManagerImpl::InterceptNavigationOnChecksComplete, &DownloadManagerImpl::InterceptNavigationOnChecksComplete,
weak_factory_.GetWeakPtr(), web_contents_getter, weak_factory_.GetWeakPtr(), frame_tree_node_id,
std::move(resource_request), std::move(url_chain), cert_status, std::move(resource_request), std::move(url_chain), cert_status,
std::move(response_head), std::move(response_body), std::move(response_head), std::move(response_body),
std::move(url_loader_client_endpoints)); std::move(url_loader_client_endpoints));
...@@ -1209,7 +1209,7 @@ void DownloadManagerImpl::DropDownload() { ...@@ -1209,7 +1209,7 @@ void DownloadManagerImpl::DropDownload() {
} }
void DownloadManagerImpl::InterceptNavigationOnChecksComplete( void DownloadManagerImpl::InterceptNavigationOnChecksComplete(
WebContents::Getter web_contents_getter, int frame_tree_node_id,
std::unique_ptr<network::ResourceRequest> resource_request, std::unique_ptr<network::ResourceRequest> resource_request,
std::vector<GURL> url_chain, std::vector<GURL> url_chain,
net::CertStatus cert_status, net::CertStatus cert_status,
...@@ -1226,13 +1226,15 @@ void DownloadManagerImpl::InterceptNavigationOnChecksComplete( ...@@ -1226,13 +1226,15 @@ void DownloadManagerImpl::InterceptNavigationOnChecksComplete(
int render_frame_id = -1; int render_frame_id = -1;
GURL site_url, tab_url, tab_referrer_url; GURL site_url, tab_url, tab_referrer_url;
RenderFrameHost* render_frame_host = nullptr; RenderFrameHost* render_frame_host = nullptr;
WebContents* web_contents = std::move(web_contents_getter).Run(); auto* ftn = FrameTreeNode::GloballyFindByID(frame_tree_node_id);
if (web_contents) { if (ftn) {
render_frame_host = web_contents->GetMainFrame(); render_frame_host = ftn->current_frame_host();
if (render_frame_host) { if (render_frame_host) {
render_process_id = render_frame_host->GetProcess()->GetID(); render_process_id = render_frame_host->GetProcess()->GetID();
render_frame_id = render_frame_host->GetRoutingID(); render_frame_id = render_frame_host->GetRoutingID();
} }
auto* web_contents = WebContentsImpl::FromFrameTreeNode(ftn);
DCHECK(web_contents);
NavigationEntry* entry = web_contents->GetController().GetVisibleEntry(); NavigationEntry* entry = web_contents->GetController().GetVisibleEntry();
if (entry) { if (entry) {
tab_url = entry->GetURL(); tab_url = entry->GetURL();
......
...@@ -264,7 +264,7 @@ class CONTENT_EXPORT DownloadManagerImpl ...@@ -264,7 +264,7 @@ class CONTENT_EXPORT DownloadManagerImpl
const GURL& site_url); const GURL& site_url);
void InterceptNavigationOnChecksComplete( void InterceptNavigationOnChecksComplete(
WebContents::Getter web_contents_getter, int frame_tree_node_id,
std::unique_ptr<network::ResourceRequest> resource_request, std::unique_ptr<network::ResourceRequest> resource_request,
std::vector<GURL> url_chain, std::vector<GURL> url_chain,
net::CertStatus cert_status, net::CertStatus cert_status,
......
Tests we properly emit Page.downloadWillBegin. Tests we properly emit Page.downloadWillBegin.
Downloading via a navigation: Downloading via a navigation:
downloadWillBegin from top frame: {
method : Page.downloadWillBegin
params : {
frameId : <string>
guid : <string>
suggestedFilename : hello.txt
url : http://127.0.0.1:8000/devtools/network/resources/resource.php?download=1
}
sessionId : <string>
}
{ {
method : Page.downloadProgress
params : {
guid : <string>
receivedBytes : <number>
state : inProgress
totalBytes : 11
}
sessionId : <string>
}
{
method : Page.downloadProgress
params : {
guid : <string>
receivedBytes : <number>
state : completed
totalBytes : 11
}
sessionId : <string>
}
Downloading via a navigation of subframe:
downloadWillBegin from child frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
...@@ -31,7 +62,38 @@ Downloading via a navigation: ...@@ -31,7 +62,38 @@ Downloading via a navigation:
sessionId : <string> sessionId : <string>
} }
Downloading by clicking a link: Downloading by clicking a link:
downloadWillBegin from top frame: {
method : Page.downloadWillBegin
params : {
frameId : <string>
guid : <string>
suggestedFilename : hello.text
url : http://127.0.0.1:8000/devtools/network/resources/resource.php
}
sessionId : <string>
}
{
method : Page.downloadProgress
params : {
guid : <string>
receivedBytes : <number>
state : inProgress
totalBytes : 11
}
sessionId : <string>
}
{ {
method : Page.downloadProgress
params : {
guid : <string>
receivedBytes : <number>
state : completed
totalBytes : 11
}
sessionId : <string>
}
Downloading by clicking a link in subframe:
downloadWillBegin from child frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
...@@ -62,7 +124,7 @@ Downloading by clicking a link: ...@@ -62,7 +124,7 @@ Downloading by clicking a link:
sessionId : <string> sessionId : <string>
} }
Downloading by clicking a link (no HTTP Content-Disposition header, a[download=""]): Downloading by clicking a link (no HTTP Content-Disposition header, a[download=""]):
{ downloadWillBegin from top frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
...@@ -73,7 +135,7 @@ Downloading by clicking a link (no HTTP Content-Disposition header, a[download=" ...@@ -73,7 +135,7 @@ Downloading by clicking a link (no HTTP Content-Disposition header, a[download="
sessionId : <string> sessionId : <string>
} }
Downloading by clicking a link (HTTP Content-Disposition header with filename=foo.txt, no a[download]): Downloading by clicking a link (HTTP Content-Disposition header with filename=foo.txt, no a[download]):
{ downloadWillBegin from top frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
...@@ -84,7 +146,7 @@ Downloading by clicking a link (HTTP Content-Disposition header with filename=fo ...@@ -84,7 +146,7 @@ Downloading by clicking a link (HTTP Content-Disposition header with filename=fo
sessionId : <string> sessionId : <string>
} }
Downloading by clicking a link (HTTP Content-Disposition header with filename=override.txt, a[download="foo.txt"]): Downloading by clicking a link (HTTP Content-Disposition header with filename=override.txt, a[download="foo.txt"]):
{ downloadWillBegin from top frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
...@@ -95,7 +157,7 @@ Downloading by clicking a link (HTTP Content-Disposition header with filename=ov ...@@ -95,7 +157,7 @@ Downloading by clicking a link (HTTP Content-Disposition header with filename=ov
sessionId : <string> sessionId : <string>
} }
Downloading by clicking a link (HTTP Content-Disposition header without filename, no a[download]): Downloading by clicking a link (HTTP Content-Disposition header without filename, no a[download]):
{ downloadWillBegin from top frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
...@@ -106,7 +168,7 @@ Downloading by clicking a link (HTTP Content-Disposition header without filename ...@@ -106,7 +168,7 @@ Downloading by clicking a link (HTTP Content-Disposition header without filename
sessionId : <string> sessionId : <string>
} }
Downloading by clicking a link (HTTP Content-Disposition header without filename, no a[download], js): Downloading by clicking a link (HTTP Content-Disposition header without filename, no a[download], js):
{ downloadWillBegin from top frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
...@@ -117,7 +179,7 @@ Downloading by clicking a link (HTTP Content-Disposition header without filename ...@@ -117,7 +179,7 @@ Downloading by clicking a link (HTTP Content-Disposition header without filename
sessionId : <string> sessionId : <string>
} }
Downloading by clicking a link (HTTP Content-Disposition header without filename, no a[download], image): Downloading by clicking a link (HTTP Content-Disposition header without filename, no a[download], image):
{ downloadWillBegin from top frame: {
method : Page.downloadWillBegin method : Page.downloadWillBegin
params : { params : {
frameId : <string> frameId : <string>
......
(async function(testRunner) { (async function(testRunner) {
var {page, session, dp} = await testRunner.startBlank('Tests we properly emit Page.downloadWillBegin.'); const {page, session, dp} = await testRunner.startBlank('Tests we properly emit Page.downloadWillBegin.');
await dp.Page.enable();
await session.evaluateAsync(`
const frame = document.createElement('iframe');
frame.src = location.href;
document.body.appendChild(frame);
new Promise(resolve => frame.onload = resolve);
`);
const frameTree = (await dp.Page.getFrameTree()).result.frameTree;
const frameNames = new Map([
[frameTree.frame.id, 'top'],
[frameTree.childFrames[0].frame.id, 'child']
]);
dp.Page.onDownloadWillBegin(event => { dp.Page.onDownloadWillBegin(event => {
testRunner.log(event); const frame = frameNames.get(event.params.frameId) || 'unknown';
testRunner.log(event, `downloadWillBegin from ${frame} frame: `);
}); });
async function waitForDownloadAndDump() { async function waitForDownloadAndDump() {
...@@ -23,19 +39,30 @@ ...@@ -23,19 +39,30 @@
await dp.Page.onceDownloadProgress(event => event.params.state === 'completed'); await dp.Page.onceDownloadProgress(event => event.params.state === 'completed');
} }
await dp.Page.enable();
testRunner.log('Downloading via a navigation: '); testRunner.log('Downloading via a navigation: ');
session.evaluate('location.href = "/devtools/network/resources/resource.php?download=1"'); session.evaluate('location.href = "/devtools/network/resources/resource.php?download=1"');
await waitForDownloadAndDump(); await waitForDownloadAndDump();
testRunner.log('Downloading by clicking a link: ');
session.evaluate(` testRunner.log('Downloading via a navigation of subframe: ');
const a = document.createElement('a'); session.evaluate(`frames[0].frameElement.src = '/devtools/network/resources/resource.php?download=1'`);
await waitForDownloadAndDump();
function createLinkAndClick(doc) {
const a = doc.createElement('a');
a.href = '/devtools/network/resources/resource.php'; a.href = '/devtools/network/resources/resource.php';
a.download = 'hello.text'; a.download = 'hello.text';
document.body.appendChild(a); doc.body.appendChild(a);
a.click(); a.click();
`); }
testRunner.log('Downloading by clicking a link: ');
session.evaluate(`(${createLinkAndClick})(document)`);
await waitForDownloadAndDump();
testRunner.log('Downloading by clicking a link in subframe: ');
session.evaluate(`(${createLinkAndClick})(frames[0].document)`);
await waitForDownloadAndDump(); await waitForDownloadAndDump();
testRunner.log( testRunner.log(
'Downloading by clicking a link (no HTTP Content-Disposition header, a[download=""]): '); 'Downloading by clicking a link (no HTTP Content-Disposition header, a[download=""]): ');
session.evaluate(` session.evaluate(`
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment