Commit 8146cbf7 authored by Fergal Daly's avatar Fergal Daly Committed by Commit Bot

Validate speculative render frame host from multiple places.

Move validation logic into ValidateSpeculativeFrameHostForBug1146573.

Also fixes the problem of dumping too often by not dumping when
replacing a crashed RFH.

Bug: 1146573
Change-Id: Ifcae2662ca7fa071440a5e1d069936683f763269
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538863
Commit-Queue: Fergal Daly <fergal@chromium.org>
Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Auto-Submit: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828146}
parent 9a7d13ae
...@@ -3203,6 +3203,8 @@ void NavigationRequest::AddOldPageInfoToCommitParamsIfNeeded() { ...@@ -3203,6 +3203,8 @@ void NavigationRequest::AddOldPageInfoToCommitParamsIfNeeded() {
} }
void NavigationRequest::CommitNavigation() { void NavigationRequest::CommitNavigation() {
frame_tree_node_->render_manager()
->ValidateSpeculativeRenderFrameHostForBug1146573();
UpdateCommitNavigationParamsHistory(); UpdateCommitNavigationParamsHistory();
DCHECK(NeedsUrlLoader() == !!response_head_ || DCHECK(NeedsUrlLoader() == !!response_head_ ||
(was_redirected_ && common_params_->url.IsAboutBlank())); (was_redirected_ && common_params_->url.IsAboutBlank()));
......
...@@ -654,6 +654,7 @@ void RenderFrameHostManager::RestoreFromBackForwardCache( ...@@ -654,6 +654,7 @@ void RenderFrameHostManager::RestoreFromBackForwardCache(
// in the long run. For now, and to avoid complex edge cases, we simply reuse // in the long run. For now, and to avoid complex edge cases, we simply reuse
// it to preserve the understood logic in CommitPending. // it to preserve the understood logic in CommitPending.
speculative_render_frame_host_ = std::move(entry->render_frame_host); speculative_render_frame_host_ = std::move(entry->render_frame_host);
ValidateSpeculativeRenderFrameHostForBug1146573();
bfcache_entry_to_restore_ = std::move(entry); bfcache_entry_to_restore_ = std::move(entry);
} }
...@@ -696,6 +697,36 @@ void RenderFrameHostManager::DidCreateNavigationRequest( ...@@ -696,6 +697,36 @@ void RenderFrameHostManager::DidCreateNavigationRequest(
} }
} }
void RenderFrameHostManager::ValidateSpeculativeRenderFrameHostForBug1146573() {
// TODO(https://crbug.com/1146573): Remove this when the bug is closed.
if (ShouldCreateNewHostForSameSiteSubframe())
return;
// This can happen during destruction after the RFH has been cleared.
if (!render_frame_host_)
return;
if (render_frame_host_->must_be_replaced())
return;
if (!speculative_render_frame_host_)
return;
if (speculative_render_frame_host_->GetSiteInstance() ==
render_frame_host_->GetSiteInstance()) {
// This should never be true.
SCOPED_CRASH_KEY_BOOL(ValidateSpeculative, HostsEqual,
speculative_render_frame_host_ == render_frame_host_);
DCHECK_NE(speculative_render_frame_host_, render_frame_host_);
SCOPED_CRASH_KEY_BOOL(ValidateSpeculative, Live,
render_frame_host_->IsRenderFrameLive());
SCOPED_CRASH_KEY_STRING256(
ValidateSpeculative, OldSiteInstance,
render_frame_host_->GetSiteInstance()->GetSiteURL().spec());
SCOPED_CRASH_KEY_STRING256(
ValidateSpeculative, NewSiteInstance,
speculative_render_frame_host_->GetSiteInstance()->GetSiteURL().spec());
DCHECK(false);
base::debug::DumpWithoutCrashing();
}
}
RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation( RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
NavigationRequest* request, NavigationRequest* request,
std::string* reason) { std::string* reason) {
...@@ -2363,6 +2394,7 @@ bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost( ...@@ -2363,6 +2394,7 @@ bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
speculative_render_frame_host_ = CreateSpeculativeRenderFrame( speculative_render_frame_host_ = CreateSpeculativeRenderFrame(
new_instance, recovering_without_early_commit); new_instance, recovering_without_early_commit);
ValidateSpeculativeRenderFrameHostForBug1146573();
// If RenderViewHost was created along with the speculative RenderFrameHost, // If RenderViewHost was created along with the speculative RenderFrameHost,
// ensure that RenderViewCreated is fired for it. It is important to do this // ensure that RenderViewCreated is fired for it. It is important to do this
...@@ -3128,6 +3160,7 @@ std::unique_ptr<RenderFrameHostImpl> RenderFrameHostManager::SetRenderFrameHost( ...@@ -3128,6 +3160,7 @@ std::unique_ptr<RenderFrameHostImpl> RenderFrameHostManager::SetRenderFrameHost(
std::unique_ptr<RenderFrameHostImpl> old_render_frame_host = std::unique_ptr<RenderFrameHostImpl> old_render_frame_host =
std::move(render_frame_host_); std::move(render_frame_host_);
render_frame_host_ = std::move(render_frame_host); render_frame_host_ = std::move(render_frame_host);
ValidateSpeculativeRenderFrameHostForBug1146573();
if (render_frame_host_ && render_frame_host_->lifecycle_state() != if (render_frame_host_ && render_frame_host_->lifecycle_state() !=
RenderFrameHostImpl::LifecycleState::kActive) { RenderFrameHostImpl::LifecycleState::kActive) {
......
...@@ -322,6 +322,10 @@ class CONTENT_EXPORT RenderFrameHostManager ...@@ -322,6 +322,10 @@ class CONTENT_EXPORT RenderFrameHostManager
// new RenderFrameHost (and potentially a new process) if needed. // new RenderFrameHost (and potentially a new process) if needed.
void DidCreateNavigationRequest(NavigationRequest* request); void DidCreateNavigationRequest(NavigationRequest* request);
// Validate the speculative RFH and DumpWithoutCrashing if it's invalid.
// TODO(https://crbug.com/1146573): Remove this when the bug is closed.
void ValidateSpeculativeRenderFrameHostForBug1146573();
// Called (possibly several times) during a navigation to select or create an // Called (possibly several times) during a navigation to select or create an
// appropriate RenderFrameHost for the provided URL. The returned pointer will // appropriate RenderFrameHost for the provided URL. The returned pointer will
// be for the current or the speculative RenderFrameHost and the instance is // be for the current or the speculative RenderFrameHost and the instance is
......
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