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

Unified Diff: chrome/browser/page_load_metrics/page_load_tracker.h

Issue 2699933003: Generalize abort tracking to page end state tracking (Closed)
Patch Set: fix comment Created 3 years, 10 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: chrome/browser/page_load_metrics/page_load_tracker.h
diff --git a/chrome/browser/page_load_metrics/page_load_tracker.h b/chrome/browser/page_load_metrics/page_load_tracker.h
index ef782b2a6406e91f60814b8ad272c4a4eae62d77..c08628d0d7236c1f13edb318241a19fee8fc8e6f 100644
--- a/chrome/browser/page_load_metrics/page_load_tracker.h
+++ b/chrome/browser/page_load_metrics/page_load_tracker.h
@@ -77,11 +77,11 @@ enum InternalErrorLoadEvent {
// occur if the browser filters loads less aggressively than the renderer.
ERR_NO_IPCS_RECEIVED,
- // Tracks frequency with which we record an abort time that occurred before
+ // Tracks frequency with which we record an end time that occurred before
// navigation start. This is expected to happen in some cases (see comments in
// cc file for details). We use this error counter to understand how often it
// happens.
- ERR_ABORT_BEFORE_NAVIGATION_START,
+ ERR_END_BEFORE_NAVIGATION_START,
// A new navigation triggers abort updates in multiple trackers in
// |aborted_provisional_loads_|, when usually there should only be one (the
@@ -104,6 +104,9 @@ enum InternalErrorLoadEvent {
// commit nor a failed provisional load.
ERR_NO_COMMIT_OR_FAILED_PROVISIONAL_LOAD,
+ // No page load end time was recorded for this page load.
+ ERR_NO_PAGE_LOAD_END_TIME,
+
// Add values before this final count.
ERR_LAST_ENTRY,
};
@@ -112,7 +115,7 @@ enum InternalErrorLoadEvent {
// metrics_web_contents_observer.cc. They are declared here to allow both files
// to access them.
void RecordInternalError(InternalErrorLoadEvent event);
-UserAbortType AbortTypeForPageTransition(ui::PageTransition transition);
+PageEndReason EndReasonForPageTransition(ui::PageTransition transition);
void LogAbortChainSameURLHistogram(int aborted_chain_size_same_url);
bool IsNavigationUserInitiated(content::NavigationHandle* handle);
@@ -138,7 +141,8 @@ class PageLoadTracker {
void WillProcessNavigationResponse(
content::NavigationHandle* navigation_handle);
void Commit(content::NavigationHandle* navigation_handle);
- void FailedProvisionalLoad(content::NavigationHandle* navigation_handle);
+ void FailedProvisionalLoad(content::NavigationHandle* navigation_handle,
+ base::TimeTicks failed_load_time);
void WebContentsHidden();
void WebContentsShown();
@@ -168,8 +172,8 @@ class PageLoadTracker {
return aborted_chain_size_same_url_;
}
- UserAbortType abort_type() const { return abort_type_; }
- base::TimeTicks abort_time() const { return abort_time_; }
+ PageEndReason page_end_reason() const { return page_end_reason_; }
+ base::TimeTicks page_end_time() const { return page_end_time_; }
void AddObserver(std::unique_ptr<PageLoadMetricsObserver> observer);
@@ -180,19 +184,19 @@ class PageLoadTracker {
// in the
// browser process or not. We need this to possibly clamp browser timestamp on
// a machine with inter process time tick skew.
- void NotifyAbort(UserAbortType abort_type,
- UserInitiatedInfo user_initiated_info,
- base::TimeTicks timestamp,
- bool is_certainly_browser_timestamp);
- void UpdateAbort(UserAbortType abort_type,
- UserInitiatedInfo user_initiated_info,
- base::TimeTicks timestamp,
- bool is_certainly_browser_timestamp);
+ void NotifyPageEnd(PageEndReason page_end_reason,
+ UserInitiatedInfo user_initiated_info,
+ base::TimeTicks timestamp,
+ bool is_certainly_browser_timestamp);
+ void UpdatePageEnd(PageEndReason page_end_reason,
+ UserInitiatedInfo user_initiated_info,
+ base::TimeTicks timestamp,
+ bool is_certainly_browser_timestamp);
// This method returns true if this page load has been aborted with type of
- // ABORT_OTHER, and the |abort_cause_time| is within a sufficiently close
+ // END_OTHER, and the |abort_cause_time| is within a sufficiently close
// delta to when it was aborted. Note that only provisional loads can be
- // aborted with ABORT_OTHER. While this heuristic is coarse, it works better
+ // aborted with END_OTHER. While this heuristic is coarse, it works better
// and is simpler than other feasible methods. See https://goo.gl/WKRG98.
bool IsLikelyProvisionalAbort(base::TimeTicks abort_cause_time) const;
@@ -228,10 +232,10 @@ class PageLoadTracker {
void ClampBrowserTimestampIfInterProcessTimeTickSkew(
base::TimeTicks* event_time);
- void UpdateAbortInternal(UserAbortType abort_type,
- UserInitiatedInfo user_initiated_info,
- base::TimeTicks timestamp,
- bool is_certainly_browser_timestamp);
+ void UpdatePageEndInternal(PageEndReason page_end_reason,
+ UserInitiatedInfo user_initiated_info,
+ base::TimeTicks timestamp,
+ bool is_certainly_browser_timestamp);
// If |final_navigation| is null, then this is an "unparented" abort chain,
// and represents a sequence of provisional aborts that never ends with a
@@ -266,21 +270,21 @@ class PageLoadTracker {
std::unique_ptr<FailedProvisionalLoadInfo> failed_provisional_load_info_;
- // Will be ABORT_NONE if we have not aborted this load yet. Otherwise will
- // be the first abort action the user performed.
- UserAbortType abort_type_;
-
- // Whether the abort for this page load was user initiated. For example, if
- // this page load was aborted by a new navigation, this field tracks whether
- // that new navigation was user-initiated. This field is only useful if this
- // page load's abort type is a value other than ABORT_NONE. Note that this
- // value is currently experimental, and is subject to change. In particular,
- // this field is never set to true for some abort types, such as stop and
- // close, since we don't yet have sufficient instrumentation to know if a stop
- // or close was caused by a user action.
- UserInitiatedInfo abort_user_initiated_info_;
-
- base::TimeTicks abort_time_;
+ // Will be END_NONE if we have not ended this load yet. Otherwise will
+ // be the first page end reason encountered.
+ PageEndReason page_end_reason_;
+
+ // Whether the page end cause for this page load was user initiated. For
+ // example, if this page load was ended by a new navigation, this field tracks
+ // whether that new navigation was user-initiated. This field is only useful
+ // if this page load's end reason is a value other than END_NONE. Note that
+ // this value is currently experimental, and is subject to change. In
+ // particular, this field is never set to true for some page end reasons, such
+ // as stop and close, since we don't yet have sufficient instrumentation to
+ // know if a stop or close was caused by a user action.
+ UserInitiatedInfo page_end_user_initiated_info_;
+
+ base::TimeTicks page_end_time_;
// We record separate metrics for events that occur after a background,
// because metrics like layout/paint are delayed artificially
« no previous file with comments | « chrome/browser/page_load_metrics/page_load_metrics_util.cc ('k') | chrome/browser/page_load_metrics/page_load_tracker.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698