[Page Load Metrics] Add page load metrics for omnibox.

chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5057c7345d3db2bd3c7f702a043dc81d8c5583a9
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.cc
@@ -0,0 +1,117 @@
+// Copyright 2017 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 "chrome/browser/page_load_metrics/observers/omnibox_suggestion_used_page_load_metrics_observer.h"
+
+#include <algorithm>
+
+#include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
+
+namespace {
+
+const char kSearchFirstContentfulPaint[] =
+    "Omnibox.SuggestionUsed.Search.NavigationToFirstContentfulPaint";
+const char kURLFirstContentfulPaint[] =
+    "Omnibox.SuggestionUsed.URL.NavigationToFirstContentfulPaint";
+const char kPrerenderSearchFirstContentfulPaint[] =
+    "Omnibox.SuggestionUsed.Search.ForegroundToFirstContentfulPaint.Prerender";
+const char kPrerenderURLFirstContentfulPaint[] =
+    "Omnibox.SuggestionUsed.URL.ForegroundToFirstContentfulPaint.Prerender";
+
+const char kSearchFirstMeaningfulPaint[] =
+    "Omnibox.SuggestionUsed.Search.Experimental."
+    "NavigationToFirstMeaningfulPaint";
+const char kURLFirstMeaningfulPaint[] =
+    "Omnibox.SuggestionUsed.URL.Experimental.NavigationToFirstMeaningfulPaint";
+const char kPrerenderSearchFirstMeaningfulPaint[] =
+    "Omnibox.SuggestionUsed.Search.Experimental."
+    "ForegroundToFirstMeaningfulPaint.Prerender";
+const char kPrerenderURLFirstMeaningfulPaint[] =
+    "Omnibox.SuggestionUsed.URL.Experimental."
+    "ForegroundToFirstMeaningfulPaint.Prerender";
+
+const char kPrerenderSearchNavigationToFirstForeground[] =
+    "Omnibox.SuggestionUsed.Search.NavigationToFirstForeground.Prerender";
+const char kPrerenderURLNavigationToFirstForeground[] =
+    "Omnibox.SuggestionUsed.URL.NavigationToFirstForeground.Prerender";

[Mark P, 2017/03/29 23:17:07]

Is it expected that search and URL navigations to first foreground for
non-prerender is 0, or at least at least really tiny and cannot be measured
using this framework?

[lpy, 2017/03/31 20:42:03]

If I understand correctly, non-prerender means we don't have is_prerender is
false, we don't log to UMA.

And I understand correctly, yes, it should be 0 because it's on foreground when
navigation starts.

[Bryan McQuade, 2017/04/03 19:34:34]

Navigation time is the time we start the prerender (as opposed to the time the
user first sees the tab), whereas 'first foreground' would be the time that the
prerender is swapped in and used. So this is essentially the amount of time we
saved the user in predicting that a result might be needed.

[Mark P, 2017/04/03 23:19:15]

Acknowledged.

+
+}  // namespace
+
+OmniboxSuggestionUsedMetricsObserver::OmniboxSuggestionUsedMetricsObserver(
+    bool is_prerender)
+    : is_prerender_(is_prerender) {}
+
+OmniboxSuggestionUsedMetricsObserver::~OmniboxSuggestionUsedMetricsObserver() {}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+OmniboxSuggestionUsedMetricsObserver::OnHidden(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  return STOP_OBSERVING;
+}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+OmniboxSuggestionUsedMetricsObserver::OnCommit(
+    content::NavigationHandle* navigation_handle) {
+  transition_type_ = navigation_handle->GetPageTransition();
+  return (transition_type_ & ui::PAGE_TRANSITION_FROM_ADDRESS_BAR) != 0
+             ? CONTINUE_OBSERVING
+             : STOP_OBSERVING;
+}
+
+void OmniboxSuggestionUsedMetricsObserver::OnFirstContentfulPaint(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  base::TimeDelta fcp = timing.first_contentful_paint.value();
+
+  if (info.started_in_foreground) {
+    if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                     ui::PAGE_TRANSITION_GENERATED)) {
+      PAGE_LOAD_HISTOGRAM(kSearchFirstContentfulPaint, fcp);
+    } else if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                            ui::PAGE_TRANSITION_TYPED)) {
+      PAGE_LOAD_HISTOGRAM(kURLFirstContentfulPaint, fcp);
+    }
+  } else if (is_prerender_ && info.first_foreground_time) {

[Mark P, 2017/03/29 23:17:07]

Are all prerenders started in the background, even if the things that spawned
them is in the foreground?
Please comment.
ditto below

[Mark P, 2017/03/29 23:17:07]

Why do you need to condition on info.first_foreground_time being there?  Under
what conditions is it missing?
(The documentation in the .h for the structure does not say.)
If this question causes you to add comments here, then add them below as well.

[lpy, 2017/03/31 20:42:03]

Bryan@, any idea about this question?

[lpy, 2017/03/31 20:42:03]

If I understand correctly, since pages are not supposed to paint while in the
background, when `OnFirstContentfulPaint` gets called, it means pages are on
foreground, thus first_foreground_time should exist, the reason we add this,
quote from Bryan:
I add a comment to explain.

[Bryan McQuade, 2017/04/03 19:34:34]

Yes, this is my understanding - the WebContents hosting a prerender is 'hidden'
until the prerender is used. We used to have a DCHECK to verify this, but it got
lost in a refactor. You could ask pasko@ if you want to be certain of this.

+    base::TimeDelta perceived_fcp =
+        std::max(base::TimeDelta(), fcp - info.first_foreground_time.value());

[Mark P, 2017/03/29 23:17:07]

optional:
This only makes sense to me if first_contentful_paint and first_foreground_time
(both timedeltas) are both relative to the same baseline.  E.g., time navigation
started.
chrome/common/page_load_metrics/page_load_timing.h doesn't say anything about
the baseline for first_* values.  (The info structure
chrome/browser/page_load_metrics/page_load_metrics_observer.h does talk about
baselines.)  The comments in the timing structure kind of imply that the that
all the first_ fields and the ones near it are all relative to the same baseline
and all the parse_* fields are relative to a different baseline.  It might help
(outside this changelist), if my interpretation is correct, to make the comments
in that file more explicit.

[lpy, 2017/03/31 20:42:03]

Yes, they are both based on navigation start.
For first_foreground_time, please see:
https://cs.chromium.org/chromium/src/chrome/browser/page_load_metrics/page_lo...

I will add some comments to the file in a separate patch.

+    if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                     ui::PAGE_TRANSITION_GENERATED)) {
+      PAGE_LOAD_HISTOGRAM(kPrerenderSearchFirstContentfulPaint, perceived_fcp);
+      PAGE_LOAD_HISTOGRAM(kPrerenderSearchNavigationToFirstForeground,

[Mark P, 2017/03/29 23:17:07]

Is it safe to assume (as this code does) that all pages that are foregrounded
observe a first contentful paint?  After all, you're only emitting the first
foreground time on pages that do.

[lpy, 2017/03/31 20:42:03]

If I understand correctly, if we don't observe a first contentful pain, we don't
even trigger this function.

https://cs.chromium.org/chromium/src/chrome/browser/page_load_metrics/page_lo...

[Bryan McQuade, 2017/04/03 19:34:34]

Not all pages that are foregrounded will observe a contentful paint. For example
a navigation to a completely blank page will never encounter an FCP. These cases
are rare and it's not clear that it's interesting to track them so we ignore
those pages. The last event in the pipeline that's guaranteed to happen for all
pages is the first layout, so if we do want a metric that we know gets logged
for all pages, even empty ones, we could hook OnFirstLayout. However, first
layout is not as well connected to the user experience as painting, which is why
we have tended to focus more on the paint metrics despite the fact that some
pages may never report contentful paints (e.g. blank pages, or pages with no
text, images, svg content, etc).

[Mark P, 2017/04/03 23:19:15]

Thanks for the explanation.  I don't think new comments are needed here, but it
does influence the histograms.xml comments.  I'll add a note there.

+                          info.first_foreground_time.value());
+    } else if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                            ui::PAGE_TRANSITION_TYPED)) {
+      PAGE_LOAD_HISTOGRAM(kPrerenderURLFirstContentfulPaint, perceived_fcp);
+      PAGE_LOAD_HISTOGRAM(kPrerenderURLNavigationToFirstForeground,
+                          info.first_foreground_time.value());
+    }
+  }
+}
+
+void OmniboxSuggestionUsedMetricsObserver::OnFirstMeaningfulPaint(
+    const page_load_metrics::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  base::TimeDelta fmp = timing.first_meaningful_paint.value();
+
+  if (info.started_in_foreground) {
+    if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                     ui::PAGE_TRANSITION_GENERATED)) {
+      PAGE_LOAD_HISTOGRAM(kSearchFirstMeaningfulPaint, fmp);
+    } else if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                            ui::PAGE_TRANSITION_TYPED)) {
+      PAGE_LOAD_HISTOGRAM(kURLFirstMeaningfulPaint, fmp);
+    }
+  } else if (is_prerender_ && info.first_foreground_time) {
+    base::TimeDelta perceived_fmp =
+        std::max(base::TimeDelta(), fmp - info.first_foreground_time.value());
+    if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                     ui::PAGE_TRANSITION_GENERATED)) {
+      PAGE_LOAD_HISTOGRAM(kPrerenderSearchFirstMeaningfulPaint, perceived_fmp);
+    } else if (ui::PageTransitionCoreTypeIs(transition_type_,
+                                            ui::PAGE_TRANSITION_TYPED)) {
+      PAGE_LOAD_HISTOGRAM(kPrerenderURLFirstMeaningfulPaint, perceived_fmp);
+    }
+  }
+}