Skip to content

Commit

Permalink
power: Add wake lock based logic to re-suspend after dark resume
Browse files Browse the repository at this point in the history
This is a rework of https://crrev.com/c/1302866 monitoring all wake
locks rather than just ARC++ wake locks.

This change adds functionality to support lock screen notifications for
ARC++, Assistant on Chrome OS and other future clients. It adds a new
module that listens to dark resume events from the power manager. It -

1. Starts a timer to check for any current app suspension wake locks.

2. After the timer in 1 expires, if no app suspension wake lock is
acquired the power manager is requested to re-suspend the system. If an
app suspension wake lock is acquired then another hard timeout timer is
set and also an observer to the wake lock being released is set.

3. If the wake lock is released, the system is re-suspended immediately.

4. If the hard timeout in 2 expires and a wake lock is still acquired
then the system is re-suspended immediately.

5. If the system transitions to a full resume all dark resume related
state and timers are cleared.

On top of this change -

1. Changes were added to fix linker errors by declaring static constants
in the source file as well.

2. Refactored some functions in the test code.

BUG=chromium:898297
TEST=Unit tests and end to end test with Android applications and
Assistant.

Change-Id: I695a7dc666b26fce00f6b0f5d0d2017f6556f83a
Reviewed-on: https://chromium-review.googlesource.com/c/1372477
Commit-Queue: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635849}
  • Loading branch information
Abhishek Bhardwaj authored and Commit Bot committed Feb 27, 2019
1 parent 56b5123 commit d98628e
Show file tree
Hide file tree
Showing 7 changed files with 482 additions and 3 deletions.
6 changes: 6 additions & 0 deletions chrome/browser/chromeos/chrome_browser_main_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
#include "chromeos/network/network_cert_loader.h"
#include "chromeos/network/network_handler.h"
#include "chromeos/network/portal_detector/network_portal_detector_stub.h"
#include "chromeos/system/dark_resume_controller.h"
#include "chromeos/system/statistics_provider.h"
#include "chromeos/tpm/install_attributes.h"
#include "chromeos/tpm/tpm_token_loader.h"
Expand Down Expand Up @@ -1039,6 +1040,10 @@ void ChromeBrowserMainPartsChromeos::PostBrowserStart() {
cros_usb_detector_->ConnectToDeviceManager();
}

dark_resume_controller_ =
std::make_unique<chromeos::system::DarkResumeController>(
content::ServiceManagerConnection::GetForProcess()->GetConnector());

ChromeBrowserMainPartsLinux::PostBrowserStart();
}

Expand Down Expand Up @@ -1096,6 +1101,7 @@ void ChromeBrowserMainPartsChromeos::PostMainMessageLoopRun() {
diagnosticsd_bridge_.reset();
scheduler_configuration_manager_.reset();
auto_screen_brightness_controller_.reset();
dark_resume_controller_.reset();

// Detach D-Bus clients before DBusThreadManager is shut down.
idle_action_warning_observer_.reset();
Expand Down
12 changes: 9 additions & 3 deletions chrome/browser/chromeos/chrome_browser_main_chromeos.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class StateController;
namespace arc {
class ArcServiceLauncher;
class VoiceInteractionControllerClient;
}
} // namespace arc

#if BUILDFLAG(ENABLE_CROS_ASSISTANT)
class AssistantClient;
Expand Down Expand Up @@ -55,11 +55,10 @@ namespace default_app_order {
class ExternalLoader;
}


namespace internal {
class DBusServices;
class SystemTokenCertDBInitializer;
}
} // namespace internal

namespace power {
namespace ml {
Expand All @@ -72,6 +71,10 @@ class Controller;
} // namespace auto_screen_brightness
} // namespace power

namespace system {
class DarkResumeController;
} // namespace system

// ChromeBrowserMainParts implementation for chromeos specific code.
// NOTE: Chromeos UI (Ash) support should be added to
// ChromeBrowserMainExtraPartsAsh instead. This class should not depend on
Expand Down Expand Up @@ -163,6 +166,9 @@ class ChromeBrowserMainPartsChromeos : public ChromeBrowserMainPartsLinux {

std::unique_ptr<CrosUsbDetector> cros_usb_detector_;

std::unique_ptr<chromeos::system::DarkResumeController>
dark_resume_controller_;

DISALLOW_COPY_AND_ASSIGN(ChromeBrowserMainPartsChromeos);
};

Expand Down
9 changes: 9 additions & 0 deletions chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@ component("chromeos") {
":chromeos_export",
"//base",
"//base:i18n",
"//chromeos/dbus",
"//components/policy/proto",
"//google_apis",
"//services/device/public/mojom",
"//services/network/public/cpp:cpp",
"//services/service_manager/public/cpp",
"//third_party/protobuf:protobuf_lite",
]
sources = [
Expand Down Expand Up @@ -75,6 +78,8 @@ component("chromeos") {
"process_proxy/process_proxy_registry.h",
"system/cpu_temperature_reader.cc",
"system/cpu_temperature_reader.h",
"system/dark_resume_controller.cc",
"system/dark_resume_controller.h",
"system/devicemode.cc",
"system/devicemode.h",
"system/factory_ping_embargo_check.cc",
Expand Down Expand Up @@ -206,8 +211,11 @@ test("chromeos_unittests") {
"//mojo/core/embedder",
"//net",
"//net:test_support",
"//services/device/public/cpp/test:test_support",
"//services/device/public/mojom",
"//services/network:test_support",
"//services/network/public/cpp",
"//services/service_manager/public/cpp/test:test_support",
"//testing/gmock",
"//testing/gtest",
"//third_party/icu",
Expand All @@ -228,6 +236,7 @@ test("chromeos_unittests") {
"process_proxy/process_output_watcher_unittest.cc",
"process_proxy/process_proxy_unittest.cc",
"system/cpu_temperature_reader_unittest.cc",
"system/dark_resume_controller_unittest.cc",
"system/factory_ping_embargo_check_unittest.cc",
"system/name_value_pairs_parser_unittest.cc",
"test/run_all_unittests.cc",
Expand Down
3 changes: 3 additions & 0 deletions chromeos/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ include_rules = [
"+crypto",
"+google_apis/gaia",
"+media/base/video_facing.h",
"+mojo/public",
"+net",
"+services/device/public",
"+services/network/public",
"+services/service_manager/public",
"+third_party/cros_system_api",
"+third_party/protobuf",

Expand Down
140 changes: 140 additions & 0 deletions chromeos/system/dark_resume_controller.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
// Copyright 2019 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.
#include "chromeos/system/dark_resume_controller.h"

#include <utility>

#include "chromeos/dbus/dbus_thread_manager.h"
#include "services/device/public/mojom/constants.mojom.h"
#include "services/device/public/mojom/wake_lock_provider.mojom.h"

namespace chromeos {
namespace system {

// static.
constexpr base::TimeDelta DarkResumeController::kDarkResumeWakeLockCheckTimeout;
constexpr base::TimeDelta DarkResumeController::kDarkResumeHardTimeout;

DarkResumeController::DarkResumeController(
service_manager::Connector* connector)
: connector_(connector),
wake_lock_observer_binding_(this),
weak_ptr_factory_(this) {
connector_->BindInterface(device::mojom::kServiceName,
mojo::MakeRequest(&wake_lock_provider_));
chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
this);
}

DarkResumeController::~DarkResumeController() {
chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(
this);
}

void DarkResumeController::DarkSuspendImminent() {
DVLOG(1) << __func__;
suspend_readiness_cb_ = chromeos::DBusThreadManager::Get()
->GetPowerManagerClient()
->GetSuspendReadinessCallback(FROM_HERE);
// Schedule task that will check for any wake locks acquired in dark resume.
DCHECK(!wake_lock_check_timer_.IsRunning());
wake_lock_check_timer_.Start(
FROM_HERE, kDarkResumeWakeLockCheckTimeout,
base::BindOnce(
&DarkResumeController::HandleDarkResumeWakeLockCheckTimeout,
weak_ptr_factory_.GetWeakPtr()));
}

void DarkResumeController::SuspendDone(const base::TimeDelta& sleep_duration) {
DVLOG(1) << __func__;
// Clear any dark resume state when the device resumes.
ClearDarkResumeState();
}

void DarkResumeController::OnWakeLockDeactivated(
device::mojom::WakeLockType type) {
// If this callback fires then one of two scenarios happened -
// 1. No wake lock was held after |kDarkResumeWakeLockCheckTimeout|.
// 2. Wake lock was held but was released before the hard timeout.
//
// The device is now ready to re-suspend after this dark resume event. Tell
// the power daemon to re-suspend and invalidate any other state associated
// with dark resume.
DVLOG(1) << __func__;
// The observer is only registered once dark resume starts.
DCHECK(suspend_readiness_cb_);
std::move(suspend_readiness_cb_).Run();
ClearDarkResumeState();
}

bool DarkResumeController::IsDarkResumeStateSetForTesting() const {
return !suspend_readiness_cb_.is_null() &&
wake_lock_observer_binding_.is_bound();
}

bool DarkResumeController::IsDarkResumeStateClearedForTesting() const {
return !weak_ptr_factory_.HasWeakPtrs() &&
!wake_lock_check_timer_.IsRunning() &&
!hard_timeout_timer_.IsRunning() && suspend_readiness_cb_.is_null() &&
!wake_lock_observer_binding_.is_bound();
}

void DarkResumeController::HandleDarkResumeWakeLockCheckTimeout() {
DVLOG(1) << __func__;
DCHECK_CALLED_ON_VALID_SEQUENCE(dark_resume_tasks_sequence_checker_);
wake_lock_check_timer_.Stop();
// Setup observer for wake lock deactivation. If a wake lock is not activated
// this calls back immediately, else whenever the wake lock is deactivated.
// The device will be suspended on a deactivation notification i.e. in
// OnDeactivation.
device::mojom::WakeLockObserverPtr observer;
wake_lock_observer_binding_.Bind(mojo::MakeRequest(&observer));
wake_lock_provider_->NotifyOnWakeLockDeactivation(
device::mojom::WakeLockType::kPreventDisplaySleepAllowDimming,
std::move(observer));

// Schedule 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.
DCHECK(!hard_timeout_timer_.IsRunning());
hard_timeout_timer_.Start(
FROM_HERE, kDarkResumeHardTimeout,
base::BindOnce(&DarkResumeController::HandleDarkResumeHardTimeout,
weak_ptr_factory_.GetWeakPtr()));
}

void DarkResumeController::HandleDarkResumeHardTimeout() {
DVLOG(1) << __func__;
DCHECK_CALLED_ON_VALID_SEQUENCE(dark_resume_tasks_sequence_checker_);
hard_timeout_timer_.Stop();
// Enough is enough. Tell power daemon it's okay to suspend.
DCHECK(suspend_readiness_cb_);
std::move(suspend_readiness_cb_).Run();
ClearDarkResumeState();
}

void DarkResumeController::ClearDarkResumeState() {
DVLOG(1) << __func__;
// Reset the callback that is used to trigger a re-suspend. Won't be needed
// if the dark resume state machine is ending.
suspend_readiness_cb_.Reset();

// This automatically invalidates any WakeLockObserver and associated callback
// in this case OnDeactivation.
wake_lock_observer_binding_.Close();

// Stops timer and invalidates HandleDarkResumeWakeLockCheckTimeout.
wake_lock_check_timer_.Stop();

// Stops timer and invalidates HandleDarkResumeHardTimeout.
hard_timeout_timer_.Stop();

// At this point all pending callbacks should be invalidated. This is a last
// fail safe to not have any lingering tasks associated with the dark resume
// state machine.
weak_ptr_factory_.InvalidateWeakPtrs();
}

} // namespace system
} // namespace chromeos
132 changes: 132 additions & 0 deletions chromeos/system/dark_resume_controller.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// 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 CHROMEOS_SYSTEM_DARK_RESUME_CONTROLLER_H_
#define CHROMEOS_SYSTEM_DARK_RESUME_CONTROLLER_H_

#include <memory>

#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/timer/timer.h"
#include "chromeos/chromeos_export.h"
#include "chromeos/dbus/power_manager_client.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "services/device/public/mojom/wake_lock.mojom.h"
#include "services/device/public/mojom/wake_lock_provider.mojom.h"
#include "services/service_manager/public/cpp/connector.h"

namespace chromeos {
namespace system {

// This class listens to dark resume events from the power manager and makes
// decisions on whether to re-suspend the device or keep the device in dark
// resume based on the wake locks activated at the time of a dark resume. It -
//
// 1. Starts a timer to check for any activated app suspension wake locks. This
// gives services time to do work in dark resume. They can activate a wake lock
// to indicate to the system that they are still doing work.
//
// 2. After the timer in 1 expires, an observer for wake lock deactivation is
// set. Also a hard timeout timer is scheduled.
//
// 3. If no wake lock is held then the observer gets notified immediately and
// the power manager is requested to re-suspend the system.
//
// 4. If an app suspension wake lock is acquired then either -
// - The observer from 2 is notified when the wake lock is deactivated and the
// power manager is requested to re-suspend the system.
// Or
// - The hard timeout timer from 2 fires and the power manager is requested to
// re-suspend the system.
//
// 5. If the system transitions to a full resume all dark resume related state
// and timers are cleared as the system wakes up.
class CHROMEOS_EXPORT DarkResumeController
: public chromeos::PowerManagerClient::Observer,
public device::mojom::WakeLockObserver {
public:
explicit DarkResumeController(service_manager::Connector* connector);
~DarkResumeController() override;

// 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);

// chromeos::PowerManagerClient::Observer overrides.
void DarkSuspendImminent() override;
void SuspendDone(const base::TimeDelta& sleep_duration) override;

// mojom::WakeLockObserver overrides.
void OnWakeLockDeactivated(device::mojom::WakeLockType type) override;

// 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 IsDarkResumeStateSetForTesting() const;

// Return true iff all dark resume related state is reset i.e. suspend
// readiness callback is null, wake lock release event has no observers,
// wake lock check timer is reset, hard timeout timer is reset and there are
// no in flight tasks. This should be true when device exits dark resume
// either by re-suspending or transitioning to full resume.
bool IsDarkResumeStateClearedForTesting() const;

private:
// Called |kDarkResumeWakeLockCheckTimeout| after a dark resume. Checks if
// app suspension wake locks are held. If no wake locks are held then
// re-suspends the device else schedules HandleDarkResumeHardTimeout.
void HandleDarkResumeWakeLockCheckTimeout();

// Called |kDarkResumeHardTimeout| after a
// HandleDarkResumeWakeLockCheckTimeout. Clears all dark resume state and
// re-suspends the device.
void HandleDarkResumeHardTimeout();

// Clears all state associated with dark resume.
void ClearDarkResumeState();

// Used for acquiring, releasing and observing wake locks.
device::mojom::WakeLockProviderPtr wake_lock_provider_;

// Not owned by this instance.
service_manager::Connector* const connector_;

// Called when system is ready to supend after a DarkSupendImminent i.e.
// after a dark resume.
base::OnceClosure suspend_readiness_cb_;

// The binding used to implement device::mojom::WakeLockObserver.
mojo::Binding<device::mojom::WakeLockObserver> wake_lock_observer_binding_;

// Timer used to schedule HandleDarkResumeWakeLockCheckTimeout.
base::OneShotTimer wake_lock_check_timer_;

// Timer used to schedule HandleDarkResumeHardTimeout.
base::OneShotTimer hard_timeout_timer_;

// Used for checking if HandleDarkResumeWakeLockCheckTimeout and
// HandleDarkResumeHardTimeout run on the same sequence.
SEQUENCE_CHECKER(dark_resume_tasks_sequence_checker_);

// This is invalidated in ClearDarkResumeState as a fail safe measure to clear
// any lingering timer callbacks or wake lock observer callbacks. This is a
// good to have but not a necessity as ClearDarkResumeState cancels any dark
// resume state machine related tasks via other means. In the future if other
// tasks or callbacks need to be added separate from the dark resume state
// machine lifetime then a separate factory needs to be created and used.
base::WeakPtrFactory<DarkResumeController> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(DarkResumeController);
};

} // namespace system
} // namespace chromeos

#endif // CHROMEOS_SYSTEM_DARK_RESUME_CONTROLLER_H_
Loading

0 comments on commit d98628e

Please sign in to comment.