New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 606996 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Out until 24 Jan
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

OOPIF: Going back to data URL or about:blank in subframe uses wrong process

Project Member Reported by creis@chromium.org, Apr 26 2016

Issue description

Version: 52.0.2717.0 and earlier
OS: All

What steps will reproduce the problem?
(0) Start Chrome with --site-per-process
(1) http://csreis.github.io/tests/cross-site-iframe.html (which loads a data URL in the subframe)
(2) Click "Go cross-site (simple page)" (Should have a subframe process now)
(3) Go back.

What is the expected output?

The data URL should load in the same SiteInstance/process as the first time we visited it (i.e., the parent page's process).

What do you see instead?

The data URL loads in the subframe process.

This means we don't respect the SiteInstance that's on the FrameNavigationEntry, probably because we think we don't need to swap for data URLs.  Applies to about:blank pages as well (if you navigate to them manually so that they stay in session history).

 

Comment 1 by nasko@chromium.org, May 2 2016

Here is a browser test that checks for this:

// Ensures that navigating to data: URLs present in session history will
// correctly commit the navigation in the same process as the parent frame.
// See  https://crbug.com/606996 .
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
                       NavigateSubframeToDataUrlInSessionHistory) {
  GURL main_url(embedded_test_server()->GetURL(
      "a.com", "/cross_site_iframe_factory.html?a(b,b)"));
  GURL about_blank_url("about:blank");
  EXPECT_TRUE(NavigateToURL(shell(), main_url));

  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
  EXPECT_EQ(2U, root->child_count());
  EXPECT_EQ(
      " Site A ------------ proxies for B\n"
      "   |--Site B ------- proxies for A\n"
      "   +--Site B ------- proxies for A\n"
      "Where A = http://a.com/\n"
      "      B = http://b.com/",
      DepictFrameTree(root));

  TestNavigationObserver observer(shell()->web_contents());
  FrameTreeNode* child = root->child_at(0);

  // Navigate iframe to a data URL, which will commit in a new SiteInstance.
  GURL data_url("data:text/html,dataurl");
  NavigateFrameToURL(child, data_url);
  EXPECT_TRUE(observer.last_navigation_succeeded());
  EXPECT_EQ(data_url, observer.last_navigation_url());
  SiteInstanceImpl* orig_site_instance =
    child->current_frame_host()->GetSiteInstance();
  EXPECT_NE(orig_site_instance, root->current_frame_host()->GetSiteInstance());

  // Navigate it to another cross-site url.
  GURL cross_site_url(embedded_test_server()->GetURL("c.com", "/title1.html"));
  NavigateFrameToURL(child, cross_site_url);
  EXPECT_TRUE(observer.last_navigation_succeeded());
  EXPECT_EQ(cross_site_url, observer.last_navigation_url());
  EXPECT_EQ(3, web_contents()->GetController().GetEntryCount());
  EXPECT_NE(orig_site_instance, child->current_frame_host()->GetSiteInstance());

  LOG(ERROR) << "Test: going back!";
  // Go back and ensure the data: URL committed in the same SiteInstance as the
  // original navigation.
  EXPECT_TRUE(web_contents()->GetController().CanGoBack());
  TestFrameNavigationObserver frame_observer(child);
  web_contents()->GetController().GoBack();
  LOG(ERROR) << "Test: going back! - initiated";
  frame_observer.WaitForCommit();
  LOG(ERROR) << "Test: going back! - complete";
  EXPECT_EQ(orig_site_instance, child->current_frame_host()->GetSiteInstance());
}

Project Member

Comment 2 by bugdroid1@chromium.org, May 6 2016

Project Member

Comment 3 by bugdroid1@chromium.org, May 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58b07f5147cb47dc85302bd3a112efb627c972ff

commit 58b07f5147cb47dc85302bd3a112efb627c972ff
Author: nasko <nasko@chromium.org>
Date: Mon May 09 22:38:35 2016

Take session history SiteInstance into account for unique origin navigations.

This CL fixes an issue with navigating to data: URLs in --site-per-process
mode, which caused the data: URL to be loaded in incorrect process.

BUG= 606996 
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/1960703003
Cr-Commit-Position: refs/heads/master@{#392450}

[modify] https://crrev.com/58b07f5147cb47dc85302bd3a112efb627c972ff/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/58b07f5147cb47dc85302bd3a112efb627c972ff/content/browser/site_per_process_browsertest.cc

Comment 4 by nasko@chromium.org, May 9 2016

Status: Fixed (was: Assigned)
This should make it in tomorrow's canary. I'm going to mark it as fixed and feel free to reopen if there are still issues.

Sign in to add a comment