Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(539)

Unified Diff: content/browser/memory/memory_coordinator_impl_unittest.cc

Issue 2943283002: Update default policy for memory coordinator (Closed)
Patch Set: Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/memory/memory_coordinator_impl.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/memory/memory_coordinator_impl_unittest.cc
diff --git a/content/browser/memory/memory_coordinator_impl_unittest.cc b/content/browser/memory/memory_coordinator_impl_unittest.cc
index ab466a175110702c1b00839ac37f039ffc809b3f..f641678a9688675d7239bd6a272e5db8cb28c1ce 100644
--- a/content/browser/memory/memory_coordinator_impl_unittest.cc
+++ b/content/browser/memory/memory_coordinator_impl_unittest.cc
@@ -111,6 +111,31 @@ class TestMemoryCoordinatorDelegate : public MemoryCoordinatorDelegate {
DISALLOW_COPY_AND_ASSIGN(TestMemoryCoordinatorDelegate);
};
+class MockMemoryCoordinatorPolicy : public MemoryCoordinatorImpl::Policy {
+ public:
+ explicit MockMemoryCoordinatorPolicy(MemoryCoordinatorImpl* coordinator)
+ : coordinator_(coordinator) {
+ DCHECK(coordinator_);
+ }
+
+ ~MockMemoryCoordinatorPolicy() override = default;
+
+ void OnCriticalCondition() override { coordinator_->DiscardTab(); }
+
+ void OnConditionChanged(MemoryCondition prev, MemoryCondition next) override {
+ EXPECT_NE(prev, next);
+ last_condition_ = next;
+ }
+
+ MemoryCondition last_condition() const { return last_condition_; }
+
+ private:
+ MemoryCoordinatorImpl* coordinator_ = nullptr;
+ MemoryCondition last_condition_ = MemoryCondition::NORMAL;
+
+ DISALLOW_COPY_AND_ASSIGN(MockMemoryCoordinatorPolicy);
+};
+
// A MemoryCoordinatorImpl that can be directly constructed.
class TestMemoryCoordinatorImpl : public MemoryCoordinatorImpl {
public:
@@ -130,6 +155,7 @@ class TestMemoryCoordinatorImpl : public MemoryCoordinatorImpl {
: MemoryCoordinatorImpl(task_runner,
base::MakeUnique<MockMemoryMonitor>()) {
SetDelegateForTesting(base::MakeUnique<TestMemoryCoordinatorDelegate>());
+ SetPolicyForTesting(base::MakeUnique<MockMemoryCoordinatorPolicy>(this));
SetTickClockForTesting(task_runner->GetMockTickClock());
}
@@ -163,6 +189,10 @@ class TestMemoryCoordinatorImpl : public MemoryCoordinatorImpl {
return static_cast<TestMemoryCoordinatorDelegate*>(delegate());
}
+ MockMemoryCoordinatorPolicy* GetPolicy() {
+ return static_cast<MockMemoryCoordinatorPolicy*>(policy());
+ }
+
// Wrapper of MemoryCoordinator::SetMemoryState that also calls RunUntilIdle.
bool SetChildMemoryState(
int render_process_id, MemoryState memory_state) {
@@ -285,31 +315,6 @@ TEST_F(MemoryCoordinatorImplTest, SetChildMemoryState) {
EXPECT_EQ(mojom::MemoryState::THROTTLED, cmc->state());
}
-// TODO(bashi): Move policy specific tests into a separate file.
-TEST_F(MemoryCoordinatorImplTest, OnChildVisibilityChanged) {
- auto* child = coordinator_->CreateChildMemoryCoordinator(1);
-
- coordinator_->memory_condition_ = MemoryCondition::NORMAL;
- coordinator_->policy_->OnChildVisibilityChanged(1, true);
- RunUntilIdle();
- EXPECT_EQ(mojom::MemoryState::NORMAL, child->state());
- coordinator_->policy_->OnChildVisibilityChanged(1, false);
- RunUntilIdle();
-#if defined(OS_ANDROID)
- EXPECT_EQ(mojom::MemoryState::THROTTLED, child->state());
-#else
- EXPECT_EQ(mojom::MemoryState::NORMAL, child->state());
-#endif
-
- coordinator_->memory_condition_ = MemoryCondition::CRITICAL;
- coordinator_->policy_->OnChildVisibilityChanged(1, true);
- RunUntilIdle();
- EXPECT_EQ(mojom::MemoryState::THROTTLED, child->state());
- coordinator_->policy_->OnChildVisibilityChanged(1, false);
- RunUntilIdle();
- EXPECT_EQ(mojom::MemoryState::THROTTLED, child->state());
-}
-
TEST_F(MemoryCoordinatorImplTest, CalculateNextCondition) {
auto* condition_observer = coordinator_->condition_observer_.get();
@@ -332,69 +337,25 @@ TEST_F(MemoryCoordinatorImplTest, CalculateNextCondition) {
condition_observer->CalculateNextCondition());
}
-TEST_F(MemoryCoordinatorImplTest, UpdateCondition) {
- auto* condition_observer = coordinator_->condition_observer_.get();
-
- auto* foreground_child = coordinator_->CreateChildMemoryCoordinator(1);
- auto* background_child = coordinator_->CreateChildMemoryCoordinator(2);
- auto iter = coordinator_->children().find(2);
- iter->second.is_visible = false;
+TEST_F(MemoryCoordinatorImplTest, UpdateConditionIfNeeded) {
+ auto* policy = coordinator_->GetPolicy();
- {
- // Transition happens (NORMAL -> CRITICAL).
- // All processes should be in THROTTLED memory state.
- MockMemoryCoordinatorClient client;
- base::MemoryCoordinatorClientRegistry::GetInstance()->Register(&client);
- GetMockMemoryMonitor()->SetFreeMemoryUntilCriticalMB(0);
- condition_observer->UpdateCondition();
- task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(40));
- RunUntilIdle();
- EXPECT_TRUE(client.did_state_changed());
- EXPECT_EQ(base::MemoryState::THROTTLED, client.state());
- EXPECT_EQ(mojom::MemoryState::THROTTLED, foreground_child->state());
- EXPECT_EQ(mojom::MemoryState::THROTTLED, background_child->state());
- base::MemoryCoordinatorClientRegistry::GetInstance()->Unregister(&client);
- }
+ // The initial condition is NORMAL.
+ EXPECT_EQ(MemoryCondition::NORMAL, coordinator_->GetMemoryCondition());
+ coordinator_->UpdateConditionIfNeeded(MemoryCondition::NORMAL);
+ RunUntilIdle();
+ EXPECT_EQ(MemoryCondition::NORMAL, policy->last_condition());
- {
- // No transtion (NORMAL -> NORMAL). OnStateChange shouldn't be called.
- MockMemoryCoordinatorClient client;
- base::MemoryCoordinatorClientRegistry::GetInstance()->Register(&client);
- coordinator_->memory_condition_ = MemoryCondition::NORMAL;
- GetMockMemoryMonitor()->SetFreeMemoryUntilCriticalMB(1000);
- condition_observer->UpdateCondition();
- RunUntilIdle();
- EXPECT_FALSE(client.did_state_changed());
- EXPECT_EQ(base::MemoryState::NORMAL, client.state());
- base::MemoryCoordinatorClientRegistry::GetInstance()->Unregister(&client);
- }
+ // Change to CRITICAL condition.
+ coordinator_->UpdateConditionIfNeeded(MemoryCondition::CRITICAL);
+ RunUntilIdle();
+ EXPECT_EQ(MemoryCondition::CRITICAL, policy->last_condition());
- {
- // Transition happends but browser state change is delayed
- // (CRITICAL -> NORMAL).
- base::TimeDelta transition_interval =
- coordinator_->minimum_state_transition_period_ +
- base::TimeDelta::FromSeconds(1);
- coordinator_->memory_condition_ = MemoryCondition::NORMAL;
- coordinator_->last_state_change_ = task_runner_->NowTicks();
- GetMockMemoryMonitor()->SetFreeMemoryUntilCriticalMB(0);
- condition_observer->UpdateCondition();
- EXPECT_EQ(base::MemoryState::THROTTLED,
- coordinator_->GetCurrentMemoryState());
-
- // Back to NORMAL condition before |minimum_state_transition_period_| is
- // passed. At this point the browser's state shouldn't be changed.
- GetMockMemoryMonitor()->SetFreeMemoryUntilCriticalMB(1000);
- condition_observer->UpdateCondition();
- task_runner_->FastForwardBy(transition_interval / 2);
- EXPECT_EQ(MemoryCondition::NORMAL, coordinator_->GetMemoryCondition());
- EXPECT_EQ(base::MemoryState::THROTTLED,
- coordinator_->GetCurrentMemoryState());
-
- // |minimum_state_transition_period_| is passed. State should be changed.
- task_runner_->FastForwardBy(transition_interval / 2);
- EXPECT_EQ(base::MemoryState::NORMAL, coordinator_->GetCurrentMemoryState());
- }
+ // Make sure that OnConditionChanged() won't get called when the condition is
+ // unchanged. CHECK_NE() in MockMemoryCoordinatorPolicy ensures it.
+ coordinator_->UpdateConditionIfNeeded(MemoryCondition::CRITICAL);
+ RunUntilIdle();
+ EXPECT_EQ(MemoryCondition::CRITICAL, policy->last_condition());
}
// TODO(bashi): Move ForceSetMemoryCondition?
@@ -468,53 +429,6 @@ TEST_F(MemoryCoordinatorImplTest, DiscardTabUnderCritical) {
EXPECT_EQ(2, delegate->discard_tab_count());
}
-// TODO(bashi): Move policy specific tests into a separate file.
-TEST_F(MemoryCoordinatorImplTest, OnCriticalCondition) {
- MockMemoryCoordinatorClient client;
- base::MemoryCoordinatorClientRegistry::GetInstance()->Register(&client);
- auto* child1 = coordinator_->CreateChildMemoryCoordinator(1);
- auto* child2 = coordinator_->CreateChildMemoryCoordinator(2);
- auto* delegate = coordinator_->GetDelegate();
- base::TimeDelta interval = base::TimeDelta::FromSeconds(31);
-
- // child1: Foreground, child2: Background
- coordinator_->policy_->OnChildVisibilityChanged(1, true);
- coordinator_->policy_->OnChildVisibilityChanged(2, false);
-
- // Purge memory from all children regardless of their visibility.
- task_runner_->FastForwardBy(interval);
- coordinator_->policy_->OnCriticalCondition();
- RunUntilIdle();
- task_runner_->FastForwardBy(interval);
- coordinator_->policy_->OnCriticalCondition();
- RunUntilIdle();
- EXPECT_EQ(2, delegate->discard_tab_count());
- EXPECT_EQ(0, client.purge_memory_calls());
- EXPECT_EQ(1, child1->purge_memory_calls());
- EXPECT_EQ(1, child2->purge_memory_calls());
-
- // Purge memory from browser process only after we asked all children to
- // purge memory.
- task_runner_->FastForwardBy(interval);
- coordinator_->policy_->OnCriticalCondition();
- RunUntilIdle();
- EXPECT_EQ(3, delegate->discard_tab_count());
- EXPECT_EQ(1, client.purge_memory_calls());
- EXPECT_EQ(1, child1->purge_memory_calls());
- EXPECT_EQ(1, child2->purge_memory_calls());
-
- // Don't request purging for a certain period of time if we already requested.
- task_runner_->FastForwardBy(interval);
- coordinator_->policy_->OnCriticalCondition();
- RunUntilIdle();
- EXPECT_EQ(4, delegate->discard_tab_count());
- EXPECT_EQ(1, client.purge_memory_calls());
- EXPECT_EQ(1, child1->purge_memory_calls());
- EXPECT_EQ(1, child2->purge_memory_calls());
-
- base::MemoryCoordinatorClientRegistry::GetInstance()->Unregister(&client);
-}
-
#if defined(OS_ANDROID)
// TODO(jcivelli): Broken on Android. http://crbug.com/678665
#define MAYBE_GetStateForProcess DISABLED_GetStateForProcess
« no previous file with comments | « content/browser/memory/memory_coordinator_impl.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698