Remove TabMetricsLogging per-source timeout

TabActivityWatcher logs URL-keyed metrics about tabs upon tab activity.
Currently, we don't log a UKM for a source if we have already logged
that source within the past 10 seconds. This was intended to ensure
the new UKM logging did not cause unexpected flooding of the UKM
service.

Analysis of Finch experiments demonstrates that removing this timeout
does not substantially affect the rate of events logged or cause UKM's
built-in event limit to be reached more often. This CL therefore
removes this timeout as well as the Finch parameter that controlled it.

The feature flag itself remains as a way to disable TabMetrics logging.

Bug: 804668
Change-Id: Iad821bd9e91ac34eedaca8b7f68436fafffed9ae
Reviewed-on: https://chromium-review.googlesource.com/880143
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532717}

chrome/browser/resource_coordinator/tab_activity_watcher.cc

@@ -4,14 +4,11 @@
 
 #include "chrome/browser/resource_coordinator/tab_activity_watcher.h"
 
-#include "base/metrics/field_trial_params.h"
-#include "base/time/time.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_metrics_logger_impl.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tabs/window_activity_watcher.h"
-#include "chrome/common/chrome_features.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_view_host.h"
@@ -27,18 +24,6 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(
 
 namespace resource_coordinator {
 
-namespace {
-
-// Default time before a tab with the same SourceId can be logged again.
-constexpr base::TimeDelta kDefaultPerSourceLogTimeout =
-    base::TimeDelta::FromSeconds(10);
-
-// Fieldtrial param to override the default per-source log timeout above.
-constexpr char kPerSourceLogTimeoutMsecParamName[] =
-    "per_source_log_timeout_msec";
-
-}  // namespace
-
 // Per-WebContents helper class that observes its WebContents, notifying
 // TabActivityWatcher when interesting events occur. Also provides
 // per-WebContents data that TabActivityWatcher uses to log the tab.
@@ -49,21 +34,12 @@ class TabActivityWatcher::WebContentsData
  public:
   ~WebContentsData() override = default;
 
-  // Call after logging to update |last_log_time_for_source_|.
-  void DidLog(base::TimeTicks log_time) {
-    last_log_time_for_source_ = log_time;
-  }
-
   ukm::SourceId ukm_source_id() const { return ukm_source_id_; }
 
   const TabMetricsLogger::TabMetrics& tab_metrics() const {
     return tab_metrics_;
   }
 
-  base::TimeTicks last_log_time_for_source() const {
-    return last_log_time_for_source_;
-  }
-
  private:
   friend class content::WebContentsUserData<WebContentsData>;
 
@@ -96,9 +72,6 @@ class TabActivityWatcher::WebContentsData
         << "Expected a unique Source ID for the navigation";
     ukm_source_id_ = new_source_id;
 
-    // Clear the per-SourceId last log time.
-    last_log_time_for_source_ = base::TimeTicks();
-
     // Reset the per-page data.
     tab_metrics_.page_metrics = {};
 
@@ -128,9 +101,6 @@ class TabActivityWatcher::WebContentsData
   // Updated when a navigation is finished.
   ukm::SourceId ukm_source_id_ = 0;
 
-  // Used to throttle event logging per SourceId.
-  base::TimeTicks last_log_time_for_source_;
-
   // Stores current stats for the tab.
   TabMetricsLogger::TabMetrics tab_metrics_;
 
@@ -140,10 +110,6 @@ class TabActivityWatcher::WebContentsData
 TabActivityWatcher::TabActivityWatcher()
     : tab_metrics_logger_(std::make_unique<TabMetricsLoggerImpl>()),
       browser_tab_strip_tracker_(this, this, nullptr) {
-  per_source_log_timeout_ =
-      base::TimeDelta::FromMilliseconds(base::GetFieldTrialParamByFeatureAsInt(
-          features::kTabMetricsLogging, kPerSourceLogTimeoutMsecParamName,
-          kDefaultPerSourceLogTimeout.InMilliseconds()));
   browser_tab_strip_tracker_.Init();
 
   // TabMetrics UKMs reference WindowMetrics UKM entries, so ensure the
@@ -193,21 +159,9 @@ void TabActivityWatcher::MaybeLogTab(content::WebContents* web_contents) {
       TabActivityWatcher::WebContentsData::FromWebContents(web_contents);
   DCHECK(web_contents_data);
 
-  // TODO(michaelpg): Convert to resource_coordinator::NowTicks().
-  base::TimeTicks now = base::TimeTicks::Now();
-  if (now - web_contents_data->last_log_time_for_source() <
-      per_source_log_timeout_) {
-    return;
-  }
-
   ukm::SourceId ukm_source_id = web_contents_data->ukm_source_id();
   tab_metrics_logger_->LogBackgroundTab(ukm_source_id,
                                         web_contents_data->tab_metrics());
-  web_contents_data->DidLog(now);
-}
-
-void TabActivityWatcher::DisableLogTimeoutForTesting() {
-  per_source_log_timeout_ = base::TimeDelta();
 }
 
 void TabActivityWatcher::ResetForTesting() {

chrome/browser/resource_coordinator/tab_activity_watcher.h

@@ -8,7 +8,6 @@
 #include <memory>
 
 #include "base/macros.h"
-#include "base/time/time.h"
 #include "chrome/browser/ui/browser_tab_strip_tracker.h"
 #include "chrome/browser/ui/browser_tab_strip_tracker_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
@@ -55,13 +54,9 @@ class TabActivityWatcher : public TabStripModelObserver,
   // Called from WebContentsData when a tab has stopped loading.
   void OnDidStopLoading(content::WebContents* web_contents);
 
-  // Logs the tab with |web_contents| if the tab hasn't been logged for the same
-  // source ID within a timeout window.
+  // Logs the tab with |web_contents|, unless it is being destroyed.
   void MaybeLogTab(content::WebContents* web_contents);
 
-  // Forces logging even when a timeout would have prevented it.
-  void DisableLogTimeoutForTesting();
-
   // Resets internal state.
   void ResetForTesting();
 
@@ -71,9 +66,6 @@ class TabActivityWatcher : public TabStripModelObserver,
   // browsers are created and destroyed.
   BrowserTabStripTracker browser_tab_strip_tracker_;
 
-  // Time before a tab with the same SourceId can be logged again.
-  base::TimeDelta per_source_log_timeout_;
-
   DISALLOW_COPY_AND_ASSIGN(TabActivityWatcher);
 };
 

chrome/browser/resource_coordinator/tab_activity_watcher_unittest.cc

@@ -72,7 +72,6 @@ blink::WebMouseEvent CreateMouseEvent(WebInputEvent::Type event_type) {
 class TabActivityWatcherTest : public TabActivityTestBase {
  protected:
   TabActivityWatcherTest() {
-    TabActivityWatcher::GetInstance()->DisableLogTimeoutForTesting();
     TabActivityWatcher::GetInstance()->ResetForTesting();
   }