Add regression test for an aborting navigation losing its nav_entry_id

Add regression test for an aborting navigation losing its nav_entry_id

BUG=631617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/c0cf770bd57739b34ae035f0fb75bbb9e81f4026
Cr-Commit-Position: refs/heads/master@{#408449}

[Charlie Harrison, 2016-07-28 18:14:11]

Description was changed from

==========
Add regression test for an aborting navigation losing its nav_entry_id

BUG=631617
==========

to

==========
Add regression test for an aborting navigation losing its nav_entry_id

BUG=631617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Charlie Harrison, 2016-07-28 18:14:14]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 18:14:36]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2016-07-28 18:16:03]

csharrison@chromium.org changed reviewers:
+ creis@chromium.org

[Charlie Harrison, 2016-07-28 18:16:04]

Hi Charlie,
Here's the test, which uses repro steps you discovered in the bug.

[Charlie Reis, 2016-07-28 18:22:29]

Thanks!  One suggestion on avoiding flakiness below.

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5014: //
Start a provisional navigation, but abort it by going back before it commits
nit: Missing period.

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5016: //
pending_navigation_params_ in RenderFrameImpl. This caused the abort-er
nit: s/abort-er/interrupting/

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5027:
shell()->LoadURL(embedded_test_server()->GetURL("/slow?30"));
Can we use a TestNavigationManager here to pause a navigation to a normal URL? 
Slow URLs tend to lead to flaky tests on slow bots.

There's an example in the ForwardInSubframeWithPendingForward test if it helps.

[Charlie Harrison, 2016-07-28 18:30:52]

Thanks. I also confirmed this test fails without your revert.

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
File content/browser/frame_host/navigation_controller_impl_browsertest.cc
(right):

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5014: //
Start a provisional navigation, but abort it by going back before it commits
On 2016/07/28 18:22:29, Charlie Reis wrote:
> nit: Missing period.

Done.

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5016: //
pending_navigation_params_ in RenderFrameImpl. This caused the abort-er
On 2016/07/28 18:22:29, Charlie Reis wrote:
> nit: s/abort-er/interrupting/

Done.

https://codereview.chromium.org/2188283002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_browsertest.cc:5027:
shell()->LoadURL(embedded_test_server()->GetURL("/slow?30"));
On 2016/07/28 18:22:29, Charlie Reis wrote:
> Can we use a TestNavigationManager here to pause a navigation to a normal URL?

> Slow URLs tend to lead to flaky tests on slow bots.
> 
> There's an example in the ForwardInSubframeWithPendingForward test if it
helps.

Yupp. Done.

[Charlie Reis, 2016-07-28 18:32:26]

That was quick!  Thanks, LGTM.

[Charlie Harrison, 2016-07-28 18:35:15]

Haha no problem. <3 tests! Crush bugs!

[Charlie Harrison, 2016-07-28 18:35:23]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2016-07-28 18:36:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 19:13:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-28 19:15:24]

Description was changed from

==========
Add regression test for an aborting navigation losing its nav_entry_id

BUG=631617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add regression test for an aborting navigation losing its nav_entry_id

BUG=631617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/c0cf770bd57739b34ae035f0fb75bbb9e81f4026
Cr-Commit-Position: refs/heads/master@{#408449}
==========

[commit-bot: I haz the power, 2016-07-28 19:15:25]

Patchset 2 (id:??) landed as
https://crrev.com/c0cf770bd57739b34ae035f0fb75bbb9e81f4026
Cr-Commit-Position: refs/heads/master@{#408449}