Tabdisarder: disable background time logging

We are not using background time logging for TabDiscarder for now;
so it's better to disable it.

We still need to enable it in some unit tests for the test to
verify the logic.

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

chrome/browser/resource_coordinator/tab_activity_watcher.cc

@@ -425,6 +425,11 @@ class TabActivityWatcher::WebContentsData
 
   // Collect current ForegroundedOrClosedMetrics and send to ukm.
   void LogForegroundedOrClosedMetrics(bool is_foregrounded) {
+    // If background time logging is disabled, then we only log the case where
+    // the label_id_ != 0 (a feature is logged and a label has not been logged).
+    if (DisableBackgroundLogWithTabRanker() && label_id_ == 0)
+      return;
+
     TabMetricsLogger::ForegroundedOrClosedMetrics metrics;
     metrics.is_foregrounded = is_foregrounded;
     metrics.is_discarded = discarded_since_backgrounded_;

chrome/browser/resource_coordinator/tab_activity_watcher_browsertest.cc

@@ -127,6 +127,9 @@ class TabActivityWatcherUkmTest : public TabActivityWatcherTest {
   }
 
   void SetUpOnMainThread() override {
+    scoped_feature_list_.InitAndEnableFeatureWithParameters(
+        features::kTabRanker,
+        {{"disable_background_log_with_TabRanker", "false"}});
     // Browser created in BrowserMain() shouldn't result in a background tab
     // being logged.
     EXPECT_EQ(0u, ukm_entry_checker_->NumEntries(kEntryName));
@@ -200,6 +203,7 @@ class TabActivityWatcherUkmTest : public TabActivityWatcherTest {
     return *(test_ukm_recorder_->GetEntryMetric(entries.back(), metric_name));
   }
 
+  base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<UkmEntryChecker> ukm_entry_checker_;
   std::unique_ptr<ukm::TestAutoSetUkmRecorder> test_ukm_recorder_;
 
@@ -382,10 +386,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(
+  base::test::ScopedFeatureList scoped_feature_list_overrides;
+  scoped_feature_list_overrides.InitAndEnableFeatureWithParameters(
       features::kTabRanker,
-      {{"number_of_oldest_tabs_to_log_with_TabRanker", "1"}});
+      {{"number_of_oldest_tabs_to_log_with_TabRanker", "1"},
+       {"disable_background_log_with_TabRanker", "false"}});
 
   ukm::SourceId ukm_source_id_for_tab_0 = 0;
   ukm::SourceId ukm_source_id_for_tab_1 = 0;
@@ -503,8 +508,8 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest, LogOldestNTabFeatures) {
   // corresponding labels should be logged.
   // (2) number of oldest tabs to log is set to 1, so that only 1 tab should be
   // logged.
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
+  base::test::ScopedFeatureList scoped_feature_list_overrides;
+  scoped_feature_list_overrides.InitAndEnableFeatureWithParameters(
       features::kTabRanker,
       {{"number_of_oldest_tabs_to_log_with_TabRanker", "1"},
        {"disable_background_log_with_TabRanker", "true"}});
@@ -526,17 +531,6 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest, LogOldestNTabFeatures) {
       0, {TabStripModel::GestureType::kOther});
   test_clock.Advance(base::TimeDelta::FromMinutes(1));
 
-  // Foregrounded event should be logged for tab@0
-  EXPECT_EQ(1, ukm_entry_checker_->NumNewEntriesRecorded(kFOCEntryName));
-  {
-    SCOPED_TRACE("");
-    ukm_entry_checker_->ExpectNewEntry(
-        kFOCEntryName, test_urls_[0],
-        {{TabManager_Background_ForegroundedOrClosed::kIsForegroundedName, 1},
-         {TabManager_Background_ForegroundedOrClosed::kLabelIdName, 0},
-         {TabManager_Background_ForegroundedOrClosed::kIsDiscardedName, 0}});
-  }
-
   // No tab metrics should be logged till now.
   EXPECT_EQ(0, ukm_entry_checker_->NumNewEntriesRecorded(kEntryName));
 
@@ -607,22 +601,14 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest, LogOldestNTabFeatures) {
          {TabManager_Background_ForegroundedOrClosed::kLabelIdName,
           4 * kIdShift},
          {TabManager_Background_ForegroundedOrClosed::kIsDiscardedName, 0}});
-
-    // Close Browser should log a ForegroundedOrClosed event for tab@0 with
-    // label_id == 0 because  when we score it, it was not in the oldest N list.
-    ukm_entry_checker_->ExpectNewEntry(
-        kFOCEntryName, test_urls_[0],
-        {{TabManager_Background_ForegroundedOrClosed::kIsForegroundedName, 0},
-         {TabManager_Background_ForegroundedOrClosed::kLabelIdName, 0},
-         {TabManager_Background_ForegroundedOrClosed::kIsDiscardedName, 0}});
   }
 }
 
 // Tests label id is recorded correctly for discarded tabs.
 IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest,
                        DiscardedTabGetsCorrectLabelId) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
+  base::test::ScopedFeatureList scoped_feature_list_overrides;
+  scoped_feature_list_overrides.InitAndEnableFeatureWithParameters(
       features::kTabRanker,
       {{"number_of_oldest_tabs_to_log_with_TabRanker", "1"},
        {"disable_background_log_with_TabRanker", "true"}});
@@ -704,8 +690,8 @@ IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest,
 // times without logging the label first.
 IN_PROC_BROWSER_TEST_F(TabActivityWatcherUkmTest,
                        TabsAlreadyHaveLabelIdGetIncrementalLabelIds) {
-  base::test::ScopedFeatureList scoped_feature_list;
-  scoped_feature_list.InitAndEnableFeatureWithParameters(
+  base::test::ScopedFeatureList scoped_feature_list_overrides;
+  scoped_feature_list_overrides.InitAndEnableFeatureWithParameters(
       features::kTabRanker,
       {{"number_of_oldest_tabs_to_log_with_TabRanker", "1"},
        {"disable_background_log_with_TabRanker", "true"}});

chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc

@@ -75,8 +75,10 @@ class TabActivityWatcherTest : public ChromeRenderViewHostTestHarness {
  public:
   TabActivityWatcherTest() {
     // Use MRUScorer for TabRanker to bypass ML model.
-    feature_list_.InitAndEnableFeatureWithParameters(features::kTabRanker,
-                                                     {{"scorer_type", "0"}});
+    feature_list_.InitAndEnableFeatureWithParameters(
+        features::kTabRanker,
+        {{"scorer_type", "0"},
+         {"disable_background_log_with_TabRanker", "false"}});
     TabActivityWatcher::GetInstance()->ResetForTesting();
   }
 

chrome/browser/resource_coordinator/tab_manager_features.cc

@@ -269,7 +269,7 @@ int GetNumOldestTabsToLogWithTabRanker() {
 
 bool DisableBackgroundLogWithTabRanker() {
   return base::GetFieldTrialParamByFeatureAsBool(
-      features::kTabRanker, "disable_background_log_with_TabRanker", false);
+      features::kTabRanker, "disable_background_log_with_TabRanker", true);
 }
 
 float GetDiscardCountPenaltyTabRanker() {