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

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

Issue 1837233002: Optional <TimeDelta> since they may be non existent. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: rebased again to make sure win_clang bot failure is not a merging issue Created 4 years, 7 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.h
diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer.h b/components/page_load_metrics/browser/metrics_web_contents_observer.h
index d58cfb003b594b0b803b1b3b7fa8befb7d2354ab..db872caf2425022fc46faba7f50a2a4edf8699b9 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.h
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.h
@@ -87,6 +87,12 @@ enum InternalErrorLoadEvent {
// latest aborted load is used to track the chain size.
ERR_NAVIGATION_SIGNALS_MULIPLE_ABORTED_LOADS,
+ // A TimeTicks value in the browser process has value less than
+ // navigation_start_. This could happen if the navigation_start_ was computed
+ // in the renderer process and the system clock has inter process time tick
+ // skew.
+ ERR_INTER_PROCESS_TIME_TICK_SKEW,
Charlie Harrison 2016/05/24 19:32:01 Looks like you need to update histograms.xml with
shivanisha 2016/05/24 20:09:56 Done. Thanks.
+
// Add values before this final count.
ERR_LAST_ENTRY
};
@@ -143,8 +149,15 @@ class PageLoadTracker {
// If the user performs some abort-like action while we are tracking this page
// load, notify the tracker. Note that we may not classify this as an abort if
// we've already performed a first paint.
- void NotifyAbort(UserAbortType abort_type, base::TimeTicks timestamp);
- void UpdateAbort(UserAbortType abort_type, base::TimeTicks timestamp);
+ // is_browser_timestamp signifies if the timestamp passed is taken 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,
+ base::TimeTicks timestamp,
+ bool is_browser_timestamp);
+ void UpdateAbort(UserAbortType abort_type,
+ base::TimeTicks timestamp,
+ bool is_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
@@ -161,11 +174,20 @@ class PageLoadTracker {
return url_;
}
+ PageLoadExtraInfo ComputePageLoadExtraInfo();
+
private:
- PageLoadExtraInfo GetPageLoadMetricsInfo();
+ // This function converts a TimeTicks value taken in the browser process
+ // to navigation_start_ if:
+ // - base::TimeTicks is not comparable across processes because the clock
+ // is not system wide monotonic.
+ // - *event_time < navigation_start_
+ void ClampBrowserTimestampIfInterProcessTimeTickSkew(
+ base::TimeTicks* event_time);
void UpdateAbortInternal(UserAbortType abort_type,
- base::TimeTicks timestamp);
+ base::TimeTicks timestamp,
+ bool is_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
@@ -251,6 +273,9 @@ class MetricsWebContentsObserver
void WasHidden() override;
void RenderProcessGone(base::TerminationStatus status) override;
+ // This getter function is required for testing.
+ const PageLoadExtraInfo GetPageLoadExtraInfoForCommittedLoad();
+
private:
friend class content::WebContentsUserData<MetricsWebContentsObserver>;
@@ -258,7 +283,8 @@ class MetricsWebContentsObserver
// that might abort them.
void NotifyAbortAllLoads(UserAbortType abort_type);
void NotifyAbortAllLoadsWithTimestamp(UserAbortType abort_type,
- base::TimeTicks timestamp);
+ base::TimeTicks timestamp,
+ bool is_browser_timestamp);
// Notify aborted provisional loads that a new navigation occurred. This is
// used for more consistent attribution tracking for aborted provisional
// loads. This method returns the provisional load that was likely aborted by

Powered by Google App Engine
This is Rietveld 408576698