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

chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc

Index: chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
diff --git a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
index 4d5a1ca1253e4e2a14eefbe8af41a381ecaa1253..1b8b1ca243df4ebc084adaa6f7bf2429ebdc5f42 100644
--- a/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
+++ b/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc
@@ -11,16 +11,21 @@
 #include "base/test/histogram_tester.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
+#include "chrome/browser/lifetime/keep_alive_types.h"
+#include "chrome/browser/lifetime/scoped_keep_alive.h"
 #include "chrome/browser/page_load_metrics/metrics_web_contents_observer.h"
 #include "chrome/browser/page_load_metrics/observers/aborts_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/document_write_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/observers/no_state_prefetch_page_load_metrics_observer.h"
+#include "chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer.h"
 #include "chrome/browser/page_load_metrics/page_load_tracker.h"
+#include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/prerender/prerender_histograms.h"
 #include "chrome/browser/prerender/prerender_origin.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_features.h"
@@ -34,6 +39,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/referrer.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/download_test_observer.h"
 #include "net/base/net_errors.h"
@@ -1049,3 +1055,184 @@ IN_PROC_BROWSER_TEST_F(PageLoadMetricsBrowserTest,
 
   histogram_tester_.ExpectUniqueSample(internal::kHistogramTotalBytes, 0, 1);
 }
+
+class SessionRestorePageLoadMetricsBrowserTest

[Bryan McQuade, 2017/06/30 20:04:38]

thanks for all these tests! i just want to make sure none are flaky - can you
run with:

./browser_tests --gtest_filter="SessionRestorePageLoadMetricsBrowserTest.*"
--gtest_repeat=20

and just make sure none report failures when run repeatedly?

[ducbui, 2017/06/30 22:03:51]

Done.

+    : public PageLoadMetricsBrowserTest {
+ public:
+  SessionRestorePageLoadMetricsBrowserTest() {}
+
+  void SetUpOnMainThread() override {
+    PageLoadMetricsBrowserTest::SetUpOnMainThread();
+    SessionStartupPref pref(SessionStartupPref::LAST);
+    SessionStartupPref::SetStartupPref(browser()->profile(), pref);
+    ASSERT_TRUE(embedded_test_server()->Start());
+  }
+
+  Browser* QuitBrowserAndRestore(Browser* browser) {
+    // Close the browser.
+    std::unique_ptr<ScopedKeepAlive> keep_alive(new ScopedKeepAlive(
+        KeepAliveOrigin::SESSION_RESTORE, KeepAliveRestartOption::DISABLED));
+    CloseBrowserSynchronously(browser);
+
+    // Create a new window, which should trigger session restore.
+    chrome::NewEmptyWindow(browser->profile());
+    ui_test_utils::BrowserAddedObserver window_observer;
+    return window_observer.WaitForSingleNewBrowser();
+  }
+
+  std::unique_ptr<PageLoadMetricsWaiter> CreatePageLoadMetricsWaiter(
+      content::WebContents* web_contents) const {
+    return base::MakeUnique<PageLoadMetricsWaiter>(web_contents);
+  }
+
+  void WaitForFirstMeaningfulPaintOfActiveTab(Browser* browser) const {
+    auto waiter = CreatePageLoadMetricsWaiter(
+        browser->tab_strip_model()->GetActiveWebContents());
+    waiter->AddPageExpectation(TimingField::FIRST_MEANINGFUL_PAINT);
+    waiter->Wait();
+  }
+
+  void WaitForTabsToLoad(Browser* browser) {
+    for (int i = 0; i < browser->tab_strip_model()->count(); ++i) {
+      content::WebContents* contents =
+          browser->tab_strip_model()->GetWebContentsAt(i);
+      contents->GetController().LoadIfNecessary();

[Bryan McQuade, 2017/06/30 20:04:38]

i notice we call this here, but we don't end up invoking LoadIfNecessary in all
tests. Is the call to LoadIfNecessary needed? If so, do you know why it's not
needed for nay of the other tests?

[ducbui, 2017/06/30 22:03:51]

WaitForTabsToLoad() is used only in the MultipleTabsSessionRestore test: the
session restore restores multiple tabs and we test that the metrics of
background tabs are not recorded.
In the test I made a session restore with 2 tabs and we need to wait until all
the tabs to be loaded before testing that no metrics were recorded. Since the
background tabs do not paint anything, I did not wait for first-paint events to
be fired by background tabs.

This code is from SessionRestoreTest
https://cs.chromium.org/chromium/src/chrome/browser/sessions/session_restore_...

+      content::WaitForLoadStop(contents);
+    }
+  }
+
+  GURL GetTestURL() const {
+    return embedded_test_server()->GetURL(
+        "/page_load_metrics/page_with_css.html");
+  }
+
+  GURL GetTestURL2() const {
+    return embedded_test_server()->GetURL(
+        "/page_load_metrics/main_frame_with_iframe.html");
+  }
+
+  void ExpectFirstPaintMetricsTotalCount(int expected_total_count) const {
+    histogram_tester_.ExpectTotalCount(
+        internal::kHistogramSessionRestoreForegroundTabFirstPaint,
+        expected_total_count);
+    histogram_tester_.ExpectTotalCount(
+        internal::kHistogramSessionRestoreForegroundTabFirstContentfulPaint,
+        expected_total_count);
+    histogram_tester_.ExpectTotalCount(
+        internal::kHistogramSessionRestoreForegroundTabFirstMeaningfulPaint,
+        expected_total_count);
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SessionRestorePageLoadMetricsBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(SessionRestorePageLoadMetricsBrowserTest,
+                       NoSessionRestore) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL());
+  // No metrics recorded because navigation is outside of session restore.
+  ExpectFirstPaintMetricsTotalCount(0);
+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestorePageLoadMetricsBrowserTest,
+                       SingleTabSessionRestore) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL());
+  Browser* new_browser = QuitBrowserAndRestore(browser());
+  WaitForFirstMeaningfulPaintOfActiveTab(new_browser);
+  ExpectFirstPaintMetricsTotalCount(1);
+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestorePageLoadMetricsBrowserTest,
+                       MultipleTabsSessionRestore) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL());
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GetTestURL(), WindowOpenDisposition::NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  Browser* new_browser = QuitBrowserAndRestore(browser());
+
+  TabStripModel* tab_strip = new_browser->tab_strip_model();
+  ASSERT_TRUE(tab_strip);
+  ASSERT_EQ(2, tab_strip->count());
+
+  // Wait for first paints on the initial foreground tab and all tabs loaded.
+  WaitForFirstMeaningfulPaintOfActiveTab(new_browser);
+  WaitForTabsToLoad(new_browser);
+
+  // Only metrics of the initial foreground tab are recorded.
+  ExpectFirstPaintMetricsTotalCount(1);
+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestorePageLoadMetricsBrowserTest,
+                       LoadingInForegroundTabInSessionRestore) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL());
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GetTestURL2(), WindowOpenDisposition::NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  Browser* new_browser = QuitBrowserAndRestore(browser());
+
+  // Load a new page in the foreground tab before it finishes loading.
+  content::WebContents* active_web_contents =
+      new_browser->tab_strip_model()->GetActiveWebContents();
+  auto waiter = CreatePageLoadMetricsWaiter(active_web_contents);
+  waiter->AddPageExpectation(TimingField::FIRST_MEANINGFUL_PAINT);
+  active_web_contents->GetController().LoadURL(
+      GetTestURL2(), content::Referrer(), ui::PAGE_TRANSITION_TYPED,
+      std::string());
+  waiter->Wait();
+
+  // We also count the new page load in the initial foreground tab.
+  ExpectFirstPaintMetricsTotalCount(1);

[Bryan McQuade, 2017/06/30 20:04:38]

interesting, is this the desired behavior? if i stop a navigation and start a
new page load to something else, that's no longer a session restore, in which
case i'd expect zero metrics logged.

perhaps if we add the PAGE_TRANSITION_RELOAD suggestion above, this would no
longer get logged here, and we could correctly assert that zero metrics are
logged?

[ducbui, 2017/06/30 22:03:51]

Done. I agree that it would better to filter out the page loads in the initial
foreground tabs. After adding the PAGE_TRANSTITION_RELOAD, these cases are
filtered out.

+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestorePageLoadMetricsBrowserTest,
+                       LoadingAfterSessionRestore) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL());
+  Browser* new_browser = QuitBrowserAndRestore(browser());
+
+  // Wait for session restore to finish (i.e., the end of the only tab).
+  WaitForFirstMeaningfulPaintOfActiveTab(new_browser);
+  ExpectFirstPaintMetricsTotalCount(1);
+
+  // Load a new page after session restore.
+  auto waiter = CreatePageLoadMetricsWaiter(
+      new_browser->tab_strip_model()->GetActiveWebContents());
+  waiter->AddPageExpectation(TimingField::FIRST_MEANINGFUL_PAINT);
+  ui_test_utils::NavigateToURL(new_browser, GetTestURL2());
+  waiter->Wait();
+
+  // No more metrics because the navigation is after session restore.
+  ExpectFirstPaintMetricsTotalCount(1);
+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestorePageLoadMetricsBrowserTest,
+                       InitialForegroundTabChanged) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL());
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GetTestURL2(), WindowOpenDisposition::NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  Browser* new_browser = QuitBrowserAndRestore(browser());
+
+  // Change the foreground tab before the first meaningful paint.
+  TabStripModel* tab_strip = new_browser->tab_strip_model();
+  ASSERT_TRUE(tab_strip);
+  ASSERT_EQ(2, tab_strip->count());
+  ASSERT_EQ(0, tab_strip->active_index());
+  tab_strip->ActivateTabAt(1, true);
+
+  // Wait for the first meaningful paint of the current foreground tab.
+  WaitForFirstMeaningfulPaintOfActiveTab(new_browser);
+  ExpectFirstPaintMetricsTotalCount(0);
+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestorePageLoadMetricsBrowserTest,
+                       MultipleSessionRestores) {
+  ui_test_utils::NavigateToURL(browser(), GetTestURL());
+
+  Browser* current_browser = browser();
+  const int num_session_restores = 3;
+  for (int i = 1; i <= num_session_restores; ++i) {
+    current_browser = QuitBrowserAndRestore(current_browser);
+    WaitForFirstMeaningfulPaintOfActiveTab(current_browser);
+    ExpectFirstPaintMetricsTotalCount(i);
+  }
+}