Navigating remote frame to about:blank has incorrect origin and doesn't go back into parent's SiteInstance |
||||||
Issue description
The following test illustrates the issue:
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
BrowserInitiatedNavigationRemoteToAboutBlank) {
GURL main_url(embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?a(b)"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
WebContents* contents = shell()->web_contents();
FrameTreeNode* root =
static_cast<WebContentsImpl*>(contents)->GetFrameTree()->root();
EXPECT_EQ(1U, root->child_count());
FrameTreeNode* child = root->child_at(0);
EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
child->current_frame_host()->GetSiteInstance());
GURL about_blank(url::kAboutBlankURL);
NavigateFrameToURL(child, about_blank);
EXPECT_EQ(main_url.GetOrigin().spec(),
child->current_origin().Serialize() + "/");
EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
child->current_frame_host()->GetSiteInstance());
}
Navigating a remote frame to about:blank should bring the frame back to its parent's SiteInstance, and its origin should be the same as the parent. This is not happening, and the last two EXPECTs are failing:
../../content/browser/site_per_process_browsertest.cc:5538: Failure
Value of: child->current_origin().Serialize() + "/"
Actual: "null/"
Expected: main_url.GetOrigin().spec()
Which is: "http://a.com:39766/"
../../content/browser/site_per_process_browsertest.cc:5540: Failure
Value of: child->current_frame_host()->GetSiteInstance()
Actual: 0x2ae0719222a0
Expected: shell()->web_contents()->GetSiteInstance()
Which is: 0x2ae07191c520
Note that this works fine for renderer-initiated subframe navigations, but breaks for browser-initiated subframe navigations (which should only be possible in browser tests).
,
Mar 2 2016
Is this the same root cause for issue 585649?
,
Mar 2 2016
,
Mar 3 2016
#2: I think that issue is slightly different, as all navigations there are renderer-initiated, so about:blank's origin should be determined either on the renderer side or via transfer logic where source site instance should be non-null. For example, I've checked the last navigation in your test for 585649 (where frame navigates self to about:blank), and that frame's origin ends up as unique because it reaches Document::initSecurityContext with a DocumentInit that doesn't have an owner() (since its parent is remote), so it ends up not inheriting the origin from anything and creating a new SecurityOrigin out of its m_url (about:blank), which results in a unique origin. It doesn't involve this browser-side logic at all, IIUC. There is also no ambiguity in this issue about which frame's SiteInstance to use for the about:blank. Since it's browser-initiated, it should probably be the parent frame, or, for main frames, the opener frame (if any). Which brings up a good point that we should check how this behaves for main frames, which Nasko also mentioned offline.
,
Apr 15 2016
I'll start looking at this one.
,
Apr 26 2016
,
Apr 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d948aa86b4ab06738c7d8c67fb13bf42d95a1ff3 commit d948aa86b4ab06738c7d8c67fb13bf42d95a1ff3 Author: nasko <nasko@chromium.org> Date: Tue Apr 26 19:58:52 2016 Add an extra check in NavigateRemoteFrameToBlankAndDataURLs. This CL adds an explicit navigation from within an iframe to about:blank. It verifies that the iframe stays in the same process after navigation has completed as the initiator of the navigation is the iframe itself. BUG=591509 Review URL: https://codereview.chromium.org/1917223002 Cr-Commit-Position: refs/heads/master@{#389862} [modify] https://crrev.com/d948aa86b4ab06738c7d8c67fb13bf42d95a1ff3/content/browser/site_per_process_browsertest.cc
,
Apr 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/faa01fb5cdfe41211cbe25154445d50dd774d97e commit faa01fb5cdfe41211cbe25154445d50dd774d97e Author: nasko <nasko@chromium.org> Date: Sat Apr 30 01:04:17 2016 Add a test for iframe srcdoc origin. This CL adds a test to ensure that iframes with srcdoc are properly navigated to the origin of the parent. BUG=591509, 585649 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation Review-Url: https://codereview.chromium.org/1922243002 Cr-Commit-Position: refs/heads/master@{#390826} [modify] https://crrev.com/faa01fb5cdfe41211cbe25154445d50dd774d97e/content/browser/frame_host/frame_tree_browsertest.cc
,
Jun 2 2016
After the above fixes, about:blank navigations in session history commit in the same SiteInstance as the original navigation. We need more work to get about:blank navigations to work "correctly" in all cases, but this will require an Intent to Implement and behavior changes. That work is not blocking launching --isolate-extensions.
,
Jan 11
You started fixing this bug over two years ago. Are you still working on it? |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by alex...@chromium.org
, Mar 2 2016Looks like there is some logic for about:blank in RenderFrameHostManager::UpdateStateForNavigate, but it doesn't seem to work for browser-initiated navigations: // If dest_url is a unique origin like about:blank, then the need for a swap // is determined by the source_instance. GURL resolved_url = dest_url; if (url::Origin(resolved_url).unique()) { // If there is no source_instance for a unique origin, then we should // avoid a process swap. if (!source_instance) return render_frame_host_.get(); ... } When running the repro test, source_instance is null, and url::Origin(resolved_url).unique() is true for about:blank, so we end up returning the current render_frame_host_ (which has b.com in the test).