Commit 5289d592 authored by Arthur Sonzogni's avatar Arthur Sonzogni Committed by Commit Bot

Instrument NavigationRequest for bug 880741.

devtools_instrumentation::OnResetNavigationRequest is probably
called with an already deleted NavigationRequest.
Add instrumentation to understand what happens.

Crash reports:
  https://crash.corp.google.com/browse?q=expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27content%3A%3Adevtools_instrumentation%3A%3AOnResetNavigationRequest%27+AND+product_name%3D%27Chrome

Experiment: NetworkService.
OS: Windows

StackTrace:
 * content::devtools_instrumentation::OnResetNavigationRequest
 * content::FrameTreeNode::TransferNavigationRequestOwnership
 * content::NavigationRequest::CommitNavigation
 * content::NavigationRequest::OnWillProcessResponseChecksComplete
 * content::NavigationHandleImpl::RunCompleteCallback
 * content::NavigationHandleImpl::WillProcessResponse
 * content::NavigationRequest::OnResponseStarted
 * content::NavigationURLLoaderImpl::OnReceiveResponse

Bug: 880741
Change-Id: Ib428283192cc3de83292f2b62c71583e2596a19f
Reviewed-on: https://chromium-review.googlesource.com/c/1316734
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605644}
parent b8400e34
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <iterator> #include <iterator>
#include "base/bind.h" #include "base/bind.h"
#include "base/debug/dump_without_crashing.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/optional.h" #include "base/optional.h"
...@@ -867,8 +868,13 @@ void NavigationHandleImpl::WillProcessResponse( ...@@ -867,8 +868,13 @@ void NavigationHandleImpl::WillProcessResponse(
// If the navigation is done processing the response, then it's ready to // If the navigation is done processing the response, then it's ready to
// commit. Inform observers that the navigation is now ready to commit, unless // commit. Inform observers that the navigation is now ready to commit, unless
// it is not set to commit (204/205s/downloads). // it is not set to commit (204/205s/downloads).
if (result.action() == NavigationThrottle::PROCEED && render_frame_host_) if (result.action() == NavigationThrottle::PROCEED && render_frame_host_) {
base::WeakPtr<NavigationHandleImpl> weak_ptr = weak_factory_.GetWeakPtr();
ReadyToCommitNavigation(render_frame_host_, false); ReadyToCommitNavigation(render_frame_host_, false);
// TODO(https://crbug.com/880741): Remove this once the bug is fixed.
if (!weak_ptr)
base::debug::DumpWithoutCrashing();
}
TRACE_EVENT_ASYNC_STEP_INTO1("navigation", "NavigationHandle", this, TRACE_EVENT_ASYNC_STEP_INTO1("navigation", "NavigationHandle", this,
"ProcessResponse", "result", result.action()); "ProcessResponse", "result", result.action());
......
...@@ -6,6 +6,8 @@ ...@@ -6,6 +6,8 @@
#include <utility> #include <utility>
#include "base/debug/alias.h"
#include "base/debug/dump_without_crashing.h"
#include "base/metrics/field_trial_params.h" #include "base/metrics/field_trial_params.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
...@@ -902,6 +904,11 @@ void NavigationRequest::OnResponseStarted( ...@@ -902,6 +904,11 @@ void NavigationRequest::OnResponseStarted(
bool is_download, bool is_download,
bool is_stream, bool is_stream,
base::Optional<SubresourceLoaderParams> subresource_loader_params) { base::Optional<SubresourceLoaderParams> subresource_loader_params) {
// TODO(https://crbug.com/880741): Remove this once the bug is fixed.
if (state_ != STARTED) {
DEBUG_ALIAS_FOR_GURL(url, navigation_handle_->GetURL());
base::debug::DumpWithoutCrashing();
}
DCHECK_EQ(state_, STARTED); DCHECK_EQ(state_, STARTED);
DCHECK(response); DCHECK(response);
TRACE_EVENT_ASYNC_STEP_INTO0("navigation", "NavigationRequest", this, TRACE_EVENT_ASYNC_STEP_INTO0("navigation", "NavigationRequest", this,
...@@ -1636,6 +1643,12 @@ void NavigationRequest::CommitNavigation() { ...@@ -1636,6 +1643,12 @@ void NavigationRequest::CommitNavigation() {
render_frame_host == render_frame_host ==
frame_tree_node_->render_manager()->speculative_frame_host()); frame_tree_node_->render_manager()->speculative_frame_host());
// TODO(https://crbug.com/880741): Remove this once the bug is fixed.
if (!frame_tree_node_->navigation_request()) {
DEBUG_ALIAS_FOR_GURL(url, navigation_handle_->GetURL());
base::debug::DumpWithoutCrashing();
}
frame_tree_node_->TransferNavigationRequestOwnership(render_frame_host); frame_tree_node_->TransferNavigationRequestOwnership(render_frame_host);
if (IsPerNavigationMojoInterfaceEnabled() && request_navigation_client_ && if (IsPerNavigationMojoInterfaceEnabled() && request_navigation_client_ &&
request_navigation_client_.is_bound()) { request_navigation_client_.is_bound()) {
......
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