[Tab Metrics] Measure FP, FCP and FMP for Foreground Tab during Session Restore

chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer.cc

Index: chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer.cc
diff --git a/chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer.cc b/chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c9e95cde40367b0d8ece846280bcfe14b3c8d902
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer.cc
@@ -0,0 +1,113 @@
+// 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/session_restore_page_load_metrics_observer.h"
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_macros.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/page_load_metrics/page_load_metrics_util.h"
+#include "chrome/browser/resource_coordinator/tab_manager.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_widget_host_view.h"
+#include "content/public/browser/restore_type.h"
+#include "content/public/browser/web_contents.h"
+#include "ui/base/page_transition_types.h"
+#include "url/gurl.h"
+
+namespace internal {
+
+const char kHistogramSessionRestoreForegroundTabFirstPaint[] =
+    "TabManager.Experimental.SessionRestore.ForegroundTab.FirstPaint";
+const char kHistogramSessionRestoreForegroundTabFirstContentfulPaint[] =
+    "TabManager.Experimental.SessionRestore.ForegroundTab.FirstContentfulPaint";
+const char kHistogramSessionRestoreForegroundTabFirstMeaningfulPaint[] =
+    "TabManager.Experimental.SessionRestore.ForegroundTab.FirstMeaningfulPaint";
+
+}  // namespace internal
+
+// static
+std::unique_ptr<SessionRestorePageLoadMetricsObserver>
+SessionRestorePageLoadMetricsObserver::CreateIfNeeded() {
+  resource_coordinator::TabManager* tab_manager =
+      g_browser_process->GetTabManager();
+  return tab_manager && tab_manager->IsSessionRestoreLoadingTabs() &&
+                 !tab_manager->HasSessionRestoreInitialForegroundTabChanged()
+             ? base::MakeUnique<SessionRestorePageLoadMetricsObserver>()
+             : nullptr;
+}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+SessionRestorePageLoadMetricsObserver::OnStart(
+    content::NavigationHandle* navigation_handle,
+    const GURL& currently_committed_url,
+    bool started_in_foreground) {
+  Browser* browser =
+      chrome::FindBrowserWithWebContents(navigation_handle->GetWebContents());
+  bool is_in_session_restored_browser =
+      browser && browser->is_used_for_session_restore();
+  // A non-empty currently-committed URL in the hosting tab indicates that
+  // there was a previous page load in this tab, and thus this load can't be a
+  // session restore.
+  if (!currently_committed_url.is_empty() || !started_in_foreground ||

[Charlie Harrison, 2017/07/21 13:59:33]

nit: I would prefer if this method looked like this:

if (!started_in_foreground || !currently_committed_url.is_empty())
  return STOP_OBSERVING;

Browser* browser = ...
...
if (!is_in_session_restored_browser)
  return STOP_OBSERVING;

DCHECK(...);
return CONTINUE_OBSERVING;

This way we can early return without doing an O(n) lookup for the common case
where this isn't the first URL loaded in the contents. If there is a session
restore of n tabs then we could hit quadratic behavior here.

[ducbui, 2017/07/25 05:11:27]

Ack. In the new patch set, I add RestoreOrigin into WebContents instead of
Browser.

+      !is_in_session_restored_browser) {
+    return STOP_OBSERVING;
+  }
+
+  // Should be restored from the last session.
+  DCHECK(navigation_handle->GetRestoreType() ==
+             content::RestoreType::LAST_SESSION_EXITED_CLEANLY ||
+         navigation_handle->GetRestoreType() ==
+             content::RestoreType::LAST_SESSION_CRASHED);
+  return CONTINUE_OBSERVING;
+}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+SessionRestorePageLoadMetricsObserver::OnCommit(
+    content::NavigationHandle* navigation_handle,
+    ukm::SourceId source_id) {
+  // Session restores use transition reload, so we only observe loads with a
+  // reload transition type.
+  return ui::PageTransitionCoreTypeIs(navigation_handle->GetPageTransition(),

[Charlie Harrison, 2017/07/21 13:59:33]

Can you comment why this isn't DCHECKable? I guess it's because the session
restore could fail so we attach an observer to subsequent non-reloads.

[ducbui, 2017/07/25 05:11:27]

Acknowledged. I put this into DCHECK in the new patch set.

+                                      ui::PAGE_TRANSITION_RELOAD)
+             ? CONTINUE_OBSERVING
+             : STOP_OBSERVING;
+}
+
+page_load_metrics::PageLoadMetricsObserver::ObservePolicy
+SessionRestorePageLoadMetricsObserver::OnHidden(
+    const page_load_metrics::mojom::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& info) {
+  return STOP_OBSERVING;
+}
+
+void SessionRestorePageLoadMetricsObserver::OnFirstPaintInPage(
+    const page_load_metrics::mojom::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  PAGE_LOAD_HISTOGRAM(internal::kHistogramSessionRestoreForegroundTabFirstPaint,
+                      timing.paint_timing->first_paint.value());
+}
+
+void SessionRestorePageLoadMetricsObserver::OnFirstContentfulPaintInPage(
+    const page_load_metrics::mojom::PageLoadTiming& timing,
+    const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  PAGE_LOAD_HISTOGRAM(
+      internal::kHistogramSessionRestoreForegroundTabFirstContentfulPaint,
+      timing.paint_timing->first_contentful_paint.value());
+}
+
+void SessionRestorePageLoadMetricsObserver::
+    OnFirstMeaningfulPaintInMainFrameDocument(
+        const page_load_metrics::mojom::PageLoadTiming& timing,
+        const page_load_metrics::PageLoadExtraInfo& extra_info) {
+  PAGE_LOAD_HISTOGRAM(
+      internal::kHistogramSessionRestoreForegroundTabFirstMeaningfulPaint,
+      timing.paint_timing->first_meaningful_paint.value());
+}
+
+SessionRestorePageLoadMetricsObserver::SessionRestorePageLoadMetricsObserver() {

[Charlie Harrison, 2017/07/21 13:59:33]

nit: let's put this at the top of the file, so method order is consistent.

[ducbui, 2017/07/25 05:11:27]

Done.

+}