Skip to content

GitLab

T
tangled
Commit e7250ebe authored by lukasza's avatar lukasza Committed by Commit bot
Browse files
Options

Avoiding going through RenderFrameProxy when targeting a *new* frame. 

BUG=647772

Review-Url: https://chromiumcodereview.appspot.com/2410303006
Cr-Commit-Position: refs/heads/master@{#426483}
parent f8003d49
Hide whitespace changes
Inline Side-by-side
Showing with 103 additions and 10 deletions
chrome/browser/chrome_site_per_process_browsertest.cc
View file @ e7250ebe
...@@ -249,3 +249,39 @@ IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest, PopupWindowFocus) { ...@@ -249,3 +249,39 @@ IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest, PopupWindowFocus) {
// The popup should be focused now. // The popup should be focused now.
EXPECT_EQ(popup, browser()->tab_strip_model()->GetActiveWebContents()); EXPECT_EQ(popup, browser()->tab_strip_model()->GetActiveWebContents());
} }
// Verify that ctrl-click of an anchor targeting a remote frame works (i.e. that
// it opens the link in a new tab). See also https://crbug.com/647772.
IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessTest,
AnchorCtrlClickWhenTargetIsCrossSite) {
// Navigate to anchor_targeting_remote_frame.html.
GURL main_url(embedded_test_server()->GetURL(
"a.com", "/frame_tree/anchor_targeting_remote_frame.html"));
ui_test_utils::NavigateToURL(browser(), main_url);
// Verify that there is only 1 active tab (with the right contents committed).
EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
content::WebContents* main_contents =
browser()->tab_strip_model()->GetWebContentsAt(0);
EXPECT_EQ(main_url, main_contents->GetLastCommittedURL());
// Ctrl-click the anchor/link in the page.
content::WebContentsAddedObserver new_tab_observer;
#if defined(OS_MACOSX)
std::string new_tab_click_script = "simulateClick({ metaKey: true });";
#else
std::string new_tab_click_script = "simulateClick({ ctrlKey: true });";
#endif
EXPECT_TRUE(ExecuteScript(main_contents, new_tab_click_script));
// Wait for a new tab to appear (the whole point of this test).
content::WebContents* new_contents = new_tab_observer.GetWebContents();
// Verify that the new tab has the right contents and is in the right, new
// place in the tab strip.
EXPECT_TRUE(WaitForLoadStop(new_contents));
EXPECT_EQ(2, browser()->tab_strip_model()->count());
EXPECT_EQ(new_contents, browser()->tab_strip_model()->GetWebContentsAt(1));
GURL expected_url(embedded_test_server()->GetURL("c.com", "/title1.html"));
EXPECT_EQ(expected_url, new_contents->GetLastCommittedURL());
}
chrome/test/data/frame_tree/anchor_targeting_remote_frame.html 0 → 100644
View file @ e7250ebe
<!doctype html>
<html>
<head>
<script>
function simulateClick(properties) {
var evt = new MouseEvent("click", properties);
target = document.getElementById("test-anchor");
return target.dispatchEvent(evt);
}
</script>
</head>
<body>
This page helps testing shift-clicking or ctrl-clicking an anchor/link
that normally (when clicked without any modifier keys) targets a remote,
cross-site frame. See also https://crbug.com/647772.
<hr>
<a href="/cross-site/c.com/title1.html"
target="cross-site-frame"
id="test-anchor">Test link to click while holding ctrl key</a>
<hr>
<iframe
name="cross-site-frame"
src="/cross-site/b.com/title1.html"></iframe>
</body>
</html>
third_party/WebKit/Source/core/loader/FrameLoader.cpp
View file @ e7250ebe
...@@ -975,6 +975,37 @@ static bool shouldOpenInNewWindow(Frame* targetFrame, ...@@ -975,6 +975,37 @@ static bool shouldOpenInNewWindow(Frame* targetFrame,
return request.form() && policy != NavigationPolicyCurrentTab; return request.form() && policy != NavigationPolicyCurrentTab;
} }
static bool shouldNavigateTargetFrame(NavigationPolicy policy) {
switch (policy) {
case NavigationPolicyCurrentTab:
return true;
// Navigation will target a *new* frame (e.g. because of a ctrl-click),
// so the target frame can be ignored.
case NavigationPolicyNewBackgroundTab:
case NavigationPolicyNewForegroundTab:
case NavigationPolicyNewWindow:
case NavigationPolicyNewPopup:
return false;
// Navigation won't really target any specific frame,
// so the target frame can be ignored.
case NavigationPolicyIgnore:
case NavigationPolicyDownload:
return false;
case NavigationPolicyHandledByClient:
// Impossible, because at this point we shouldn't yet have called
// client()->decidePolicyForNavigation(...).
NOTREACHED();
return true;
default:
NOTREACHED() << policy;
return true;
}
}
static NavigationType determineNavigationType(FrameLoadType frameLoadType, static NavigationType determineNavigationType(FrameLoadType frameLoadType,
bool isFormSubmission, bool isFormSubmission,
bool haveEvent) { bool haveEvent) {
...@@ -1072,17 +1103,18 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest, ...@@ -1072,17 +1103,18 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
if (!prepareRequestForThisFrame(request)) if (!prepareRequestForThisFrame(request))
return; return;
Frame* targetFrame = request.form()
? nullptr
: m_frame->findFrameForNavigation(
AtomicString(request.frameName()), *m_frame);
if (isBackForwardLoadType(frameLoadType)) { if (isBackForwardLoadType(frameLoadType)) {
DCHECK(historyItem); DCHECK(historyItem);
m_provisionalItem = historyItem; m_provisionalItem = historyItem;
} }
if (targetFrame && targetFrame != m_frame) { Frame* targetFrame = request.form()
? nullptr
: m_frame->findFrameForNavigation(
AtomicString(request.frameName()), *m_frame);
NavigationPolicy policy = navigationPolicyForRequest(request);
if (targetFrame && targetFrame != m_frame &&
shouldNavigateTargetFrame(policy)) {
bool wasInSamePage = targetFrame->page() == m_frame->page(); bool wasInSamePage = targetFrame->page() == m_frame->page();
request.setFrameName("_self"); request.setFrameName("_self");
...@@ -1095,10 +1127,6 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest, ...@@ -1095,10 +1127,6 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
setReferrerForFrameRequest(request); setReferrerForFrameRequest(request);
FrameLoadType newLoadType = (frameLoadType == FrameLoadTypeStandard)
? determineFrameLoadType(request)
: frameLoadType;
NavigationPolicy policy = navigationPolicyForRequest(request);
if (shouldOpenInNewWindow(targetFrame, request, policy)) { if (shouldOpenInNewWindow(targetFrame, request, policy)) {
if (policy == NavigationPolicyDownload) { if (policy == NavigationPolicyDownload) {
client()->loadURLExternally(request.resourceRequest(), client()->loadURLExternally(request.resourceRequest(),
...@@ -1111,6 +1139,9 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest, ...@@ -1111,6 +1139,9 @@ void FrameLoader::load(const FrameLoadRequest& passedRequest,
} }
const KURL& url = request.resourceRequest().url(); const KURL& url = request.resourceRequest().url();
FrameLoadType newLoadType = (frameLoadType == FrameLoadTypeStandard)
? determineFrameLoadType(request)
: frameLoadType;
bool sameDocumentHistoryNavigation = bool sameDocumentHistoryNavigation =
isBackForwardLoadType(newLoadType) && isBackForwardLoadType(newLoadType) &&
historyLoadType == HistorySameDocumentLoad; historyLoadType == HistorySameDocumentLoad;
......
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