[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_unittest.cc

Index: chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer_unittest.cc
diff --git a/chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer_unittest.cc b/chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b3c3d2288950ef618a78839b21c8adb47e14f7d1
--- /dev/null
+++ b/chrome/browser/page_load_metrics/observers/session_restore_page_load_metrics_observer_unittest.cc
@@ -0,0 +1,250 @@
+// 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 "chrome/browser/browser_process.h"
+#include "chrome/browser/page_load_metrics/observers/page_load_metrics_observer_test_harness.h"
+#include "chrome/browser/resource_coordinator/tab_manager.h"
+#include "chrome/browser/sessions/tab_loader.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/test_tab_strip_model_delegate.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/public/browser/browser_url_handler.h"
+#include "content/public/common/referrer.h"
+#include "content/public/test/web_contents_tester.h"
+
+using RestoredTab = SessionRestoreDelegate::RestoredTab;
+
+namespace {
+
+const char kDefaultTestUrl[] = "https://google.com";
+const char kDefaultTestUrl2[] = "https://example.com";
+
+}  // namespace
+
+class SessionRestorePageLoadMetricsObserverTest
+    : public page_load_metrics::PageLoadMetricsObserverTestHarness {
+ protected:
+  void RegisterObservers(page_load_metrics::PageLoadTracker* tracker) override {
+    std::unique_ptr<SessionRestorePageLoadMetricsObserver> observer =

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

#include <memory>

[ducbui, 2017/08/03 20:41:27]

Done.

+        SessionRestorePageLoadMetricsObserver::CreateIfNeeded();
+    if (observer)
+      tracker->AddObserver(std::move(observer));

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

#include <utility> for std::move

[ducbui, 2017/08/03 20:41:27]

Done.

+  }
+
+  void SetUp() override {
+    page_load_metrics::PageLoadMetricsObserverTestHarness::SetUp();
+
+    // Stop the initial loading.
+    StopLoading(web_contents());
+
+    // Create the tab manager to register its SessionRestoreObserver before
+    // session restore starts.
+    g_browser_process->GetTabManager();
+
+    restored_tabs_.emplace_back(web_contents(), false, false, false);

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

Would you mind documenting these "false" args e.g.
restored_tabs_.emplace_back(web_contents(), false /* is_active */, false /*
is_app */, false /* is_pinned */);

[ducbui, 2017/08/03 20:41:27]

Done.

+    PopulateAllFirstPaintTimings(&timing_);
+  }
+
+  void TearDown() override {
+    page_load_metrics::PageLoadMetricsObserverTestHarness::TearDown();
+    restored_tabs_.clear();

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

Is this really needed?

[ducbui, 2017/08/03 20:41:27]

No, I will remove it. Each RestoredTab element, which does not hold
dynamically-allocated memory, in the restore_tabs_ vector will be deallocated in
the Test class destructor.

+  }
+
+  // Populate first paint and first [contentful,meaningful] paint timings.
+  void PopulateAllFirstPaintTimings(
+      page_load_metrics::mojom::PageLoadTiming* timing_) {
+    page_load_metrics::InitPageLoadTimingForTest(timing_);
+    timing_->navigation_start = base::Time::FromDoubleT(1);
+    timing_->paint_timing->first_meaningful_paint =
+        base::TimeDelta::FromMilliseconds(10);
+    PopulateRequiredTimingFields(timing_);
+  }
+
+  void ExpectFirstPaintMetricsTotalCount(int expected_total_count) {
+    histogram_tester().ExpectTotalCount(
+        internal::kHistogramSessionRestoreForegroundTabFirstPaint,
+        expected_total_count);
+    histogram_tester().ExpectTotalCount(
+        internal::kHistogramSessionRestoreForegroundTabFirstContentfulPaint,
+        expected_total_count);
+    histogram_tester().ExpectTotalCount(
+        internal::kHistogramSessionRestoreForegroundTabFirstMeaningfulPaint,
+        expected_total_count);
+  }
+
+  void RestoreTabs() {
+    TabLoader::RestoreTabs(restored_tabs_, base::TimeTicks());

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

#include "base/time/time.h"

[ducbui, 2017/08/03 20:41:27]

Done.

+  }
+
+  void SimulateTimingUpdate() {
+    PageLoadMetricsObserverTestHarness::SimulateTimingUpdate(timing_);
+  }
+
+  void StopLoading(content::WebContents* contents) {
+    content::WebContentsTester::For(contents)->TestSetIsLoading(false);

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

Does WebContents::Stop work in this case? It seems better than just resetting
the loading state.

[ducbui, 2017/08/03 20:41:26]

WebContents::Stop() seems not sufficient. It crashed
MultipleSessionRestoresWithoutInitialForegroundTabChange test because TabLoader
has not received stop or destroy signal from the tab. This caused duplicated
registration in TabLoader's NotificationRegistrar.

I will add WebContents::Stop() in addition to TestSetIsLoading(false).

+  }
+
+  // Navigate with a RELOAD page transition to emulate session restore.
+  void NavigateAndCommitWithReloadTransition(content::WebContents* contents,

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

These days it is preferable to use content::NavigationSimulator to do these
sorts of tests. If possible it would be nice to use that.

[ducbui, 2017/08/07 03:55:43]

Done.

+                                             const GURL& url) {
+    contents->GetController().LoadURL(

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

#include <string>

#include "content/public/browser/web_contents.h"
#include "content/public/browser/navigation_controller.h"
#include "ui/base/page_transition_types.h"

[ducbui, 2017/08/03 20:41:27]

Done.

+        url, content::Referrer(), ui::PAGE_TRANSITION_RELOAD, std::string());
+    GURL loaded_url(url);

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

nit: #include "url/gurl.h"

[ducbui, 2017/08/03 20:41:27]

Done.

+    bool reverse_on_redirect = false;
+    content::BrowserURLHandler::GetInstance()->RewriteURLIfNecessary(
+        &loaded_url, contents->GetBrowserContext(), &reverse_on_redirect);
+    // LoadURL created a navigation entry, now simulate the RenderView sending
+    // a notification that it actually navigated.
+    content::WebContentsTester::For(contents)->CommitPendingNavigation();
+  }
+
+ private:
+  page_load_metrics::mojom::PageLoadTiming timing_;
+  std::vector<RestoredTab> restored_tabs_;

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

#include <vector>

[ducbui, 2017/08/03 20:41:27]

Done.

+};

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

DISALLOW_COPY_AND_ASSIGN

[ducbui, 2017/08/03 20:41:27]

Done.

+
+TEST_F(SessionRestorePageLoadMetricsObserverTest, NoMetrics) {
+  ExpectFirstPaintMetricsTotalCount(0);
+}
+
+TEST_F(SessionRestorePageLoadMetricsObserverTest,
+       FirstPaintsOutOfSessionRestore) {
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl));
+  SimulateTimingUpdate();
+  ExpectFirstPaintMetricsTotalCount(0);
+}
+
+TEST_F(SessionRestorePageLoadMetricsObserverTest, InitialForegroundTabChanged) {

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

Hmm, overall I'm not sure that the PLM observer test harness is robust enough
for multiple tab cases. Internally, it stores and accesses only a single metrics
web contents observer, and doesn't create new ones for new tabs.

Can you create a test that exercises this? e.g. Close all tabs then start a
session restore expecting metrics to arrive for a new tab.

[ducbui, 2017/08/03 20:41:27]

Acknowledged. I will add a PageLoadMetricsObserverTester to record metrics of
multiple web contents.

[ducbui, 2017/08/07 03:55:43]

Done. I use an additional PageLoadMetricsObserverTester to record metrics of a
second tab. I also add a test case MultipleForegroundTabsLoadedInSessionRestore
to check UMA recorded multiple tabs.

+  TestTabStripModelDelegate tab_strip_delegate;
+  TabStripModel tab_strip(&tab_strip_delegate, profile());
+  tab_strip.AddObserver(g_browser_process->GetTabManager());
+
+  std::unique_ptr<content::WebContents> test_contents(CreateTestWebContents());
+  std::vector<RestoredTab> restored_tabs{
+      RestoredTab(test_contents.get(), false, false, false),
+      RestoredTab(web_contents(), false, false, false)};
+
+  // Tab strip with 2 tabs: tab 0 is foreground, tab 1 is background.
+  tab_strip.AppendWebContents(restored_tabs[0].contents(), true);
+  tab_strip.AppendWebContents(restored_tabs[1].contents(), false);
+
+  // Start loading tab 0 but do not finish loading it.

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

nit: Start loading tab 0 but switch tabs before any paint events occur.

[ducbui, 2017/08/03 20:41:27]

Done.

+  TabLoader::RestoreTabs(restored_tabs, base::TimeTicks());
+  NavigateAndCommitWithReloadTransition(restored_tabs[0].contents(),
+                                        GURL(kDefaultTestUrl));
+
+  // Switch to tab 1 and load it completely.
+  tab_strip.ActivateTabAt(1, true);
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl));
+  SimulateTimingUpdate();
+
+  // No paint timings recorded because the initial foreground tab was changed.
+  ExpectFirstPaintMetricsTotalCount(0);
+}
+
+TEST_F(SessionRestorePageLoadMetricsObserverTest,
+       ForegroundTabLoadedInSessionRestore) {
+  // Restore one tab which finishes loading in foreground.
+  RestoreTabs();
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl));
+  SimulateTimingUpdate();
+  ExpectFirstPaintMetricsTotalCount(1);
+}
+
+TEST_F(SessionRestorePageLoadMetricsObserverTest,
+       BackgroundTabLoadedInSessionRestore) {
+  // Set the tab to background before the PageLoadMetricsObserver was created.
+  web_contents()->WasHidden();
+
+  // Load the restored tab in background.
+  RestoreTabs();
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl));
+  SimulateTimingUpdate();
+
+  // No paint timings recorded because the loaded tab was in background.
+  ExpectFirstPaintMetricsTotalCount(0);
+}
+
+TEST_F(SessionRestorePageLoadMetricsObserverTest,
+       TabIsHiddenBeforeFirstPaints) {
+  // Hide the tab after it starts loading but before it finishes loading.
+  RestoreTabs();
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl));
+  web_contents()->WasHidden();
+  SimulateTimingUpdate();
+
+  // No paint timings recorded because tab was hidden before the first paints.
+  ExpectFirstPaintMetricsTotalCount(0);
+}
+
+TEST_F(SessionRestorePageLoadMetricsObserverTest,
+       MultipleSessionRestoresWithoutInitialForegroundTabChange) {
+  // First session restore.
+  RestoreTabs();
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl));
+  SimulateTimingUpdate();
+  StopLoading(web_contents());
+  ExpectFirstPaintMetricsTotalCount(1);
+
+  // Clear committed URL so the next session restore starts from an emtpy URL.

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

s/emtpy/empty

[ducbui, 2017/08/03 20:41:27]

Done.

+  NavigateAndCommitWithReloadTransition(web_contents(), GURL());
+  StopLoading(web_contents());
+
+  // Second session restore.
+  RestoreTabs();
+
+  // Load the only foreground tab with first paints.
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl2));
+  SimulateTimingUpdate();
+  ExpectFirstPaintMetricsTotalCount(2);
+}
+
+TEST_F(SessionRestorePageLoadMetricsObserverTest,
+       MultipleSessionRestoresWithInitialForegroundTabChange) {
+  TestTabStripModelDelegate tab_strip_delegate;
+  TabStripModel tab_strip(&tab_strip_delegate, profile());
+  tab_strip.AddObserver(g_browser_process->GetTabManager());
+
+  std::unique_ptr<content::WebContents> test_contents(CreateTestWebContents());
+  std::vector<RestoredTab> restored_tabs{
+      RestoredTab(test_contents.get(), false, false, false),
+      RestoredTab(web_contents(), false, false, false)};
+
+  // Tab strip with 2 tabs: tab 0 is foreground, tab 1 is background.
+  tab_strip.AppendWebContents(restored_tabs[0].contents(), true);
+  tab_strip.AppendWebContents(restored_tabs[1].contents(), false);
+
+  // First session restore.
+  TabLoader::RestoreTabs(restored_tabs, base::TimeTicks());
+
+  // Start and stop loading foreground tab 0 without first paints.
+  NavigateAndCommitWithReloadTransition(restored_tabs[0].contents(),
+                                        GURL(kDefaultTestUrl));
+  StopLoading(restored_tabs[0].contents());
+
+  // Switch to tab 1 to change the initial foreground tab.
+  tab_strip.ActivateTabAt(1, true);
+
+  // Load the foreground tab with first paints.
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl));
+  SimulateTimingUpdate();
+  StopLoading(web_contents());
+
+  // No paint timings recorded because the initial foreground tab was changed.
+  ExpectFirstPaintMetricsTotalCount(0);
+
+  // Clear committed URL so the next session restore starts from an emtpy URL.
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL());
+  StopLoading(web_contents());
+
+  // Second session restore.
+  TabLoader::RestoreTabs(restored_tabs, base::TimeTicks());
+
+  // Load the foreground tab with first paints.
+  NavigateAndCommitWithReloadTransition(web_contents(), GURL(kDefaultTestUrl2));
+  SimulateTimingUpdate();
+  ExpectFirstPaintMetricsTotalCount(1);
+}