Refactor TabActivityTestBase into TabActivitySimulator

TabActivityTestBase simulates tab navigation and tab switching in a few
unit tests. Split it into its own file, since it's not directly related
to UKM. Also make it a standalone class instead of a test base class.

It was originally a test itself, then spun out into a base class. Now
it uses nothing from ChromeRenderViewHostTestHarness except profile(),
which can be gotten from the TabStripModel. This makes the class's
purpose clearer and allows the UKM helper in tab_ukm_test_helper.h to
be used in browser_tests without including extra test classes.

The remaining class, UkmEntryChecker, should probably be moved
into a renamed file somewhere else in the future, depending on how
useful it is for other teams.

Bug: 784639
Change-Id: I62ca7bb1feaa00e06a39241741696d1f217ed26b
Reviewed-on: https://chromium-review.googlesource.com/903582
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535972}

chrome/browser/BUILD.gn

@@ -4709,6 +4709,10 @@ static_library("test_support") {
       "sessions/session_restore_test_helper.h",
       "sessions/session_service_test_helper.cc",
       "sessions/session_service_test_helper.h",
+      "ui/tabs/tab_activity_simulator.cc",
+      "ui/tabs/tab_activity_simulator.h",
+      "ui/tabs/tab_ukm_test_helper.cc",
+      "ui/tabs/tab_ukm_test_helper.h",
     ]
   }
 

chrome/browser/chromeos/power/ml/user_activity_logger_delegate_ukm_unittest.cc

@@ -10,8 +10,10 @@
 #include "chrome/browser/chromeos/power/ml/user_activity_event.pb.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/tabs/tab_activity_simulator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tabs/tab_ukm_test_helper.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/test_browser_window_aura.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/web_contents.h"
@@ -27,7 +29,8 @@ using ukm::builders::UserActivity;
 using ukm::builders::UserActivityId;
 using content::WebContentsTester;
 
-class UserActivityLoggerDelegateUkmTest : public TabActivityTestBase {
+class UserActivityLoggerDelegateUkmTest
+    : public ChromeRenderViewHostTestHarness {
  public:
   UserActivityLoggerDelegateUkmTest() {
     // These values are arbitrary but must correspond with the values
@@ -99,7 +102,7 @@ class UserActivityLoggerDelegateUkmTest : public TabActivityTestBase {
     DCHECK(tab_strip_model);
     DCHECK(!url.is_empty());
     content::WebContents* contents =
-        AddWebContentsAndNavigate(tab_strip_model, url);
+        tab_activity_simulator_.AddWebContentsAndNavigate(tab_strip_model, url);
     if (is_active) {
       tab_strip_model->ActivateTabAt(tab_strip_model->count() - 1, false);
     }
@@ -108,6 +111,8 @@ class UserActivityLoggerDelegateUkmTest : public TabActivityTestBase {
 
  protected:
   UkmEntryChecker ukm_entry_checker_;
+  TabActivitySimulator tab_activity_simulator_;
+
   const GURL url1_ = GURL("https://example1.com/");
   const GURL url2_ = GURL("https://example2.com/");
   const GURL url3_ = GURL("https://example3.com/");

chrome/browser/ui/tabs/tab_activity_simulator.cc

+// Copyright 2018 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/ui/tabs/tab_activity_simulator.h"
+
+#include "base/logging.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/navigation_simulator.h"
+#include "content/public/test/web_contents_tester.h"
+
+// Helper class to respond to WebContents lifecycle events we can't
+// trigger/simulate.
+class TabActivitySimulator::TestWebContentsObserver
+    : public content::WebContentsObserver {
+ public:
+  explicit TestWebContentsObserver(content::WebContents* web_contents);
+
+  // content::WebContentsObserver:
+  void WebContentsDestroyed() override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestWebContentsObserver);
+};
+
+TabActivitySimulator::TestWebContentsObserver::TestWebContentsObserver(
+    content::WebContents* web_contents)
+    : content::WebContentsObserver(web_contents) {}
+
+void TabActivitySimulator::TestWebContentsObserver::WebContentsDestroyed() {
+  // Simulate the WebContents hiding during destruction. This lets tests
+  // validate what is logged when a tab is destroyed.
+  web_contents()->WasHidden();
+}
+
+TabActivitySimulator::TabActivitySimulator() = default;
+TabActivitySimulator::~TabActivitySimulator() = default;
+
+void TabActivitySimulator::Navigate(
+    content::WebContents* web_contents,
+    const GURL& url,
+    ui::PageTransition page_transition = ui::PAGE_TRANSITION_LINK) {
+  std::unique_ptr<content::NavigationSimulator> navigation =
+      content::NavigationSimulator::CreateBrowserInitiated(url, web_contents);
+  navigation->SetTransition(page_transition);
+  navigation->Commit();
+}
+
+content::WebContents* TabActivitySimulator::AddWebContentsAndNavigate(
+    TabStripModel* tab_strip_model,
+    const GURL& initial_url,
+    ui::PageTransition page_transition) {
+  content::WebContents::CreateParams params(tab_strip_model->profile(),
+                                            nullptr);
+  // Create as a background tab if there are other tabs in the tab strip.
+  params.initially_hidden = tab_strip_model->count() > 0;
+  content::WebContents* test_contents =
+      content::WebContentsTester::CreateTestWebContents(params);
+
+  // Create the TestWebContentsObserver to observe |test_contents|. When the
+  // WebContents is destroyed, the observer will be reset automatically.
+  observers_.push_back(
+      std::make_unique<TestWebContentsObserver>(test_contents));
+
+  tab_strip_model->AppendWebContents(test_contents, false);
+  Navigate(test_contents, initial_url, page_transition);
+  return test_contents;
+}
+
+void TabActivitySimulator::SwitchToTabAt(TabStripModel* tab_strip_model,
+                                         int new_index) {
+  int active_index = tab_strip_model->active_index();
+  CHECK(new_index != active_index);
+
+  content::WebContents* active_contents =
+      tab_strip_model->GetWebContentsAt(active_index);
+  CHECK(active_contents);
+  content::WebContents* new_contents =
+      tab_strip_model->GetWebContentsAt(new_index);
+  CHECK(new_contents);
+
+  // Activate the tab. Normally this would hide the active tab's aura::Window,
+  // which is what actually triggers TabActivityWatcher to log the change. For
+  // a TestWebContents, we must manually call WasHidden(), and do the reverse
+  // for the newly activated tab.
+  tab_strip_model->ActivateTabAt(new_index, true /* user_gesture */);
+  active_contents->WasHidden();
+  new_contents->WasShown();
+}

chrome/browser/ui/tabs/tab_activity_simulator.h

+// Copyright 2018 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.
+
+#ifndef CHROME_BROWSER_UI_TABS_TAB_ACTIVITY_SIMULATOR_H_
+#define CHROME_BROWSER_UI_TABS_TAB_ACTIVITY_SIMULATOR_H_
+
+#include <memory>
+#include <vector>
+
+#include "base/macros.h"
+#include "ui/base/page_transition_types.h"
+
+class GURL;
+class TabStripModel;
+
+namespace content {
+class WebContents;
+}  // namespace content
+
+// Helper class for unit tests that rely on more realistically simulated tab
+// activity, such as showing and hiding tabs when switching between them.
+class TabActivitySimulator {
+ public:
+  class TestWebContentsObserver;
+
+  TabActivitySimulator();
+  ~TabActivitySimulator();
+
+  // Simulates a navigation to |url| using the given transition type.
+  void Navigate(content::WebContents* web_contents,
+                const GURL& url,
+                ui::PageTransition page_transition);
+
+  // Creates a new WebContents suitable for testing, adds it to the tab strip
+  // and commits a navigation to |initial_url|. The WebContents is owned by the
+  // TabStripModel, so its tab must be closed later, e.g. via CloseAllTabs().
+  content::WebContents* AddWebContentsAndNavigate(
+      TabStripModel* tab_strip_model,
+      const GURL& initial_url,
+      ui::PageTransition page_transition = ui::PAGE_TRANSITION_LINK);
+
+  // Sets |new_index| as the active tab in its tab strip, hiding the previously
+  // active tab.
+  void SwitchToTabAt(TabStripModel* tab_strip_model, int new_index);
+
+ private:
+  // Owns the observers we've created.
+  std::vector<std::unique_ptr<TestWebContentsObserver>> observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabActivitySimulator);
+};
+
+#endif  // CHROME_BROWSER_UI_TABS_TAB_ACTIVITY_SIMULATOR_H_

chrome/browser/ui/tabs/tab_ukm_test_helper.cc

@@ -4,13 +4,9 @@
 
 #include "chrome/browser/ui/tabs/tab_ukm_test_helper.h"
 
-#include "chrome/test/base/testing_profile.h"
 #include "components/ukm/ukm_source.h"
-#include "content/public/test/navigation_simulator.h"
-#include "content/public/test/web_contents_tester.h"
 #include "services/metrics/public/interfaces/ukm_interface.mojom.h"
-
-using content::WebContentsTester;
+#include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
@@ -33,83 +29,6 @@ void ExpectEntryMetrics(const ukm::mojom::UkmEntry& entry,
 
 }  // namespace
 
-// Helper class to respond to WebContents lifecycle events we can't
-// trigger/simulate.
-class TestWebContentsObserver : public content::WebContentsObserver {
- public:
-  explicit TestWebContentsObserver(content::WebContents* web_contents);
-
-  // content::WebContentsObserver:
-  void WebContentsDestroyed() override;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TestWebContentsObserver);
-};
-
-TestWebContentsObserver::TestWebContentsObserver(
-    content::WebContents* web_contents)
-    : content::WebContentsObserver(web_contents) {}
-
-void TestWebContentsObserver::WebContentsDestroyed() {
-  // Simulate the WebContents hiding during destruction. This lets tests
-  // validate what is logged when a tab is destroyed.
-  web_contents()->WasHidden();
-}
-
-TabActivityTestBase::TabActivityTestBase() = default;
-TabActivityTestBase::~TabActivityTestBase() = default;
-
-void TabActivityTestBase::Navigate(
-    content::WebContents* web_contents,
-    const GURL& url,
-    ui::PageTransition page_transition = ui::PAGE_TRANSITION_LINK) {
-  std::unique_ptr<content::NavigationSimulator> navigation =
-      content::NavigationSimulator::CreateBrowserInitiated(url, web_contents);
-  navigation->SetTransition(page_transition);
-  navigation->Commit();
-}
-
-content::WebContents* TabActivityTestBase::AddWebContentsAndNavigate(
-    TabStripModel* tab_strip_model,
-    const GURL& initial_url,
-    ui::PageTransition page_transition) {
-  content::WebContents::CreateParams params(profile(), nullptr);
-  // Create as a background tab if there are other tabs in the tab strip.
-  params.initially_hidden = tab_strip_model->count() > 0;
-  content::WebContents* test_contents =
-      WebContentsTester::CreateTestWebContents(params);
-
-  // Create the TestWebContentsObserver to observe |test_contents|. When the
-  // WebContents is destroyed, the observer will be reset automatically.
-  observers_.push_back(
-      std::make_unique<TestWebContentsObserver>(test_contents));
-
-  tab_strip_model->AppendWebContents(test_contents, false);
-  Navigate(test_contents, initial_url, page_transition);
-  return test_contents;
-}
-
-void TabActivityTestBase::SwitchToTabAt(TabStripModel* tab_strip_model,
-                                        int new_index) {
-  int active_index = tab_strip_model->active_index();
-  EXPECT_NE(new_index, active_index);
-
-  content::WebContents* active_contents =
-      tab_strip_model->GetWebContentsAt(active_index);
-  ASSERT_TRUE(active_contents);
-  content::WebContents* new_contents =
-      tab_strip_model->GetWebContentsAt(new_index);
-  ASSERT_TRUE(new_contents);
-
-  // Activate the tab. Normally this would hide the active tab's aura::Window,
-  // which is what actually triggers TabActivityWatcher to log the change. For
-  // a TestWebContents, we must manually call WasHidden(), and do the reverse
-  // for the newly activated tab.
-  tab_strip_model->ActivateTabAt(new_index, true /* user_gesture */);
-  active_contents->WasHidden();
-  new_contents->WasShown();
-}
-
 UkmEntryChecker::UkmEntryChecker() = default;
 UkmEntryChecker::~UkmEntryChecker() = default;
 

chrome/browser/ui/tabs/tab_ukm_test_helper.h

@@ -6,17 +6,10 @@
 #define CHROME_BROWSER_UI_TABS_TAB_UKM_TEST_HELPER_H_
 
 #include <map>
-#include <memory>
-#include <utility>
-#include <vector>
 
 #include "base/macros.h"
 #include "base/optional.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "components/ukm/test_ukm_recorder.h"
-#include "content/public/browser/web_contents.h"
-#include "ui/base/page_transition_types.h"
 
 // A UKM entry consists of named metrics with int64_t values. Use a map to
 // specify expected metrics to test against an actual entry for tests.
@@ -26,38 +19,6 @@ using UkmMetricMap = std::map<const char*, base::Optional<int64_t>>;
 using SourceUkmMetricMap =
     std::map<ukm::SourceId, std::pair<GURL, UkmMetricMap>>;
 
-class TestWebContentsObserver;
-
-// Base class for tests that rely on logging tab activity. Inherits from
-// ChromeRenderViewHostTestHarness to use TestWebContents and Profile.
-class TabActivityTestBase : public ChromeRenderViewHostTestHarness {
- public:
-  TabActivityTestBase();
-  ~TabActivityTestBase() override;
-
-  // Simulates a navigation to |url| using the given transition type.
-  void Navigate(content::WebContents* web_contents,
-                const GURL& url,
-                ui::PageTransition page_transition);
-
-  // Creates a new WebContents suitable for testing, adds it to the tab strip
-  // and commits a navigation to |initial_url|. The WebContents is owned by the
-  // TabStripModel, so its tab must be closed later, e.g. via CloseAllTabs().
-  content::WebContents* AddWebContentsAndNavigate(
-      TabStripModel* tab_strip_model,
-      const GURL& initial_url,
-      ui::PageTransition page_transition = ui::PAGE_TRANSITION_LINK);
-
-  // Sets |new_index| as the active tab in its tab strip, hiding the previously
-  // active tab.
-  void SwitchToTabAt(TabStripModel* tab_strip_model, int new_index);
-
- private:
-  // Owns the observers we've created.
-  std::vector<std::unique_ptr<TestWebContentsObserver>> observers_;
-  DISALLOW_COPY_AND_ASSIGN(TabActivityTestBase);
-};
-
 // Helper class to check entries have been logged as expected into UKM.
 class UkmEntryChecker {
  public:

chrome/browser/ui/tabs/window_activity_watcher_unittest.cc

@@ -13,7 +13,9 @@
 #include "chrome/browser/resource_coordinator/tab_metrics_event.pb.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/tabs/tab_activity_simulator.h"
 #include "chrome/browser/ui/tabs/tab_ukm_test_helper.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/test_browser_window.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/web_contents_tester.h"
@@ -100,7 +102,7 @@ class FakeBrowserWindow : public TestBrowserWindow {
 
 // Tests UKM entries generated by WindowMetricsLogger at the request of
 // WindowActivityWatcher.
-class WindowActivityWatcherTest : public TabActivityTestBase {
+class WindowActivityWatcherTest : public ChromeRenderViewHostTestHarness {
  protected:
   WindowActivityWatcherTest() = default;
   ~WindowActivityWatcherTest() override { EXPECT_FALSE(WasNewEntryRecorded()); }
@@ -108,7 +110,8 @@ class WindowActivityWatcherTest : public TabActivityTestBase {
   // Adds a tab and simulates a basic navigation.
   void AddTab(Browser* browser) {
     content::WebContentsTester::For(
-        AddWebContentsAndNavigate(browser->tab_strip_model(), GURL(kTestUrl)))
+        tab_activity_simulator_.AddWebContentsAndNavigate(
+            browser->tab_strip_model(), GURL(kTestUrl)))
         ->TestSetIsLoading(false);
   }
 
@@ -120,6 +123,7 @@ class WindowActivityWatcherTest : public TabActivityTestBase {
 
  private:
   UkmEntryChecker ukm_entry_checker_;
+  TabActivitySimulator tab_activity_simulator_;
 
   DISALLOW_COPY_AND_ASSIGN(WindowActivityWatcherTest);
 };

chrome/test/BUILD.gn

@@ -2929,8 +2929,6 @@ test("unit_tests") {
       "../browser/ui/tabs/tab_menu_model_unittest.cc",
       "../browser/ui/tabs/tab_strip_model_stats_recorder_unittest.cc",
       "../browser/ui/tabs/tab_strip_model_unittest.cc",
-      "../browser/ui/tabs/tab_ukm_test_helper.cc",
-      "../browser/ui/tabs/tab_ukm_test_helper.h",
       "../browser/ui/tabs/test_tab_strip_model_delegate.cc",
       "../browser/ui/tabs/test_tab_strip_model_delegate.h",
       "../browser/ui/tabs/window_activity_watcher_unittest.cc",