Commit 2c02e0b4 authored by Fergal Daly's avatar Fergal Daly Committed by Commit Bot

Fix handling of speculative render frames that die without a navigation request.

NavigationControllerBrowserTest.SameDocumentNavigationToHttpPortZero
exposed this by killing the renderer for having sent a bad message
from ValidateDidCommitParams. By that point the |navigation_request_|
has been consumed so ResetNavigationRequest does nothing and so
CancelPendingIfNecessary does not destroy the speculative render
frame.

This test only fails when RenderDocument for subframes is enabled however,
it seems it could happen without that.

The fix is to ensure that CleanUpNavigation gets called even if there
is no navigation_request_.

Bug: 1100745
Change-Id: Icdcc17822be04237e9f038063d45ec993dc7ed92
Fixed: 1100745
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2275640
Commit-Queue: Fergal Daly <fergal@chromium.org>
Reviewed-by: default avatarArthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791699}
parent 8719f165
...@@ -1042,9 +1042,16 @@ void RenderFrameHostManager::CancelPendingIfNecessary( ...@@ -1042,9 +1042,16 @@ void RenderFrameHostManager::CancelPendingIfNecessary(
if (render_frame_host == speculative_render_frame_host_.get()) { if (render_frame_host == speculative_render_frame_host_.get()) {
// TODO(nasko, clamy): This should just clean up the speculative RFH // TODO(nasko, clamy): This should just clean up the speculative RFH
// without canceling the request. See https://crbug.com/636119. // without canceling the request. See https://crbug.com/636119.
if (frame_tree_node_->navigation_request()) if (frame_tree_node_->navigation_request()) {
frame_tree_node_->navigation_request()->set_net_error(net::ERR_ABORTED); frame_tree_node_->navigation_request()->set_net_error(net::ERR_ABORTED);
frame_tree_node_->ResetNavigationRequest(false); frame_tree_node_->ResetNavigationRequest(false);
} else {
// If we are far enough into the navigation that
// TransferNavigationRequestOwnership has already been called then the
// FrameTreeNode no longer owns the NavigationRequest and we need to clean
// up the speculative RenderFrameHost.
CleanUpNavigation();
}
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment