Add sequence_id for TabManager.ForegroundedOrClosed event

Bug: 843891

Change-Id: I7fba61c7a46a8d0b6a3dabdc37b9209510c9cb15
Reviewed-on: https://chromium-review.googlesource.com/1064651Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Andrew Moylan <amoylan@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Commit-Queue: Charles . <charleszhao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567089}

chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc

@@ -101,14 +101,12 @@ class TabMetricsTest : public TabActivityWatcherTest {
     ukm_entry_checker_.ExpectNewEntry(kEntryName, source_url, expected_metrics);
 
     const size_t num_entries = ukm_entry_checker_.NumEntries(kEntryName);
-    const ukm::mojom::UkmEntry* last_entry =
-        ukm_entry_checker_.LastUkmEntry(kEntryName);
-    ukm::TestUkmRecorder::ExpectEntryMetric(
-        last_entry, TabManager_TabMetrics::kSequenceIdName, num_entries);
+    EXPECT_EQ(num_entries, ++num_previous_entries);
   }
 
  protected:
   const char* kEntryName = TabManager_TabMetrics::kEntryName;
+  size_t num_previous_entries = 0;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TabMetricsTest);

chrome/browser/resource_coordinator/tab_metrics_logger.cc

@@ -58,11 +58,13 @@ void LogBackgroundTabForegroundedOrClosed(
     ukm::SourceId ukm_source_id,
     base::TimeDelta inactive_duration,
     const tab_ranker::MRUFeatures& mru_features,
-    bool is_foregrounded) {
+    bool is_foregrounded,
+    int sequence_id) {
   if (!ukm_source_id)
     return;
 
   ukm::builders::TabManager_Background_ForegroundedOrClosed(ukm_source_id)
+      .SetSequenceId(sequence_id)
       .SetIsForegrounded(is_foregrounded)
       .SetMRUIndex(mru_features.index)
       .SetTimeFromBackgrounded(inactive_duration.InMilliseconds())
@@ -217,7 +219,8 @@ void TabMetricsLogger::LogBackgroundTabShown(
     base::TimeDelta inactive_duration,
     const tab_ranker::MRUFeatures& mru_features) {
   LogBackgroundTabForegroundedOrClosed(ukm_source_id, inactive_duration,
-                                       mru_features, true /* is_shown */);
+                                       mru_features, true /* is_shown */,
+                                       ++sequence_id_);
 }
 
 void TabMetricsLogger::LogBackgroundTabClosed(
@@ -225,7 +228,8 @@ void TabMetricsLogger::LogBackgroundTabClosed(
     base::TimeDelta inactive_duration,
     const tab_ranker::MRUFeatures& mru_features) {
   LogBackgroundTabForegroundedOrClosed(ukm_source_id, inactive_duration,
-                                       mru_features, false /* is_shown */);
+                                       mru_features, false /* is_shown */,
+                                       ++sequence_id_);
 }
 
 void TabMetricsLogger::LogTabLifetime(ukm::SourceId ukm_source_id,

chrome/browser/resource_coordinator/tab_metrics_logger_unittest.cc

@@ -4,8 +4,10 @@
 
 #include "chrome/browser/resource_coordinator/tab_metrics_logger.h"
 
+#include "base/test/scoped_task_environment.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
+#include "chrome/browser/resource_coordinator/tab_ranker/mru_features.h"
 #include "chrome/browser/resource_coordinator/tab_ranker/tab_features.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_activity_simulator.h"
@@ -13,6 +15,7 @@
 #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 "content/public/browser/web_contents.h"
 #include "content/public/test/web_contents_tester.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -121,3 +124,107 @@ TEST_F(TabMetricsLoggerTest, TabFeatures) {
 
   tab_strip_model->CloseAllTabs();
 }
+
+// Checks that ForegroundedOrClosed event is logged correctly.
+// TODO(charleszhao): add checks for TabMetrics event.
+class TabMetricsLoggerUKMTest : public ::testing::Test {
+ protected:
+  TabMetricsLoggerUKMTest() = default;
+
+  // Returns a new source_id associated with the test url.
+  ukm::SourceId GetSourceId() {
+    const ukm::SourceId source_id = ukm::UkmRecorder::GetNewSourceID();
+    test_ukm_recorder_.UpdateSourceURL(source_id,
+                                       GURL("https://www.chromium.org"));
+    return source_id;
+  }
+
+  // Returns the fake UKM test recorder.
+  ukm::TestUkmRecorder* GetTestUkmRecorder() { return &test_ukm_recorder_; }
+
+  // Returns the TabMetricsLogger being tested.
+  TabMetricsLogger* GetLogger() { return &logger_; }
+
+ private:
+  // Sets up the task scheduling/task-runner environment for each test.
+  base::test::ScopedTaskEnvironment scoped_task_environment_;
+  // Sets itself as the global UkmRecorder on construction.
+  ukm::TestAutoSetUkmRecorder test_ukm_recorder_;
+  // The object being tested:
+  TabMetricsLogger logger_;
+
+  DISALLOW_COPY_AND_ASSIGN(TabMetricsLoggerUKMTest);
+};
+
+// Checks the foregrounded event is logged correctly.
+TEST_F(TabMetricsLoggerUKMTest, LogBackgroundTabShown) {
+  const tab_ranker::MRUFeatures& mru_metrics{4, 7};
+  const int64_t inactive_duration_ms = 1234;
+
+  GetLogger()->LogBackgroundTabShown(
+      GetSourceId(), base::TimeDelta::FromMilliseconds(inactive_duration_ms),
+      mru_metrics);
+
+  // Checks that the size is logged correctly.
+  EXPECT_EQ(1U, GetTestUkmRecorder()->sources_count());
+  EXPECT_EQ(1U, GetTestUkmRecorder()->entries_count());
+  const std::vector<const ukm::mojom::UkmEntry*> entries =
+      GetTestUkmRecorder()->GetEntriesByName(
+          "TabManager.Background.ForegroundedOrClosed");
+  EXPECT_EQ(1U, entries.size());
+
+  // Checks that all the fields are logged correctly.
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "SequenceId", 1);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "IsForegrounded", 1);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "MRUIndex",
+                                          mru_metrics.index);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "TimeFromBackgrounded",
+                                          inactive_duration_ms);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "TotalTabCount",
+                                          mru_metrics.total);
+}
+
+// Checks the closed event is logged correctly.
+TEST_F(TabMetricsLoggerUKMTest, LogBackgroundTabClosed) {
+  const tab_ranker::MRUFeatures& mru_metrics{4, 7};
+  const int64_t inactive_duration_ms = 1234;
+  GetLogger()->LogBackgroundTabClosed(
+      GetSourceId(), base::TimeDelta::FromMilliseconds(inactive_duration_ms),
+      mru_metrics);
+
+  // Checks that the size is logged correctly.
+  EXPECT_EQ(1U, GetTestUkmRecorder()->sources_count());
+  EXPECT_EQ(1U, GetTestUkmRecorder()->entries_count());
+  const std::vector<const ukm::mojom::UkmEntry*> entries =
+      GetTestUkmRecorder()->GetEntriesByName(
+          "TabManager.Background.ForegroundedOrClosed");
+  EXPECT_EQ(1U, entries.size());
+
+  // Checks that all the fields are logged correctly.
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "SequenceId", 1);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "IsForegrounded", 0);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "MRUIndex",
+                                          mru_metrics.index);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "TimeFromBackgrounded",
+                                          inactive_duration_ms);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "TotalTabCount",
+                                          mru_metrics.total);
+}
+
+// Checks the sequence id is logged as sequentially incremental sequence across
+// different events.
+TEST_F(TabMetricsLoggerUKMTest, SequenceIdShouldBeLoggedSequentially) {
+  GetLogger()->LogBackgroundTabShown(GetSourceId(), base::TimeDelta(),
+                                     tab_ranker::MRUFeatures());
+  GetLogger()->LogBackgroundTabClosed(GetSourceId(), base::TimeDelta(),
+                                      tab_ranker::MRUFeatures());
+
+  EXPECT_EQ(2U, GetTestUkmRecorder()->sources_count());
+  EXPECT_EQ(2U, GetTestUkmRecorder()->entries_count());
+  const std::vector<const ukm::mojom::UkmEntry*> entries =
+      GetTestUkmRecorder()->GetEntriesByName(
+          "TabManager.Background.ForegroundedOrClosed");
+  EXPECT_EQ(2U, entries.size());
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[0], "SequenceId", 1);
+  GetTestUkmRecorder()->ExpectEntryMetric(entries[1], "SequenceId", 2);
+}

tools/metrics/ukm/ukm.xml

@@ -3187,6 +3187,13 @@ be describing additional metrics about the same event.
       closed.
     </summary>
   </metric>
+  <metric name="SequenceId">
+    <summary>
+      The sequence of this event and TabManager.TabMetrics event in the current
+      session. Incremented by 1 each time one of the two events is logged to
+      provide an ordering of events.
+    </summary>
+  </metric>
   <metric name="TimeFromBackgrounded">
     <summary>
       Duration in MS from when the tab is backgrounded to when it is brought to
@@ -3957,8 +3964,9 @@ be describing additional metrics about the same event.
   </metric>
   <metric name="SequenceId">
     <summary>
-      The sequence of this event in the current session. Incremented by 1 each
-      time this event is logged to provide an ordering of events.
+      The sequence of this event and TabManager.ForegroundedOrClosed event in
+      the current session. Incremented by 1 each time one of the two events is
+      logged to provide an ordering of events.
     </summary>
   </metric>
   <metric name="SiteEngagementScore">