Commit d8809db3 authored by Wolfgang Beyer's avatar Wolfgang Beyer Committed by Commit Bot

[DevTools] Add 'openerFrameId' to targetInfo

We want to show which frame caused the opening of an additional window
in the DevTools frontend.

We cannot really use FrameTreeNode's 'opener', since this is reset if
the opened window itself does not have access to its opener ('noopener'
or COOP). We cannot use 'original_opener' either, because it tracks
main frames only. Therefore a new field 'opener_frame_id' is
introduced.

This is a follow-up to https://crrev.com/c/2309698

Bug: chromium:1107766

Change-Id: Idf24776a234ec029736bb74920d80c8da3c4c98a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340976Reviewed-by: default avatarCamille Lamy <clamy@chromium.org>
Reviewed-by: default avatarSigurd Schneider <sigurds@chromium.org>
Reviewed-by: default avatarAndrey Kosyakov <caseq@chromium.org>
Commit-Queue: Wolfgang Beyer <wolfi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812093}
parent 21a6cf8c
...@@ -240,6 +240,10 @@ std::string DevToolsAgentHostImpl::GetOpenerId() { ...@@ -240,6 +240,10 @@ std::string DevToolsAgentHostImpl::GetOpenerId() {
return std::string(); return std::string();
} }
std::string DevToolsAgentHostImpl::GetOpenerFrameId() {
return std::string();
}
bool DevToolsAgentHostImpl::CanAccessOpener() { bool DevToolsAgentHostImpl::CanAccessOpener() {
return false; return false;
} }
......
...@@ -45,6 +45,7 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost { ...@@ -45,6 +45,7 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
scoped_refptr<base::RefCountedMemory> data) override; scoped_refptr<base::RefCountedMemory> data) override;
std::string GetParentId() override; std::string GetParentId() override;
std::string GetOpenerId() override; std::string GetOpenerId() override;
std::string GetOpenerFrameId() override;
bool CanAccessOpener() override; bool CanAccessOpener() override;
std::string GetDescription() override; std::string GetDescription() override;
GURL GetFaviconURL() override; GURL GetFaviconURL() override;
......
...@@ -90,6 +90,8 @@ std::unique_ptr<Target::TargetInfo> CreateInfo(DevToolsAgentHost* host) { ...@@ -90,6 +90,8 @@ std::unique_ptr<Target::TargetInfo> CreateInfo(DevToolsAgentHost* host) {
.Build(); .Build();
if (!host->GetOpenerId().empty()) if (!host->GetOpenerId().empty())
target_info->SetOpenerId(host->GetOpenerId()); target_info->SetOpenerId(host->GetOpenerId());
if (!host->GetOpenerFrameId().empty())
target_info->SetOpenerFrameId(host->GetOpenerFrameId());
if (host->GetBrowserContext()) if (host->GetBrowserContext())
target_info->SetBrowserContextId(host->GetBrowserContext()->UniqueId()); target_info->SetBrowserContextId(host->GetBrowserContext()->UniqueId());
return target_info; return target_info;
......
...@@ -674,6 +674,15 @@ std::string RenderFrameDevToolsAgentHost::GetOpenerId() { ...@@ -674,6 +674,15 @@ std::string RenderFrameDevToolsAgentHost::GetOpenerId() {
return opener ? opener->devtools_frame_token().ToString() : std::string(); return opener ? opener->devtools_frame_token().ToString() : std::string();
} }
std::string RenderFrameDevToolsAgentHost::GetOpenerFrameId() {
if (!frame_tree_node_)
return std::string();
const base::Optional<base::UnguessableToken>& opener_devtools_frame_token =
frame_tree_node_->opener_devtools_frame_token();
return opener_devtools_frame_token ? opener_devtools_frame_token->ToString()
: std::string();
}
bool RenderFrameDevToolsAgentHost::CanAccessOpener() { bool RenderFrameDevToolsAgentHost::CanAccessOpener() {
return (frame_tree_node_ && frame_tree_node_->opener()); return (frame_tree_node_ && frame_tree_node_->opener());
} }
......
...@@ -93,6 +93,7 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost ...@@ -93,6 +93,7 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
WebContents* GetWebContents() override; WebContents* GetWebContents() override;
std::string GetParentId() override; std::string GetParentId() override;
std::string GetOpenerId() override; std::string GetOpenerId() override;
std::string GetOpenerFrameId() override;
bool CanAccessOpener() override; bool CanAccessOpener() override;
std::string GetType() override; std::string GetType() override;
std::string GetTitle() override; std::string GetTitle() override;
......
...@@ -276,6 +276,13 @@ void FrameTreeNode::SetOpener(FrameTreeNode* opener) { ...@@ -276,6 +276,13 @@ void FrameTreeNode::SetOpener(FrameTreeNode* opener) {
} }
} }
void FrameTreeNode::SetOpenerDevtoolsFrameToken(
base::UnguessableToken opener_devtools_frame_token) {
DCHECK(!opener_devtools_frame_token_ ||
opener_devtools_frame_token_->is_empty());
opener_devtools_frame_token_ = std::move(opener_devtools_frame_token);
}
void FrameTreeNode::SetOriginalOpener(FrameTreeNode* opener) { void FrameTreeNode::SetOriginalOpener(FrameTreeNode* opener) {
// The original opener tracks main frames only. // The original opener tracks main frames only.
DCHECK(opener == nullptr || !opener->parent()); DCHECK(opener == nullptr || !opener->parent());
......
...@@ -126,6 +126,10 @@ class CONTENT_EXPORT FrameTreeNode { ...@@ -126,6 +126,10 @@ class CONTENT_EXPORT FrameTreeNode {
FrameTreeNode* original_opener() const { return original_opener_; } FrameTreeNode* original_opener() const { return original_opener_; }
const base::Optional<base::UnguessableToken>& opener_devtools_frame_token() {
return opener_devtools_frame_token_;
}
// Gets the total number of descendants to this FrameTreeNode in addition to // Gets the total number of descendants to this FrameTreeNode in addition to
// this node. // this node.
size_t GetFrameTreeSize() const; size_t GetFrameTreeSize() const;
...@@ -142,6 +146,12 @@ class CONTENT_EXPORT FrameTreeNode { ...@@ -142,6 +146,12 @@ class CONTENT_EXPORT FrameTreeNode {
// It is not possible to change the opener once it was set. // It is not possible to change the opener once it was set.
void SetOriginalOpener(FrameTreeNode* opener); void SetOriginalOpener(FrameTreeNode* opener);
// Assigns an opener frame id for this node. This string id is only set once
// and cannot be changed. It persists, even if the |opener| is destroyed. It
// is used for attribution in the DevTools frontend.
void SetOpenerDevtoolsFrameToken(
base::UnguessableToken opener_devtools_frame_token);
FrameTreeNode* child_at(size_t index) const { FrameTreeNode* child_at(size_t index) const {
return current_frame_host()->child_at(index); return current_frame_host()->child_at(index);
} }
...@@ -478,6 +488,10 @@ class CONTENT_EXPORT FrameTreeNode { ...@@ -478,6 +488,10 @@ class CONTENT_EXPORT FrameTreeNode {
// cannot be changed unless the original opener is destroyed. // cannot be changed unless the original opener is destroyed.
FrameTreeNode* original_opener_; FrameTreeNode* original_opener_;
// The devtools frame token of the frame which opened this frame. This is
// not cleared even if the opener is destroyed or disowns the frame.
base::Optional<base::UnguessableToken> opener_devtools_frame_token_;
// An observer that clears this node's |original_opener_| if the opener is // An observer that clears this node's |original_opener_| if the opener is
// destroyed. // destroyed.
std::unique_ptr<OpenerDestroyedObserver> original_opener_observer_; std::unique_ptr<OpenerDestroyedObserver> original_opener_observer_;
......
...@@ -8839,6 +8839,7 @@ void WebContentsImpl::SetOpenerForNewContents(FrameTreeNode* opener, ...@@ -8839,6 +8839,7 @@ void WebContentsImpl::SetOpenerForNewContents(FrameTreeNode* opener,
// by spawning from a subframe and deleting the subframe. // by spawning from a subframe and deleting the subframe.
// https://crbug.com/705316 // https://crbug.com/705316
new_root->SetOriginalOpener(opener->frame_tree()->root()); new_root->SetOriginalOpener(opener->frame_tree()->root());
new_root->SetOpenerDevtoolsFrameToken(opener->devtools_frame_token());
if (!opener_suppressed) { if (!opener_suppressed) {
new_root->SetOpener(opener); new_root->SetOpener(opener);
......
...@@ -161,6 +161,10 @@ class CONTENT_EXPORT DevToolsAgentHost ...@@ -161,6 +161,10 @@ class CONTENT_EXPORT DevToolsAgentHost
// when using 'noopener' or with enabled COOP). // when using 'noopener' or with enabled COOP).
virtual bool CanAccessOpener() = 0; virtual bool CanAccessOpener() = 0;
// Returns the DevTools token of this window's opener, or empty string if no
// opener.
virtual std::string GetOpenerFrameId() = 0;
// Returns web contents instance for this host if any. // Returns web contents instance for this host if any.
virtual WebContents* GetWebContents() = 0; virtual WebContents* GetWebContents() = 0;
......
...@@ -7397,8 +7397,10 @@ domain Target ...@@ -7397,8 +7397,10 @@ domain Target
boolean attached boolean attached
# Opener target Id # Opener target Id
optional TargetID openerId optional TargetID openerId
# Whether the opened window has access to the originating window. # Whether the target has access to the originating window.
experimental boolean canAccessOpener experimental boolean canAccessOpener
# Frame id of originating window (is only set if target has an opener).
experimental optional Page.FrameId openerFrameId
experimental optional Browser.BrowserContextID browserContextId experimental optional Browser.BrowserContextID browserContextId
experimental type RemoteLocation extends object experimental type RemoteLocation extends object
......
...@@ -18,7 +18,7 @@ var TestRunner = class { ...@@ -18,7 +18,7 @@ var TestRunner = class {
'backendNodeId', 'parentId', 'frameId', 'loaderId', 'baseURL', 'backendNodeId', 'parentId', 'frameId', 'loaderId', 'baseURL',
'documentURL', 'styleSheetId', 'executionContextId', 'openerId', 'documentURL', 'styleSheetId', 'executionContextId', 'openerId',
'targetId', 'browserContextId', 'sessionId', 'receivedBytes', 'targetId', 'browserContextId', 'sessionId', 'receivedBytes',
'ownerNode', 'guid', 'requestId']; 'ownerNode', 'guid', 'requestId', 'openerFrameId'];
} }
startDumpingProtocolMessages() { startDumpingProtocolMessages() {
......
...@@ -5,64 +5,82 @@ Opening without new browsing context ...@@ -5,64 +5,82 @@ Opening without new browsing context
attached : false attached : false
browserContextId : <string> browserContextId : <string>
canAccessOpener : true canAccessOpener : true
openerFrameId : <string>
openerId : <string> openerId : <string>
targetId : <string> targetId : <string>
title : title :
type : page type : page
url : url :
} }
PASS: Correct openerFrameId
PASS: Correct openerId
{ {
attached : false attached : false
browserContextId : <string> browserContextId : <string>
canAccessOpener : true canAccessOpener : true
openerFrameId : <string>
openerId : <string> openerId : <string>
targetId : <string> targetId : <string>
title : 127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html title : 127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html
type : page type : page
url : http://127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html url : http://127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html
} }
PASS: Correct openerFrameId
PASS: Correct openerId
Opening with new browsing context Opening with new browsing context
{ {
attached : false attached : false
browserContextId : <string> browserContextId : <string>
canAccessOpener : false canAccessOpener : false
openerFrameId : <string>
openerId : <string> openerId : <string>
targetId : <string> targetId : <string>
title : title :
type : page type : page
url : url :
} }
PASS: Correct openerFrameId
PASS: Correct openerId
{ {
attached : false attached : false
browserContextId : <string> browserContextId : <string>
canAccessOpener : false canAccessOpener : false
openerFrameId : <string>
openerId : <string> openerId : <string>
targetId : <string> targetId : <string>
title : 127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html title : 127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html
type : page type : page
url : http://127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html url : http://127.0.0.1:8000/inspector-protocol/resources/resources/test-page.html
} }
PASS: Correct openerFrameId
PASS: Correct openerId
Opening with COOP header Opening with COOP header
{ {
attached : false attached : false
browserContextId : <string> browserContextId : <string>
canAccessOpener : true canAccessOpener : true
openerFrameId : <string>
openerId : <string> openerId : <string>
targetId : <string> targetId : <string>
title : title :
type : page type : page
url : url :
} }
PASS: Correct openerFrameId
PASS: Correct openerId
{ {
attached : false attached : false
browserContextId : <string> browserContextId : <string>
canAccessOpener : false canAccessOpener : false
openerFrameId : <string>
openerId : <string> openerId : <string>
targetId : <string> targetId : <string>
title : https://127.0.0.1:8443/inspector-protocol/resources/resources/test-page.html title : https://127.0.0.1:8443/inspector-protocol/resources/resources/test-page.html
type : page type : page
url : https://127.0.0.1:8443/inspector-protocol/resources/resources/test-page.html url : https://127.0.0.1:8443/inspector-protocol/resources/resources/test-page.html
} }
PASS: Correct openerFrameId
PASS: Correct openerId
...@@ -3,27 +3,50 @@ ...@@ -3,27 +3,50 @@
await dp.Target.setDiscoverTargets({discover: true}); await dp.Target.setDiscoverTargets({discover: true});
await dp.Page.enable(); await dp.Page.enable();
session.navigate('./resources/empty.html');
var response = await dp.Target.onceTargetInfoChanged();
const targetId = response.params.targetInfo.targetId;
function compareTargetIds(expectedId, actualId, keyName) {
if (expectedId === actualId)
testRunner.log(`PASS: Correct ${keyName}`);
else
testRunner.log(`FAIL: Incorrect ${keyName}`);
}
testRunner.log(`\nOpening without new browsing context`); testRunner.log(`\nOpening without new browsing context`);
session.evaluate(`window.open('./resources/test-page.html', '_blank'); undefined`); session.evaluate(`window.open('./resources/test-page.html', '_blank'); undefined`);
var response = await dp.Target.onceTargetCreated(); response = await dp.Target.onceTargetCreated();
testRunner.log(response.params.targetInfo); testRunner.log(response.params.targetInfo);
compareTargetIds(targetId, response.params.targetInfo.openerFrameId, 'openerFrameId');
compareTargetIds(targetId, response.params.targetInfo.openerId, 'openerId');
response = await dp.Target.onceTargetInfoChanged(); response = await dp.Target.onceTargetInfoChanged();
testRunner.log(response.params.targetInfo); testRunner.log(response.params.targetInfo);
compareTargetIds(targetId, response.params.targetInfo.openerFrameId, 'openerFrameId');
compareTargetIds(targetId, response.params.targetInfo.openerId, 'openerId');
testRunner.log(`\nOpening with new browsing context`); testRunner.log(`\nOpening with new browsing context`);
session.evaluate(`window.open('./resources/test-page.html', '_blank', 'noopener'); undefined`); session.evaluate(`window.open('./resources/test-page.html', '_blank', 'noopener'); undefined`);
response = await dp.Target.onceTargetCreated(); response = await dp.Target.onceTargetCreated();
testRunner.log(response.params.targetInfo); testRunner.log(response.params.targetInfo);
compareTargetIds(targetId, response.params.targetInfo.openerFrameId, 'openerFrameId');
compareTargetIds(targetId, response.params.targetInfo.openerId, 'openerId');
response = await dp.Target.onceTargetInfoChanged(); response = await dp.Target.onceTargetInfoChanged();
testRunner.log(response.params.targetInfo); testRunner.log(response.params.targetInfo);
compareTargetIds(targetId, response.params.targetInfo.openerFrameId, 'openerFrameId');
compareTargetIds(targetId, response.params.targetInfo.openerId, 'openerId');
testRunner.log(`\nOpening with COOP header`); testRunner.log(`\nOpening with COOP header`);
await dp.Page.navigate({ url: testRunner.url('https://127.0.0.1:8443/inspector-protocol/resources/coop.php')}); await dp.Page.navigate({ url: testRunner.url('https://127.0.0.1:8443/inspector-protocol/resources/coop.php')});
session.evaluate(`window.open('./resources/test-page.html', '_blank'); undefined`); session.evaluate(`window.open('./resources/test-page.html', '_blank'); undefined`);
response = await dp.Target.onceTargetCreated(); response = await dp.Target.onceTargetCreated();
testRunner.log(response.params.targetInfo); testRunner.log(response.params.targetInfo);
var response = await dp.Target.onceTargetInfoChanged(); compareTargetIds(targetId, response.params.targetInfo.openerFrameId, 'openerFrameId');
compareTargetIds(targetId, response.params.targetInfo.openerId, 'openerId');
response = await dp.Target.onceTargetInfoChanged();
testRunner.log(response.params.targetInfo); testRunner.log(response.params.targetInfo);
compareTargetIds(targetId, response.params.targetInfo.openerFrameId, 'openerFrameId');
compareTargetIds(targetId, response.params.targetInfo.openerId, 'openerId');
testRunner.completeTest(); testRunner.completeTest();
}) })
\ No newline at end of file
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