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

Issue 657896 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 0
Type: Bug



Sign in to add a comment

Injected iframe not loading after back navigation

Project Member Reported by creis@chromium.org, Oct 20 2016

Issue description

Version: 54.0.2840.59
OS: All

What steps will reproduce the problem?
(1) Visit https://storage.googleapis.com/mps-storage/miyamoto/support/biglobe/gpt_async.html
(2) Click link to go "Next page"
(3) Go back.

What is the expected output?
Ad should render in the iframe.

What do you see instead?
iframe stays at about:blank.

This is due to switching on the new navigation path in r410150.
 

Comment 1 by creis@chromium.org, Oct 20 2016

Cc: skyos...@chromium.org
Might be related to  issue 637373 , which has a tentative CL (https://codereview.chromium.org/2415013002/).  Haven't looked at the details yet.

Comment 2 by creis@chromium.org, Oct 20 2016

Some early observations:

On the first visit, the test page appears to create 3 iframes, two of which aren't visible.  They all start at about:blank and then get navigated.

When we leave and come back, the test page creates 2 iframes with initial URLs of about:blank.  We send an IPC to the browser process to find the history items for them (which is new as of r410150, rather than having the history items in the renderer process already).  The browser process appears to respond with history items for about:blank, causing us to hit the staying_at_about_blank logic we added in r410748 to fix the similar issue 626416.

I'll try to confirm and compare against what happens in the old mode.

Comment 3 by creis@chromium.org, Oct 20 2016

Confirmed that the browser process has history items of about:blank for the two iframes we create when going back.  Their unique names look like google_ads_iframe_/27979278/BL-TOP-IBA-A_0 and google_ads_iframe_/27979278/BL-TOP-IBA-A_0.  Nothing committed under those frame names the first time we visited the page, so it's correct to consider them about:blank, and it's correct to avoid loading about:blank in them a second time with the staying_at_about_blank logic from issue 626416.

However, during the first visit, there were commits under other frame unique names, which never show up when coming back:
1) 1-0-4;41991;<!doctype html><html><head></head><body leftMargin="0" topMargin="0" marginwidth="0" mar...
2) google_pubads_beacon_iframe
3) <!--framePath //1-0-4;41991;<!doctype html><html><head></head><body leftMargin="0" topMargin="0" mar...
4) google_osd_static_frame

My guess is that one of the frames' unique names changed and then committed a new document.  Not sure why that would have worked before, so I'll try disabling the new navigation path to see if the old path still works well enough to compare.

Comment 4 by creis@chromium.org, Oct 20 2016

Cc: dcheng@chromium.org lukasza@chromium.org
Ok, I think I have a theory.

In the old navigation path, we were creating the about:blank iframes and they were committing right away.  That somehow allowed the script code to continue with loading the subsequent pages into the frames (probably because it was waiting for the frame to commit?).

In the new navigation path, we don't actually get a commit for the about:blank frame (since it looks like the loading case), and we simulate a DidStopLoading instead when we get to the staying_at_about_blank logic.  That lack of commit is probably disrupting the script.  (Not sure how yet, but seems probable.)

Ideally, the unique name change wouldn't make us forget that we have a history item for this frame under a different name, and we'd just load the iframe URL that the user was on the last time.  (That's related to  issue 607205  and whether unique names could be made fully immutable.)

However, a much more practical fix might involve tweaking r421972 (from  issue 649345 ) to have the renderer not ask the browser process for a history item for a frame if we know it's just going to stay at about:blank.  In that fix, the browser told the renderer which unique names it had FrameNavigationEntries for, such that the renderer only had to send an IPC when loading one of those frames.

That list of unique names includes FrameNavigationEntries for about:blank.  If the renderer is just going to stay at about:blank, that's unnecessary work and apparently causes this regression.  We could know this in advance and omit that unique name, so that the renderer just proceeds with loading and committing about:blank without stopping to ask the browser process.  I've tested this in a quick hack and it appears to fix the bug.

(We just need to be careful not to overshoot and ignore cases where a frame started at a real URL and was later navigated to about:blank.  Those cases should still ask the browser process for a history item.  Should be possible to detect.)

Comment 5 by creis@chromium.org, Oct 20 2016

Hmm, that last case is hard to get right, since we only know the parent's URL at the time we're generating the set of subframe unique names.  Maybe it's ok to ignore for now, given that navigating a frame from a real URL to about:blank is likely rare, and then we can fix it in a followup.

Comment 6 by creis@chromium.org, Oct 20 2016

Fix in progress: https://codereview.chromium.org/2437173002

I'll see what it would take to write a test, though I'm not sure how to minimize what the script code is doing yet (to make a similar page that will fail).

Comment 7 by creis@chromium.org, Oct 20 2016

Hmm, that fix is failing some tests in try jobs.  I'll take a closer look.

Comment 8 by creis@chromium.org, Oct 20 2016

Quick update: The current fix failing the following tests:
NavigationControllerBrowserTest.ForwardInSubframeWithPendingForward
NavigationControllerBrowserTest.FrameNavigationEntry_RecreatedBlankSubframe
RenderFrameHostManagerTest.RestoreSubframeFileAccessForHistoryNavigation

The first two are actually failing on TODOs that this fix would address, so that's a good thing.  We would be improving the behavior with the fix.

The third failure is different; investigating now.

Just to have some options on the table, I've also spun up a CL that would revert r410150 and go back to the old navigation path, just to see if that's viable.  (That path has been untested for a while, so it's a bit risky.)  https://codereview.chromium.org/2427413006/ is how it would look if we landed it, but that's basically a no-op on M56 since --isolate-extensions mode is on by default there, meaning that we'd always use the new navigation path anyway.  It would only change behavior on M54 or in the Finch control group, where --isolate-extensions is turned off.  I've also started try jobs on a "do not commit" https://codereview.chromium.org/2441803002/, which simulates turning off --isolate-extensions on trunk so that we can see if the old navigation path still passes all the tests.

These CLs are only useful if we decide the only option is to turn off the new navigation path on M54.  Not clear yet if that will be necessary-- hopefully I can fix the issue with the third test and it won't be needed.

Comment 9 by creis@chromium.org, Oct 20 2016

Third test is failing because it has in iframe srcdoc (which looks like about:blank when it commits) that gets granted access to a file.  When we leave and come back, we expect the process to get granted access to that file again, which happens when the browser tells the renderer to navigate to that history item.

With my proposed fix, we notice that it would just be staying on about:blank, so we let the renderer handle the navigation to about:blank on its own and don't hit the browser-side logic that grants access to the file.

A few options to consider:
1) Identify the srcdoc case as different and exclude it from the fix.  This isn't right: file access could also be granted to a normal about:blank iframe, and we would want to restore file access in that case as well.
2) When the iframe commits, if we notice that it's missing file access to anything in the existing history item, we grant it at that time.

I need to look closer to see if that's reasonable.

Comment 10 by creis@chromium.org, Oct 20 2016

I can get the test to pass with strategy 2 from comment 9, but it's still doing the wrong thing.  Because we don't send the PageState down, the frame doesn't know what files were previously selected in its form, so the form is left empty.  (The process ends up getting granted access to the file, but it never uses that access.)

I'm looking more closely at why the original case is failing, since I'm not sure why the lack of a commit affects the script.  It appears that the page is able to script the outer about:blank iframe, but if it injects an iframe with a src of its own, that simply doesn't load.  That suggests some FrameLoader behavior might be involved?  I wonder if we should work around that aspect?

Alternatively, we could finish up issue 639842 and just give all same-process PageStates to the renderer, letting it handle it synchronously.  That's bigger and harder to merge, but it's the right thing to do.

Comment 11 by creis@chromium.org, Oct 20 2016

I noticed that the original bug doesn't appear to affect srcdoc pages.  This would be good for two reasons: it might provide a workaround until the Chrome fix makes it to M54, and it means I can solve the file access test by excluding srcdoc cases from the fix.

(I mentioned in comment 9 that this is the wrong thing to do because file access could have been granted to about:blank iframes, but that apparently never worked in the first place-- forms injected into about:blank iframes didn't restore their values when you went back, even in the old navigation path.)

I'm trying that out in PS2 of https://codereview.chromium.org/2437173002/.  Detecting iframe srcdoc is a bit tricky because it requires peeking into the PageState, but it's possible.

As for the workaround, we can avoid the bug on the 54.0.2840.59 if we use an iframe srcdoc for the inner URL.

Here's the minimal HTML that repros the bug:
<!doctype html><body>
  <div id="myDiv"></div>
  <script>
    var innerUrl = "https://csreis.github.io";
    var outerIframeHtml = '<iframe width="350" height="300" src="about:blank" id="outer"><\/iframe>';
    var innerIframeHtml = '<iframe width="300" height="250" src="' + innerUrl + '"><\/iframe>';
    //var innerIframeHtml = "foo";

    // Create an empty about:blank iframe inside the preexisting div.
    document.getElementById('myDiv').innerHTML = outerIframeHtml;

    // Populate that empty iframe via doc.write, in a way that causes a request.
    var iDoc = document.getElementById('outer').contentWindow.document;
    iDoc.open();
    iDoc.write(innerIframeHtml);
    iDoc.close();
  </script>
  <br>
  <a href="//www.google.com">Link away</a>
</body></html>


And here's a workaround version that works today:
<!doctype html><body>
  <div id="myDiv"></div>
  <script>
    var innerUrl = "https://csreis.github.io";
    var outerIframeHtml = '<iframe width="350" height="300" srcdoc="<iframe width=300 height=250 src=\'' + innerUrl + '\'><\/iframe>" id="outer"><\/iframe>';

    // Create an empty about:blank iframe inside the preexisting div.
    document.getElementById('myDiv').innerHTML = outerIframeHtml;
  </script>
  <br>
  <a href="//www.google.com">Link away</a>
</body></html>

Issue 657745 has been merged into this issue.

Comment 13 by creis@chromium.org, Oct 21 2016

Quick update on where things stand:

I have two versions of the fix up for review.
https://codereview.chromium.org/2437173002/ is smaller and simpler, but has a known issue (as mentioned in comment 5 on this bug).
https://codereview.chromium.org/2438743005/ is a bit larger, but fixes the known issue.  This CL lets the renderer know whether the history item for each child is about:blank or not, letting the renderer decide whether asking for that history item is a no-op or not.  This is more correct and is also a step towards fixing issue 639842.

--

Things we understand:

* The actual bug we're seeing is that nested iframes aren't making requests if their parent frame is an about:blank document that hasn't yet committed.  While it's unclear why the requests don't happen, it is clear why the new navigation path made this case happen (thanks to the staying_at_about_blank logic in RenderFrameImpl).

* The bug affects about:blank iframes with injected contents, but *not* srcdoc iframes.  srcdoc iframes commit when we go back to them (unlike about:blank iframes), because we exclude them from the staying_at_about_blank logic in RenderFrameImpl.  As a result, we don't exercise the nested-iframe-request bug mentioned above.  This explains how srcdoc can be a workaround for the bug until we get a fix landed/merged.

* about:blank frames with injected contents appear to not use what's stored in their PageState, at least for form state (even in the old navigation path).  This means it's ok to not get the PageState from the browser and just commit the initial about:blank in the renderer (unlike what I thought in comment 9).

* srcdoc iframes *do* use what's stored in their PageState, including form state.  (This explains the third test failure in comment 8.)  Thus, we do have to get the PageState for them from the browser, so it's a good thing the bug doesn't apply to them.


Things we don't yet understand:

* Why aren't we making requests from nested iframes when their parent hasn't committed?  Is this a behavior in Blink?  Does it apply to nested iframes in slowly loading frames, even in the old navigation logic?  Would fixing it carry higher risk than the fixes I have in progress?

* Why are the try jobs failing on the CLs for going back to the old navigation path (https://codereview.chromium.org/2427413006/ and https://codereview.chromium.org/2441803002/)?  Did that code rot in M54, or not until M56?  I'm trying to build/test M54 with the old path just to see if that's an option at all.
I haven't been following along closely (sorry I've been OOO most of the day) but one question:

> In the new navigation path, we don't actually get a commit for the about:blank frame (since it looks like the loading case), and we simulate a DidStopLoading instead when we get to the staying_at_about_blank logic.  That lack of commit is probably disrupting the script.  (Not sure how yet, but seems probable.)

Is this related to how we don't always get a didCommitProvisionalLoad() for the initial empty document in an iframe? Or is this a separate issue?

Comment 15 by creis@chromium.org, Oct 21 2016

Comment 14: I'm pretty sure it's related-- if we had a commit for the initial empty document whether a load starts or not, I don't think this bug would occur.  Then again, I'm less certain because I just found that the "nested iframe requests don't work" issue doesn't happen in the slow URL case.  I don't know what's different between that case and this back/forward case.
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 21 2016

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

commit 37c954965f2f221aec294a0a28dfbba1c7543f2a
Author: Charles Reis <creis@chromium.org>
Date: Fri Oct 21 20:38:58 2016

Fix history nav to a script-injected about:blank frame.

This CL tracks whether the browser process has an about:blank history
item for subframes during a history navigation.  If the renderer would
be staying on about:blank, it can skip asking the browser and commit
the initial about:blank page synchronously.

BUG= 657896 , 639842
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=alexmos@chromium.org

Review URL: https://codereview.chromium.org/2438743005 .

Cr-Commit-Position: refs/heads/master@{#426880}

[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/common/navigation_params.cc
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/common/navigation_params.h
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/renderer/render_frame_impl.h
[add] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/test/data/navigation_controller/inject_iframe_srcdoc_with_nested_frame.html
[add] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/content/test/data/navigation_controller/inject_subframe_into_blank_iframe.html
[modify] https://crrev.com/37c954965f2f221aec294a0a28dfbba1c7543f2a/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

Comment 17 by creis@chromium.org, Oct 21 2016

Status: Fixed (was: Started)
All of the test pages seem to be working as of r426880.  I expect this to be in tomorrow's Canary (most likely 56.0.2898.0).  If it does well over the weekend, we can start requesting merges.

Comment 18 by creis@chromium.org, Oct 24 2016

Labels: M-55 Merge-Request-55
I've verified the fix on 56.0.2898.0 (as has the ads team).  The fix seems to be doing well after baking over the weekend-- I don't see any new crashes due to it.

Requesting merge to M55, since this is a high priority bug affecting ads.  (See b/32296457.)
Is this change applicable to all os or any specific OS?

Comment 20 by creis@chromium.org, Oct 24 2016

Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Ah, sorry for not checking those boxes.  All platforms (except iOS, most likely).

Comment 21 by dimu@chromium.org, Oct 24 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 22 by bugdroid1@chromium.org, Oct 24 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/411325b228248f37b4977f304550d42f0ce04638

commit 411325b228248f37b4977f304550d42f0ce04638
Author: Charles Reis <creis@chromium.org>
Date: Mon Oct 24 18:22:22 2016

Fix history nav to a script-injected about:blank frame.

This CL tracks whether the browser process has an about:blank history
item for subframes during a history navigation.  If the renderer would
be staying on about:blank, it can skip asking the browser and commit
the initial about:blank page synchronously.

BUG= 657896 , 639842
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=alexmos@chromium.org

Review URL: https://codereview.chromium.org/2438743005 .

Cr-Commit-Position: refs/heads/master@{#426880}
(cherry picked from commit 37c954965f2f221aec294a0a28dfbba1c7543f2a)

Review URL: https://codereview.chromium.org/2445053002 .

Cr-Commit-Position: refs/branch-heads/2883@{#243}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/common/navigation_params.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/common/navigation_params.h
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/renderer/render_frame_impl.h
[add] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/test/data/navigation_controller/inject_iframe_srcdoc_with_nested_frame.html
[add] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/test/data/navigation_controller/inject_subframe_into_blank_iframe.html
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

Verified in Android - M56-56.0.2900.3 build
Labels: TE-Verified-M55 TE-Verified-55.0.2883.28
Tested the same on win10 mac 10.11.6 Linux 14.04 using chrome version 55.0.2883.28 and Cros peppy using chrome version 55.0.2883.25/8872.22.0 - Observed that the Ad renders in the iframe after clicking back button.

Please find the screencast
657896.mov
7.6 MB Download

Comment 25 by creis@chromium.org, Oct 27 2016

Cc: bustamante@chromium.org
Labels: Merge-Request-54
I'll get the merge discussion going for M54, since this is a high severity functional bug.

The fix reached M55 Beta in 55.0.2883.28, which went live yesterday.  No fallout in crash reports so far, and the fix has been verified both here (comments 23-24) and on b/32296457 (comment 45).  We'll plan on a postmortem.

Comment 26 by dimu@chromium.org, Oct 27 2016

Labels: -Merge-Request-54 Merge-Review-54 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M54), manual review required.
Labels: -Merge-Review-54 Merge-Approved-54
Per discussion, approving for merge into M54.
Labels: ReleaseBlock-Stable
Project Member

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

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6dbf3923c80c88beab18436e77125b52d446ca08

commit 6dbf3923c80c88beab18436e77125b52d446ca08
Author: Charles Reis <creis@chromium.org>
Date: Thu Oct 27 21:34:20 2016

Fix history nav to a script-injected about:blank frame.

This CL tracks whether the browser process has an about:blank history
item for subframes during a history navigation.  If the renderer would
be staying on about:blank, it can skip asking the browser and commit
the initial about:blank page synchronously.

BUG= 657896 , 639842
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=alexmos@chromium.org

Review URL: https://codereview.chromium.org/2438743005 .

Cr-Commit-Position: refs/heads/master@{#426880}
(cherry picked from commit 37c954965f2f221aec294a0a28dfbba1c7543f2a)

Review URL: https://codereview.chromium.org/2454233002 .

Cr-Commit-Position: refs/branch-heads/2840@{#786}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/common/navigation_params.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/common/navigation_params.h
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/renderer/render_frame_impl.h
[add] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/test/data/navigation_controller/inject_iframe_srcdoc_with_nested_frame.html
[add] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/test/data/navigation_controller/inject_subframe_into_blank_iframe.html
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

Project Member

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

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

commit 411325b228248f37b4977f304550d42f0ce04638
Author: Charles Reis <creis@chromium.org>
Date: Mon Oct 24 18:22:22 2016

Fix history nav to a script-injected about:blank frame.

This CL tracks whether the browser process has an about:blank history
item for subframes during a history navigation.  If the renderer would
be staying on about:blank, it can skip asking the browser and commit
the initial about:blank page synchronously.

BUG= 657896 , 639842
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=alexmos@chromium.org

Review URL: https://codereview.chromium.org/2438743005 .

Cr-Commit-Position: refs/heads/master@{#426880}
(cherry picked from commit 37c954965f2f221aec294a0a28dfbba1c7543f2a)

Review URL: https://codereview.chromium.org/2445053002 .

Cr-Commit-Position: refs/branch-heads/2883@{#243}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/common/navigation_params.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/common/navigation_params.h
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/renderer/render_frame_impl.h
[add] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/test/data/navigation_controller/inject_iframe_srcdoc_with_nested_frame.html
[add] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/content/test/data/navigation_controller/inject_subframe_into_blank_iframe.html
[modify] https://crrev.com/411325b228248f37b4977f304550d42f0ce04638/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

Project Member

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

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

commit 6dbf3923c80c88beab18436e77125b52d446ca08
Author: Charles Reis <creis@chromium.org>
Date: Thu Oct 27 21:34:20 2016

Fix history nav to a script-injected about:blank frame.

This CL tracks whether the browser process has an about:blank history
item for subframes during a history navigation.  If the renderer would
be staying on about:blank, it can skip asking the browser and commit
the initial about:blank page synchronously.

BUG= 657896 , 639842
TEST=See bug for repro steps.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
R=alexmos@chromium.org

Review URL: https://codereview.chromium.org/2438743005 .

Cr-Commit-Position: refs/heads/master@{#426880}
(cherry picked from commit 37c954965f2f221aec294a0a28dfbba1c7543f2a)

Review URL: https://codereview.chromium.org/2454233002 .

Cr-Commit-Position: refs/branch-heads/2840@{#786}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_entry_impl.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_entry_impl.h
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/browser/frame_host/navigation_request.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/common/navigation_params.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/common/navigation_params.h
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/renderer/render_frame_impl.h
[add] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/test/data/navigation_controller/inject_iframe_srcdoc_with_nested_frame.html
[add] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/content/test/data/navigation_controller/inject_subframe_into_blank_iframe.html
[modify] https://crrev.com/6dbf3923c80c88beab18436e77125b52d446ca08/testing/buildbot/filters/browser-side-navigation.linux.content_browsertests.filter

Status: Verified (was: Fixed)
Ad renders after clicking back button. 
Verified on ChromeOS 8872.67.0, 55.0.2883.82

Sign in to add a comment