RC: Use Chrome usage time since hidden to decide if tab can be proactively discarded (reland).

This is a reland of https://chromium-review.googlesource.com/1130470.
The double end session issue was fixed by
https://chromium-review.googlesource.com/1136725.

Previously, a tab was proactively discarded when it had spent T time hidden,
where T depends on the number of non-discarded tabs and the amount of
physical memory. Unfortunately, that meant that if a user kept Chrome
open during the night without touching it, all hidden tabs were discarded
in the morning.

With this CL, we use the "Chrome usage time since hidden" to determine
if a tab should be proactively discarded. That will prevent the unwanted
behavior of finding all tabs discarded after a long period of inactivity.

TBR=sebmarchand@chromium.org

Bug: 775644
Change-Id: I57a08a47aad76087b54277d19849228e1ed00305
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1136791
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575032}

chrome/browser/resource_coordinator/tab_manager.cc

@@ -190,12 +190,19 @@ TabManager::TabManager()
       GetStaticProactiveTabFreezeAndDiscardParams();
   TabLoadTracker::Get()->AddObserver(this);
   intervention_policy_database_.reset(new InterventionPolicyDatabase());
+
+  // TabManager works in the absence of DesktopSessionDurationTracker for tests.
+  if (metrics::DesktopSessionDurationTracker::IsInitialized())
+    metrics::DesktopSessionDurationTracker::Get()->AddObserver(this);
 }
 
 TabManager::~TabManager() {
   TabLoadTracker::Get()->RemoveObserver(this);
   resource_coordinator_signal_observer_.reset();
   Stop();
+
+  if (metrics::DesktopSessionDurationTracker::IsInitialized())
+    metrics::DesktopSessionDurationTracker::Get()->RemoveObserver(this);
 }
 
 void TabManager::Start() {
@@ -621,6 +628,12 @@ void TabManager::OnStopTracking(content::WebContents* web_contents,
   GetWebContentsData(web_contents)->SetTabLoadingState(loading_state);
 }
 
+void TabManager::OnSessionStarted(base::TimeTicks session_start) {
+  // LifecycleUnits might become eligible for proactive discarding when Chrome
+  // starts being used.
+  SchedulePerformStateTransitions(base::TimeDelta());
+}
+
 // static
 TabManager::WebContentsData* TabManager::GetWebContentsData(
     content::WebContents* contents) {
@@ -945,7 +958,7 @@ void TabManager::PerformStateTransitions() {
     return;
 
   base::TimeTicks next_state_transition_time = base::TimeTicks::Max();
-  base::TimeTicks now = NowTicks();
+  const base::TimeTicks now = NowTicks();
   LifecycleUnit* oldest_discardable_lifecycle_unit = nullptr;
   DecisionDetails oldest_discardable_lifecycle_unit_decision_details;
 
@@ -954,10 +967,11 @@ void TabManager::PerformStateTransitions() {
     // |kBackgroundTabFreezeTimeout|.
     DecisionDetails freeze_details;
     if (lifecycle_unit->CanFreeze(&freeze_details)) {
-      const base::TimeDelta time_not_visible =
+      const base::TimeDelta wall_time_not_visible =
           now - lifecycle_unit->GetWallTimeWhenHidden();
       const base::TimeDelta time_until_freeze =
-          proactive_freeze_discard_params_.freeze_timeout - time_not_visible;
+          proactive_freeze_discard_params_.freeze_timeout -
+          wall_time_not_visible;
 
       if (time_until_freeze <= base::TimeDelta()) {
         auto old_state = lifecycle_unit->GetState();
@@ -978,8 +992,9 @@ void TabManager::PerformStateTransitions() {
     if (lifecycle_unit->CanDiscard(DiscardReason::kProactive,
                                    &discard_details)) {
       if (!oldest_discardable_lifecycle_unit ||
-          lifecycle_unit->GetWallTimeWhenHidden() <
-              oldest_discardable_lifecycle_unit->GetWallTimeWhenHidden()) {
+          lifecycle_unit->GetChromeUsageTimeWhenHidden() <
+              oldest_discardable_lifecycle_unit
+                  ->GetChromeUsageTimeWhenHidden()) {
         oldest_discardable_lifecycle_unit = lifecycle_unit;
         oldest_discardable_lifecycle_unit_decision_details =
             std::move(discard_details);
@@ -997,10 +1012,11 @@ void TabManager::PerformStateTransitions() {
   // discarding all LifecycleUnits that have been non-visible for at least
   // GetTimeInBackgroundBeforeProactiveDiscard().
   if (ShouldProactivelyDiscardTabs() && oldest_discardable_lifecycle_unit) {
-    const base::TimeDelta time_not_visible =
-        now - oldest_discardable_lifecycle_unit->GetWallTimeWhenHidden();
+    const base::TimeDelta usage_time_not_visible =
+        usage_clock_.GetTotalUsageTime() -
+        oldest_discardable_lifecycle_unit->GetChromeUsageTimeWhenHidden();
     const base::TimeDelta time_until_discard =
-        GetTimeInBackgroundBeforeProactiveDiscard() - time_not_visible;
+        GetTimeInBackgroundBeforeProactiveDiscard() - usage_time_not_visible;
 
     if (time_until_discard <= base::TimeDelta()) {
       auto old_state = oldest_discardable_lifecycle_unit->GetState();
@@ -1015,16 +1031,15 @@ void TabManager::PerformStateTransitions() {
       // As mentioned above, call PeformStateTransitions() again after a
       // discard.
       next_state_transition_time = base::TimeTicks();
-    } else {
+    } else if (usage_clock_.IsInUse()) {
       next_state_transition_time =
           std::min(now + time_until_discard, next_state_transition_time);
     }
   }
 
   // Schedule the next call to PerformStateTransitions().
-  if (next_state_transition_time.is_max() && state_transitions_timer_)
-    state_transitions_timer_->Stop();
-  else
+  DCHECK(!state_transitions_timer_->IsRunning());
+  if (!next_state_transition_time.is_max())
     SchedulePerformStateTransitions(next_state_transition_time - now);
 }
 

chrome/browser/resource_coordinator/tab_manager.h

@@ -19,6 +19,7 @@
 #include "base/strings/string16.h"
 #include "base/timer/timer.h"
 #include "build/build_config.h"
+#include "chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker.h"
 #include "chrome/browser/resource_coordinator/discard_reason.h"
 #include "chrome/browser/resource_coordinator/intervention_policy_database.h"
 #include "chrome/browser/resource_coordinator/lifecycle_unit.h"
@@ -82,7 +83,8 @@ class TabManagerStatsCollector;
 class TabManager : public LifecycleUnitObserver,
                    public LifecycleUnitSourceObserver,
                    public TabLoadTracker::Observer,
-                   public TabStripModelObserver {
+                   public TabStripModelObserver,
+                   public metrics::DesktopSessionDurationTracker::Observer {
  public:
   // Forward declaration of resource coordinator signal observer.
   class ResourceCoordinatorSignalObserver;
@@ -309,7 +311,7 @@ class TabManager : public LifecycleUnitObserver,
   void OnMemoryPressure(
       base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level);
 
-  // TabStripModelObserver overrides.
+  // TabStripModelObserver:
   void ActiveTabChanged(content::WebContents* old_contents,
                         content::WebContents* new_contents,
                         int index,
@@ -323,7 +325,7 @@ class TabManager : public LifecycleUnitObserver,
                      content::WebContents* new_contents,
                      int index) override;
 
-  // TabLoadTracker::Observer implementation:
+  // TabLoadTracker::Observer:
   void OnStartTracking(content::WebContents* web_contents,
                        LoadingState loading_state) override;
   void OnLoadingStateChange(content::WebContents* web_contents,
@@ -332,6 +334,9 @@ class TabManager : public LifecycleUnitObserver,
   void OnStopTracking(content::WebContents* web_contents,
                       LoadingState loading_state) override;
 
+  // DesktopSessionDurationTracker::Observer:
+  void OnSessionStarted(base::TimeTicks session_start) override;
+
   // Returns the WebContentsData associated with |contents|. Also takes care of
   // creating one if needed.
   static WebContentsData* GetWebContentsData(content::WebContents* contents);

chrome/browser/resource_coordinator/tab_manager_unittest.cc

@@ -23,6 +23,7 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/resource_coordinator/background_tab_navigation_throttle.h"
 #include "chrome/browser/resource_coordinator/local_site_characteristics_data_unittest_utils.h"
@@ -133,6 +134,9 @@ class TabManagerTest : public testing::ChromeTestHarnessWithLocalDB {
                 task_runner_)),
         scoped_set_tick_clock_for_testing_(task_runner_->GetMockTickClock()) {
     base::MessageLoopCurrent::Get()->SetTaskRunner(task_runner_);
+
+    // Start with a non-zero time.
+    task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(42));
   }
 
   std::unique_ptr<WebContents> CreateWebContents() {
@@ -316,6 +320,10 @@ class TabManagerWithProactiveDiscardExperimentEnabledTest
     scoped_feature_list_.InitAndEnableFeature(
         features::kProactiveTabFreezeAndDiscard);
 
+    // Pretend that Chrome is in use.
+    metrics::DesktopSessionDurationTracker::Initialize();
+    MarkChromeInUse(true);
+
     TabManagerTest::SetUp();
 
     // Use test constants for proactive discarding parameters.
@@ -323,6 +331,21 @@ class TabManagerWithProactiveDiscardExperimentEnabledTest
         GetTestProactiveDiscardParams();
   }
 
+  void TearDown() override {
+    TabManagerTest::TearDown();
+    metrics::DesktopSessionDurationTracker::CleanupForTesting();
+  }
+
+  void MarkChromeInUse(bool in_use) {
+    auto* tracker = metrics::DesktopSessionDurationTracker::Get();
+    if (in_use) {
+      tracker->OnVisibilityChanged(true, base::TimeDelta());
+      tracker->OnUserEvent();
+    } else {
+      tracker->OnVisibilityChanged(false, base::TimeDelta());
+    }
+  }
+
   ProactiveTabFreezeAndDiscardParams GetTestProactiveDiscardParams() {
     // Return a ProactiveTabFreezeAndDiscardParams struct with default test
     // parameters.
@@ -1548,6 +1571,77 @@ TEST_F(TabManagerWithProactiveDiscardExperimentEnabledTest,
   tab_strip->CloseAllTabs();
 }
 
+TEST_F(TabManagerWithProactiveDiscardExperimentEnabledTest,
+       NoProactiveDiscardWhenChromeNotInUse) {
+  auto window = std::make_unique<TestBrowserWindow>();
+  Browser::CreateParams params(profile(), true);
+  params.type = Browser::TYPE_TABBED;
+  params.window = window.get();
+  auto browser = std::make_unique<Browser>(params);
+  TabStripModel* tab_strip = browser->tab_strip_model();
+
+  // Create 2 tabs.
+  tab_strip->AppendWebContents(CreateWebContents(), /*foreground=*/true);
+  tab_strip->GetWebContentsAt(0)->WasShown();
+  TabLoadTracker::Get()->TransitionStateForTesting(
+      tab_strip->GetWebContentsAt(0), TabLoadTracker::LoadingState::LOADED);
+
+  tab_strip->AppendWebContents(CreateWebContents(), /*foreground=*/false);
+  tab_strip->GetWebContentsAt(1)->WasHidden();
+  TabLoadTracker::Get()->TransitionStateForTesting(
+      tab_strip->GetWebContentsAt(1), TabLoadTracker::LoadingState::LOADED);
+
+  // Run tasks to let state transitions happen.
+  task_runner_->RunUntilIdle();
+
+  // No tab should be frozen or discarded initially.
+  EXPECT_FALSE(IsTabFrozen(tab_strip->GetWebContentsAt(0)));
+  EXPECT_FALSE(IsTabFrozen(tab_strip->GetWebContentsAt(1)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(0)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(1)));
+
+  // Fast-forward time when Chrome is not in use.
+  MarkChromeInUse(false);
+  task_runner_->FastForwardBy(kFreezeTimeout);
+
+  // The background tab should be frozen normally.
+  EXPECT_FALSE(IsTabFrozen(tab_strip->GetWebContentsAt(0)));
+  EXPECT_TRUE(IsTabFrozen(tab_strip->GetWebContentsAt(1)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(0)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(1)));
+
+  // Fast-forward time again when Chrome is not in use.
+  task_runner_->FastForwardBy(base::TimeDelta::FromDays(1));
+
+  // No discard should happen when Chrome is not in use.
+  EXPECT_FALSE(IsTabFrozen(tab_strip->GetWebContentsAt(0)));
+  EXPECT_TRUE(IsTabFrozen(tab_strip->GetWebContentsAt(1)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(0)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(1)));
+
+  // Fast-forward time less than the discard timeout when Chrome is in use.
+  constexpr base::TimeDelta kShortDelay = base::TimeDelta::FromSeconds(42);
+  MarkChromeInUse(true);
+  task_runner_->FastForwardBy(kShortDelay);
+
+  // No discard should happen yet.
+  EXPECT_FALSE(IsTabFrozen(tab_strip->GetWebContentsAt(0)));
+  EXPECT_TRUE(IsTabFrozen(tab_strip->GetWebContentsAt(1)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(0)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(1)));
+
+  // Fast-forward time enough for the discard timeout to expire.
+  task_runner_->FastForwardBy(kLowOccludedTimeout - kShortDelay);
+
+  // The background tab should be discarded.
+  EXPECT_FALSE(IsTabFrozen(tab_strip->GetWebContentsAt(0)));
+  EXPECT_FALSE(IsTabFrozen(tab_strip->GetWebContentsAt(1)));
+  EXPECT_FALSE(IsTabDiscarded(tab_strip->GetWebContentsAt(0)));
+  EXPECT_TRUE(IsTabDiscarded(tab_strip->GetWebContentsAt(1)));
+
+  tab_strip->CloseAllTabs();
+}
+
 TEST_F(TabManagerTest, NoProactiveDiscardWhenFeatureDisabled) {
   auto window = std::make_unique<TestBrowserWindow>();
   Browser::CreateParams params(profile(), true);