Commit ab5a3160 authored by Abhishek Bhardwaj's avatar Abhishek Bhardwaj Committed by Commit Bot

Revert "Reland "arc: Add wake lock based logic to re-suspend after dark resume""

This reverts commit 91b547aa. The same
functionality will be submitted in a separate CL that will re-suspend
based on system wide wake locks and not just ARC++ wake locks.

BUG=chromium:898297
TEST=ArcWakeLockBridgeTest unit tests.

Change-Id: I626733d89dfa7aadeb620997091abd0a2f958c10
Reviewed-on: https://chromium-review.googlesource.com/c/1372333
Commit-Queue: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: default avatarYusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634598}
parent 81d80354
......@@ -68,7 +68,6 @@ static_library("arc") {
"volume_mounter/arc_volume_mounter_bridge.h",
"wake_lock/arc_wake_lock_bridge.cc",
"wake_lock/arc_wake_lock_bridge.h",
"wake_lock/wake_lock_observer.h",
]
public_deps = [
......
......@@ -8,9 +8,6 @@
#include "base/bind.h"
#include "base/logging.h"
#include "base/memory/singleton.h"
#include "base/observer_list.h"
#include "base/task/post_task.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/power_policy_controller.h"
#include "components/arc/arc_bridge_service.h"
#include "components/arc/arc_browser_context_keyed_service_factory_base.h"
......@@ -50,8 +47,7 @@ class ArcWakeLockBridgeFactory
// WakeLockRequester requests a wake lock from the device service in response
// to wake lock requests of a given type from Android. A count is kept of
// outstanding Android requests so that only a single actual wake lock is
// used.
// outstanding Android requests so that only a single actual wake lock is used.
class ArcWakeLockBridge::WakeLockRequester {
public:
WakeLockRequester(device::mojom::WakeLockType type,
......@@ -81,9 +77,6 @@ class ArcWakeLockBridge::WakeLockRequester {
}
wake_lock_->RequestWakeLock();
for (auto& observer : observers_)
observer.OnWakeLockAcquire();
}
// Decrements the number of outstanding Android requests. Cancels the device
......@@ -102,27 +95,10 @@ class ArcWakeLockBridge::WakeLockRequester {
}
DCHECK(wake_lock_);
DVLOG(1) << "Partial wake lock force release. Count: " << wake_lock_count_;
DVLOG(1) << "Partial wake force release. Count: " << wake_lock_count_;
wake_lock_->CancelWakeLock();
for (auto& observer : observers_)
observer.OnWakeLockRelease();
}
bool IsWakeLockHeld() const { return wake_lock_count_ > 0; }
void AddObserver(WakeLockObserver* observer) {
DCHECK(observer);
observers_.AddObserver(observer);
}
void RemoveObserver(WakeLockObserver* observer) {
DCHECK(observer);
observers_.RemoveObserver(observer);
}
bool HasObservers() const { return observers_.might_have_observers(); }
// Runs the message loop until replies have been received for all pending
// requests on |wake_lock_|.
void FlushForTesting() {
......@@ -143,8 +119,6 @@ class ArcWakeLockBridge::WakeLockRequester {
// Lazily initialized in response to first request.
device::mojom::WakeLockPtr wake_lock_;
base::ObserverList<WakeLockObserver>::Unchecked observers_;
DISALLOW_COPY_AND_ASSIGN(WakeLockRequester);
};
......@@ -165,9 +139,6 @@ ArcWakeLockBridge* ArcWakeLockBridge::GetForBrowserContextForTesting(
return ArcWakeLockBridgeFactory::GetForBrowserContextForTesting(context);
}
constexpr base::TimeDelta ArcWakeLockBridge::kDarkResumeWakeLockCheckTimeout;
constexpr base::TimeDelta ArcWakeLockBridge::kDarkResumeHardTimeout;
ArcWakeLockBridge::ArcWakeLockBridge(content::BrowserContext* context,
ArcBridgeService* bridge_service)
: arc_bridge_service_(bridge_service),
......@@ -175,16 +146,11 @@ ArcWakeLockBridge::ArcWakeLockBridge(content::BrowserContext* context,
weak_ptr_factory_(this) {
arc_bridge_service_->wake_lock()->SetHost(this);
arc_bridge_service_->wake_lock()->AddObserver(this);
chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
this);
}
ArcWakeLockBridge::~ArcWakeLockBridge() {
arc_bridge_service_->wake_lock()->RemoveObserver(this);
arc_bridge_service_->wake_lock()->SetHost(nullptr);
// In case some this wasn't cleared while handling a dark resume.
GetWakeLockRequester(device::mojom::WakeLockType::kPreventAppSuspension)
->RemoveObserver(this);
}
void ArcWakeLockBridge::OnConnectionClosed() {
......@@ -192,6 +158,11 @@ void ArcWakeLockBridge::OnConnectionClosed() {
wake_lock_requesters_.clear();
}
void ArcWakeLockBridge::FlushWakeLocksForTesting() {
for (const auto& it : wake_lock_requesters_)
it.second->FlushForTesting();
}
void ArcWakeLockBridge::AcquirePartialWakeLock(
AcquirePartialWakeLockCallback callback) {
GetWakeLockRequester(device::mojom::WakeLockType::kPreventAppSuspension)
......@@ -206,104 +177,6 @@ void ArcWakeLockBridge::ReleasePartialWakeLock(
std::move(callback).Run(true);
}
void ArcWakeLockBridge::DarkSuspendImminent() {
DVLOG(1) << __func__;
suspend_readiness_cb_ = chromeos::DBusThreadManager::Get()
->GetPowerManagerClient()
->GetSuspendReadinessCallback(FROM_HERE);
// Post task that will check for any wake locks acquired in dark resume.
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&ArcWakeLockBridge::HandleDarkResumeWakeLockCheckTimeout,
dark_resume_weak_ptr_factory_.GetWeakPtr()),
kDarkResumeWakeLockCheckTimeout);
}
void ArcWakeLockBridge::SuspendDone(const base::TimeDelta& sleep_duration) {
DVLOG(1) << __func__;
// Clear any dark resume state when the device resumes.
ClearDarkResumeState();
}
void ArcWakeLockBridge::OnWakeLockRelease() {
// This observer is only registered once dark resume starts.
DCHECK(suspend_readiness_cb_);
DVLOG(1) << __func__;
// At this point the instance has done its work, tell the power daemon to
// re-suspend.
std::move(suspend_readiness_cb_).Run();
ClearDarkResumeState();
}
void ArcWakeLockBridge::FlushWakeLocksForTesting() {
for (const auto& it : wake_lock_requesters_)
it.second->FlushForTesting();
}
bool ArcWakeLockBridge::IsSuspendReadinessStateSetForTesting() const {
return !suspend_readiness_cb_.is_null();
}
bool ArcWakeLockBridge::WakeLockHasObserversForTesting(
device::mojom::WakeLockType type) {
return GetWakeLockRequester(
device::mojom::WakeLockType::kPreventAppSuspension)
->HasObservers();
}
void ArcWakeLockBridge::HandleDarkResumeWakeLockCheckTimeout() {
DVLOG(1) << __func__;
DCHECK_CALLED_ON_VALID_SEQUENCE(dark_resume_tasks_sequence_checker_);
// Check if any wake locks are held at this point. If not, then it's assumed
// the instance either acquired and released one or had no reason to acquire
// one in the first place. If it wants to after this then too bad, tell the
// power daemon to re-suspend and invalidate any other state associated with
// dark resume.
if (!GetWakeLockRequester(device::mojom::WakeLockType::kPreventAppSuspension)
->IsWakeLockHeld()) {
DVLOG(1) << "Wake lock not held during check";
std::move(suspend_readiness_cb_).Run();
ClearDarkResumeState();
return;
}
DVLOG(1) << "Wake lock held during check";
// If a wake lock is held then register for a wake lock release
// notification. As soon as it's released tell power daemon to re-suspend.
// If the instance takes a long time then tell powerd daemon to re-suspend
// after a hard timeout irrespective of wake locks held.
GetWakeLockRequester(device::mojom::WakeLockType::kPreventAppSuspension)
->AddObserver(this);
// Post task that will tell the power daemon to re-suspend after a dark
// resume irrespective of any state. This is a last resort timeout to ensure
// the device doesn't stay up indefinitely in dark resume.
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&ArcWakeLockBridge::HandleDarkResumeHardTimeout,
dark_resume_weak_ptr_factory_.GetWeakPtr()),
kDarkResumeHardTimeout);
}
void ArcWakeLockBridge::HandleDarkResumeHardTimeout() {
DVLOG(1) << __func__;
DCHECK_CALLED_ON_VALID_SEQUENCE(dark_resume_tasks_sequence_checker_);
// Enough is enough. Tell power daemon it's okay to suspend.
DCHECK(suspend_readiness_cb_);
std::move(suspend_readiness_cb_).Run();
ClearDarkResumeState();
}
void ArcWakeLockBridge::ClearDarkResumeState() {
DVLOG(1) << __func__;
// Invalidate all other state associated with dark resume.
suspend_readiness_cb_.Reset();
GetWakeLockRequester(device::mojom::WakeLockType::kPreventAppSuspension)
->RemoveObserver(this);
dark_resume_weak_ptr_factory_.InvalidateWeakPtrs();
}
ArcWakeLockBridge::WakeLockRequester* ArcWakeLockBridge::GetWakeLockRequester(
device::mojom::WakeLockType type) {
auto it = wake_lock_requesters_.find(type);
......
......@@ -10,11 +10,8 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/sequence_checker.h"
#include "chromeos/dbus/power_manager_client.h"
#include "components/arc/common/wake_lock.mojom.h"
#include "components/arc/connection_observer.h"
#include "components/arc/wake_lock/wake_lock_observer.h"
#include "components/keyed_service/core/keyed_service.h"
#include "content/public/common/service_manager_connection.h"
#include "mojo/public/cpp/bindings/binding.h"
......@@ -33,9 +30,7 @@ class ArcBridgeService;
// Sets wake up timers / alarms based on calls from the instance.
class ArcWakeLockBridge : public KeyedService,
public ConnectionObserver<mojom::WakeLockInstance>,
public mojom::WakeLockHost,
public chromeos::PowerManagerClient::Observer,
public WakeLockObserver {
public mojom::WakeLockHost {
public:
// Returns the factory instance for this class.
static BrowserContextKeyedServiceFactory* GetFactory();
......@@ -59,36 +54,13 @@ class ArcWakeLockBridge : public KeyedService,
// ConnectionObserver<mojom::WakeLockInstance>::Observer overrides.
void OnConnectionClosed() override;
// mojom::WakeLockHost overrides.
void AcquirePartialWakeLock(AcquirePartialWakeLockCallback callback) override;
void ReleasePartialWakeLock(ReleasePartialWakeLockCallback callback) override;
// chromeos::PowerManagerClient::Observer overrides.
void DarkSuspendImminent() override;
void SuspendDone(const base::TimeDelta& sleep_duration) override;
// WakeLockObserver override.
void OnWakeLockRelease() override;
// Runs the message loop until replies have been received for all pending
// device service requests in |wake_lock_requesters_|.
void FlushWakeLocksForTesting();
// Checks if |suspend_readiness_cb_| is set.
bool IsSuspendReadinessStateSetForTesting() const;
// Returns true iff wake lock of |type| has observers.
bool WakeLockHasObserversForTesting(device::mojom::WakeLockType type);
// Time after a dark resume when wake lock count is checked and a decision is
// made to re-suspend or wait for wake lock release.
static constexpr base::TimeDelta kDarkResumeWakeLockCheckTimeout =
base::TimeDelta::FromSeconds(3);
// Max time to wait for wake lock release after a wake lock check after a dark
// resume. After this time the system is asked to re-suspend.
static constexpr base::TimeDelta kDarkResumeHardTimeout =
base::TimeDelta::FromSeconds(10);
// mojom::WakeLockHost overrides.
void AcquirePartialWakeLock(AcquirePartialWakeLockCallback callback) override;
void ReleasePartialWakeLock(ReleasePartialWakeLockCallback callback) override;
private:
class WakeLockRequester;
......@@ -96,20 +68,6 @@ class ArcWakeLockBridge : public KeyedService,
// Returns the WakeLockRequester for |type|, creating one if needed.
WakeLockRequester* GetWakeLockRequester(device::mojom::WakeLockType type);
// Runs |kDarkResumeWakeLockCheckTimeout| time delta after a dark resume.
// Checks if app suspension wake locks (partial wake locks for Android) are
// held. If no wake locks are held then re-suspends the device else schedules
// |HandleDarkResumeHardTimeout|.
void HandleDarkResumeWakeLockCheckTimeout();
// Runs |kDarkResumeHardTimeout| time delta after a
// |HandleDarkResumeWakeLockCheckTimeout|. Clears all dark resume state and
// re-suspends the device.
void HandleDarkResumeHardTimeout();
// Clears all state associated with dark resume.
void ClearDarkResumeState();
ArcBridgeService* const arc_bridge_service_; // Owned by ArcServiceManager.
// If non-null, used instead of the process-wide connector to fetch services.
......@@ -120,21 +78,8 @@ class ArcWakeLockBridge : public KeyedService,
std::map<device::mojom::WakeLockType, std::unique_ptr<WakeLockRequester>>
wake_lock_requesters_;
// Called when system is ready to supend after a |DarkSupendImminent| i.e.
// after a dark resume.
base::OnceClosure suspend_readiness_cb_;
mojo::Binding<mojom::WakeLockHost> binding_;
// Used for checking if |DarkResumeWakeLockCheckTimeout| and
// |DarkResumeHardTimeout| run on the same sequence.
SEQUENCE_CHECKER(dark_resume_tasks_sequence_checker_);
// Factory used to schedule and cancel
// |HandleDarkResumeWakeLockCheckTimeout| and |HandleDarkResumeHardTimeout|.
// At any point either none or one of these tasks is in flight.
base::WeakPtrFactory<ArcWakeLockBridge> dark_resume_weak_ptr_factory_{this};
base::WeakPtrFactory<ArcWakeLockBridge> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(ArcWakeLockBridge);
......
......@@ -10,8 +10,6 @@
#include "base/bind.h"
#include "base/run_loop.h"
#include "base/test/scoped_task_environment.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_power_manager_client.h"
#include "components/arc/arc_bridge_service.h"
#include "components/arc/common/power.mojom.h"
#include "components/arc/test/connection_holder_util.h"
......@@ -33,10 +31,6 @@ class ArcWakeLockBridgeTest : public testing::Test {
base::test::ScopedTaskEnvironment::MainThreadType::MOCK_TIME),
wake_lock_provider_(
connector_factory_.RegisterInstance(device::mojom::kServiceName)) {
fake_power_manager_client_ = new chromeos::FakePowerManagerClient;
chromeos::DBusThreadManager::GetSetterForTesting()->SetPowerManagerClient(
base::WrapUnique(fake_power_manager_client_));
bridge_service_ = std::make_unique<ArcBridgeService>();
wake_lock_bridge_ =
std::make_unique<ArcWakeLockBridge>(nullptr, bridge_service_.get());
......@@ -48,27 +42,6 @@ class ArcWakeLockBridgeTest : public testing::Test {
~ArcWakeLockBridgeTest() override { DestroyWakeLockInstance(); }
protected:
// Creates a FakeWakeLockInstance for |bridge_service_|. This results in
// ArcWakeLockBridge::OnInstanceReady() being called.
void CreateWakeLockInstance() {
instance_ = std::make_unique<FakeWakeLockInstance>();
bridge_service_->wake_lock()->SetInstance(instance_.get());
WaitForInstanceReady(bridge_service_->wake_lock());
}
// Destroys the FakeWakeLockInstance. This results in
// ArcWakeLockBridge::OnInstanceClosed() being called.
void DestroyWakeLockInstance() {
if (!instance_)
return;
bridge_service_->wake_lock()->CloseInstance(instance_.get());
instance_.reset();
}
device::TestWakeLockProvider* GetWakeLockProvider() {
return &wake_lock_provider_;
}
// Returns true iff there is no failure acquiring a system wake lock.
bool AcquirePartialWakeLock() {
base::RunLoop loop;
......@@ -91,29 +64,30 @@ class ArcWakeLockBridgeTest : public testing::Test {
return result;
}
// Return true iff all dark resume related state is set i.e the suspend
// readiness callback is set and wake lock release event has observers.
bool IsDarkResumeStateSet() const {
return wake_lock_bridge_->IsSuspendReadinessStateSetForTesting() &&
wake_lock_bridge_->WakeLockHasObserversForTesting(
WakeLockType::kPreventAppSuspension);
// Creates a FakeWakeLockInstance for |bridge_service_|. This results in
// ArcWakeLockBridge::OnInstanceReady() being called.
void CreateWakeLockInstance() {
instance_ = std::make_unique<FakeWakeLockInstance>();
bridge_service_->wake_lock()->SetInstance(instance_.get());
WaitForInstanceReady(bridge_service_->wake_lock());
}
// Destroys the FakeWakeLockInstance. This results in
// ArcWakeLockBridge::OnInstanceClosed() being called.
void DestroyWakeLockInstance() {
if (!instance_)
return;
bridge_service_->wake_lock()->CloseInstance(instance_.get());
instance_.reset();
}
// Return true iff all dark resume related state is reset. This should be true
// when device exits dark resume either by re-suspending or transitioning to
// full resume.
bool IsDarkResumeStateReset() const {
return !wake_lock_bridge_->WakeLockHasObserversForTesting(
WakeLockType::kPreventAppSuspension) &&
!wake_lock_bridge_->IsSuspendReadinessStateSetForTesting();
device::TestWakeLockProvider* GetWakeLockProvider() {
return &wake_lock_provider_;
}
private:
base::test::ScopedTaskEnvironment scoped_task_environment_;
// Owned by chromeos::DBusThreadManager.
chromeos::FakePowerManagerClient* fake_power_manager_client_;
private:
service_manager::TestConnectorFactory connector_factory_;
device::TestWakeLockProvider wake_lock_provider_;
......@@ -178,94 +152,4 @@ TEST_F(ArcWakeLockBridgeTest, ReleaseWakeLockOnInstanceClosed) {
WakeLockType::kPreventAppSuspension));
}
TEST_F(ArcWakeLockBridgeTest, CheckSuspendAfterDarkResumeNoWakeLocksHeld) {
// Trigger a dark resume event, move time forward to trigger a wake lock check
// and check if a re-suspend happened if no wake locks were acquired.
fake_power_manager_client_->SendDarkSuspendImminent();
scoped_task_environment_.FastForwardBy(
ArcWakeLockBridge::kDarkResumeWakeLockCheckTimeout);
base::RunLoop run_loop;
run_loop.RunUntilIdle();
EXPECT_TRUE(IsDarkResumeStateReset());
// Trigger a dark resume event, acquire and release a wake lock and move time
// forward to trigger a wake lock check. The device should re-suspend in this
// case since no wake locks were held at the time of the wake lock check.
fake_power_manager_client_->SendDarkSuspendImminent();
EXPECT_TRUE(AcquirePartialWakeLock());
EXPECT_TRUE(ReleasePartialWakeLock());
scoped_task_environment_.FastForwardBy(
ArcWakeLockBridge::kDarkResumeWakeLockCheckTimeout);
base::RunLoop run_loop2;
run_loop2.RunUntilIdle();
EXPECT_TRUE(IsDarkResumeStateReset());
}
TEST_F(ArcWakeLockBridgeTest, CheckSuspendAfterDarkResumeWakeLocksHeld) {
// Trigger a dark resume event, acquire a wake lock and move time forward to a
// wake lock check. At this point the system shouldn't re-suspend i.e. the
// suspend readiness callback should be set and wake lock release should have
// observers.
fake_power_manager_client_->SendDarkSuspendImminent();
EXPECT_TRUE(AcquirePartialWakeLock());
scoped_task_environment_.FastForwardBy(
ArcWakeLockBridge::kDarkResumeWakeLockCheckTimeout);
base::RunLoop run_loop;
run_loop.RunUntilIdle();
EXPECT_TRUE(IsDarkResumeStateSet());
// Move time forward by < |kDarkResumeHardTimeout| and release the
// partial wake lock.This should instantaneously re-suspend the device.
scoped_task_environment_.FastForwardBy(
ArcWakeLockBridge::kDarkResumeHardTimeout -
base::TimeDelta::FromSeconds(1));
EXPECT_TRUE(ReleasePartialWakeLock());
base::RunLoop run_loop2;
run_loop2.RunUntilIdle();
EXPECT_TRUE(IsDarkResumeStateReset());
}
TEST_F(ArcWakeLockBridgeTest, CheckSuspendAfterDarkResumeHardTimeout) {
// Trigger a dark resume event, acquire a wake lock and move time forward to a
// wake lock check. At this point the system shouldn't re-suspend i.e. the
// suspend readiness callback should be set and wake lock release should have
// observers.
fake_power_manager_client_->SendDarkSuspendImminent();
EXPECT_TRUE(AcquirePartialWakeLock());
scoped_task_environment_.FastForwardBy(
ArcWakeLockBridge::kDarkResumeWakeLockCheckTimeout);
base::RunLoop run_loop;
run_loop.RunUntilIdle();
EXPECT_TRUE(IsDarkResumeStateSet());
// Move time forward by |kDarkResumeHardTimeout|. At this point the
// device should re-suspend even though the wake lock is acquired.
scoped_task_environment_.FastForwardBy(
ArcWakeLockBridge::kDarkResumeHardTimeout);
EXPECT_EQ(1, GetWakeLockProvider()->GetActiveWakeLocksOfType(
WakeLockType::kPreventAppSuspension));
base::RunLoop run_loop2;
run_loop2.RunUntilIdle();
EXPECT_TRUE(IsDarkResumeStateReset());
}
TEST_F(ArcWakeLockBridgeTest, CheckStateResetAfterSuspendDone) {
// Trigger a dark resume event, acquire a wake lock and move time forward to a
// wake lock check. At this point the system shouldn't re-suspend i.e. the
// suspend readiness callback should be set and wake lock release should have
// observers.
fake_power_manager_client_->SendDarkSuspendImminent();
EXPECT_TRUE(AcquirePartialWakeLock());
scoped_task_environment_.FastForwardBy(
ArcWakeLockBridge::kDarkResumeWakeLockCheckTimeout);
base::RunLoop run_loop;
run_loop.RunUntilIdle();
EXPECT_TRUE(IsDarkResumeStateSet());
// Trigger suspend done event. Check if state is reset as dark resume would be
// exited.
fake_power_manager_client_->SendSuspendDone();
EXPECT_TRUE(IsDarkResumeStateReset());
}
} // namespace arc
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef COMPONENTS_ARC_WAKE_LOCK_WAKE_LOCK_OBSERVER_H_
#define COMPONENTS_ARC_WAKE_LOCK_WAKE_LOCK_OBSERVER_H_
namespace arc {
// This is an interface for classes that want to learn when Android wake locks
// are acquired or released. Observer should register themselves by calling the
// overriding class's AddObserver() method.
class WakeLockObserver {
public:
virtual ~WakeLockObserver() = default;
// Called when the tracked wake lock is acquired the first time i.e.
// number of holders increases to 1.
virtual void OnWakeLockAcquire() {}
// Called when the tracked wake lock is released the last time i.e. the number
// of holders goes to 0.
virtual void OnWakeLockRelease() {}
};
} // namespace arc
#endif // COMPONENTS_ARC_WAKE_LOCK_WAKE_LOCK_OBSERVER_H_
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