Clear failed browser-initiated navigations if blank tab is modified.

Clear failed browser-initiated navigations if blank tab is modified.

BUG=355537
TEST=Create a popup tab with a slow URL, type in a 204 URL, and then have
     the opener try to inject content into the popup.  Omnibox should
     reset to about:blank.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264916

[Charlie Reis, 2014-04-18 19:27:18]

Here's the fix for the second independent issue mentioned in
https://codereview.chromium.org/232463007/.

[Peter Kasting, 2014-04-18 20:39:11]

So how does this compare with when the user takes some existing tab and tries to
navigate it to a slow-loading page, while the current page manipulates the
visible content?  It seems kinda odd that in that case we wouldn't reset the
URL, but in this case we do.

https://codereview.chromium.org/243493002/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/navigation_controller_impl_unittest.cc (right):

https://codereview.chromium.org/243493002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/navigation_controller_impl_unittest.cc:2921:
FrameHostMsg_DidFailProvisionalLoadWithError(0,  // routing_id
Nit: I think this EOL comment just makes reading the call tougher; I'd probably
just remove it and move the following param up to this line.

[Charlie Reis, 2014-04-18 21:05:28]

On 2014/04/18 20:39:11, Peter Kasting wrote:
> So how does this compare with when the user takes some existing tab and tries
to
> navigate it to a slow-loading page, while the current page manipulates the
> visible content?  It seems kinda odd that in that case we wouldn't reset the
> URL, but in this case we do.

In both cases, the typed pending URL is visible at least until the navigation
fails.

In the case you describe (where there's an existing committed page), we do reset
the URL when the navigation fails (or rather, we will as of
https://codereview.chromium.org/232463007/), so that we don't leave it above
someone else's content.

In the case handled here (where it's essentially a new blank tab), we'll
continue to show it until content can appear under it.

https://codereview.chromium.org/243493002/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/navigation_controller_impl_unittest.cc (right):

https://codereview.chromium.org/243493002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/navigation_controller_impl_unittest.cc:2921:
FrameHostMsg_DidFailProvisionalLoadWithError(0,  // routing_id
On 2014/04/18 20:39:12, Peter Kasting wrote:
> Nit: I think this EOL comment just makes reading the call tougher; I'd
probably
> just remove it and move the following param up to this line.

Done.

[Peter Kasting, 2014-04-18 21:07:28]

OK.  LGTM then.

[Charlie Reis, 2014-04-18 21:44:53]

Nasko, John's OOO for a bit.  Can you review the details for this one?

[nasko, 2014-04-18 22:30:50]

LGTM

[Charlie Reis, 2014-04-18 22:35:10]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2014-04-18 22:35:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/creis@chromium.org/243493002/40001

[commit-bot: I haz the power, 2014-04-18 23:48:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-18 23:48:11]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[Charlie Reis, 2014-04-18 23:52:56]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2014-04-18 23:53:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/creis@chromium.org/243493002/40001

[commit-bot: I haz the power, 2014-04-19 03:26:34]

Change committed as 264916