Commit a85c146b authored by Francois Doray's avatar Francois Doray Committed by Commit Bot

RC: Validate state transitions in TabLifecycleUnit.

This CL adds a function that determines if a state transition is valid
in TabLifecycleUnit. This will help us ensure that things work properly
as we add more possible state transitions (e.g. periodic unfreeze).

Bug: 775644
Change-Id: I89e90021415f68edbed1aca1d6f0052057cd44e9
Reviewed-on: https://chromium-review.googlesource.com/1097550Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567995}
parent fd8a5495
...@@ -58,19 +58,23 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, DiscardReloadCount) { ...@@ -58,19 +58,23 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, DiscardReloadCount) {
histograms_.ExpectTotalCount(kDiscardCountHistogram, 0); histograms_.ExpectTotalCount(kDiscardCountHistogram, 0);
histograms_.ExpectTotalCount(kReloadCountHistogram, 0); histograms_.ExpectTotalCount(kReloadCountHistogram, 0);
lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED); lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTotalCount(kDiscardCountHistogram, 1); histograms_.ExpectTotalCount(kDiscardCountHistogram, 1);
histograms_.ExpectTotalCount(kReloadCountHistogram, 0); histograms_.ExpectTotalCount(kReloadCountHistogram, 0);
lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE); lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTotalCount(kDiscardCountHistogram, 1); histograms_.ExpectTotalCount(kDiscardCountHistogram, 1);
histograms_.ExpectTotalCount(kReloadCountHistogram, 1); histograms_.ExpectTotalCount(kReloadCountHistogram, 1);
lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED); lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTotalCount(kDiscardCountHistogram, 2); histograms_.ExpectTotalCount(kDiscardCountHistogram, 2);
histograms_.ExpectTotalCount(kReloadCountHistogram, 1); histograms_.ExpectTotalCount(kReloadCountHistogram, 1);
lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE); lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTotalCount(kDiscardCountHistogram, 2); histograms_.ExpectTotalCount(kDiscardCountHistogram, 2);
histograms_.ExpectTotalCount(kReloadCountHistogram, 2); histograms_.ExpectTotalCount(kReloadCountHistogram, 2);
} }
...@@ -78,11 +82,13 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, DiscardReloadCount) { ...@@ -78,11 +82,13 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, DiscardReloadCount) {
TEST_F(DiscardMetricsLifecycleUnitObserverTest, DiscardToReloadTime) { TEST_F(DiscardMetricsLifecycleUnitObserverTest, DiscardToReloadTime) {
histograms_.ExpectTotalCount(kDiscardToReloadTimeHistogram, 0); histograms_.ExpectTotalCount(kDiscardToReloadTimeHistogram, 0);
lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED); lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
test_clock_.Advance(kShortDelay); test_clock_.Advance(kShortDelay);
histograms_.ExpectTotalCount(kDiscardToReloadTimeHistogram, 0); histograms_.ExpectTotalCount(kDiscardToReloadTimeHistogram, 0);
lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE); lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTimeBucketCount(kDiscardToReloadTimeHistogram, kShortDelay, histograms_.ExpectTimeBucketCount(kDiscardToReloadTimeHistogram, kShortDelay,
1); 1);
} }
...@@ -93,11 +99,13 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, InactiveToReloadTime) { ...@@ -93,11 +99,13 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, InactiveToReloadTime) {
const base::TimeTicks last_focused_time = NowTicks(); const base::TimeTicks last_focused_time = NowTicks();
lifecycle_unit_->SetLastFocusedTime(last_focused_time); lifecycle_unit_->SetLastFocusedTime(last_focused_time);
test_clock_.Advance(kShortDelay); test_clock_.Advance(kShortDelay);
lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED); lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
test_clock_.Advance(kShortDelay); test_clock_.Advance(kShortDelay);
histograms_.ExpectTotalCount(kInactiveToReloadTimeHistogram, 0); histograms_.ExpectTotalCount(kInactiveToReloadTimeHistogram, 0);
lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE); lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTimeBucketCount(kInactiveToReloadTimeHistogram, histograms_.ExpectTimeBucketCount(kInactiveToReloadTimeHistogram,
2 * kShortDelay, 1); 2 * kShortDelay, 1);
} }
...@@ -113,7 +121,8 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, ...@@ -113,7 +121,8 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest,
ReloadToCloseTimeDiscardedButNotReloaded) { ReloadToCloseTimeDiscardedButNotReloaded) {
histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0); histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0);
lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED); lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0); histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0);
lifecycle_unit_.reset(); lifecycle_unit_.reset();
...@@ -124,11 +133,13 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, ReloadToCloseTime) { ...@@ -124,11 +133,13 @@ TEST_F(DiscardMetricsLifecycleUnitObserverTest, ReloadToCloseTime) {
histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0); histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0);
test_clock_.Advance(kShortDelay * 1); test_clock_.Advance(kShortDelay * 1);
lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED); lifecycle_unit_->SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0); histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0);
test_clock_.Advance(kShortDelay * 2); test_clock_.Advance(kShortDelay * 2);
lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE); lifecycle_unit_->SetState(LifecycleUnitState::ACTIVE,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0); histograms_.ExpectTotalCount(kReloadToCloseTimeHistogram, 0);
test_clock_.Advance(kShortDelay * 4); test_clock_.Advance(kShortDelay * 4);
......
...@@ -40,18 +40,20 @@ ukm::SourceId LifecycleUnitBase::GetUkmSourceId() const { ...@@ -40,18 +40,20 @@ ukm::SourceId LifecycleUnitBase::GetUkmSourceId() const {
return ukm::kInvalidSourceId; return ukm::kInvalidSourceId;
} }
void LifecycleUnitBase::SetState(LifecycleUnitState state) { void LifecycleUnitBase::SetState(LifecycleUnitState state,
LifecycleUnitStateChangeReason reason) {
if (state == state_) if (state == state_)
return; return;
LifecycleUnitState last_state = state_; LifecycleUnitState last_state = state_;
state_ = state; state_ = state;
OnLifecycleUnitStateChanged(last_state); OnLifecycleUnitStateChanged(last_state, reason);
for (auto& observer : observers_) for (auto& observer : observers_)
observer.OnLifecycleUnitStateChanged(this, last_state); observer.OnLifecycleUnitStateChanged(this, last_state);
} }
void LifecycleUnitBase::OnLifecycleUnitStateChanged( void LifecycleUnitBase::OnLifecycleUnitStateChanged(
LifecycleUnitState last_state) {} LifecycleUnitState last_state,
LifecycleUnitStateChangeReason reason) {}
void LifecycleUnitBase::OnLifecycleUnitVisibilityChanged( void LifecycleUnitBase::OnLifecycleUnitVisibilityChanged(
content::Visibility visibility) { content::Visibility visibility) {
......
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
namespace resource_coordinator { namespace resource_coordinator {
using ::mojom::LifecycleUnitState; using ::mojom::LifecycleUnitState;
using ::mojom::LifecycleUnitStateChangeReason;
// Base class for a LifecycleUnit. // Base class for a LifecycleUnit.
class LifecycleUnitBase : public LifecycleUnit { class LifecycleUnitBase : public LifecycleUnit {
...@@ -30,12 +31,17 @@ class LifecycleUnitBase : public LifecycleUnit { ...@@ -30,12 +31,17 @@ class LifecycleUnitBase : public LifecycleUnit {
protected: protected:
// Sets the state of this LifecycleUnit to |state| and notifies observers. // Sets the state of this LifecycleUnit to |state| and notifies observers.
void SetState(LifecycleUnitState state); // |reason| indicates what caused the state change.
void SetState(LifecycleUnitState state,
LifecycleUnitStateChangeReason reason);
// Invoked when the state of the LifecycleUnit changes, before external // Invoked when the state of the LifecycleUnit changes, before external
// observers are notified. Derived classes can override to add their own // observers are notified. Derived classes can override to add their own
// logic. The default implementation is empty. // logic. The default implementation is empty. |last_state| is the state
virtual void OnLifecycleUnitStateChanged(LifecycleUnitState last_state); // before the change and |reason| indicates what caused the change.
virtual void OnLifecycleUnitStateChanged(
LifecycleUnitState last_state,
LifecycleUnitStateChangeReason reason);
// Notifies observers that the visibility of the LifecycleUnit has changed. // Notifies observers that the visibility of the LifecycleUnit has changed.
void OnLifecycleUnitVisibilityChanged(content::Visibility visibility); void OnLifecycleUnitVisibilityChanged(content::Visibility visibility);
......
...@@ -59,11 +59,13 @@ TEST(LifecycleUnitBaseTest, SetStateNotifiesObservers) { ...@@ -59,11 +59,13 @@ TEST(LifecycleUnitBaseTest, SetStateNotifiesObservers) {
// Observer is notified when the state changes. // Observer is notified when the state changes.
EXPECT_CALL(observer, OnLifecycleUnitStateChanged(&lifecycle_unit, EXPECT_CALL(observer, OnLifecycleUnitStateChanged(&lifecycle_unit,
lifecycle_unit.GetState())); lifecycle_unit.GetState()));
lifecycle_unit.SetState(LifecycleUnitState::DISCARDED); lifecycle_unit.SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
testing::Mock::VerifyAndClear(&observer); testing::Mock::VerifyAndClear(&observer);
// Observer isn't notified when the state stays the same. // Observer isn't notified when the state stays the same.
lifecycle_unit.SetState(LifecycleUnitState::DISCARDED); lifecycle_unit.SetState(LifecycleUnitState::DISCARDED,
LifecycleUnitStateChangeReason::BROWSER_INITIATED);
lifecycle_unit.RemoveObserver(&observer); lifecycle_unit.RemoveObserver(&observer);
} }
......
...@@ -143,8 +143,13 @@ class TabLifecycleUnitSource::TabLifecycleUnit ...@@ -143,8 +143,13 @@ class TabLifecycleUnitSource::TabLifecycleUnit
// Returns the RenderProcessHost associated with this tab. // Returns the RenderProcessHost associated with this tab.
content::RenderProcessHost* GetRenderProcessHost() const; content::RenderProcessHost* GetRenderProcessHost() const;
// Initializes |freeze_timeout_timer_| if not already initialized.
void EnsureFreezeTimeoutTimerInitialized();
// LifecycleUnitBase: // LifecycleUnitBase:
void OnLifecycleUnitStateChanged(LifecycleUnitState last_state) override; void OnLifecycleUnitStateChanged(
LifecycleUnitState last_state,
LifecycleUnitStateChangeReason reason) override;
// content::WebContentsObserver: // content::WebContentsObserver:
void DidStartLoading() override; void DidStartLoading() override;
......
...@@ -206,7 +206,7 @@ class TabManager : public LifecycleUnitObserver, ...@@ -206,7 +206,7 @@ class TabManager : public LifecycleUnitObserver,
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, PauseAndResumeBackgroundTabOpening); FRIEND_TEST_ALL_PREFIXES(TabManagerTest, PauseAndResumeBackgroundTabOpening);
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
ProactiveFastShutdownSharedTabProcess); ProactiveFastShutdownSharedTabProcess);
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, FRIEND_TEST_ALL_PREFIXES(TabManagerTestWithTwoTabs,
ProactiveFastShutdownSingleTabProcess); ProactiveFastShutdownSingleTabProcess);
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
ProactiveFastShutdownWithBeforeunloadHandler); ProactiveFastShutdownWithBeforeunloadHandler);
...@@ -229,7 +229,8 @@ class TabManager : public LifecycleUnitObserver, ...@@ -229,7 +229,8 @@ class TabManager : public LifecycleUnitObserver,
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
TrackingNumberOfLoadedLifecycleUnits); TrackingNumberOfLoadedLifecycleUnits);
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, UrgentFastShutdownSharedTabProcess); FRIEND_TEST_ALL_PREFIXES(TabManagerTest, UrgentFastShutdownSharedTabProcess);
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, UrgentFastShutdownSingleTabProcess); FRIEND_TEST_ALL_PREFIXES(TabManagerTestWithTwoTabs,
UrgentFastShutdownSingleTabProcess);
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, FRIEND_TEST_ALL_PREFIXES(TabManagerTest,
UrgentFastShutdownWithBeforeunloadHandler); UrgentFastShutdownWithBeforeunloadHandler);
FRIEND_TEST_ALL_PREFIXES(TabManagerTest, UrgentFastShutdownWithUnloadHandler); FRIEND_TEST_ALL_PREFIXES(TabManagerTest, UrgentFastShutdownWithUnloadHandler);
......
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