Enforcing freezing a tab prior to proactive discard.

This CL enforces that a tab is frozen prior to being proactively discarded. This will allow the website to have enough time to persist any state prior to being proactively discarded. Urgent discard behavior is unchanged.

Bug: 775644
Change-Id: Ia3ec9e8f53f80d2c940edd065090415cbcdd1f6c
Reviewed-on: https://chromium-review.googlesource.com/1041138
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557209}

chrome/browser/resource_coordinator/lifecycle_state.h

@@ -10,8 +10,12 @@ namespace resource_coordinator {
 enum class LifecycleState {
   // The LifecycleUnit is alive and active.
   ACTIVE,
+  // The LifecycleUnit is pending a freeze.
+  PENDING_FREEZE,
   // The LifecycleUnit is frozen.
   FROZEN,
+  // The LifecycleUnit is pending a discard.
+  PENDING_DISCARD,
   // The LifecycleUnit is discarded, and is consuming no system resources.
   DISCARDED,
 };

chrome/browser/resource_coordinator/tab_lifecycle_unit.cc

@@ -24,22 +24,6 @@
 
 namespace resource_coordinator {
 
-namespace {
-
-// Translates a mojom::LifecycleState to resource_coordinator::LifecycleState.
-LifecycleState GetLifecycleStateFromMojoState(mojom::LifecycleState state) {
-  switch (state) {
-    case mojom::LifecycleState::kDiscarded:
-      return LifecycleState::DISCARDED;
-    case mojom::LifecycleState::kFrozen:
-      return LifecycleState::FROZEN;
-    case mojom::LifecycleState::kRunning:
-      return LifecycleState::ACTIVE;
-  }
-}
-
-}  // namespace
-
 TabLifecycleUnitSource::TabLifecycleUnit::TabLifecycleUnit(
     base::ObserverList<TabLifecycleObserver>* observers,
     content::WebContents* web_contents,
@@ -73,14 +57,26 @@ void TabLifecycleUnitSource::TabLifecycleUnit::SetFocused(bool focused) {
     return;
   last_focused_time_ = focused ? base::TimeTicks::Max() : NowTicks();
 
-  if (focused && GetState() == LifecycleState::DISCARDED) {
+  if (focused && (GetState() == LifecycleState::DISCARDED ||
+                  GetState() == LifecycleState::PENDING_DISCARD)) {
+    bool was_discarded = GetState() == LifecycleState::DISCARDED;
     SetState(LifecycleState::ACTIVE);
-    // See comment in Discard() for an explanation of why "needs reload" is
-    // false when a tab is discarded.
-    // TODO(fdoray): Remove NavigationControllerImpl::needs_reload_ once session
-    // restore is handled by LifecycleManager.
-    GetWebContents()->GetController().SetNeedsReload();
-    GetWebContents()->GetController().LoadIfNecessary();
+
+    // If the tab was fully discarded, the tab needs to be reloaded.
+    if (was_discarded) {
+      // See comment in Discard() for an explanation of why "needs reload" is
+      // false when a tab is discarded.
+      // TODO(fdoray): Remove NavigationControllerImpl::needs_reload_ once
+      // session restore is handled by LifecycleManager.
+      GetWebContents()->GetController().SetNeedsReload();
+      GetWebContents()->GetController().LoadIfNecessary();
+    }
+
+    // Stop |freeze_timeout_timer_| to avoid completing the discard, as the tab
+    // was focused.
+    if (freeze_timeout_timer_)
+      freeze_timeout_timer_->Stop();
+
     OnDiscardedStateChange();
   }
 }
@@ -96,7 +92,48 @@ void TabLifecycleUnitSource::TabLifecycleUnit::SetRecentlyAudible(
 void TabLifecycleUnitSource::TabLifecycleUnit::UpdateLifecycleState(
     mojom::LifecycleState state) {
   DCHECK_NE(mojom::LifecycleState::kDiscarded, state);
-  SetState(GetLifecycleStateFromMojoState(state));
+  if (state == mojom::LifecycleState::kFrozen) {
+    if (GetState() == LifecycleState::PENDING_DISCARD) {
+      freeze_timeout_timer_->Stop();
+      FinishDiscard(discard_reason_);
+    } else {
+      SetState(LifecycleState::FROZEN);
+    }
+  } else if (state == mojom::LifecycleState::kRunning) {
+    SetState(LifecycleState::ACTIVE);
+  } else {
+    NOTREACHED();
+  }
+}
+
+void TabLifecycleUnitSource::TabLifecycleUnit::RequestFreezeForDiscard(
+    DiscardReason reason) {
+  // Ensure this isn't happening on an urgent discard.
+  DCHECK_NE(reason, DiscardReason::kUrgent);
+
+  // Ensure the tab is not already pending a discard.
+  DCHECK_NE(GetState(), LifecycleState::PENDING_DISCARD);
+
+  SetState(LifecycleState::PENDING_DISCARD);
+
+  // External observers should now view this tab as discarded, hiding the
+  // pending discard implementation detail.
+  // TODO(fdoray) This should be done as part of SetState() instead of manually
+  // after each SetState() that might change IsDiscarded(). We would have less
+  // chances of breaking things in a future change.
+  OnDiscardedStateChange();
+
+  if (!freeze_timeout_timer_) {
+    freeze_timeout_timer_ =
+        std::make_unique<base::OneShotTimer>(GetTickClock());
+  }
+
+  freeze_timeout_timer_->Start(
+      FROM_HERE, kProactiveDiscardFreezeTimeout,
+      base::BindRepeating(&TabLifecycleUnit::FinishDiscard,
+                          base::Unretained(this), reason));
+
+  Freeze();
 }
 
 TabLifecycleUnitExternal*
@@ -134,6 +171,12 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::Freeze() {
   if (GetState() == LifecycleState::DISCARDED)
     return false;
 
+  // If currently in PENDING_DISCARD, the tab IsDiscarded() to external
+  // observers. To maintain this, don't set it to PENDING_FREEZE, but keep the
+  // tab in the PENDING_DISCARD state.
+  if (GetState() != LifecycleState::PENDING_DISCARD)
+    SetState(LifecycleState::PENDING_FREEZE);
+
   GetWebContents()->FreezePage();
   return true;
 }
@@ -225,9 +268,44 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanDiscard(
 
 bool TabLifecycleUnitSource::TabLifecycleUnit::Discard(
     DiscardReason discard_reason) {
-  if (!tab_strip_model_ || GetState() == LifecycleState::DISCARDED)
+  // Can't discard a tab when it isn't in a tabstrip.
+  if (!tab_strip_model_)
+    return false;
+
+  // Can't discard a tab if it is already discarded.
+  if (GetState() == LifecycleState::DISCARDED)
     return false;
 
+  // If a non-urgent discard is requested when the state is PENDING_DISCARD,
+  // returns false to indicate that it is incorrect to request a non-urgent
+  // discard again.
+  if (GetState() == LifecycleState::PENDING_DISCARD &&
+      discard_reason != DiscardReason::kUrgent) {
+    return false;
+  }
+
+  discard_reason_ = discard_reason;
+
+  // If the tab is not going through an urgent discard, it should be frozen
+  // first. Freeze the tab and set a timer to callback to FinishDiscard() incase
+  // the freeze callback takes too long.
+  if (discard_reason != DiscardReason::kUrgent &&
+      GetState() != LifecycleState::FROZEN) {
+    RequestFreezeForDiscard(discard_reason);
+
+    // Returning true because even though the discard did not happen yet, the
+    // tab is in PENDING_DISCARD state and will be discarded either when the
+    // freeze callback occurs, or the kProactiveDiscardFreezeTimeout timeout is
+    // reached.
+    return true;
+  }
+
+  FinishDiscard(discard_reason);
+  return true;
+}
+
+void TabLifecycleUnitSource::TabLifecycleUnit::FinishDiscard(
+    DiscardReason discard_reason) {
   UMA_HISTOGRAM_BOOLEAN(
       "TabManager.Discarding.DiscardedTabHasBeforeUnloadHandler",
       GetWebContents()->NeedToFireBeforeUnload());
@@ -295,11 +373,11 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::Discard(
   // RenderFrameProxyHosts.
   old_contents_deleter.reset();
 
+  LifecycleState previous_state = GetState();
   SetState(LifecycleState::DISCARDED);
+  if (previous_state != LifecycleState::PENDING_DISCARD)
+    OnDiscardedStateChange();
   ++discard_count_;
-  OnDiscardedStateChange();
-
-  return true;
 }
 
 content::WebContents* TabLifecycleUnitSource::TabLifecycleUnit::GetWebContents()
@@ -351,11 +429,21 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::FreezeTab() {
 }
 
 bool TabLifecycleUnitSource::TabLifecycleUnit::IsDiscarded() const {
-  return GetState() == LifecycleState::DISCARDED;
+  // External code does not need to know about the intermediary PENDING_DISCARD
+  // state. To external callers, the tab is discarded while in the
+  // PENDING_DISCARD state.
+  LifecycleState current_state = GetState();
+  return current_state == LifecycleState::PENDING_DISCARD ||
+         current_state == LifecycleState::DISCARDED;
 }
 
 bool TabLifecycleUnitSource::TabLifecycleUnit::IsFrozen() const {
-  return GetState() == LifecycleState::FROZEN;
+  // External code does not need to know about the intermediary PENDING_FREEZE
+  // state.  To external callers, the tab is frozen while in the PENDING_FREEZE
+  // state.
+  LifecycleState current_state = GetState();
+  return current_state == LifecycleState::PENDING_FREEZE ||
+         current_state == LifecycleState::FROZEN;
 }
 
 int TabLifecycleUnitSource::TabLifecycleUnit::GetDiscardCount() const {

chrome/browser/resource_coordinator/tab_lifecycle_unit.h

@@ -8,6 +8,7 @@
 #include "base/macros.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
+#include "base/timer/timer.h"
 #include "chrome/browser/resource_coordinator/lifecycle_unit_base.h"
 #include "chrome/browser/resource_coordinator/tab_lifecycle_unit_external.h"
 #include "chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h"
@@ -30,6 +31,11 @@ class TabLifecycleObserver;
 static constexpr base::TimeDelta kTabAudioProtectionTime =
     base::TimeDelta::FromMinutes(1);
 
+// Timeout after which a tab is proactively discarded if the freeze callback
+// hasn't been received.
+static constexpr base::TimeDelta kProactiveDiscardFreezeTimeout =
+    base::TimeDelta::FromMilliseconds(500);
+
 // Represents a tab.
 class TabLifecycleUnitSource::TabLifecycleUnit
     : public LifecycleUnitBase,
@@ -66,6 +72,10 @@ class TabLifecycleUnitSource::TabLifecycleUnit
   // "recently audible" state of the tab changes.
   void SetRecentlyAudible(bool recently_audible);
 
+  // Updates the tab's lifecycle state when changed outside the tab lifecycle
+  // unit.
+  void UpdateLifecycleState(mojom::LifecycleState state);
+
   // LifecycleUnit:
   TabLifecycleUnitExternal* AsTabLifecycleUnitExternal() override;
   base::string16 GetTitle() const override;
@@ -90,17 +100,29 @@ class TabLifecycleUnitSource::TabLifecycleUnit
   int GetDiscardCount() const override;
 
  protected:
+  friend class TabManagerTest;
+
   // TabLifecycleUnitSource needs to update the state when a external lifecycle
   // state change is observed.
   friend class TabLifecycleUnitSource;
-  // Updates the tab's lifecycle state when changed outside the tab lifecycle
-  // unit.
-  void UpdateLifecycleState(mojom::LifecycleState state);
 
  private:
-  // Invoked when the state goes from DISCARDED to non-DISCARDED and vice-versa.
+  // For non-urgent discarding, sends a request for freezing to occur prior to
+  // discarding the tab.
+  void RequestFreezeForDiscard(DiscardReason reason);
+
+  // Invoked when the state goes from DISCARDED or PENDING_DISCARD to any
+  // non-DISCARDED state and vice-versa.
   void OnDiscardedStateChange();
 
+  // Finishes a tab discard. For an urgent discard, this is invoked by
+  // Discard(). For a proactive or external discard, where the tab is frozen
+  // prior to being discarded, this is called by UpdateLifecycleState() once the
+  // callback has been received, or by |freeze_timeout_timer_| if the
+  // kProactiveDiscardFreezeTimeout timeout has passed without receiving the
+  // callback.
+  void FinishDiscard(DiscardReason discard_reason);
+
   // Returns the RenderProcessHost associated with this tab.
   content::RenderProcessHost* GetRenderProcessHost() const;
 
@@ -130,6 +152,13 @@ class TabLifecycleUnitSource::TabLifecycleUnit
   // When this is false, CanDiscard() always returns false.
   bool auto_discardable_ = true;
 
+  // Maintains the most recent DiscardReason that was pased into Discard().
+  DiscardReason discard_reason_;
+
+  // Timer that ensures that this tab does not wait forever for the callback
+  // when it is being frozen.
+  std::unique_ptr<base::OneShotTimer> freeze_timeout_timer_;
+
   // TimeTicks::Max() if the tab is currently "recently audible", null
   // TimeTicks() if the tab was never "recently audible", last time at which the
   // tab was "recently audible" otherwise.

chrome/browser/resource_coordinator/tab_lifecycle_unit_source.h

@@ -54,7 +54,20 @@ class TabLifecycleUnitSource : public BrowserListObserver,
 
  private:
   friend class TabLifecycleUnitTest;
-
+  friend class TabManagerTest;
+  FRIEND_TEST_ALL_PREFIXES(TabLifecycleUnitSourceTest,
+                           TabProactiveDiscardedByFrozenCallback);
+  FRIEND_TEST_ALL_PREFIXES(TabManagerTest, TabManagerWasDiscarded);
+  FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
+                           TabManagerWasDiscardedCrossSiteSubFrame);
+  FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
+                           ProactiveFastShutdownSingleTabProcess);
+  FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
+                           ProactiveFastShutdownSharedTabProcess);
+  FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
+                           ProactiveFastShutdownWithUnloadHandler);
+  FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
+                           ProactiveFastShutdownWithBeforeunloadHandler);
   class TabLifecycleUnit;
 
   // Returns the TabLifecycleUnit instance associated with |web_contents|, or

chrome/browser/resource_coordinator/tab_manager.cc

@@ -119,8 +119,11 @@ std::unique_ptr<base::trace_event::ConvertableToTraceFormat> DataAsTraceValue(
 int GetNumLoadedLifecycleUnits(LifecycleUnitSet lifecycle_unit_set) {
   int num_loaded_lifecycle_units = 0;
   for (auto* lifecycle_unit : lifecycle_unit_set) {
-    if (lifecycle_unit->GetState() != LifecycleState::DISCARDED)
+    LifecycleState state = lifecycle_unit->GetState();
+    if (state != LifecycleState::DISCARDED &&
+        state != LifecycleState::PENDING_DISCARD) {
       num_loaded_lifecycle_units++;
+    }
   }
   return num_loaded_lifecycle_units;
 }
@@ -1029,9 +1032,15 @@ void TabManager::UpdateProactiveDiscardTimerIfNecessary() {
 
 void TabManager::OnLifecycleUnitStateChanged(LifecycleUnit* lifecycle_unit,
                                              LifecycleState last_state) {
-  if (lifecycle_unit->GetState() == LifecycleState::DISCARDED)
+  LifecycleState state = lifecycle_unit->GetState();
+  bool was_discarded = (last_state == LifecycleState::PENDING_DISCARD ||
+                        last_state == LifecycleState::DISCARDED);
+  bool is_discarded = (state == LifecycleState::PENDING_DISCARD ||
+                       state == LifecycleState::DISCARDED);
+
+  if (is_discarded && !was_discarded)
     num_loaded_lifecycle_units_--;
-  else if (last_state == LifecycleState::DISCARDED)
+  else if (was_discarded && !is_discarded)
     num_loaded_lifecycle_units_++;
 
   DCHECK_EQ(num_loaded_lifecycle_units_,
@@ -1047,8 +1056,10 @@ void TabManager::OnLifecycleUnitVisibilityChanged(
 }
 
 void TabManager::OnLifecycleUnitDestroyed(LifecycleUnit* lifecycle_unit) {
-  if (lifecycle_unit->GetState() != LifecycleState::DISCARDED)
+  if (lifecycle_unit->GetState() != LifecycleState::DISCARDED &&
+      lifecycle_unit->GetState() != LifecycleState::PENDING_DISCARD) {
     num_loaded_lifecycle_units_--;
+  }
   lifecycle_units_.erase(lifecycle_unit);
 
   DCHECK_EQ(num_loaded_lifecycle_units_,

chrome/browser/resource_coordinator/tab_manager_browsertest.cc

@@ -12,6 +12,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/media/webrtc/media_capture_devices_dispatcher.h"
 #include "chrome/browser/media/webrtc/media_stream_capture_indicator.h"
+#include "chrome/browser/resource_coordinator/tab_lifecycle_unit.h"
 #include "chrome/browser/resource_coordinator/tab_lifecycle_unit_external.h"
 #include "chrome/browser/resource_coordinator/tab_manager.h"
 #include "chrome/browser/resource_coordinator/tab_manager_web_contents_data.h"
@@ -52,6 +53,19 @@ namespace {
 constexpr char kBlinkPageLifecycleFeature[] = "PageLifecycle";
 constexpr base::TimeDelta kShortDelay = base::TimeDelta::FromSeconds(1);
 
+bool ObserveNavEntryCommitted(const GURL& expected_url,
+                              const content::NotificationSource& source,
+                              const content::NotificationDetails& details) {
+  return content::Details<content::LoadCommittedDetails>(details)
+             ->entry->GetURL() == expected_url;
+}
+
+bool IsTabDiscarded(content::WebContents* web_contents) {
+  return TabLifecycleUnitExternal::FromWebContents(web_contents)->IsDiscarded();
+}
+
+}  // namespace
+
 class TabManagerTest : public InProcessBrowserTest {
  public:
   TabManagerTest() : scoped_set_tick_clock_for_testing_(&test_clock_) {
@@ -93,23 +107,19 @@ class TabManagerTest : public InProcessBrowserTest {
     ASSERT_EQ(2, browser()->tab_strip_model()->count());
   }
 
+  // Gets the TabLifecycleUnit from |contents| and sends the signal that
+  // indicates that the page is frozen. In production, this is sent by the
+  // renderer process. This is done to finish a proactive tab discard.
+  void SimulateFreezeSignal(content::WebContents* contents) {
+    static_cast<TabLifecycleUnitSource::TabLifecycleUnit*>(
+        TabLifecycleUnitExternal::FromWebContents(contents))
+        ->UpdateLifecycleState(mojom::LifecycleState::kFrozen);
+  }
+
   base::SimpleTestTickClock test_clock_;
   ScopedSetTickClockForTesting scoped_set_tick_clock_for_testing_;
 };
 
-bool ObserveNavEntryCommitted(const GURL& expected_url,
-                              const content::NotificationSource& source,
-                              const content::NotificationDetails& details) {
-  return content::Details<content::LoadCommittedDetails>(details)
-             ->entry->GetURL() == expected_url;
-}
-
-bool IsTabDiscarded(content::WebContents* web_contents) {
-  return TabLifecycleUnitExternal::FromWebContents(web_contents)->IsDiscarded();
-}
-
-}  // namespace
-
 IN_PROC_BROWSER_TEST_F(TabManagerTest, TabManagerBasics) {
   using content::WindowedNotificationObserver;
   TabManager* tab_manager = g_browser_process->GetTabManager();
@@ -174,14 +184,14 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, TabManagerBasics) {
 
   // Discard a tab.  It should kill the first tab, since it was the oldest
   // and was not selected.
-  EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kProactive));
+  EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kUrgent));
   EXPECT_EQ(3, tsm->count());
   EXPECT_TRUE(IsTabDiscarded(tsm->GetWebContentsAt(0)));
   EXPECT_FALSE(IsTabDiscarded(tsm->GetWebContentsAt(1)));
   EXPECT_FALSE(IsTabDiscarded(tsm->GetWebContentsAt(2)));
 
   // Run discard again, make sure it kills the second tab.
-  EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kProactive));
+  EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kUrgent));
   EXPECT_EQ(3, tsm->count());
   EXPECT_TRUE(IsTabDiscarded(tsm->GetWebContentsAt(0)));
   EXPECT_TRUE(IsTabDiscarded(tsm->GetWebContentsAt(1)));
@@ -189,7 +199,7 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, TabManagerBasics) {
 
   // Kill the third tab. It should not kill the last tab, since it is active
   // tab.
-  EXPECT_FALSE(tab_manager->DiscardTabImpl(DiscardReason::kProactive));
+  EXPECT_FALSE(tab_manager->DiscardTabImpl(DiscardReason::kUrgent));
   EXPECT_TRUE(IsTabDiscarded(tsm->GetWebContentsAt(0)));
   EXPECT_TRUE(IsTabDiscarded(tsm->GetWebContentsAt(1)));
   EXPECT_FALSE(IsTabDiscarded(tsm->GetWebContentsAt(2)));
@@ -199,7 +209,7 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, TabManagerBasics) {
 
   EXPECT_EQ(1, tsm->active_index());
   EXPECT_FALSE(IsTabDiscarded(tsm->GetWebContentsAt(1)));
-  tab_manager->DiscardTabImpl(DiscardReason::kProactive);
+  tab_manager->DiscardTabImpl(DiscardReason::kUrgent);
   EXPECT_TRUE(IsTabDiscarded(tsm->GetWebContentsAt(2)));
 
   // Force creation of the FindBarController.
@@ -589,6 +599,8 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, ProactiveFastShutdownSingleTabProcess) {
       content::NotificationService::AllSources());
   base::HistogramTester tester;
   EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kProactive));
+  SimulateFreezeSignal(browser()->tab_strip_model()->GetWebContentsAt(1));
+
   tester.ExpectUniqueSample(
       "TabManager.Discarding.DiscardedTabCouldFastShutdown", true, 1);
   observer.Wait();
@@ -625,6 +637,8 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, ProactiveFastShutdownSharedTabProcess) {
   // will be made.
   base::HistogramTester tester;
   EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kProactive));
+  SimulateFreezeSignal(browser()->tab_strip_model()->GetWebContentsAt(1));
+
   tester.ExpectUniqueSample(
       "TabManager.Discarding.DiscardedTabCouldFastShutdown", false, 1);
 }
@@ -664,6 +678,8 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, ProactiveFastShutdownWithUnloadHandler) {
   // one of them is current, and the other has an unload handler. No unsafe
   // attempts will be made.
   EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kProactive));
+  SimulateFreezeSignal(browser()->tab_strip_model()->GetWebContentsAt(1));
+
   tester.ExpectUniqueSample(
       "TabManager.Discarding.DiscardedTabCouldFastShutdown", false, 1);
 }
@@ -712,6 +728,8 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest,
   // attempts will be made.
   base::HistogramTester tester;
   EXPECT_TRUE(tab_manager->DiscardTabImpl(DiscardReason::kProactive));
+  SimulateFreezeSignal(browser()->tab_strip_model()->GetWebContentsAt(1));
+
   tester.ExpectUniqueSample(
       "TabManager.Discarding.DiscardedTabCouldFastShutdown", false, 1);
 }
@@ -839,6 +857,7 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest, TabManagerWasDiscarded) {
   TabLifecycleUnitExternal::FromWebContents(
       browser()->tab_strip_model()->GetActiveWebContents())
       ->DiscardTab();
+  SimulateFreezeSignal(browser()->tab_strip_model()->GetActiveWebContents());
 
   // Here we simulate re-focussing the tab causing reload with navigation,
   // the navigation will reload the tab.
@@ -895,6 +914,7 @@ IN_PROC_BROWSER_TEST_F(TabManagerTest,
 
   // Discard the tab. This simulates a tab discard.
   TabLifecycleUnitExternal::FromWebContents(contents)->DiscardTab();
+  SimulateFreezeSignal(contents);
 
   // Here we simulate re-focussing the tab causing reload with navigation,
   // the navigation will reload the tab.

chrome/browser/resource_coordinator/tab_manager_delegate_chromeos.cc

@@ -381,6 +381,7 @@ void TabManagerDelegate::OnFocusTabScoreAdjustmentTimeout() {
           << " for focused tab " << pid;
   std::map<int, int> dict;
   dict[pid] = chrome::kLowestRendererOomScore;
+  DCHECK(GetDebugDaemonClient());
   GetDebugDaemonClient()->SetOomScoreAdj(dict, base::Bind(&OnSetOomScoreAdj));
 }
 

chrome/browser/resource_coordinator/tab_manager_unittest.cc

@@ -478,7 +478,7 @@ TEST_F(TabManagerTest, MAYBE_DiscardTabWithNonVisibleTabs) {
   tab_strip2->GetWebContentsAt(1)->WasHidden();
 
   for (int i = 0; i < 4; ++i)
-    tab_manager_->DiscardTab(DiscardReason::kProactive);
+    tab_manager_->DiscardTab(DiscardReason::kUrgent);
 
   // Active tab in a visible window should not be discarded.
   EXPECT_FALSE(IsTabDiscarded(tab_strip1->GetWebContentsAt(0)));

chrome/browser/sync/sessions/browser_list_router_helper_unittest.cc

@@ -8,8 +8,12 @@
 #include <vector>
 
 #include "base/stl_util.h"
+#include "base/test/simple_test_tick_clock.h"
+#include "base/test/test_mock_time_task_runner.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/resource_coordinator/tab_lifecycle_unit.h"
 #include "chrome/browser/resource_coordinator/tab_manager.h"
+#include "chrome/browser/resource_coordinator/time.h"
 #include "chrome/browser/sync/sessions/sync_sessions_web_contents_router.h"
 #include "chrome/browser/sync/sessions/sync_sessions_web_contents_router_factory.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -42,8 +46,6 @@ class MockLocalSessionEventHandler : public LocalSessionEventHandler {
 
 class BrowserListRouterHelperTest : public BrowserWithTestWindowTest {
  protected:
-  ~BrowserListRouterHelperTest() override {}
-
   MockLocalSessionEventHandler handler_1;
   MockLocalSessionEventHandler handler_2;
 };
@@ -137,8 +139,24 @@ TEST_F(BrowserListRouterHelperTest, NotifyOnDiscardTab) {
   handler_1.seen_urls()->clear();
   handler_1.seen_ids()->clear();
 
-  g_browser_process->GetTabManager()->DiscardTabByExtension(
-      browser()->tab_strip_model()->GetWebContentsAt(0));
+  {
+    auto task_runner = base::MakeRefCounted<base::TestMockTimeTaskRunner>();
+    base::TestMockTimeTaskRunner::ScopedContext scoped_context(task_runner);
+    resource_coordinator::ScopedSetTickClockForTesting scoped_set_tick_clock(
+        task_runner->GetMockTickClock());
+
+    g_browser_process->GetTabManager()->DiscardTabByExtension(
+        browser()->tab_strip_model()->GetWebContentsAt(0));
+
+    // After a tab goes through a non-urgent discard, such as by an extension,
+    // it is frozen prior to being fully discarded.
+    // kProactiveDiscardFreezeTimeout is the duration before the tab finishes
+    // discarding after beginning a discard. Fast forward
+    // kProactiveDiscardFreezeTimeout amount of time to ensure the discard is
+    // complete.
+    task_runner->FastForwardBy(
+        resource_coordinator::kProactiveDiscardFreezeTimeout);
+  }
 
   // We're typically notified twice while discarding tabs. Once for the
   // destruction of the old web contents, and once for the new. This test case