TabDiscarder: fix source_id for discarded tabs.

Currently, when a tab is discarded, it has 0 ukm_source_id in
TabActivityWatcher; which eventually leads to no logging.

Fix: assign the old ukm_source_id to the new WebContentsData when
      replace is called.

Bug: 967558
Change-Id: Ic989baafc6850c47d8d53c14bc775a2475e1f12e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1631914Reviewed-by: François Doray <fdoray@chromium.org>
Commit-Queue: Charles . <charleszhao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664033}

chrome/browser/resource_coordinator/tab_activity_watcher.cc

@@ -103,8 +103,8 @@ class TabActivityWatcher::WebContentsData
     // Copy the replaced tab's stats.
     page_metrics_ = replaced_tab.page_metrics_;
 
-    // Record previous ukm_source_id from the |replaced_tab|.
-    previous_ukm_source_id_ = replaced_tab.ukm_source_id_;
+    // Recover the ukm_source_id from the |replaced_tab|.
+    ukm_source_id_ = replaced_tab.ukm_source_id_;
 
     // Copy the replaced label_id_.
     label_id_ = replaced_tab.label_id_;
@@ -426,11 +426,8 @@ class TabActivityWatcher::WebContentsData
     metrics.total_tab_count = mru.total;
     metrics.label_id = label_id_;
 
-    const ukm::SourceId source_id = discarded_since_backgrounded_
-                                        ? previous_ukm_source_id_
-                                        : ukm_source_id_;
     TabActivityWatcher::GetInstance()
-        ->tab_metrics_logger_->LogForegroundedOrClosedMetrics(source_id,
+        ->tab_metrics_logger_->LogForegroundedOrClosedMetrics(ukm_source_id_,
                                                               metrics);
     // label_id_ is reset whenever a label is logged.
     // A new label_id_ is generated when a query happens inside
@@ -445,9 +442,6 @@ class TabActivityWatcher::WebContentsData
   // Updated when a navigation is finished.
   ukm::SourceId ukm_source_id_ = 0;
 
-  // Recorded when a WebContents is replaced by another.
-  ukm::SourceId previous_ukm_source_id_ = 0;
-
   // When the tab was created.
   base::TimeTicks creation_time_;
 

chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc

@@ -381,6 +381,11 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest, TabDrag) {
 // Tests discarded tab is recorded correctly.
 IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest,
                        DiscardedTabGetsPreviousSourceId) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeatureWithParameters(
+      features::kTabRanker,
+      {{"number_of_oldest_tabs_to_log_with_TabRanker", "1"}});
+
   ukm::SourceId ukm_source_id_for_tab_0 = 0;
   ukm::SourceId ukm_source_id_for_tab_1 = 0;
 
@@ -407,6 +412,29 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest,
       ->GetTabLifecycleUnitExternal(first_contents)
       ->DiscardTab();
 
+  // Call LogOldestNTabFeatures should log the oldest tab which is tab@0.
+  TabActivityWatcher::GetInstance()->LogOldestNTabFeatures();
+  {
+    SCOPED_TRACE("");
+    // tab feature of tab@0 should be logged correctly.
+    UkmMetricMap expected_tab_feature_values = kBasicMetricValues;
+    expected_tab_feature_values[TabManager_TabMetrics::kMRUIndexName] = 1;
+    expected_tab_feature_values
+        [TabManager_TabMetrics::kNumReactivationBeforeName] = 0;
+    expected_tab_feature_values[TabManager_TabMetrics::kTotalTabCountName] = 2;
+    expected_tab_feature_values[TabManager_TabMetrics::kWindowTabCountName] = 2;
+    expected_tab_feature_values[TabManager_TabMetrics::kLabelIdName] = 2;
+    expected_tab_feature_values[TabManager_TabMetrics::kQueryIdName] = 1;
+    expected_tab_feature_values
+        [TabManager_TabMetrics::kNavigationEntryCountName] = 2;
+
+    ukm_entry_checker_->ExpectNewEntry(kEntryName, test_urls_[0],
+                                       expected_tab_feature_values);
+    ExpectNewEntryWithSourceId(test_urls_[0], kEntryName, 2,
+                               expected_tab_feature_values,
+                               ukm_source_id_for_tab_0);
+  }
+
   // Switching to first tab logs a forgrounded event for test_urls_[0]
   // and a backgrounded event for test_urls_[1].
   browser()->tab_strip_model()->ActivateTabAt(
@@ -417,7 +445,7 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest,
                                        kBasicMetricValues);
 
     ukm_source_id_for_tab_1 = ExpectNewEntryWithSourceId(
-        test_urls_[1], kEntryName, 2, kBasicMetricValues);
+        test_urls_[1], kEntryName, 3, kBasicMetricValues);
 
     ExpectNewEntryWithSourceId(
         test_urls_[0], kFOCEntryName, 1,

chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc

@@ -602,7 +602,9 @@ TEST_F(TabMetricsTest, ReplaceForegroundTab) {
   tab_activity_simulator_.SwitchToTabAt(tab_strip_model, 1);
   {
     SCOPED_TRACE("");
-    ExpectNewEntry(kTestUrls[1], kBasicMetricValues);
+    // Replaced tab uses the orig source_id; so the metrics is logged to
+    // kTestUrls[0].
+    ExpectNewEntry(kTestUrls[0], kBasicMetricValues);
   }
 
   tab_strip_model->CloseAllTabs();