Commit 2b96359c authored by Chris Hamilton's avatar Chris Hamilton Committed by Commit Bot

[RC] Allow repeated discards.

Allow tabs to be discarded more than once. The heuristics have changed
so much in in the last 6 months that this additional protection isn't
needed.

BUG=794622

Change-Id: I094af452b5a7190f229fefbc77f4571ac0601a17
Reviewed-on: https://chromium-review.googlesource.com/1142479
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: default avatarSébastien Marchand <sebmarchand@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576485}
parent 27cee15c
...@@ -532,23 +532,6 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanDiscard( ...@@ -532,23 +532,6 @@ bool TabLifecycleUnitSource::TabLifecycleUnit::CanDiscard(
return false; return false;
} }
// Do not discard a tab that has already been discarded. Since this is being
// removed there is no way to communicate that to the heuristic. Treat this
// as a "trivial" rejection reason for now and return with an empty decision
// details.
// TODO(fdoray): Allow tabs to be discarded more than once.
// https://crbug.com/794622
if (discard_count_ > 0) {
#if defined(OS_CHROMEOS)
// On ChromeOS this can be ignored for urgent discards, where running out of
// memory leads to a kernel panic.
if (reason != DiscardReason::kUrgent)
return false;
#else
return false;
#endif // defined(OS_CHROMEOS)
}
// We deliberately run through all of the logic without early termination. // We deliberately run through all of the logic without early termination.
// This ensures that the decision details lists all possible reasons that the // This ensures that the decision details lists all possible reasons that the
// transition can be denied. // transition can be denied.
......
...@@ -413,58 +413,6 @@ class TabLifecycleUnitSourceTest ...@@ -413,58 +413,6 @@ class TabLifecycleUnitSourceTest
.GetPendingEntry()); .GetPendingEntry());
} }
void CanOnlyDiscardOnceTest(DiscardReason reason) {
LifecycleUnit* background_lifecycle_unit = nullptr;
LifecycleUnit* foreground_lifecycle_unit = nullptr;
CreateTwoTabs(true /* focus_tab_strip */, &background_lifecycle_unit,
&foreground_lifecycle_unit);
content::WebContents* initial_web_contents =
tab_strip_model_->GetWebContentsAt(0);
// It should be possible to discard the background tab.
ExpectCanDiscardTrueAllReasons(background_lifecycle_unit);
// Discard the tab.
EXPECT_EQ(LifecycleUnitState::ACTIVE,
background_lifecycle_unit->GetState());
EXPECT_CALL(tab_observer_, OnDiscardedStateChange(::testing::_, true));
background_lifecycle_unit->Discard(reason);
::testing::Mock::VerifyAndClear(&tab_observer_);
TransitionFromPendingDiscardToDiscardedIfNeeded(reason,
background_lifecycle_unit);
EXPECT_NE(initial_web_contents, tab_strip_model_->GetWebContentsAt(0));
EXPECT_FALSE(tab_strip_model_->GetWebContentsAt(0)
->GetController()
.GetPendingEntry());
// Explicitly reload the tab. Expect the state to be LOADED.
EXPECT_CALL(tab_observer_, OnDiscardedStateChange(::testing::_, false));
tab_strip_model_->GetWebContentsAt(0)->GetController().Reload(
content::ReloadType::NORMAL, false);
::testing::Mock::VerifyAndClear(&tab_observer_);
EXPECT_EQ(LifecycleUnitState::ACTIVE,
background_lifecycle_unit->GetState());
EXPECT_TRUE(tab_strip_model_->GetWebContentsAt(0)
->GetController()
.GetPendingEntry());
// It shouldn't be possible to discard the background tab again, except for
// an urgent discard on ChromeOS.
ExpectCanDiscardFalseTrivial(background_lifecycle_unit,
DiscardReason::kExternal);
ExpectCanDiscardFalseTrivial(background_lifecycle_unit,
DiscardReason::kProactive);
#if defined(OS_CHROMEOS)
ExpectCanDiscardTrue(background_lifecycle_unit, DiscardReason::kUrgent);
#else
ExpectCanDiscardFalseTrivial(background_lifecycle_unit,
DiscardReason::kUrgent);
#endif
}
TabLifecycleUnitSource* source_ = nullptr; TabLifecycleUnitSource* source_ = nullptr;
::testing::StrictMock<MockLifecycleUnitSourceObserver> source_observer_; ::testing::StrictMock<MockLifecycleUnitSourceObserver> source_observer_;
::testing::StrictMock<MockTabLifecycleObserver> tab_observer_; ::testing::StrictMock<MockTabLifecycleObserver> tab_observer_;
...@@ -645,18 +593,6 @@ TEST_F(TabLifecycleUnitSourceTest, DiscardAndExplicitlyReload_External) { ...@@ -645,18 +593,6 @@ TEST_F(TabLifecycleUnitSourceTest, DiscardAndExplicitlyReload_External) {
DiscardAndExplicitlyReloadTest(DiscardReason::kExternal); DiscardAndExplicitlyReloadTest(DiscardReason::kExternal);
} }
TEST_F(TabLifecycleUnitSourceTest, CanOnlyDiscardOnce_Urgent) {
CanOnlyDiscardOnceTest(DiscardReason::kUrgent);
}
TEST_F(TabLifecycleUnitSourceTest, CanOnlyDiscardOnce_Proactive) {
CanOnlyDiscardOnceTest(DiscardReason::kProactive);
}
TEST_F(TabLifecycleUnitSourceTest, CanOnlyDiscardOnce_External) {
CanOnlyDiscardOnceTest(DiscardReason::kExternal);
}
TEST_F(TabLifecycleUnitSourceTest, CannotFreezeADiscardedTab) { TEST_F(TabLifecycleUnitSourceTest, CannotFreezeADiscardedTab) {
LifecycleUnit* background_lifecycle_unit = nullptr; LifecycleUnit* background_lifecycle_unit = nullptr;
LifecycleUnit* foreground_lifecycle_unit = nullptr; LifecycleUnit* foreground_lifecycle_unit = nullptr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment