chromeos: Make dark resume hard timeout configurable

This change makes the max timeout, after which DarkResumeController
notifies the  power daemon that it's okay to resuspend, configurable.
This is needed for upcoming boards that need to do more work in dark
resume and will set the command line option added in this change.

BUG=924762
TEST=Unit tests.

Change-Id: I658f4c77d2e6dd3a4391fb60971447426875600d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1649035
Commit-Queue: Abhishek Bhardwaj <abhishekbh@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688274}

chromeos/components/power/dark_resume_controller.cc

@@ -11,15 +11,25 @@
 namespace chromeos {
 namespace system {
 
+namespace {
+
+// The default value of |dark_resume_hard_timeout_| till
+// |PowerManagerInitialized| is called.
+constexpr base::TimeDelta kDefaultDarkResumeHardTimeout =
+    base::TimeDelta::FromSeconds(20);
+
+}  // namespace
+
 // static.
 constexpr base::TimeDelta DarkResumeController::kDarkResumeWakeLockCheckTimeout;
-constexpr base::TimeDelta DarkResumeController::kDarkResumeHardTimeout;
 
 DarkResumeController::DarkResumeController(
     service_manager::Connector* connector)
     : connector_(connector),
       wake_lock_observer_binding_(this),
+      dark_resume_hard_timeout_(kDefaultDarkResumeHardTimeout),
       weak_ptr_factory_(this) {
+  DCHECK(!dark_resume_hard_timeout_.is_zero());
   connector_->BindInterface(device::mojom::kServiceName,
                             mojo::MakeRequest(&wake_lock_provider_));
   PowerManagerClient::Get()->AddObserver(this);
@@ -29,6 +39,11 @@ DarkResumeController::~DarkResumeController() {
   PowerManagerClient::Get()->RemoveObserver(this);
 }
 
+void DarkResumeController::PowerManagerInitialized() {
+  dark_resume_hard_timeout_ =
+      PowerManagerClient::Get()->GetDarkSuspendDelayTimeout();
+}
+
 void DarkResumeController::DarkSuspendImminent() {
   DVLOG(1) << __func__;
   block_suspend_token_ = base::UnguessableToken::Create();
@@ -70,6 +85,10 @@ bool DarkResumeController::IsDarkResumeStateSetForTesting() const {
   return block_suspend_token_ && wake_lock_observer_binding_.is_bound();
 }
 
+base::TimeDelta DarkResumeController::GetHardTimeoutForTesting() const {
+  return dark_resume_hard_timeout_;
+}
+
 bool DarkResumeController::IsDarkResumeStateClearedForTesting() const {
   return !weak_ptr_factory_.HasWeakPtrs() &&
          !wake_lock_check_timer_.IsRunning() &&
@@ -95,7 +114,7 @@ void DarkResumeController::HandleDarkResumeWakeLockCheckTimeout() {
   // the device doesn't stay up indefinitely in dark resume.
   DCHECK(!hard_timeout_timer_.IsRunning());
   hard_timeout_timer_.Start(
-      FROM_HERE, kDarkResumeHardTimeout,
+      FROM_HERE, dark_resume_hard_timeout_,
       base::BindOnce(&DarkResumeController::HandleDarkResumeHardTimeout,
                      weak_ptr_factory_.GetWeakPtr()));
 }

chromeos/components/power/dark_resume_controller.h

@@ -56,12 +56,8 @@ class COMPONENT_EXPORT(CHROMEOS_POWER) DarkResumeController
   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 PowerManagerInitialized() override;
   void DarkSuspendImminent() override;
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
 
@@ -79,6 +75,9 @@ class COMPONENT_EXPORT(CHROMEOS_POWER) DarkResumeController
   // either by re-suspending or transitioning to full resume.
   bool IsDarkResumeStateClearedForTesting() const;
 
+  // Returns |dark_resume_hard_timeout_|.
+  base::TimeDelta GetHardTimeoutForTesting() const;
+
  private:
   // Called |kDarkResumeWakeLockCheckTimeout| after a dark resume. Checks if
   // app suspension wake locks are held. If no wake locks are held then
@@ -112,6 +111,13 @@ class COMPONENT_EXPORT(CHROMEOS_POWER) DarkResumeController
   // Timer used to schedule HandleDarkResumeHardTimeout.
   base::OneShotTimer hard_timeout_timer_;
 
+  // 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. This is
+  // initialized via PowerManagerClient when it's initialization is complete in
+  // |PowerManagerInitialized|. Till then there may be a very small window after
+  // booth when it takes a default value.
+  base::TimeDelta dark_resume_hard_timeout_;
+
   // Used for checking if HandleDarkResumeWakeLockCheckTimeout and
   // HandleDarkResumeHardTimeout run on the same sequence.
   SEQUENCE_CHECKER(dark_resume_tasks_sequence_checker_);

chromeos/components/power/dark_resume_controller_unittest.cc

@@ -9,6 +9,7 @@
 
 #include "base/run_loop.h"
 #include "base/test/scoped_task_environment.h"
+#include "base/time/time.h"
 #include "chromeos/dbus/power/fake_power_manager_client.h"
 #include "services/device/public/cpp/test/test_wake_lock_provider.h"
 #include "services/device/public/mojom/constants.mojom.h"
@@ -127,10 +128,12 @@ TEST_F(DarkResumeControllerTest, CheckSuspendAfterDarkResumeWakeLocksHeld) {
   run_loop.RunUntilIdle();
   EXPECT_TRUE(dark_resume_controller_->IsDarkResumeStateSetForTesting());
 
-  // Move time forward by < |kDarkResumeHardTimeout| and release the
+  // Move time forward by < |dark_resume_hard_timeout_| and release the
   // partial wake lock. This should instantaneously re-suspend the device.
-  task_environment_.FastForwardBy(DarkResumeController::kDarkResumeHardTimeout -
-                                  base::TimeDelta::FromSeconds(1));
+  base::TimeDelta small_delay = base::TimeDelta::FromSeconds(1);
+  ASSERT_GT(dark_resume_controller_->GetHardTimeoutForTesting(), small_delay);
+  task_environment_.FastForwardBy(
+      dark_resume_controller_->GetHardTimeoutForTesting() - small_delay);
   wake_lock_->CancelWakeLock();
   base::RunLoop run_loop2;
   run_loop2.RunUntilIdle();
@@ -153,9 +156,10 @@ TEST_F(DarkResumeControllerTest, CheckSuspendAfterDarkResumeHardTimeout) {
   run_loop.RunUntilIdle();
   EXPECT_TRUE(dark_resume_controller_->IsDarkResumeStateSetForTesting());
 
-  // Move time forward by |kDarkResumeHardTimeout|. At this point the
+  // Move time forward by |dark_resume_hard_timeout_|. At this point the
   // device should re-suspend even though the wake lock is acquired.
-  task_environment_.FastForwardBy(DarkResumeController::kDarkResumeHardTimeout);
+  task_environment_.FastForwardBy(
+      dark_resume_controller_->GetHardTimeoutForTesting());
   EXPECT_EQ(1, GetActiveWakeLocks(WakeLockType::kPreventAppSuspension));
   base::RunLoop run_loop2;
   run_loop2.RunUntilIdle();

chromeos/dbus/constants/dbus_switches.cc

@@ -42,5 +42,10 @@ const char kSmsTestMessages[] = "sms-test-messages";
 // session manager.
 const char kSystemDevMode[] = "system-developer-mode";
 
+// Makes Chrome register the maximum dark suspend delay possible on Chrome OS
+// i.e. give the device the maximum amount of time to do its work in dark
+// resume as is allowed by the power manager.
+const char kRegisterMaxDarkSuspendDelay[] = "register-max-dark-suspend-delay";
+
 }  // namespace switches
 }  // namespace chromeos

chromeos/dbus/constants/dbus_switches.h

@@ -22,6 +22,8 @@ COMPONENT_EXPORT(CHROMEOS_DBUS_CONSTANTS)
 extern const char kSmsTestMessages[];
 COMPONENT_EXPORT(CHROMEOS_DBUS_CONSTANTS)
 extern const char kSystemDevMode[];
+COMPONENT_EXPORT(CHROMEOS_DBUS_CONSTANTS)
+extern const char kRegisterMaxDarkSuspendDelay[];
 
 }  // namespace switches
 }  // namespace chromeos

chromeos/dbus/power/fake_power_manager_client.cc

@@ -27,6 +27,10 @@ FakePowerManagerClient* g_instance = nullptr;
 // Minimum power for a USB power source to be classified as AC.
 constexpr double kUsbMinAcWatts = 24;
 
+// The time power manager will wait before resuspending from a dark resume.
+constexpr base::TimeDelta kDarkSuspendDelayTimeout =
+    base::TimeDelta::FromSeconds(20);
+
 // Callback fired when timer started through |StartArcTimer| expires. In
 // non-test environments this does a potentially blocking call on the UI
 // thread. However, the clients that exercise this code path don't run in
@@ -97,6 +101,7 @@ FakePowerManagerClient::~FakePowerManagerClient() {
 void FakePowerManagerClient::AddObserver(Observer* observer) {
   observers_.AddObserver(observer);
   observer->PowerManagerBecameAvailable(true);
+  observer->PowerManagerInitialized();
 }
 
 void FakePowerManagerClient::RemoveObserver(Observer* observer) {
@@ -361,6 +366,10 @@ void FakePowerManagerClient::DeleteArcTimers(const std::string& tag,
       FROM_HERE, base::BindOnce(std::move(callback), true));
 }
 
+base::TimeDelta FakePowerManagerClient::GetDarkSuspendDelayTimeout() {
+  return kDarkSuspendDelayTimeout;
+}
+
 bool FakePowerManagerClient::PopVideoActivityReport() {
   CHECK(!video_activity_reports_.empty());
   bool fullscreen = video_activity_reports_.front();

chromeos/dbus/power/fake_power_manager_client.h

@@ -127,6 +127,7 @@ class COMPONENT_EXPORT(DBUS_POWER) FakePowerManagerClient
                      VoidDBusMethodCallback callback) override;
   void DeleteArcTimers(const std::string& tag,
                        VoidDBusMethodCallback callback) override;
+  base::TimeDelta GetDarkSuspendDelayTimeout() override;
 
   // Pops the first report from |video_activity_reports_|, returning whether the
   // activity was fullscreen or not. There must be at least one report.

chromeos/dbus/power/power_manager_client.cc

@@ -8,10 +8,12 @@
 
 #include <algorithm>
 #include <memory>
+#include <unordered_map>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
@@ -22,8 +24,10 @@
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/platform_thread.h"
+#include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "base/unguessable_token.h"
+#include "chromeos/dbus/constants/dbus_switches.h"
 #include "chromeos/dbus/power/fake_power_manager_client.h"
 #include "chromeos/dbus/power_manager/backlight.pb.h"
 #include "chromeos/dbus/power_manager/idle.pb.h"
@@ -537,6 +541,10 @@ class PowerManagerClientImpl : public PowerManagerClient {
         base::BindOnce(&OnVoidDBusMethod, std::move(callback)));
   }
 
+  base::TimeDelta GetDarkSuspendDelayTimeout() override {
+    return max_dark_suspend_delay_timeout_;
+  }
+
  private:
   // Returns true if the current thread is the origin thread.
   bool OnOriginThread() {
@@ -584,6 +592,11 @@ class PowerManagerClientImpl : public PowerManagerClient {
     }
   }
 
+  void NotifiyIntiailization() {
+    for (auto& observer : observers_)
+      observer.PowerManagerInitialized();
+  }
+
   void ScreenBrightnessChangedReceived(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
     power_manager::BacklightBrightnessChange proto;
@@ -674,6 +687,12 @@ class PowerManagerClientImpl : public PowerManagerClient {
   }
 
   void OnGetPowerSupplyPropertiesMethod(dbus::Response* response) {
+    // This is the last callback to run after all the initialization in |Init|.
+    // Notify all observers that the initialization is complete.
+    base::ScopedClosureRunner(
+        base::BindOnce(&PowerManagerClientImpl::NotifiyIntiailization,
+                       base::Unretained(this)));
+
     if (!response) {
       POWER_LOG(ERROR) << "Error calling "
                        << power_manager::kGetPowerSupplyPropertiesMethod;
@@ -800,6 +819,12 @@ class PowerManagerClientImpl : public PowerManagerClient {
     if (dark_suspend) {
       dark_suspend_delay_id_ = protobuf.delay_id();
       has_dark_suspend_delay_id_ = true;
+
+      // Set |max_dark_suspend_delay_timeout_| to the minimum time power manager
+      // guarantees before resuspending.
+      max_dark_suspend_delay_timeout_ =
+          base::TimeDelta::FromMilliseconds(protobuf.min_delay_timeout_ms());
+
       POWER_LOG(EVENT) << "Registered dark suspend delay "
                        << dark_suspend_delay_id_;
     } else {
@@ -1027,6 +1052,12 @@ class PowerManagerClientImpl : public PowerManagerClient {
         base::BindOnce(&PowerManagerClientImpl::HandleRegisterSuspendDelayReply,
                        weak_ptr_factory_.GetWeakPtr(), false,
                        power_manager::kRegisterSuspendDelayMethod));
+
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            chromeos::switches::kRegisterMaxDarkSuspendDelay)) {
+      // Negative timeout means request maximum delay.
+      protobuf_request.set_timeout(-1);
+    }
     RegisterSuspendDelayImpl(
         power_manager::kRegisterDarkSuspendDelayMethod, protobuf_request,
         base::BindOnce(&PowerManagerClientImpl::HandleRegisterSuspendDelayReply,
@@ -1098,6 +1129,10 @@ class PowerManagerClientImpl : public PowerManagerClient {
   int32_t dark_suspend_delay_id_ = -1;
   bool has_dark_suspend_delay_id_ = false;
 
+  // The maximum time power manager will wait before resuspending from a dark
+  // resume.
+  base::TimeDelta max_dark_suspend_delay_timeout_;
+
   // powerd-supplied ID corresponding to an imminent (either regular or dark)
   // suspend attempt that is currently being delayed.
   int32_t pending_suspend_id_ = -1;

chromeos/dbus/power/power_manager_client.h

@@ -72,6 +72,9 @@ class COMPONENT_EXPORT(DBUS_POWER) PowerManagerClient {
     // PowerManagerClient if the service's availability is already known.
     virtual void PowerManagerBecameAvailable(bool available) {}
 
+    // Called when the power manager is completely initialized.
+    virtual void PowerManagerInitialized() {}
+
     // Called if the power manager process restarts.
     virtual void PowerManagerRestarted() {}
 
@@ -318,6 +321,9 @@ class COMPONENT_EXPORT(DBUS_POWER) PowerManagerClient {
   virtual void DeleteArcTimers(const std::string& tag,
                                VoidDBusMethodCallback callback) = 0;
 
+  // The time power manager will wait before resuspending from a dark resume.
+  virtual base::TimeDelta GetDarkSuspendDelayTimeout() = 0;
+
   PowerManagerClient();
   virtual ~PowerManagerClient();