Record duplicate navs & renderer-initiated new nav discard reasons
Duplicate navigations are navigations that are the same as the current ongoing navigations, and likely to be triggered accidentally by e.g. double link/button clicks. This CL makes it so that we record duplicate navs as a separate NavigationDiscardReason. Also, currently when a new renderer-initiated navigation is triggered while there is already another navigation initiated by the same renderer ongoing, the older navigation is canceled by overwriting the NavigationClient. This cancellation is incorrectly marked as kInternalCancellation because the overwrite didn't set a reason. This CL fixes that by setting the reason to indicate that the navigation client is disconnected because of a new navigation / duplicate navigation. This CL also makes form submission-initiated cancellation mark the disconnection correctly (as "new navigation"). Bug: 366060351 Change-Id: I084ca4dbd56805192cf4d553ab866e011d7d1917 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5856745 Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org> Commit-Queue: Rakina Zata Amni <rakina@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1355642}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
40b8770870
commit
63a7ab05fe
chrome/browser/page_load_metrics/observers
components/page_load_metrics/browser/observers
content
browser
common
public
browser
renderer
third_party/blink
public
renderer
tools/metrics/histograms/metadata/page
130
chrome/browser/page_load_metrics/observers/gws_abandoned_page_load_metrics_observer_browsertest.cc
130
chrome/browser/page_load_metrics/observers/gws_abandoned_page_load_metrics_observer_browsertest.cc
@@ -1213,4 +1213,134 @@ IN_PROC_BROWSER_TEST_F(GWSAbandonedPageLoadMetricsObserverBrowserTest,
|
|||||||
ExpectTotalCountForAllNavigationMilestones(/*include_redirect=*/false, 1);
|
ExpectTotalCountForAllNavigationMilestones(/*include_redirect=*/false, 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
IN_PROC_BROWSER_TEST_F(GWSAbandonedPageLoadMetricsObserverBrowserTest,
|
||||||
|
DuplicateNavigation_BrowserInitiated) {
|
||||||
|
EXPECT_TRUE(content::NavigateToURL(web_contents(), url_non_srp()));
|
||||||
|
|
||||||
|
// 1. Start browser-initiated navigation to `url_srp()`
|
||||||
|
content::TestNavigationManager nav_manager(web_contents(), url_srp());
|
||||||
|
web_contents()->GetController().LoadURL(
|
||||||
|
url_srp(), content::Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
|
||||||
|
// Pause the navigation at request start.
|
||||||
|
EXPECT_TRUE(nav_manager.WaitForRequestStart());
|
||||||
|
|
||||||
|
// 2. Navigate again, also to `url_srp()`.
|
||||||
|
web_contents()->GetController().LoadURL(
|
||||||
|
url_srp(), content::Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
|
||||||
|
// Wait for the first navigation to finish.
|
||||||
|
EXPECT_TRUE(nav_manager.WaitForNavigationFinished());
|
||||||
|
// Ensure that the first_navigation didn't commit.
|
||||||
|
EXPECT_FALSE(nav_manager.was_committed());
|
||||||
|
|
||||||
|
EXPECT_TRUE(content::WaitForLoadStop(web_contents()));
|
||||||
|
EXPECT_EQ(url_srp(),
|
||||||
|
web_contents()->GetPrimaryMainFrame()->GetLastCommittedURL());
|
||||||
|
|
||||||
|
// Expect that the first navigation didn't get to LoaderStart.
|
||||||
|
histogram_tester().ExpectTotalCount(
|
||||||
|
GetMilestoneHistogramName(NavigationMilestone::kNavigationStart), 2);
|
||||||
|
histogram_tester().ExpectTotalCount(
|
||||||
|
GetMilestoneHistogramName(NavigationMilestone::kLoaderStart), 1);
|
||||||
|
|
||||||
|
// Check that the abandonment reason is set correctly.
|
||||||
|
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix(
|
||||||
|
GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart)),
|
||||||
|
testing::UnorderedElementsAreArray(
|
||||||
|
ExpandHistograms({GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart)})));
|
||||||
|
histogram_tester().ExpectUniqueSample(
|
||||||
|
GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart),
|
||||||
|
AbandonReason::kNewDuplicateNavigation, 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
IN_PROC_BROWSER_TEST_F(GWSAbandonedPageLoadMetricsObserverBrowserTest,
|
||||||
|
DuplicateNavigation_RendererInitiated) {
|
||||||
|
EXPECT_TRUE(content::NavigateToURL(web_contents(), url_non_srp()));
|
||||||
|
|
||||||
|
// 1. Start renderer-initiated navigation to `url_srp()`
|
||||||
|
content::TestNavigationManager nav_manager(web_contents(), url_srp());
|
||||||
|
EXPECT_TRUE(ExecJs(web_contents(),
|
||||||
|
content::JsReplace("location.href = $1;", url_srp())));
|
||||||
|
// Pause the navigation at request start.
|
||||||
|
EXPECT_TRUE(nav_manager.WaitForRequestStart());
|
||||||
|
|
||||||
|
// 2. Navigate again, also to `url_srp()`.
|
||||||
|
EXPECT_TRUE(ExecJs(web_contents(),
|
||||||
|
content::JsReplace("location.href = $1;", url_srp())));
|
||||||
|
// Wait for the first navigation to finish.
|
||||||
|
EXPECT_TRUE(nav_manager.WaitForNavigationFinished());
|
||||||
|
// Ensure that the first_navigation didn't commit.
|
||||||
|
EXPECT_FALSE(nav_manager.was_committed());
|
||||||
|
|
||||||
|
EXPECT_TRUE(content::WaitForLoadStop(web_contents()));
|
||||||
|
EXPECT_EQ(url_srp(),
|
||||||
|
web_contents()->GetPrimaryMainFrame()->GetLastCommittedURL());
|
||||||
|
|
||||||
|
// Expect that the first navigation didn't get to LoaderStart.
|
||||||
|
histogram_tester().ExpectTotalCount(
|
||||||
|
GetMilestoneHistogramName(NavigationMilestone::kNavigationStart), 2);
|
||||||
|
histogram_tester().ExpectTotalCount(
|
||||||
|
GetMilestoneHistogramName(NavigationMilestone::kLoaderStart), 1);
|
||||||
|
|
||||||
|
// Check that the abandonment reason is set correctly.
|
||||||
|
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix(
|
||||||
|
GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart)),
|
||||||
|
testing::UnorderedElementsAreArray(
|
||||||
|
ExpandHistograms({GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart)})));
|
||||||
|
histogram_tester().ExpectUniqueSample(
|
||||||
|
GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart),
|
||||||
|
AbandonReason::kNewDuplicateNavigation, 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
IN_PROC_BROWSER_TEST_F(GWSAbandonedPageLoadMetricsObserverBrowserTest,
|
||||||
|
MultipleDifferentRendererInitiatedNavigations) {
|
||||||
|
EXPECT_TRUE(content::NavigateToURL(web_contents(), url_non_srp()));
|
||||||
|
|
||||||
|
// 1. Start renderer-initiated navigation to `url_srp_redirect()`
|
||||||
|
content::TestNavigationManager nav_manager(web_contents(),
|
||||||
|
url_srp_redirect());
|
||||||
|
EXPECT_TRUE(ExecJs(web_contents(), content::JsReplace("location.href = $1;",
|
||||||
|
url_srp_redirect())));
|
||||||
|
// Pause the navigation at request start.
|
||||||
|
EXPECT_TRUE(nav_manager.WaitForRequestStart());
|
||||||
|
|
||||||
|
// 2. Navigate again but to `url_srp()`.
|
||||||
|
EXPECT_TRUE(ExecJs(web_contents(),
|
||||||
|
content::JsReplace("location.href = $1;", url_srp())));
|
||||||
|
// Run script to ensure that the second link click is already processed.
|
||||||
|
EXPECT_TRUE(ExecJs(web_contents(), "console.log('Success');"));
|
||||||
|
|
||||||
|
// Wait for the first navigation to finish.
|
||||||
|
EXPECT_TRUE(nav_manager.WaitForNavigationFinished());
|
||||||
|
// Ensure that the first_navigationdidn't commit.
|
||||||
|
EXPECT_FALSE(nav_manager.was_committed());
|
||||||
|
|
||||||
|
EXPECT_TRUE(content::WaitForLoadStop(web_contents()));
|
||||||
|
EXPECT_EQ(url_srp(),
|
||||||
|
web_contents()->GetPrimaryMainFrame()->GetLastCommittedURL());
|
||||||
|
|
||||||
|
// Expect that the first navigation didn't get to LoaderStart.
|
||||||
|
histogram_tester().ExpectTotalCount(
|
||||||
|
GetMilestoneHistogramName(NavigationMilestone::kNavigationStart), 2);
|
||||||
|
histogram_tester().ExpectTotalCount(
|
||||||
|
GetMilestoneHistogramName(NavigationMilestone::kLoaderStart), 1);
|
||||||
|
|
||||||
|
// Check that the abandonment reason is setcorrectly.
|
||||||
|
EXPECT_THAT(histogram_tester().GetTotalCountsForPrefix(
|
||||||
|
GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart)),
|
||||||
|
testing::UnorderedElementsAreArray(
|
||||||
|
ExpandHistograms({GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart)})));
|
||||||
|
histogram_tester().ExpectUniqueSample(
|
||||||
|
GetAbandonReasonAtMilestoneHistogramName(
|
||||||
|
NavigationMilestone::kNavigationStart),
|
||||||
|
AbandonReason::kNewOtherNavigationRendererInitiated, 1);
|
||||||
|
}
|
||||||
|
|
||||||
// TODO(https://crbug.com/347706997): Test backgrounded case.
|
// TODO(https://crbug.com/347706997): Test backgrounded case.
|
||||||
|
@@ -44,6 +44,8 @@ AbandonReason DiscardReasonToAbandonReason(
|
|||||||
return AbandonReason::kNeverStarted;
|
return AbandonReason::kNeverStarted;
|
||||||
case content::NavigationDiscardReason::kFailedSecurityCheck:
|
case content::NavigationDiscardReason::kFailedSecurityCheck:
|
||||||
return AbandonReason::kFailedSecurityCheck;
|
return AbandonReason::kFailedSecurityCheck;
|
||||||
|
case content::NavigationDiscardReason::kNewDuplicateNavigation:
|
||||||
|
return AbandonReason::kNewDuplicateNavigation;
|
||||||
// Other cases like kCommittedNavigation and kRenderFrameHostDestruction
|
// Other cases like kCommittedNavigation and kRenderFrameHostDestruction
|
||||||
// should be obsolete, so just use "other" as the reason.
|
// should be obsolete, so just use "other" as the reason.
|
||||||
case content::NavigationDiscardReason::kCommittedNavigation:
|
case content::NavigationDiscardReason::kCommittedNavigation:
|
||||||
@@ -74,6 +76,7 @@ const char kAbandonReasonNewOtherNavigationBrowserInitiated[] =
|
|||||||
"NewOtherNavigationBrowserInitiated";
|
"NewOtherNavigationBrowserInitiated";
|
||||||
const char kAbandonReasonNewOtherNavigationRendererInitiated[] =
|
const char kAbandonReasonNewOtherNavigationRendererInitiated[] =
|
||||||
"NewOtherNavigationRendererInitiated";
|
"NewOtherNavigationRendererInitiated";
|
||||||
|
const char kAbandonReasonNewDuplicateNavigation[] = "NewDuplicateNavigation";
|
||||||
const char kAbandonReasonFrameRemoved[] = "FrameRemoved";
|
const char kAbandonReasonFrameRemoved[] = "FrameRemoved";
|
||||||
const char kAbandonReasonExplicitCancellation[] = "ExplicitCancellation";
|
const char kAbandonReasonExplicitCancellation[] = "ExplicitCancellation";
|
||||||
const char kAbandonReasonInternalCancellation[] = "InternalCancellation";
|
const char kAbandonReasonInternalCancellation[] = "InternalCancellation";
|
||||||
@@ -137,6 +140,8 @@ std::string AbandonedPageLoadMetricsObserver::AbandonReasonToString(
|
|||||||
return internal::kAbandonReasonNewOtherNavigationBrowserInitiated;
|
return internal::kAbandonReasonNewOtherNavigationBrowserInitiated;
|
||||||
case AbandonReason::kNewOtherNavigationRendererInitiated:
|
case AbandonReason::kNewOtherNavigationRendererInitiated:
|
||||||
return internal::kAbandonReasonNewOtherNavigationRendererInitiated;
|
return internal::kAbandonReasonNewOtherNavigationRendererInitiated;
|
||||||
|
case AbandonReason::kNewDuplicateNavigation:
|
||||||
|
return internal::kAbandonReasonNewDuplicateNavigation;
|
||||||
case AbandonReason::kFrameRemoved:
|
case AbandonReason::kFrameRemoved:
|
||||||
return internal::kAbandonReasonFrameRemoved;
|
return internal::kAbandonReasonFrameRemoved;
|
||||||
case AbandonReason::kExplicitCancellation:
|
case AbandonReason::kExplicitCancellation:
|
||||||
|
@@ -82,7 +82,8 @@ class AbandonedPageLoadMetricsObserver
|
|||||||
kNewHistoryNavigation = 11,
|
kNewHistoryNavigation = 11,
|
||||||
kNewOtherNavigationBrowserInitiated = 12,
|
kNewOtherNavigationBrowserInitiated = 12,
|
||||||
kNewOtherNavigationRendererInitiated = 13,
|
kNewOtherNavigationRendererInitiated = 13,
|
||||||
kMaxValue = kNewOtherNavigationRendererInitiated,
|
kNewDuplicateNavigation = 14,
|
||||||
|
kMaxValue = kNewDuplicateNavigation,
|
||||||
};
|
};
|
||||||
// LINT.ThenChange(//tools/metrics/histograms/metadata/page/enums.xml:NavigationAbandonReasonEnum)
|
// LINT.ThenChange(//tools/metrics/histograms/metadata/page/enums.xml:NavigationAbandonReasonEnum)
|
||||||
|
|
||||||
|
@@ -573,7 +573,8 @@ void FrameTreeNode::TakeNavigationRequest(
|
|||||||
}
|
}
|
||||||
ResetNavigationRequestButKeepState(
|
ResetNavigationRequestButKeepState(
|
||||||
navigation_request->GetTypeForNavigationDiscardReason());
|
navigation_request->GetTypeForNavigationDiscardReason());
|
||||||
} else if (navigation_request_) {
|
} else if (navigation_request_ &&
|
||||||
|
!navigation_request_->GetNavigationDiscardReason().has_value()) {
|
||||||
navigation_request_->set_navigation_discard_reason(
|
navigation_request_->set_navigation_discard_reason(
|
||||||
navigation_request->GetTypeForNavigationDiscardReason());
|
navigation_request->GetTypeForNavigationDiscardReason());
|
||||||
}
|
}
|
||||||
@@ -633,8 +634,9 @@ void FrameTreeNode::ResetNavigationRequestButKeepState(
|
|||||||
// accidentally complete a navigation that should be reset.
|
// accidentally complete a navigation that should be reset.
|
||||||
CancelRestartingBackForwardCacheNavigation();
|
CancelRestartingBackForwardCacheNavigation();
|
||||||
devtools_instrumentation::OnResetNavigationRequest(navigation_request_.get());
|
devtools_instrumentation::OnResetNavigationRequest(navigation_request_.get());
|
||||||
|
if (!navigation_request_->GetNavigationDiscardReason().has_value()) {
|
||||||
navigation_request_->set_navigation_discard_reason(reason);
|
navigation_request_->set_navigation_discard_reason(reason);
|
||||||
|
}
|
||||||
navigation_request_.reset();
|
navigation_request_.reset();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@@ -7085,15 +7085,6 @@ void NavigationRequest::RendererRequestedNavigationCancellationForTesting() {
|
|||||||
void NavigationRequest::OnNavigationClientDisconnected(
|
void NavigationRequest::OnNavigationClientDisconnected(
|
||||||
uint32_t reason,
|
uint32_t reason,
|
||||||
const std::string& description) {
|
const std::string& description) {
|
||||||
if (reason == mojom::NavigationClient::kResetForSwap) {
|
|
||||||
// If the RenderFrame that initiated this navigation request is swapped out
|
|
||||||
// (disconnecting its NavigationClient for this request), do not treat it as
|
|
||||||
// a cancellation. Otherwise, if a previous navigation before `this` is slow
|
|
||||||
// to commit, it would unexpectedly cancel `this` subsequent attempt to
|
|
||||||
// navigate elsewhere.
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Renderer-initiated navigation cancellations can only happen before the
|
// Renderer-initiated navigation cancellations can only happen before the
|
||||||
// navigation gets into the READY_TO_COMMIT state, because
|
// navigation gets into the READY_TO_COMMIT state, because
|
||||||
// RendererCancellationThrottle will prevent renderer-initiated navigations
|
// RendererCancellationThrottle will prevent renderer-initiated navigations
|
||||||
@@ -7108,16 +7099,44 @@ void NavigationRequest::OnNavigationClientDisconnected(
|
|||||||
// anymore. Fix tests that expect this behavior.
|
// anymore. Fix tests that expect this behavior.
|
||||||
// 2. The target renderer had crashed, so the speculative RenderFrame is not
|
// 2. The target renderer had crashed, so the speculative RenderFrame is not
|
||||||
// live anymore, because the navigation can't commit in a crashed renderer.
|
// live anymore, because the navigation can't commit in a crashed renderer.
|
||||||
NavigationDiscardReason discard_reason =
|
std::optional<NavigationDiscardReason> discard_reason;
|
||||||
(HasRenderFrameHost() && !GetRenderFrameHost()->IsRenderFrameLive())
|
if (HasRenderFrameHost() && !GetRenderFrameHost()->IsRenderFrameLive()) {
|
||||||
? NavigationDiscardReason::kRenderProcessGone
|
discard_reason = NavigationDiscardReason::kRenderProcessGone;
|
||||||
: (reason == mojom::NavigationClient::kResetForAbort
|
} else {
|
||||||
? NavigationDiscardReason::kExplicitCancellation
|
switch (static_cast<mojom::NavigationClientDisconnectReason>(reason)) {
|
||||||
: NavigationDiscardReason::kInternalCancellation);
|
case mojom::NavigationClientDisconnectReason::kResetForSwap:
|
||||||
|
// If the RenderFrame that initiated this navigation request is swapped
|
||||||
|
// out (disconnecting its NavigationClient for this request), do not
|
||||||
|
// treat it as a cancellation. Otherwise, if a previous navigation
|
||||||
|
// before `this` is slow to commit, it would unexpectedly cancel `this`
|
||||||
|
// subsequent attempt to navigate elsewhere.
|
||||||
|
return;
|
||||||
|
case mojom::NavigationClientDisconnectReason::kNoExplicitReason:
|
||||||
|
discard_reason = NavigationDiscardReason::kInternalCancellation;
|
||||||
|
break;
|
||||||
|
case mojom::NavigationClientDisconnectReason::kResetForAbort:
|
||||||
|
discard_reason = NavigationDiscardReason::kExplicitCancellation;
|
||||||
|
break;
|
||||||
|
case mojom::NavigationClientDisconnectReason::kResetForNewNavigation:
|
||||||
|
discard_reason =
|
||||||
|
NavigationDiscardReason::kNewOtherNavigationRendererInitiated;
|
||||||
|
break;
|
||||||
|
case mojom::NavigationClientDisconnectReason::
|
||||||
|
kResetForDuplicateNavigation:
|
||||||
|
discard_reason = NavigationDiscardReason::kNewDuplicateNavigation;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
if (!discard_reason.has_value()) {
|
||||||
|
// TODO(https://crbug.com/366060351): An invalid value was used. Kill
|
||||||
|
// either the requesting or committing client's process.
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if (!IsWaitingToCommit()) {
|
if (!IsWaitingToCommit()) {
|
||||||
// The cancellation happens before READY_TO_COMMIT.
|
// The cancellation happens before READY_TO_COMMIT.
|
||||||
frame_tree_node_->navigator().CancelNavigation(frame_tree_node_,
|
frame_tree_node_->navigator().CancelNavigation(frame_tree_node_,
|
||||||
discard_reason);
|
discard_reason.value());
|
||||||
} else if (GetRenderFrameHost() ==
|
} else if (GetRenderFrameHost() ==
|
||||||
frame_tree_node_->render_manager()->current_frame_host() ||
|
frame_tree_node_->render_manager()->current_frame_host() ||
|
||||||
!GetRenderFrameHost()->IsRenderFrameLive()) {
|
!GetRenderFrameHost()->IsRenderFrameLive()) {
|
||||||
@@ -7125,10 +7144,11 @@ void NavigationRequest::OnNavigationClientDisconnected(
|
|||||||
// `render_frame_host_` owns `this`. Cache any needed state in stack
|
// `render_frame_host_` owns `this`. Cache any needed state in stack
|
||||||
// variables to avoid a use-after-free.
|
// variables to avoid a use-after-free.
|
||||||
FrameTreeNode* frame_tree_node = frame_tree_node_;
|
FrameTreeNode* frame_tree_node = frame_tree_node_;
|
||||||
GetRenderFrameHost()->NavigationRequestCancelled(this, discard_reason);
|
GetRenderFrameHost()->NavigationRequestCancelled(this,
|
||||||
|
discard_reason.value());
|
||||||
// Ensure that the speculative RFH, if any, is also cleaned up.
|
// Ensure that the speculative RFH, if any, is also cleaned up.
|
||||||
frame_tree_node->render_manager()->DiscardSpeculativeRFHIfUnused(
|
frame_tree_node->render_manager()->DiscardSpeculativeRFHIfUnused(
|
||||||
discard_reason);
|
discard_reason.value());
|
||||||
}
|
}
|
||||||
|
|
||||||
// Do not add code after this, NavigationRequest might have been destroyed.
|
// Do not add code after this, NavigationRequest might have been destroyed.
|
||||||
|
@@ -1359,6 +1359,9 @@ class CONTENT_EXPORT NavigationRequest
|
|||||||
navigation_discard_reason_ = navigation_discard_reason;
|
navigation_discard_reason_ = navigation_discard_reason;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Returns the type of this navigation (e.g. history, browser-initiated, etc)
|
||||||
|
// to set as a discard reason on another navigation that is being discarded
|
||||||
|
// because this navigation is taking its place in the FrameTreeNode.
|
||||||
NavigationDiscardReason GetTypeForNavigationDiscardReason();
|
NavigationDiscardReason GetTypeForNavigationDiscardReason();
|
||||||
|
|
||||||
void set_force_no_https_upgrade() { force_no_https_upgrade_ = true; }
|
void set_force_no_https_upgrade() { force_no_https_upgrade_ = true; }
|
||||||
|
@@ -834,11 +834,15 @@ void Navigator::Navigate(std::unique_ptr<NavigationRequest> request,
|
|||||||
is_duplicate_navigation = true;
|
is_duplicate_navigation = true;
|
||||||
}
|
}
|
||||||
base::UmaHistogramBoolean("Navigation.IsDuplicate", is_duplicate_navigation);
|
base::UmaHistogramBoolean("Navigation.IsDuplicate", is_duplicate_navigation);
|
||||||
if (is_duplicate_navigation &&
|
if (is_duplicate_navigation) {
|
||||||
base::FeatureList::IsEnabled(features::kIgnoreDuplicateNavs)) {
|
if (base::FeatureList::IsEnabled(features::kIgnoreDuplicateNavs)) {
|
||||||
request->set_navigation_discard_reason(
|
request->set_navigation_discard_reason(
|
||||||
NavigationDiscardReason::kNeverStarted);
|
NavigationDiscardReason::kNeverStarted);
|
||||||
return;
|
return;
|
||||||
|
} else {
|
||||||
|
ongoing_navigation_request->set_navigation_discard_reason(
|
||||||
|
NavigationDiscardReason::kNewDuplicateNavigation);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
metrics_data_ = std::make_unique<NavigationMetricsData>(
|
metrics_data_ = std::make_unique<NavigationMetricsData>(
|
||||||
|
@@ -219,6 +219,30 @@ struct StorageInfo {
|
|||||||
pending_remote<blink.mojom.StorageArea>? session_storage_area;
|
pending_remote<blink.mojom.StorageArea>? session_storage_area;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
enum NavigationClientDisconnectReason {
|
||||||
|
// No explicit reason set while calling.
|
||||||
|
kNoExplicitReason = 0,
|
||||||
|
|
||||||
|
// Explicit reset reason when the NavigationClient needs to be reset when
|
||||||
|
// swapping frames. In this case, it is often desirable to *not* cancel the
|
||||||
|
// navigation: otherwise, this may interfere with the navigation queueing
|
||||||
|
// logic (by unexpectedly cancelling the queued navigation).
|
||||||
|
kResetForSwap,
|
||||||
|
|
||||||
|
// Explicit reset reason when the NavigationClient needs to be reset because
|
||||||
|
// the renderer explicitly aborted the navigation (instead of the connection
|
||||||
|
// being automatically disconncted due to e.g. crashing).
|
||||||
|
kResetForAbort,
|
||||||
|
|
||||||
|
// Explicit reset reason when the NavigationClient needs to be reset because
|
||||||
|
// a new renderer-initiated navigation started.
|
||||||
|
kResetForNewNavigation,
|
||||||
|
|
||||||
|
// Similar to above but the navigation is a duplicate navigation (i.e. the
|
||||||
|
// URL, method, other params are all the same).
|
||||||
|
kResetForDuplicateNavigation,
|
||||||
|
};
|
||||||
|
|
||||||
// Implemented by the renderer and called by the browser. Used for two purposes:
|
// Implemented by the renderer and called by the browser. Used for two purposes:
|
||||||
//
|
//
|
||||||
// - When the renderer requests a navigation in a RenderFrame in the same
|
// - When the renderer requests a navigation in a RenderFrame in the same
|
||||||
@@ -230,16 +254,6 @@ struct StorageInfo {
|
|||||||
// or `CommitFailedNavigation()`. Similar to requesting a navigation above,
|
// or `CommitFailedNavigation()`. Similar to requesting a navigation above,
|
||||||
// disconnecting this interface is used as a signal to cancel the navigation.
|
// disconnecting this interface is used as a signal to cancel the navigation.
|
||||||
interface NavigationClient {
|
interface NavigationClient {
|
||||||
// Explicit reset reason when the NavigationClient needs to be reset when
|
|
||||||
// swapping frames. In this case, it is often desirable to *not* cancel the
|
|
||||||
// navigation: otherwise, this may interfere with the navigation queueing
|
|
||||||
// logic (by unexpectedly cancelling the queued navigation).
|
|
||||||
const uint32 kResetForSwap = 1;
|
|
||||||
|
|
||||||
// Explicit reset reason when the NavigationClient needs to be reset because
|
|
||||||
// the renderer explicitly aborted the navigation (instead of e.g. crashing).
|
|
||||||
const uint32 kResetForAbort = 2;
|
|
||||||
|
|
||||||
// Tells the renderer that a navigation is ready to commit.
|
// Tells the renderer that a navigation is ready to commit.
|
||||||
//
|
//
|
||||||
// The renderer should bind the |url_loader_client_endpoints| to an
|
// The renderer should bind the |url_loader_client_endpoints| to an
|
||||||
|
@@ -48,6 +48,9 @@ enum class NavigationDiscardReason {
|
|||||||
// the response failed security checks (e.g. claiming an incompatible origin),
|
// the response failed security checks (e.g. claiming an incompatible origin),
|
||||||
// so the renderer got killed and the commit didn't go through.
|
// so the renderer got killed and the commit didn't go through.
|
||||||
kFailedSecurityCheck,
|
kFailedSecurityCheck,
|
||||||
|
// Cancelled by a new navigation with identical conditions to the discarded
|
||||||
|
// navigation.
|
||||||
|
kNewDuplicateNavigation,
|
||||||
};
|
};
|
||||||
|
|
||||||
} // namespace content
|
} // namespace content
|
||||||
|
@@ -138,12 +138,26 @@ void NavigationClient::SetUpRendererInitiatedNavigation(
|
|||||||
|
|
||||||
void NavigationClient::ResetWithoutCancelling() {
|
void NavigationClient::ResetWithoutCancelling() {
|
||||||
navigation_client_receiver_.ResetWithReason(
|
navigation_client_receiver_.ResetWithReason(
|
||||||
mojom::NavigationClient::kResetForSwap, "");
|
base::to_underlying(
|
||||||
|
mojom::NavigationClientDisconnectReason::kResetForSwap),
|
||||||
|
"");
|
||||||
|
}
|
||||||
|
|
||||||
|
void NavigationClient::ResetForNewNavigation(bool is_duplicate_navigation) {
|
||||||
|
navigation_client_receiver_.ResetWithReason(
|
||||||
|
base::to_underlying(is_duplicate_navigation
|
||||||
|
? mojom::NavigationClientDisconnectReason::
|
||||||
|
kResetForDuplicateNavigation
|
||||||
|
: mojom::NavigationClientDisconnectReason::
|
||||||
|
kResetForNewNavigation),
|
||||||
|
"");
|
||||||
}
|
}
|
||||||
|
|
||||||
void NavigationClient::ResetForAbort() {
|
void NavigationClient::ResetForAbort() {
|
||||||
navigation_client_receiver_.ResetWithReason(
|
navigation_client_receiver_.ResetWithReason(
|
||||||
mojom::NavigationClient::kResetForAbort, "");
|
base::to_underlying(
|
||||||
|
mojom::NavigationClientDisconnectReason::kResetForAbort),
|
||||||
|
"");
|
||||||
}
|
}
|
||||||
|
|
||||||
void NavigationClient::NotifyNavigationCancellationWindowEnded() {
|
void NavigationClient::NotifyNavigationCancellationWindowEnded() {
|
||||||
|
@@ -87,6 +87,8 @@ class NavigationClient : mojom::NavigationClient {
|
|||||||
|
|
||||||
void ResetWithoutCancelling();
|
void ResetWithoutCancelling();
|
||||||
|
|
||||||
|
void ResetForNewNavigation(bool is_duplicate_navigation);
|
||||||
|
|
||||||
void ResetForAbort();
|
void ResetForAbort();
|
||||||
|
|
||||||
bool HasBeginNavigationParams() const { return !!begin_params_; }
|
bool HasBeginNavigationParams() const { return !!begin_params_; }
|
||||||
|
@@ -2192,7 +2192,11 @@ void RenderFrameImpl::BindNavigationClient(
|
|||||||
void RenderFrameImpl::BindNavigationClientWithParams(
|
void RenderFrameImpl::BindNavigationClientWithParams(
|
||||||
mojo::PendingAssociatedReceiver<mojom::NavigationClient> receiver,
|
mojo::PendingAssociatedReceiver<mojom::NavigationClient> receiver,
|
||||||
blink::mojom::BeginNavigationParamsPtr begin_params,
|
blink::mojom::BeginNavigationParamsPtr begin_params,
|
||||||
blink::mojom::CommonNavigationParamsPtr common_params) {
|
blink::mojom::CommonNavigationParamsPtr common_params,
|
||||||
|
bool is_duplicate_navigation) {
|
||||||
|
if (navigation_client_impl_) {
|
||||||
|
navigation_client_impl_->ResetForNewNavigation(is_duplicate_navigation);
|
||||||
|
}
|
||||||
navigation_client_impl_ = std::make_unique<NavigationClient>(
|
navigation_client_impl_ = std::make_unique<NavigationClient>(
|
||||||
this, std::move(begin_params), std::move(common_params));
|
this, std::move(begin_params), std::move(common_params));
|
||||||
navigation_client_impl_->Bind(std::move(receiver));
|
navigation_client_impl_->Bind(std::move(receiver));
|
||||||
@@ -4467,7 +4471,7 @@ base::UnguessableToken RenderFrameImpl::GetDevToolsFrameToken() {
|
|||||||
return devtools_frame_token_;
|
return devtools_frame_token_;
|
||||||
}
|
}
|
||||||
|
|
||||||
void RenderFrameImpl::AbortClientNavigation() {
|
void RenderFrameImpl::AbortClientNavigation(bool for_new_navigation) {
|
||||||
CHECK(in_frame_tree_);
|
CHECK(in_frame_tree_);
|
||||||
is_requesting_navigation_ = false;
|
is_requesting_navigation_ = false;
|
||||||
if (mhtml_body_loader_client_) {
|
if (mhtml_body_loader_client_) {
|
||||||
@@ -4479,7 +4483,12 @@ void RenderFrameImpl::AbortClientNavigation() {
|
|||||||
// Note: This might not actually cancel the navigation if the navigation is
|
// Note: This might not actually cancel the navigation if the navigation is
|
||||||
// already in the process of committing to a different RenderFrame.
|
// already in the process of committing to a different RenderFrame.
|
||||||
|
|
||||||
navigation_client_impl_->ResetForAbort();
|
if (for_new_navigation) {
|
||||||
|
navigation_client_impl_->ResetForNewNavigation(
|
||||||
|
/*is_duplicate_navigation=*/false);
|
||||||
|
} else {
|
||||||
|
navigation_client_impl_->ResetForAbort();
|
||||||
|
}
|
||||||
navigation_client_impl_.reset();
|
navigation_client_impl_.reset();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -6306,7 +6315,7 @@ void RenderFrameImpl::BeginNavigationInternal(
|
|||||||
navigation_client_remote;
|
navigation_client_remote;
|
||||||
BindNavigationClientWithParams(
|
BindNavigationClientWithParams(
|
||||||
navigation_client_remote.InitWithNewEndpointAndPassReceiver(),
|
navigation_client_remote.InitWithNewEndpointAndPassReceiver(),
|
||||||
begin_params.Clone(), common_params.Clone());
|
begin_params.Clone(), common_params.Clone(), is_duplicate_navigation);
|
||||||
mojo::PendingReceiver<mojom::NavigationRendererCancellationListener>
|
mojo::PendingReceiver<mojom::NavigationRendererCancellationListener>
|
||||||
renderer_cancellation_listener_receiver;
|
renderer_cancellation_listener_receiver;
|
||||||
navigation_client_impl_->SetUpRendererInitiatedNavigation(
|
navigation_client_impl_->SetUpRendererInitiatedNavigation(
|
||||||
|
@@ -606,7 +606,7 @@ class CONTENT_EXPORT RenderFrameImpl
|
|||||||
void NotifyCurrentHistoryItemChanged() override;
|
void NotifyCurrentHistoryItemChanged() override;
|
||||||
void DidUpdateCurrentHistoryItem() override;
|
void DidUpdateCurrentHistoryItem() override;
|
||||||
base::UnguessableToken GetDevToolsFrameToken() override;
|
base::UnguessableToken GetDevToolsFrameToken() override;
|
||||||
void AbortClientNavigation() override;
|
void AbortClientNavigation(bool for_new_navigation) override;
|
||||||
void DidChangeSelection(bool is_empty_selection,
|
void DidChangeSelection(bool is_empty_selection,
|
||||||
blink::SyncCondition force_sync) override;
|
blink::SyncCondition force_sync) override;
|
||||||
void FocusedElementChanged(const blink::WebElement& element) override;
|
void FocusedElementChanged(const blink::WebElement& element) override;
|
||||||
@@ -754,7 +754,8 @@ class CONTENT_EXPORT RenderFrameImpl
|
|||||||
void BindNavigationClientWithParams(
|
void BindNavigationClientWithParams(
|
||||||
mojo::PendingAssociatedReceiver<mojom::NavigationClient> receiver,
|
mojo::PendingAssociatedReceiver<mojom::NavigationClient> receiver,
|
||||||
blink::mojom::BeginNavigationParamsPtr begin_params,
|
blink::mojom::BeginNavigationParamsPtr begin_params,
|
||||||
blink::mojom::CommonNavigationParamsPtr common_params);
|
blink::mojom::CommonNavigationParamsPtr common_params,
|
||||||
|
bool is_duplicate_navigation);
|
||||||
|
|
||||||
// Virtual so that a TestRenderFrame can mock out the interface.
|
// Virtual so that a TestRenderFrame can mock out the interface.
|
||||||
virtual mojom::FrameHost* GetFrameHost();
|
virtual mojom::FrameHost* GetFrameHost();
|
||||||
|
@@ -504,7 +504,10 @@ class BLINK_EXPORT WebLocalFrameClient {
|
|||||||
|
|
||||||
// PlzNavigate
|
// PlzNavigate
|
||||||
// Called to abort a navigation that is being handled by the browser process.
|
// Called to abort a navigation that is being handled by the browser process.
|
||||||
virtual void AbortClientNavigation() {}
|
// `for_new_navigation` is true iff the abort is the result of beginning a
|
||||||
|
// new navigation, since a document should only have one navigation in-flight
|
||||||
|
// at a time.
|
||||||
|
virtual void AbortClientNavigation(bool for_new_navigation) {}
|
||||||
|
|
||||||
// InstalledApp API ----------------------------------------------------
|
// InstalledApp API ----------------------------------------------------
|
||||||
|
|
||||||
|
@@ -342,7 +342,7 @@ class CORE_EXPORT LocalFrameClient : public FrameClient {
|
|||||||
|
|
||||||
virtual void NotifyUserActivation() {}
|
virtual void NotifyUserActivation() {}
|
||||||
|
|
||||||
virtual void AbortClientNavigation() {}
|
virtual void AbortClientNavigation(bool for_new_navigation) {}
|
||||||
|
|
||||||
virtual WebSpellCheckPanelHostClient* SpellCheckPanelHostClient() const = 0;
|
virtual WebSpellCheckPanelHostClient* SpellCheckPanelHostClient() const = 0;
|
||||||
|
|
||||||
|
@@ -1040,9 +1040,10 @@ void LocalFrameClientImpl::NotifyUserActivation() {
|
|||||||
autofill_client->UserGestureObserved();
|
autofill_client->UserGestureObserved();
|
||||||
}
|
}
|
||||||
|
|
||||||
void LocalFrameClientImpl::AbortClientNavigation() {
|
void LocalFrameClientImpl::AbortClientNavigation(bool for_new_navigation) {
|
||||||
if (web_frame_->Client())
|
if (web_frame_->Client()) {
|
||||||
web_frame_->Client()->AbortClientNavigation();
|
web_frame_->Client()->AbortClientNavigation(for_new_navigation);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
WebSpellCheckPanelHostClient* LocalFrameClientImpl::SpellCheckPanelHostClient()
|
WebSpellCheckPanelHostClient* LocalFrameClientImpl::SpellCheckPanelHostClient()
|
||||||
|
@@ -222,7 +222,7 @@ class CORE_EXPORT LocalFrameClientImpl final : public LocalFrameClient {
|
|||||||
|
|
||||||
void NotifyUserActivation() override;
|
void NotifyUserActivation() override;
|
||||||
|
|
||||||
void AbortClientNavigation() override;
|
void AbortClientNavigation(bool for_new_navigation) override;
|
||||||
|
|
||||||
WebSpellCheckPanelHostClient* SpellCheckPanelHostClient() const override;
|
WebSpellCheckPanelHostClient* SpellCheckPanelHostClient() const override;
|
||||||
|
|
||||||
|
@@ -1691,7 +1691,8 @@ void FrameLoader::CancelClientNavigation(CancelNavigationReason reason) {
|
|||||||
ClearClientNavigation();
|
ClearClientNavigation();
|
||||||
if (WebPluginContainerImpl* plugin = frame_->GetWebPluginContainer())
|
if (WebPluginContainerImpl* plugin = frame_->GetWebPluginContainer())
|
||||||
plugin->DidFailLoading(error);
|
plugin->DidFailLoading(error);
|
||||||
Client()->AbortClientNavigation();
|
Client()->AbortClientNavigation(reason ==
|
||||||
|
CancelNavigationReason::kNewNavigation);
|
||||||
}
|
}
|
||||||
|
|
||||||
void FrameLoader::DispatchDocumentElementAvailable() {
|
void FrameLoader::DispatchDocumentElementAvailable() {
|
||||||
|
@@ -63,6 +63,8 @@ enum class CancelNavigationReason {
|
|||||||
kDropped,
|
kDropped,
|
||||||
// Navigate event is fired.
|
// Navigate event is fired.
|
||||||
kNavigateEvent,
|
kNavigateEvent,
|
||||||
|
// New navigation is starting (e.g. form submission).
|
||||||
|
kNewNavigation,
|
||||||
// Anything else (including error cases that don't drop the navigation).
|
// Anything else (including error cases that don't drop the navigation).
|
||||||
kOther
|
kOther
|
||||||
};
|
};
|
||||||
|
@@ -179,6 +179,7 @@ chromium-metrics-reviews@google.com.
|
|||||||
<int value="11" label="kNewHistoryNavigation"/>
|
<int value="11" label="kNewHistoryNavigation"/>
|
||||||
<int value="12" label="kNewOtherNavigationBrowserInitiated"/>
|
<int value="12" label="kNewOtherNavigationBrowserInitiated"/>
|
||||||
<int value="13" label="kNewOtherNavigationRendererInitiated"/>
|
<int value="13" label="kNewOtherNavigationRendererInitiated"/>
|
||||||
|
<int value="14" label="kNewDuplicateNavigation"/>
|
||||||
</enum>
|
</enum>
|
||||||
|
|
||||||
<!-- LINT.ThenChange(//components/page_load_metrics/browser/observers/abandoned_page_load_metrics_observer.h:AbandonReason) -->
|
<!-- LINT.ThenChange(//components/page_load_metrics/browser/observers/abandoned_page_load_metrics_observer.h:AbandonReason) -->
|
||||||
|
Reference in New Issue
Block a user