New issue
Advanced search Search tips
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 607205: Can't navigate back/forward in subframes after changing window.name

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

Issue description

Version: 52.0.2718.0 (default Chrome or --site-per-process)
OS: All

What steps will reproduce the problem?
(1) Visit a page with an iframe (e.g., https://csreis.github.io/tests/cross-site-iframe.html).
(2) In DevTools, assign the iframe a name (window.name = "foo" in the context of the iframe).
(3) Navigate to https://csreis.github.io/tests/ in the iframe.
(4) Navigate to https://csreis.github.io/tests/simple.html in the iframe.
(5) Go back.

What is the expected output?

You should see https://csreis.github.io/tests/ in the iframe (and simple.html if you go forward again).

What do you see instead?

You see the "Initial page" data URL after going back (and after going forward again).

This happens because we track session history items for subframes using solely the unique name of the frame, and the unique name can change when the frame's name changes.  This prevents us from doing *any* history navigations in a frame that changes its name after creation.  We don't add the history item to the tree, so when we try to go back or forward, we find no frames to navigate and reload the main frame instead.  This causes us to load the initial URL for the iframe.

This applies in both default Chrome (due to HistoryEntry::HistoryNode::CloneAndReplace) and --site-per-process mode (due to NavigationEntryImpl::TreeNode::CloneAndReplace).

We should make sure the history item can be added to the tree even if the name has changed.
 

Comment 1 by creis@chromium.org, Apr 27 2016

This has been true for a long time, so I'll propose fixing it in the new navigation path (NavigationEntryImpl::TreeNode::CloneAndReplace), since we're close to switching over.

Comment 2 by lukasza@chromium.org, Aug 31 2016

Cc: lukasza@chromium.org

Comment 3 by bugdroid1@chromium.org, Sep 13 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ae993fad864e5c683d009ebdfbb0bdae3faef44

commit 2ae993fad864e5c683d009ebdfbb0bdae3faef44
Author: lukasza <lukasza@chromium.org>
Date: Tue Sep 13 20:28:10 2016

Avoid mutating frame unique name after first real commit.

History navigations depend on frame unique names to identify frames that
need to load their state from history.  This identification is broken if
a frame's unique name can change throughout frame's lifetime.  For
example - the newly added layout tests fails (before FrameTree.cpp
changes introduced by this CL) to correctly navigate the subframe after
a back navigation.

BUG= 607205 

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

[modify] https://crrev.com/2ae993fad864e5c683d009ebdfbb0bdae3faef44/third_party/WebKit/LayoutTests/fast/forms/form-and-frame-interaction-retains-values-expected.txt
[add] https://crrev.com/2ae993fad864e5c683d009ebdfbb0bdae3faef44/third_party/WebKit/LayoutTests/http/tests/navigation/rename-subframe-goback-expected.txt
[add] https://crrev.com/2ae993fad864e5c683d009ebdfbb0bdae3faef44/third_party/WebKit/LayoutTests/http/tests/navigation/rename-subframe-goback.html
[modify] https://crrev.com/2ae993fad864e5c683d009ebdfbb0bdae3faef44/third_party/WebKit/Source/core/page/FrameTree.cpp
[modify] https://crrev.com/2ae993fad864e5c683d009ebdfbb0bdae3faef44/third_party/WebKit/Source/web/tests/WebFrameTest.cpp

Comment 5 by creis@chromium.org, Sep 15 2016

Owner: lukasza@chromium.org
Status: Fixed (was: Assigned)
These repro steps are working now in 55.0.2861.0.  Thanks, lukasza@!

Comment 6 by bugdroid1@chromium.org, Oct 6 2016

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77df108d260b0f877cfae096bf136397fce6b4f8

commit 77df108d260b0f877cfae096bf136397fce6b4f8
Author: dcheng <dcheng@chromium.org>
Date: Thu Oct 06 02:54:17 2016

Add UMA to measure feasibility of making unique names immutable

r418350 changed Chrome so that unique names are fixed once the frame has
committed the first real load. Unfortunately, the bug still occurs if
there are in-page navigations on the initial empty document mixed with
window.name changes, since the first real load has not yet committed.

One possible simplification is to just make unique name completely
immutable. However, it's possible that some sites depend on creating a
browsing context, naming it, and then navigating it. This measures how
many sites depend on that behavior (though it doesn't measure how many
sites might be broken if this behavior changes).

BUG= 607205 

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

[modify] https://crrev.com/77df108d260b0f877cfae096bf136397fce6b4f8/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/77df108d260b0f877cfae096bf136397fce6b4f8/content/renderer/render_frame_impl.h
[modify] https://crrev.com/77df108d260b0f877cfae096bf136397fce6b4f8/tools/metrics/histograms/histograms.xml

Comment 7 by bugdroid1@chromium.org, Oct 27 2016

Project Member
Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77df108d260b0f877cfae096bf136397fce6b4f8

commit 77df108d260b0f877cfae096bf136397fce6b4f8
Author: dcheng <dcheng@chromium.org>
Date: Thu Oct 06 02:54:17 2016

Add UMA to measure feasibility of making unique names immutable

r418350 changed Chrome so that unique names are fixed once the frame has
committed the first real load. Unfortunately, the bug still occurs if
there are in-page navigations on the initial empty document mixed with
window.name changes, since the first real load has not yet committed.

One possible simplification is to just make unique name completely
immutable. However, it's possible that some sites depend on creating a
browsing context, naming it, and then navigating it. This measures how
many sites depend on that behavior (though it doesn't measure how many
sites might be broken if this behavior changes).

BUG= 607205 

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

[modify] https://crrev.com/77df108d260b0f877cfae096bf136397fce6b4f8/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/77df108d260b0f877cfae096bf136397fce6b4f8/content/renderer/render_frame_impl.h
[modify] https://crrev.com/77df108d260b0f877cfae096bf136397fce6b4f8/tools/metrics/histograms/histograms.xml

Comment 8 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment