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

Issue 608402 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Create subframe FrameNavigationEntries in all cases

Project Member Reported by creis@chromium.org, May 2 2016

Issue description

Version: 52.0.2722.0
OS: All

There are several cases in which we do not create subframe FrameNavigationEntries in OOPIF-enabled modes of Chrome.  This mostly matches the behavior of HistoryController in default Chrome, where WebHistoryItems are not tracked for these frames.  In practice this means we cannot go back/forward in these subframes.

This case also led to crashes in issue 600743 when trying to avoid creating pending entries for transfer navigations in r384009.  We can probably avoid that crash to re-land that CL, and then fix this separately.

Repro 1: No commit in parent.
1) Create an iframe on a page that does not commit (e.g., https://www.google.com/csi for a 204 response).
2) Create a nested iframe inside it (e.g., to https://csreis.github.io/tests).
3) Click the link to simple.html.
Bug: Going back reloads the main frame rather than going back in the subframe.
Note: Same thing happens with a slow URL in the parent, rather than a 204 response.  Same for download.

Repro 2: No record of the parent.
1) Do an in-page navigation to #foo on a page.
2) Create an iframe on the page (e.g., https://csreis.github.io).
3) Go back.
4) Create a nested iframe inside the iframe (e.g., to https://csreis.github.io/tests).
5) Click the link to simple.html.
Bug: Incorrectly triggers the mixed content blocker.  If you use HTTP URLs instead, then going back reloads the main frame rather than going back in the subframe.

Repro 3: Change name of parent.
1) Create an iframe on the page (e.g., https://csreis.github.io).
2) Change the name of the iframe (window.name = "foo" inside the iframe).
3) Create a nested iframe inside the iframe (e.g., to https://csreis.github.io/tests).
4) Click the link to simple.html.
Bug: Going back reloads the main frame rather than going back in the subframe.

These all have the same root cause: not finding the parent frame's FrameNavigationEntry, and thus not having anywhere to put the subframe's FrameNavigationEntry.  See also  issue 607205  (for window.name changes) and issue 524208 (for an initial blank NavigationEntry, since these bugs also apply in that case).
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 4 2016

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

commit c717927a3712cf6391962971e0e4084a8f159ba0
Author: creis <creis@chromium.org>
Date: Wed May 04 03:35:58 2016

Don't use pending NavigationEntries for navigation transfers (try #2).

Cross-process transfers for navigations that are about to commit
had been using NavigationControllerImpl::LoadURLWithParams (as
an artifact of previously going through OpenURL).  Most of that
code is unnecessary, in particular the fact that it creates a
new pending NavigationEntry.  That's problematic for subframe
transfers, where we should not affect any existing pending
NavigationEntry (e.g., for a slow main frame navigation).

This CL shortcuts the transfer by creating a NavigationEntry
without making it the pending one.  A future CL can eliminate
the entry entirely by validating and passing the parameters
to the right RenderFrameHost directly.

This also makes it possible to do cross-process navigations
in subframes of the initial blank page, fixing a crash.

This CL is a second attempt that fixes crashes seen after
https://crrev.com/384009, and adds tests for those cases.
It works around https://crbug.com/608402 for the time being.

BUG= 495161 , 584739, 608402
TEST=Create OOPIF on initial blank page of a tab.
TEST=Cross-process subframe navigation during slow main frame navigation.
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/c717927a3712cf6391962971e0e4084a8f159ba0/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/c717927a3712cf6391962971e0e4084a8f159ba0/content/browser/frame_host/navigator_impl.cc

Sign in to add a comment