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

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

Issue 1357403003: Separate page load metrics for backgrounded pages (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Used DidFinishLoad instead of DidStopLoading Created 5 years, 3 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 080f00b076152f9052e85a3fe361e88888520c83..c5975ca3f7c85171e7648558209583d7edfa805b 100644
--- a/components/page_load_metrics/browser/metrics_web_contents_observer.cc
+++ b/components/page_load_metrics/browser/metrics_web_contents_observer.cc
@@ -51,7 +51,10 @@ bool IsValidPageLoadTiming(const PageLoadTiming& timing) {
MetricsWebContentsObserver::MetricsWebContentsObserver(
content::WebContents* web_contents)
- : content::WebContentsObserver(web_contents) {}
+ : content::WebContentsObserver(web_contents),
+ current_navigation_stayed_in_foreground_(false),
+ previous_navigation_stayed_in_foreground_(false),
+ in_foreground_(false) {}
Bryan McQuade 2015/09/22 23:27:47 can webcontents ever start foregrounded (such that
Charlie Harrison 2015/09/23 22:15:23 I couldn't find a case where we don't get the WasS
// This object is tied to a single WebContents for its entire lifetime.
MetricsWebContentsObserver::~MetricsWebContentsObserver() {
@@ -86,6 +89,41 @@ void MetricsWebContentsObserver::RenderProcessGone(
RecordTimingHistograms();
}
+void MetricsWebContentsObserver::DidStartNavigation(
+ content::NavigationHandle* navigation_handle) {
+ if (!navigation_handle->IsInMainFrame())
+ return;
+ current_navigation_stayed_in_foreground_ = in_foreground_;
clamy 2015/09/22 23:54:31 This could break down in certain edge cases. You m
clamy 2015/09/23 00:06:18 Similarly, we can have the following case: 1) You
+}
+
+void MetricsWebContentsObserver::DidFinishLoad(
+ content::RenderFrameHost* render_frame_host,
+ const GURL& validated_url) {
+ if (render_frame_host == web_contents()->GetMainFrame()) {
+ previous_navigation_stayed_in_foreground_ =
+ current_navigation_stayed_in_foreground_;
+ }
+}
+
+void MetricsWebContentsObserver::DidFailLoad(
+ content::RenderFrameHost* render_frame_host,
+ const GURL& validated_url,
+ int error_code,
+ const base::string16& error_description,
+ bool was_ignored_by_handler) {
+ if (render_frame_host == web_contents()->GetMainFrame()) {
+ previous_navigation_stayed_in_foreground_ =
+ current_navigation_stayed_in_foreground_;
+ }
+}
+void MetricsWebContentsObserver::WasShown() {
+ in_foreground_ = true;
+}
+void MetricsWebContentsObserver::WasHidden() {
+ in_foreground_ = false;
+ current_navigation_stayed_in_foreground_ = false;
+}
+
#define PAGE_LOAD_HISTOGRAM(name, sample) \
UMA_HISTOGRAM_CUSTOM_TIMES(name, sample, \
base::TimeDelta::FromMilliseconds(10), \
@@ -123,19 +161,35 @@ void MetricsWebContentsObserver::RecordTimingHistograms() {
return;
if (!current_timing_->dom_content_loaded_event_start.is_zero()) {
- PAGE_LOAD_HISTOGRAM(
- "PageLoad.Timing.NavigationToDOMContentLoadedEventFired",
- current_timing_->dom_content_loaded_event_start);
+ if (previous_navigation_stayed_in_foreground_) {
+ PAGE_LOAD_HISTOGRAM(
+ "PageLoad.Timing.NavigationToDOMContentLoadedEventFired",
+ current_timing_->dom_content_loaded_event_start);
+ } else {
+ PAGE_LOAD_HISTOGRAM(
+ "PageLoad.Timing.BG.NavigationToDOMContentLoadedEventFired",
+ current_timing_->dom_content_loaded_event_start);
+ }
}
if (!current_timing_->load_event_start.is_zero()) {
- PAGE_LOAD_HISTOGRAM("PageLoad.Timing.NavigationToLoadEventFired",
- current_timing_->load_event_start);
+ if (previous_navigation_stayed_in_foreground_) {
+ PAGE_LOAD_HISTOGRAM("PageLoad.Timing.NavigationToLoadEventFired",
+ current_timing_->load_event_start);
+ } else {
+ PAGE_LOAD_HISTOGRAM("PageLoad.Timing.BG.NavigationToLoadEventFired",
+ current_timing_->load_event_start);
+ }
}
if (!current_timing_->first_layout.is_zero()) {
- PAGE_LOAD_HISTOGRAM("PageLoad.Timing.NavigationToFirstLayout",
- current_timing_->first_layout);
+ if (previous_navigation_stayed_in_foreground_) {
+ PAGE_LOAD_HISTOGRAM("PageLoad.Timing.NavigationToFirstLayout",
+ current_timing_->first_layout);
+ } else {
+ PAGE_LOAD_HISTOGRAM("PageLoad.Timing.BG.NavigationToFirstLayout",
+ current_timing_->first_layout);
+ }
}
current_timing_.reset();
}

Powered by Google App Engine
This is Rietveld 408576698