Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(562)

Unified Diff: components/page_load_metrics/browser/metrics_web_contents_observer.cc

Issue 2132603002: [page_load_metrics] Add a NavigationThrottle for richer abort metrics (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Attach the throttle first so it gets all notifications before any DEFERs Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/page_load_metrics/browser/metrics_web_contents_observer.cc
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.cc b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
index 0c586b7c65e6a9842bfa0a0f6fced893633f73f1..890bf00031c47851b7af01cddc265411ef49202a 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -251,6 +251,7 @@ PageLoadTracker::PageLoadTracker(
url_(navigation_handle->GetURL()),
abort_type_(ABORT_NONE),
started_in_foreground_(in_foreground),
+ page_transition_(navigation_handle->GetPageTransition()),
aborted_chain_size_(aborted_chain_size),
aborted_chain_size_same_url_(aborted_chain_size_same_url),
embedder_interface_(embedder_interface) {
@@ -276,11 +277,14 @@ PageLoadTracker::~PageLoadTracker() {
PAGE_LOAD_HISTOGRAM(internal::kCommitToCompleteNoTimingIPCs,
base::TimeTicks::Now() - commit_time);
}
- // Recall that trackers that are given ABORT_UNKNOWN_NAVIGATION have their
- // chain length added to the next navigation. Take care not to double count
- // them. Also do not double count committed loads, which call this already.
- if (commit_time_.is_null() && abort_type_ != ABORT_UNKNOWN_NAVIGATION)
+
+ // Don't include any aborts that resulted in a new navigation, as the chain
+ // length will be included in the aborter PageLoadTracker.
+ if (commit_time_.is_null() && abort_type_ != ABORT_RELOAD &&
+ abort_type_ != ABORT_FORWARD_BACK &&
+ abort_type_ != ABORT_NEW_NAVIGATION) {
LogAbortChainHistograms(nullptr);
+ }
for (const auto& observer : observers_) {
if (failed_provisional_load_info_) {
@@ -372,6 +376,8 @@ void PageLoadTracker::Commit(content::NavigationHandle* navigation_handle) {
commit_time_ = base::TimeTicks::Now();
ClampBrowserTimestampIfInterProcessTimeTickSkew(&commit_time_);
url_ = navigation_handle->GetURL();
+ // Some transitions (like CLIENT_REDIRECT) are only known at commit time.
+ page_transition_ = navigation_handle->GetPageTransition();
for (const auto& observer : observers_) {
observer->OnCommit(navigation_handle);
}
@@ -546,7 +552,7 @@ void PageLoadTracker::UpdateAbort(UserAbortType abort_type,
}
bool PageLoadTracker::IsLikelyProvisionalAbort(
- base::TimeTicks abort_cause_time) {
+ base::TimeTicks abort_cause_time) const {
// Note that |abort_cause_time - abort_time| can be negative.
return abort_type_ == ABORT_OTHER &&
(abort_cause_time - abort_time_).InMilliseconds() < 100;
@@ -647,7 +653,7 @@ bool MetricsWebContentsObserver::OnMessageReceived(
return handled;
}
-void MetricsWebContentsObserver::DidStartNavigation(
+void MetricsWebContentsObserver::WillStartNavigationRequest(
content::NavigationHandle* navigation_handle) {
if (!navigation_handle->IsInMainFrame())
return;
@@ -866,17 +872,19 @@ void MetricsWebContentsObserver::NotifyAbortAllLoadsWithTimestamp(
UserAbortType abort_type,
base::TimeTicks timestamp,
bool is_certainly_browser_timestamp) {
- if (committed_load_)
+ if (committed_load_) {
committed_load_->NotifyAbort(abort_type, timestamp,
is_certainly_browser_timestamp);
+ }
for (const auto& kv : provisional_loads_) {
kv.second->NotifyAbort(abort_type, timestamp,
is_certainly_browser_timestamp);
}
for (const auto& tracker : aborted_provisional_loads_) {
- if (tracker->IsLikelyProvisionalAbort(timestamp))
+ if (tracker->IsLikelyProvisionalAbort(timestamp)) {
tracker->UpdateAbort(abort_type, timestamp,
is_certainly_browser_timestamp);
+ }
}
aborted_provisional_loads_.clear();
}
@@ -897,8 +905,11 @@ MetricsWebContentsObserver::NotifyAbortedProvisionalLoadsNewNavigation(
aborted_provisional_loads_.pop_back();
base::TimeTicks timestamp = new_navigation->NavigationStart();
- if (last_aborted_load->IsLikelyProvisionalAbort(timestamp))
- last_aborted_load->UpdateAbort(ABORT_UNKNOWN_NAVIGATION, timestamp, false);
+ if (last_aborted_load->IsLikelyProvisionalAbort(timestamp)) {
+ last_aborted_load->UpdateAbort(
+ AbortTypeForPageTransition(new_navigation->GetPageTransition()),
+ timestamp, false);
+ }
aborted_provisional_loads_.clear();
return last_aborted_load;

Powered by Google App Engine
This is Rietveld 408576698