Test UKMs in TabActivityWatcher

Adds unit tests that simulate tab activity and test the UKMs logged by
TabMetricsLogger.

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

chrome/browser/ui/tabs/tab_activity_watcher.cc

@@ -97,14 +97,6 @@ void TabActivityWatcher::DisableLogTimeoutForTest() {
   per_source_log_timeout_ = base::TimeDelta();
 }
 
-void TabActivityWatcher::SetTabMetricsLoggerForTest(
-    std::unique_ptr<TabMetricsLogger> tab_metrics_logger) {
-  if (tab_metrics_logger)
-    tab_metrics_logger_ = std::move(tab_metrics_logger);
-  else
-    tab_metrics_logger_ = std::make_unique<TabMetricsLoggerImpl>();
-}
-
 void TabActivityWatcher::TabPinnedStateChanged(TabStripModel* tab_strip_model,
                                                content::WebContents* contents,
                                                int index) {

chrome/browser/ui/tabs/tab_activity_watcher.h

@@ -32,9 +32,6 @@ class TabActivityWatcher : public TabStripModelObserver,
   // Forces logging even when a timeout would have prevented it.
   void DisableLogTimeoutForTest();
 
-  void SetTabMetricsLoggerForTest(
-      std::unique_ptr<TabMetricsLogger> tab_metrics_logger);
-
   // Returns the single instance, creating it if necessary.
   static TabActivityWatcher* GetInstance();
 

chrome/browser/ui/tabs/tab_activity_watcher_unittest.cc

@@ -7,39 +7,49 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "chrome/browser/engagement/site_engagement_service.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/tabs/tab_metrics_logger.h"
+#include "chrome/browser/ui/tabs/tab_activity_watcher.h"
+#include "chrome/browser/ui/tabs/tab_metrics_event.pb.h"
+#include "chrome/browser/ui/tabs/tab_metrics_logger_impl.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.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 "components/ukm/test_ukm_recorder.h"
+#include "components/ukm/ukm_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/web_contents_tester.h"
-#include "testing/gmock/include/gmock/gmock.h"
+#include "services/metrics/public/cpp/ukm_builders.h"
+#include "services/metrics/public/interfaces/ukm_interface.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::WebContentsTester;
-using testing::_;
-using testing::StrictMock;
-
-namespace {
+using metrics::TabMetricsEvent;
+using ukm::builders::TabManager_TabMetrics;
 
-const char* kTestUrls[] = {"http://example.com", "https://google.fake"};
+// 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.
+using UkmMetricMap = std::map<const char*, int64_t>;
 
-// Mocks the LogBackgroundTab method for verification.
-class MockTabMetricsLogger : public TabMetricsLogger {
- public:
-  MockTabMetricsLogger() = default;
+namespace {
 
-  // TabMetricsLogger:
-  MOCK_METHOD2(LogBackgroundTab,
-               void(ukm::SourceId ukm_source_id,
-                    content::WebContents* web_contents));
+const char* kEntryName = TabManager_TabMetrics::kEntryName;
 
- private:
-  DISALLOW_COPY_AND_ASSIGN(MockTabMetricsLogger);
+const GURL kTestUrls[] = {
+    GURL("https://example.com/"), GURL("https://google.fake"),
+    GURL("https://example3.com"),
 };
 
+// The default metric values for a tab.
+const UkmMetricMap kBasicMetricValues({
+    {TabManager_TabMetrics::kContentTypeName,
+     TabMetricsEvent::CONTENT_TYPE_TEXT_HTML},
+    {TabManager_TabMetrics::kHasFormEntryName, 0},
+    {TabManager_TabMetrics::kIsPinnedName, 0},
+    {TabManager_TabMetrics::kSiteEngagementScoreName, 0},
+});
+
 // Helper class to respond to WebContents lifecycle events we can't
 // trigger/simulate.
 class TestWebContentsObserver : public content::WebContentsObserver {
@@ -59,24 +69,16 @@ class TestWebContentsObserver : public content::WebContentsObserver {
 
 }  // namespace
 
+// Tests UKM entries generated by TabMetricsLogger at the request of
+// TabActivityWatcher.
 // Inherits from ChromeRenderViewHostTestHarness to use TestWebContents and
 // Profile.
 class TabActivityWatcherTest : public ChromeRenderViewHostTestHarness {
  protected:
   TabActivityWatcherTest() {
-    // Create and pass a unique_ptr, but retain a pointer to the object.
-    auto mock_logger_transient =
-        std::make_unique<StrictMock<MockTabMetricsLogger>>();
-    mock_logger_ = mock_logger_transient.get();
-    TabActivityWatcher::GetInstance()->SetTabMetricsLoggerForTest(
-        std::move(mock_logger_transient));
     TabActivityWatcher::GetInstance()->DisableLogTimeoutForTest();
   }
 
-  ~TabActivityWatcherTest() override {
-    TabActivityWatcher::GetInstance()->SetTabMetricsLoggerForTest(nullptr);
-  }
-
   // Creates a new WebContents suitable for testing, adds it to the tab strip
   // and navigates it to |initial_url|. The result is owned by the
   // TabStripModel, so its tab must be closed later (e.g. via CloseAllTabs()).
@@ -92,14 +94,54 @@ class TabActivityWatcherTest : public ChromeRenderViewHostTestHarness {
         std::make_unique<TestWebContentsObserver>(test_contents));
 
     tab_strip_model->AppendWebContents(test_contents, false);
+    WebContentsTester::For(test_contents)->NavigateAndCommit(initial_url);
     return test_contents;
   }
 
-  StrictMock<MockTabMetricsLogger>* mock_logger() { return mock_logger_; }
+  // Expects that the most recently added TabManager.TabMetrics entry matches
+  // the values and the given URL.
+  void ExpectEntry(const GURL& source_url,
+                   const UkmMetricMap& expected_metrics) {
+    std::vector<const ukm::mojom::UkmEntry*> entries =
+        ukm_recorder()->GetEntriesByName(kEntryName);
+    ASSERT_GT(entries.size(), 0u);
+
+    // Verify the entry is associated with the correct URL.
+    const ukm::mojom::UkmEntry* entry = entries.back();
+    ukm_recorder()->ExpectEntrySourceHasUrl(entry, source_url);
+
+    // Each expected metric should match a named value in UkmRecorder.
+    for (const std::pair<const char*, uint64_t>& pair : expected_metrics)
+      ukm::TestUkmRecorder::ExpectEntryMetric(entry, pair.first, pair.second);
+  }
+
+  // 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) {
+    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, /*user_gesture=*/true);
+    active_contents->WasHidden();
+    new_contents->WasShown();
+  }
+
+  ukm::TestUkmRecorder* ukm_recorder() { return &ukm_recorder_; }
 
  private:
-  // Owned by TabActivityWatcher.
-  StrictMock<MockTabMetricsLogger>* mock_logger_ = nullptr;
+  ukm::TestAutoSetUkmRecorder ukm_recorder_;
+  TabMetricsLoggerImpl tab_metrics_logger_;
 
   // Owns the observers we've created.
   std::vector<std::unique_ptr<TestWebContentsObserver>> observers_;
@@ -111,38 +153,128 @@ TEST_F(TabActivityWatcherTest, Basic) {
   Browser::CreateParams params(profile(), true);
   auto browser = CreateBrowserWithTestWindowForParams(&params);
 
+  TabStripModel* tab_strip_model = browser->tab_strip_model();
+  AddWebContentsAndNavigate(tab_strip_model, GURL(kTestUrls[0]));
+  AddWebContentsAndNavigate(tab_strip_model, GURL(kTestUrls[1]));
+
+  // Start with the leftmost tab activated.
+  tab_strip_model->ActivateTabAt(0, false);
+  EXPECT_EQ(0u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+
+  // Activating a tab logs the deactivated tab.
+  SwitchToTabAt(tab_strip_model, 1);
+  {
+    SCOPED_TRACE("");
+    EXPECT_EQ(1u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+    ExpectEntry(kTestUrls[0], kBasicMetricValues);
+  }
+
+  SwitchToTabAt(tab_strip_model, 0);
+  {
+    SCOPED_TRACE("");
+    EXPECT_EQ(2u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+    ExpectEntry(kTestUrls[1], kBasicMetricValues);
+  }
+
+  // Closing the tabs destroys the WebContentses but should not trigger logging.
+  // The TestWebContentsObserver simulates hiding these tabs as they are closed;
+  // verify that no logging occurs.
+  tab_strip_model->CloseAllTabs();
+  EXPECT_EQ(2u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+}
+
+// Tests when tab events like pinning and navigating trigger logging.
+TEST_F(TabActivityWatcherTest, TabEvents) {
+  Browser::CreateParams params(profile(), true);
+  auto browser = CreateBrowserWithTestWindowForParams(&params);
+
   TabStripModel* tab_strip_model = browser->tab_strip_model();
   content::WebContents* test_contents_1 =
       AddWebContentsAndNavigate(tab_strip_model, GURL(kTestUrls[0]));
   content::WebContents* test_contents_2 =
       AddWebContentsAndNavigate(tab_strip_model, GURL(kTestUrls[1]));
+  tab_strip_model->ActivateTabAt(0, false);
 
-  // Start with the leftmost tab activated.
+  // Navigating the active tab doesn't trigger logging.
+  WebContentsTester::For(test_contents_1)->NavigateAndCommit(kTestUrls[2]);
+  EXPECT_EQ(0u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+
+  // Pinning the active tab doesn't trigger logging.
+  tab_strip_model->SetTabPinned(0, true);
+  EXPECT_EQ(0u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+
+  // Pinning and unpinning the background tab triggers logging.
+  tab_strip_model->SetTabPinned(1, true);
+  EXPECT_EQ(1u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+  tab_strip_model->SetTabPinned(1, false);
+  EXPECT_EQ(2u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+
+  // Navigating the background tab doesn't trigger logging.
+  // TODO(michaelpg): Logging should occur once the page loads.
+  WebContentsTester::For(test_contents_2)->NavigateAndCommit(kTestUrls[0]);
+  EXPECT_EQ(2u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+
+  tab_strip_model->CloseAllTabs();
+  EXPECT_EQ(2u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+}
+
+// Tests setting and changing tab metrics.
+TEST_F(TabActivityWatcherTest, TabMetrics) {
+  Browser::CreateParams params(profile(), true);
+  auto browser = CreateBrowserWithTestWindowForParams(&params);
+
+  TabStripModel* tab_strip_model = browser->tab_strip_model();
+  content::WebContents* test_contents_1 =
+      AddWebContentsAndNavigate(tab_strip_model, GURL(kTestUrls[0]));
+  content::WebContents* test_contents_2 =
+      AddWebContentsAndNavigate(tab_strip_model, GURL(kTestUrls[1]));
   tab_strip_model->ActivateTabAt(0, false);
+  EXPECT_EQ(0u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+
+  // Expected metrics for tab event.
+  UkmMetricMap expected_metrics(kBasicMetricValues);
 
-  WebContentsTester::For(test_contents_1)
-      ->NavigateAndCommit(GURL(kTestUrls[0]));
-  WebContentsTester::For(test_contents_2)
-      ->NavigateAndCommit(GURL(kTestUrls[1]));
+  // Site engagement score should round down to the nearest 10.
+  SiteEngagementService::Get(profile())->ResetBaseScoreForURL(kTestUrls[1], 45);
+  expected_metrics[TabManager_TabMetrics::kSiteEngagementScoreName] = 40;
 
-  // Pinning or unpinning test_contents_2 triggers the log.
-  // Note that pinning the tab moves it to index 0.
-  EXPECT_CALL(*mock_logger(), LogBackgroundTab(_, test_contents_2));
+  // Pin the background tab to log an event. (This moves it to index 0.)
   tab_strip_model->SetTabPinned(1, true);
+  expected_metrics[TabManager_TabMetrics::kIsPinnedName] = 1;
+  {
+    SCOPED_TRACE("");
+    EXPECT_EQ(1u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+    ExpectEntry(kTestUrls[1], expected_metrics);
+  }
 
-  EXPECT_CALL(*mock_logger(), LogBackgroundTab(_, test_contents_2));
+  // Navigate the background tab to a new domain.
+  // Site engagement score for the new domain is 0.
+  WebContentsTester::For(test_contents_2)->NavigateAndCommit(kTestUrls[2]);
+  EXPECT_EQ(1u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+  expected_metrics[TabManager_TabMetrics::kSiteEngagementScoreName] = 0;
+
+  // Unpin the background tab to log an event.
   tab_strip_model->SetTabPinned(0, false);
+  expected_metrics[TabManager_TabMetrics::kIsPinnedName] = 0;
+  {
+    SCOPED_TRACE("");
+    EXPECT_EQ(2u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+    ExpectEntry(kTestUrls[2], expected_metrics);
+  }
 
-  // Activate test_contents_2. Normally this would hide the current tab's
-  // aura::Window, which is what actually triggers TabActivityWatcher to log the
-  // change.
-  EXPECT_CALL(*mock_logger(), LogBackgroundTab(_, test_contents_1));
-  tab_strip_model->ActivateTabAt(0, true);
-  test_contents_1->WasHidden();
+  // Navigate the active tab and switch away from it. The entry should reflect
+  // the new URL.
+  WebContentsTester::For(test_contents_1)->NavigateAndCommit(kTestUrls[2]);
+  SwitchToTabAt(tab_strip_model, 0);
+  {
+    SCOPED_TRACE("");
+    EXPECT_EQ(3u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+    // This tab still has the default metrics.
+    ExpectEntry(kTestUrls[2], kBasicMetricValues);
+  }
 
-  // Closing the tabs destroys the WebContentses and should not trigger logging.
-  EXPECT_CALL(*mock_logger(), LogBackgroundTab(_, _)).Times(0);
   tab_strip_model->CloseAllTabs();
+  EXPECT_EQ(3u, ukm_recorder()->GetEntriesByName(kEntryName).size());
 }
 
 // Tests that logging happens when the browser window is hidden (even if the
@@ -152,14 +284,22 @@ TEST_F(TabActivityWatcherTest, HideWindow) {
   auto browser = CreateBrowserWithTestWindowForParams(&params);
 
   TabStripModel* tab_strip_model = browser->tab_strip_model();
-  content::WebContents* test_contents_1 =
+  content::WebContents* test_contents =
       AddWebContentsAndNavigate(tab_strip_model, GURL(kTestUrls[0]));
   tab_strip_model->ActivateTabAt(0, false);
 
   // Hiding the window triggers the log.
-  EXPECT_CALL(*mock_logger(), LogBackgroundTab(_, test_contents_1));
-  test_contents_1->WasHidden();
+  test_contents->WasHidden();
+  {
+    SCOPED_TRACE("");
+    EXPECT_EQ(1u, ukm_recorder()->GetEntriesByName(kEntryName).size());
+    ExpectEntry(kTestUrls[0], kBasicMetricValues);
+  }
+
+  // Showing the window does not.
+  test_contents->WasShown();
+  EXPECT_EQ(1u, ukm_recorder()->GetEntriesByName(kEntryName).size());
 
-  EXPECT_CALL(*mock_logger(), LogBackgroundTab(_, _)).Times(0);
   tab_strip_model->CloseAllTabs();
+  EXPECT_EQ(1u, ukm_recorder()->GetEntriesByName(kEntryName).size());
 }

chrome/browser/ui/tabs/tab_metrics_logger_impl.h

@@ -11,7 +11,6 @@
 // Logs a TabManager.TabMetrics UKM for a tab when requested. Includes
 // information relevant to the tab and its WebContents.
 // Must be used on the UI thread.
-// TODO(michaelpg): Unit test for UKMs.
 class TabMetricsLoggerImpl : public TabMetricsLogger {
  public:
   TabMetricsLoggerImpl();