PlzNavigate: destroy the speculative RFH on renderer-initiated navigations

PlzNavigate: destroy the speculative RFH on renderer-initiated navigations

This CL makes sure that the speculative RenderFrameHost is destroyed when a new
renderer-intiated navigation cancels the current one. This fixes the
NavigationControllerTest.LoadURL_NewPending when run with browser-side
navigation enabled.

BUG=376094
Committed: https://crrev.com/f8a56aa27183fceb0489e686a663d980ff76f97f
Cr-Commit-Position: refs/heads/master@{#321178}

[clamy, 2015-03-16 16:27:30]

clamy@chromium.org changed reviewers:
+ carlosk@chromium.org, nasko@chromium.org

[clamy, 2015-03-16 16:27:31]

@nasko, carlosk: PTAL
This fixes an issue with the cancelation of the speculative RFH on
renderer-initiated navigations. Note that for the test to work properly,
carlosk@ patch for not having renderer-initiated navigations create a
speculative RFH must land as well.

[nasko, 2015-03-16 18:48:37]

LGTM with a couple of nits.

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

https://codereview.chromium.org/1009993002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_unittest.cc:793:
main_test_rfh()->SendRendererInitiatedNavigationRequest(kNewURL, true);
I think this is already in Carlos' recent CL.

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

https://codereview.chromium.org/1009993002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:655:
CancelNavigation(frame_tree_node);
nit: Blank line after this new CancelNavigation call.

[carlosk, 2015-03-17 14:42:59]

Cool and thanks.

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

https://codereview.chromium.org/1009993002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigation_controller_impl_unittest.cc:793:
main_test_rfh()->SendRendererInitiatedNavigationRequest(kNewURL, true);
On 2015/03/16 18:48:37, nasko wrote:
> I think this is already in Carlos' recent CL.

It is. Rebasing will sort it out.

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

https://codereview.chromium.org/1009993002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:655:
CancelNavigation(frame_tree_node);
Don't we need to do the same in NavigatorImpl::RequestNavigation as we might
also replace an ongoing navigation there?

[clamy, 2015-03-18 13:55:47]

Patchset #3 (id:40001) has been deleted

[clamy, 2015-03-18 13:55:55]

Thanks!

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

https://codereview.chromium.org/1009993002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:655:
CancelNavigation(frame_tree_node);
On 2015/03/17 14:42:59, carlosk wrote:
> Don't we need to do the same in NavigatorImpl::RequestNavigation as we might
> also replace an ongoing navigation there?

Done.

https://codereview.chromium.org/1009993002/diff/1/content/browser/frame_host/...
content/browser/frame_host/navigator_impl.cc:655:
CancelNavigation(frame_tree_node);
On 2015/03/16 18:48:37, nasko wrote:
> nit: Blank line after this new CancelNavigation call.

Done.

[carlosk, 2015-03-18 14:23:29]

Thanks and LGTM.

[clamy, 2015-03-18 17:40:51]

The CQ bit was checked by clamy@chromium.org

[clamy, 2015-03-18 17:40:52]

The patchset sent to the CQ was uploaded after l-g-t-m from nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1009993002/#ps60001
(title: "Addressed comments")

[commit-bot: I haz the power, 2015-03-18 17:41:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1009993002/60001

[commit-bot: I haz the power, 2015-03-18 18:51:48]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-03-18 18:52:31]

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