Separate page load metrics for backgrounded pages

components/page_load_metrics/browser/metrics_web_contents_observer.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/page_load_metrics/browser/metrics_web_contents_observer.h"
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "components/page_load_metrics/common/page_load_metrics_messages.h"
 #include "components/page_load_metrics/common/page_load_timing.h"
@@ skipping 33 matching lines @@
       timing.response_start <= timing.load_event_start &&
       timing.dom_content_loaded_event_start <= timing.load_event_start);
 
   return true;
 }
 
 }  // namespace
 
 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 we would not get an initial
WasShown callback)?

[Charlie Harrison, 2015/09/23 22:15:23]

I couldn't find a case where we don't get the WasShown callback, but it's worth
investigating. I could put a DCHECK in WasHidden to assert that we were
foregrounded.

 
 // This object is tied to a single WebContents for its entire lifetime.
 MetricsWebContentsObserver::~MetricsWebContentsObserver() {
   RecordTimingHistograms();
 }
 
 bool MetricsWebContentsObserver::OnMessageReceived(
     const IPC::Message& message,
     content::RenderFrameHost* render_frame_host) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ skipping 14 matching lines @@
     current_timing_.reset(new PageLoadTiming());
 }
 
 // This will occur when the process for the main RenderFrameHost exits.
 // This will happen with a normal exit or a crash.
 void MetricsWebContentsObserver::RenderProcessGone(
     base::TerminationStatus status) {
   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 make the assumption that there
is always one navigation in the WebContents at the same time. However this is
not true. Here is what can happen:
1) You start on the page slow.com with the tab in the background.
2) A javascript navigation to slow.com/foo starts. ->
current_navigation_stayed_in_foreground_ is false.
3) The tab gets in the foreground before slow.com commits.
4) You type in the omnibox and start a second concurrent navigation to
slowaswell.com. It's cross-process so it can go on at the same time as the
navigation to slow.com/foo. -> current_navigation_stayed_in_foreground_ is true.
5) The navigation to slow.com/foo commits first. This cancels the navigation to
slowaswell.com. However current_navigation_stayed_in_foreground_ is still true,
while this navigation actually started in the background.

If you want to do things cleanly, I don't think you can escape tracking the 2
possible NavigationHandles and whether they stayed in foreground all the time
separately, and only get to a general value for the tab once one of them commits
(which with my patch corresponds to DidFinishNavigation). Not to mention that
there is a special case in which you will see a commit from a same-document
navigation, but it will not cancel an ongoing cross-site navigation.

[clamy, 2015/09/23 00:06:18]

Similarly, we can have the following case:
1) You start a prerender to foo.com. The prerender contents is in background.
2) You start a request to foo.com which means your prerender WebContents become
the actual WebContents for the tab. The prerender has not finished loading but
it has committed the navigation. current_navigation_stayed_in_foreground_ is
false.
3) You start a cross-site navigation to bar.com from the omnibox.
current_navigation_stayed_in_foreground_ becomes true.
4) You cancel it before it commits. current_navigation_stayed_in_foreground_  is
unchanged.
5) The prerender navigation to foo.com finally finishes ->
current_navigation_stayed_in_foreground_ is still true when it should have been
false.

+}
+
+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), \
                              base::TimeDelta::FromMinutes(10), 100);
 
 void MetricsWebContentsObserver::OnTimingUpdated(
     content::RenderFrameHost* render_frame_host,
     const PageLoadTiming& timing) {
   if (!current_timing_)
     return;
@@ skipping 17 matching lines @@
   }
 
   *current_timing_ = timing;
 }
 
 void MetricsWebContentsObserver::RecordTimingHistograms() {
   if (!current_timing_ || !IsValidPageLoadTiming(*current_timing_))
     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();
 }
 
 bool MetricsWebContentsObserver::IsRelevantNavigation(
     content::NavigationHandle* navigation_handle) {
   // The url we see from the renderer side is not always the same as what
   // we see from the browser side (e.g. chrome://newtab). We want to be
   // sure here that we aren't logging UMA for internal pages.
   const GURL& browser_url = web_contents()->GetLastCommittedURL();
   return navigation_handle->IsInMainFrame() &&
          !navigation_handle->IsSamePage() &&
          navigation_handle->HasCommittedDocument() &&
          navigation_handle->GetURL().SchemeIsHTTPOrHTTPS() &&
          browser_url.SchemeIsHTTPOrHTTPS();
 }
 
 }  // namespace page_load_metrics