diff options
author | csharrison <csharrison@chromium.org> | 2015-11-17 08:22:05 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-11-17 16:23:44 +0000 |
commit | 54a2332a8a1f5299329c445f126ccbd2c7e1fef8 (patch) | |
tree | 25e44f6f8a1b2eedcddc0668de99b89f287b1da6 /components/page_load_metrics | |
parent | 5c6599d17ebb019a21c3e4152d83e00ab4cc4c02 (diff) | |
download | chromium_src-54a2332a8a1f5299329c445f126ccbd2c7e1fef8.zip chromium_src-54a2332a8a1f5299329c445f126ccbd2c7e1fef8.tar.gz chromium_src-54a2332a8a1f5299329c445f126ccbd2c7e1fef8.tar.bz2 |
[page_load_metrics] Get navigation_start from NavigationHandle
This change also exposes GetNavigationStart() to the public content interface, with PageLoadTracker as the first consumer.
This gives us much better resolution time for events logged relative to
navigation start on the browser side (backgrounding, etc.)
This change is needed to implement abort timing.
BUG=517209
Review URL: https://codereview.chromium.org/1433533007
Cr-Commit-Position: refs/heads/master@{#360084}
Diffstat (limited to 'components/page_load_metrics')
3 files changed, 29 insertions, 35 deletions
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 6e2feb7..ffda133 100644 --- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc +++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc @@ -77,11 +77,6 @@ bool IsValidPageLoadTiming(const PageLoadTiming& timing) { return true; } -base::Time WallTimeFromTimeTicks(const base::TimeTicks& time) { - return base::Time::FromDoubleT( - (time - base::TimeTicks::UnixEpoch()).InSecondsF()); -} - void RecordInternalError(InternalErrorLoadEvent event) { UMA_HISTOGRAM_ENUMERATION(kErrorEvents, event, ERR_LAST_ENTRY); } @@ -122,8 +117,10 @@ uint64_t RapporHistogramBucketIndex(const base::TimeDelta& time) { PageLoadTracker::PageLoadTracker( bool in_foreground, PageLoadMetricsEmbedderInterface* embedder_interface, + content::NavigationHandle* navigation_handle, base::ObserverList<PageLoadMetricsObserver, true>* observers) : has_commit_(false), + navigation_start_(navigation_handle->NavigationStart()), started_in_foreground_(in_foreground), embedder_interface_(embedder_interface), observers_(observers) {} @@ -187,14 +184,10 @@ bool PageLoadTracker::HasBackgrounded() { PageLoadExtraInfo PageLoadTracker::GetPageLoadMetricsInfo() { base::TimeDelta first_background_time; base::TimeDelta first_foreground_time; - if (!background_time_.is_null() && started_in_foreground_) { - first_background_time = - WallTimeFromTimeTicks(background_time_) - timing_.navigation_start; - } - if (!foreground_time_.is_null() && !started_in_foreground_) { - first_foreground_time = - WallTimeFromTimeTicks(foreground_time_) - timing_.navigation_start; - } + if (!background_time_.is_null() && started_in_foreground_) + first_background_time = background_time_ - navigation_start_; + if (!foreground_time_.is_null() && !started_in_foreground_) + first_foreground_time = foreground_time_ - navigation_start_; return PageLoadExtraInfo(first_background_time, first_foreground_time, started_in_foreground_); } @@ -218,9 +211,8 @@ const GURL& PageLoadTracker::GetCommittedURL() { // backgrounded. base::TimeDelta PageLoadTracker::GetBackgroundDelta() { if (started_in_foreground_) { - if (background_time_.is_null()) - return base::TimeDelta::Max(); - return WallTimeFromTimeTicks(background_time_) - timing_.navigation_start; + return background_time_.is_null() ? base::TimeDelta::Max() + : background_time_ - navigation_start_; } return base::TimeDelta(); } @@ -306,9 +298,8 @@ void PageLoadTracker::RecordTimingHistograms() { if (!background_time_.is_null()) { PAGE_LOAD_HISTOGRAM(kHistogramFirstBackground, background_delta); } else if (!foreground_time_.is_null()) { - PAGE_LOAD_HISTOGRAM( - kHistogramFirstForeground, - WallTimeFromTimeTicks(foreground_time_) - timing_.navigation_start); + PAGE_LOAD_HISTOGRAM(kHistogramFirstForeground, + foreground_time_ - navigation_start_); } } @@ -427,10 +418,10 @@ void MetricsWebContentsObserver::DidStartNavigation( // Passing raw pointers to observers_ and embedder_interface_ is safe because // the MetricsWebContentsObserver owns them both list and they are torn down // after the PageLoadTracker. - provisional_loads_.insert( - navigation_handle, - make_scoped_ptr(new PageLoadTracker( - in_foreground_, embedder_interface_.get(), &observers_))); + provisional_loads_.insert(navigation_handle, + make_scoped_ptr(new PageLoadTracker( + in_foreground_, embedder_interface_.get(), + navigation_handle, &observers_))); } void MetricsWebContentsObserver::DidFinishNavigation( 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 c9685b6..1c8a4e5 100644 --- a/components/page_load_metrics/browser/metrics_web_contents_observer.h +++ b/components/page_load_metrics/browser/metrics_web_contents_observer.h @@ -195,6 +195,7 @@ class PageLoadTracker { // outlives this class. PageLoadTracker(bool in_foreground, PageLoadMetricsEmbedderInterface* embedder_interface, + content::NavigationHandle* navigation_handle, base::ObserverList<PageLoadMetricsObserver, true>* observers); ~PageLoadTracker(); void Redirect(content::NavigationHandle* navigation_handle); @@ -219,6 +220,9 @@ class PageLoadTracker { bool has_commit_; + // The navigation start in TimeTicks, not the wall time reported by Blink. + const base::TimeTicks navigation_start_; + // We record separate metrics for events that occur after a background, // because metrics like layout/paint are delayed artificially // when they occur in the background. diff --git a/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc b/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc index 769c237..b1f3228 100644 --- a/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc +++ b/components/page_load_metrics/browser/metrics_web_contents_observer_unittest.cc @@ -302,8 +302,7 @@ TEST_F(MetricsWebContentsObserverTest, BackgroundDifferentHistogram) { base::TimeDelta first_layout = base::TimeDelta::FromSeconds(2); PageLoadTiming timing; - timing.navigation_start = base::Time::FromDoubleT( - (base::TimeTicks::Now() - base::TimeTicks::UnixEpoch()).InSecondsF()); + timing.navigation_start = base::Time::FromDoubleT(1); timing.first_layout = first_layout; content::WebContentsTester* web_contents_tester = @@ -356,11 +355,11 @@ TEST_F(MetricsWebContentsObserverTest, DontLogPrerender) { TEST_F(MetricsWebContentsObserverTest, OnlyBackgroundLaterEvents) { PageLoadTiming timing; - timing.navigation_start = base::Time::FromDoubleT( - (base::TimeTicks::Now() - base::TimeTicks::UnixEpoch()).InSecondsF() - 1); - - timing.response_start = base::TimeDelta::FromMilliseconds(1); - timing.dom_content_loaded_event_start = base::TimeDelta::FromMilliseconds(1); + timing.navigation_start = base::Time::FromDoubleT(1); + // Set these events at 1 microsecond so they are definitely occur before we + // background the tab later in the test. + timing.response_start = base::TimeDelta::FromMicroseconds(1); + timing.dom_content_loaded_event_start = base::TimeDelta::FromMicroseconds(1); content::WebContentsTester* web_contents_tester = content::WebContentsTester::For(web_contents()); @@ -402,7 +401,9 @@ TEST_F(MetricsWebContentsObserverTest, OnlyBackgroundLaterEvents) { } TEST_F(MetricsWebContentsObserverTest, DontBackgroundQuickerLoad) { - base::TimeDelta first_layout = base::TimeDelta::FromMilliseconds(1); + // Set this event at 1 microsecond so it occurs before we foreground later in + // the test. + base::TimeDelta first_layout = base::TimeDelta::FromMicroseconds(1); PageLoadTiming timing; timing.navigation_start = base::Time::FromDoubleT(1); @@ -430,7 +431,7 @@ TEST_F(MetricsWebContentsObserverTest, DontBackgroundQuickerLoad) { main_rfh()); rfh_tester->SimulateNavigationStop(); - // Navigate again to see if the timing updated for a subframe message. + // Navigate again to see if the timing updated for the foregrounded load. web_contents_tester->NavigateAndCommit(GURL(kDefaultTestUrl)); histogram_tester_.ExpectTotalCount(kHistogramDomContentLoaded, 0); @@ -578,9 +579,7 @@ TEST_F(MetricsWebContentsObserverTest, SuccessfulFirstLayoutInForegroundEvent) { TEST_F(MetricsWebContentsObserverTest, SuccessfulFirstLayoutInBackgroundEvent) { PageLoadTiming timing; - timing.navigation_start = base::Time::FromDoubleT( - (base::TimeTicks::Now() - base::TimeTicks::UnixEpoch()).InSecondsF() - 1); - + timing.navigation_start = base::Time::FromDoubleT(1); timing.first_layout = base::TimeDelta::FromSeconds(30); content::WebContentsTester* web_contents_tester = |