[Cast] Abstract screen power off logic to an interface.

* Extract original implementation from display_settings_manager_impl.cc to
screen_power_controller_aura.cc
* This allows the display to be enabled/disabled when allow_screen_power_off_ is
set to be false/true after screen is off.
* This handles multiple power requests at the same time (e.g. SetScreenOn() and
SetScreenOff() at the same time).
* Add logs when screen is successfully powered on/off.
* Add unittest to ScreenPowerControllerAura.

Bug: b/162546246
Bug: b/173504008
Test: cast_display_settings_unittests --single-process-tests
--gtest_filter=ScreenPowerControllerAuraTest.*
Test: logcat | grep "screen_power_controller", mute the mic, in low light:
display off mode, and toggle the display timeout on and off. Logs should show
that "Screen is powered on/off".

Change-Id: If470d72eeb11c53e6dea8eefbd63f71e0f6c4dd2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391333
Commit-Queue: Mingyu Kang <kenkangxgwe@google.com>
Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Reviewed-by: Sean Topping <seantopping@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835044}

chromecast/BUILD.gn

@@ -82,6 +82,7 @@ cast_test_group("cast_tests") {
     "//chromecast/media:cast_media_unittests",
     "//chromecast/metrics:cast_metrics_unittest",
     "//chromecast/net:cast_net_unittests",
+    "//chromecast/ui/display_settings:cast_display_settings_unittests",
     "//chromecast/system/reboot:cast_reboot_unittests",
     "//content/test:content_unittests",
     "//crypto:crypto_unittests",

chromecast/ui/display_settings/BUILD.gn

@@ -11,6 +11,7 @@ cast_source_set("display_settings") {
     "brightness_animation.h",
     "color_temperature_animation.cc",
     "color_temperature_animation.h",
+    "screen_power_controller.h",
   ]
 
   deps = [
@@ -30,6 +31,61 @@ cast_source_set("display_settings") {
       "gamma_configurator.h",
     ]
 
-    deps += [ "//ui/aura" ]
+    deps += [
+      ":screen_power_controller_aura",
+      "//ui/aura"
+    ]
+  } else {
+    deps += [
+      ":screen_power_controller_default",
+    ]
+  }
+}
+
+cast_source_set("screen_power_controller_default") {
+  sources = [
+    "screen_power_controller.h",
+    "screen_power_controller_default.h",
+    "screen_power_controller_default.cc",
+  ]
+
+  deps = [
+    "//base",
+  ]
+}
+
+cast_source_set("screen_power_controller_aura") {
+  sources = [
+    "screen_power_controller.h",
+    "screen_power_controller_aura.h",
+    "screen_power_controller_aura.cc",
+  ]
+
+  deps = [
+    "//base",
+  ]
+}
+
+test("cast_display_settings_unittests") {
+  testonly = true
+
+  sources = []
+
+  deps = [
+    "//base",
+    "//base/test:run_all_unittests",
+    "//base/test:test_support",
+    "//testing/gmock",
+    "//testing/gtest",
+  ]
+
+  if (use_aura) {
+    sources += [
+      "screen_power_controller_aura_unittest.cc",
+    ]
+
+    deps += [
+      ":screen_power_controller_aura",
+    ]
   }
 }

chromecast/ui/display_settings/screen_power_controller.h

+// Copyright 2020 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 CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_H_
+#define CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_H_
+
+#include <memory>
+
+#include "base/callback.h"
+#include "base/time/time.h"
+
+namespace chromecast {
+
+// This is an interface that controls the screen power. There are two stages
+// when the screen is off: brightness off (zero) and power off. Whether the
+// screen backlight is set to zero or the screen is powered off is abstracted in
+// this class. Higher-level services generally do not care which mode it's in.
+// The interface doesn't perform actual actions but invokes its delegate to turn
+// on/off the screen, which is easy to isolate with mock classes and write
+// unittest.
+class ScreenPowerController {
+ public:
+  // The actual class that controls the brightness and the power of the
+  // screen.
+  class Delegate {
+   public:
+    using PowerToggleCallback = base::OnceCallback<void(bool)>;
+
+    virtual ~Delegate() = default;
+    // Turns the screen power on and calls the |callback| with a bool that
+    // indicates if the action is successful.
+    virtual void SetScreenPowerOn(PowerToggleCallback callback) = 0;
+    // Turns the screen power off and calls the |callback| with a bool that
+    // indicates if the action is successful.
+    virtual void SetScreenPowerOff(PowerToggleCallback callback) = 0;
+    // Turns the screen brightness on/off according to |brightness_on| with an
+    // animation of |duration|.
+    virtual void SetScreenBrightnessOn(bool brightness_on,
+                                       base::TimeDelta duration) = 0;
+  };
+
+  // The factory method that creates the derived implementation class.
+  static std::unique_ptr<ScreenPowerController> Create(Delegate* delegate);
+
+  virtual ~ScreenPowerController() = default;
+
+  // Turns on the screen.
+  virtual void SetScreenOn() = 0;
+  // Turns off the screen and lets the derived class decide which stage should
+  // the screen transfer to.
+  virtual void SetScreenOff() = 0;
+  // Sets whether the screen is allow to power off.
+  virtual void SetAllowScreenPowerOff(bool allow_power_off) = 0;
+  // Returns the state whether the screen is on/off.
+  virtual bool IsScreenOn() const = 0;
+};
+
+}  // namespace chromecast
+
+#endif  // CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_H_

chromecast/ui/display_settings/screen_power_controller_aura.cc

+// Copyright 2020 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 "chromecast/ui/display_settings/screen_power_controller_aura.h"
+
+#include "base/callback.h"
+#include "base/check.h"
+#include "base/logging.h"
+#include "base/threading/sequenced_task_runner_handle.h"
+#include "base/time/time.h"
+
+namespace chromecast {
+
+namespace {
+
+constexpr base::TimeDelta kScreenOnOffDuration =
+    base::TimeDelta::FromMilliseconds(200);
+// These delays are needed to ensure there are no visible artifacts due to the
+// backlight turning on prior to the LCD fully initializing or vice-versa.
+// TODO(b/161140301): Make this configurable for different products
+// TODO(b/161268188): Remove these if the delays can be handled by the kernel
+constexpr base::TimeDelta kDisplayPowerOnDelay =
+    base::TimeDelta::FromMilliseconds(35);
+constexpr base::TimeDelta kDisplayPowerOffDelay =
+    base::TimeDelta::FromMilliseconds(85);
+
+}  // namespace
+
+std::unique_ptr<ScreenPowerController> ScreenPowerController::Create(
+    Delegate* delegate) {
+  return std::make_unique<ScreenPowerControllerAura>(delegate);
+}
+
+ScreenPowerControllerAura::ScreenPowerControllerAura(
+    ScreenPowerController::Delegate* delegate)
+    : screen_on_(true),
+      screen_power_on_(true),
+      allow_screen_power_off_(false),
+      pending_task_(PendingTask::kNone),
+      delegate_(delegate),
+      weak_factory_(this) {
+  DCHECK(delegate_);
+}
+
+ScreenPowerControllerAura::~ScreenPowerControllerAura() = default;
+
+void ScreenPowerControllerAura::SetScreenOn() {
+  if (pending_task_ != PendingTask::kNone) {
+    pending_task_ = PendingTask::kOn;
+    return;
+  }
+
+  if (!screen_on_) {
+    pending_task_ = PendingTask::kOn;
+    TriggerPendingTask();
+  }
+}
+
+void ScreenPowerControllerAura::SetScreenOff() {
+  if (pending_task_ != PendingTask::kNone) {
+    if (!allow_screen_power_off_) {
+      pending_task_ = PendingTask::kBrightnessOff;
+    } else {
+      pending_task_ = PendingTask::kPowerOff;
+    }
+    return;
+  }
+
+  if (!allow_screen_power_off_ && (screen_on_ || !screen_power_on_)) {
+    pending_task_ = PendingTask::kBrightnessOff;
+    TriggerPendingTask();
+  } else if (allow_screen_power_off_ && screen_power_on_) {
+    pending_task_ = PendingTask::kPowerOff;
+    TriggerPendingTask();
+  }
+}
+
+void ScreenPowerControllerAura::SetAllowScreenPowerOff(bool allow_power_off) {
+  LOG(INFO) << "SetAllowScreenPowerOff set to "
+            << (allow_power_off ? "true" : "false");
+  allow_screen_power_off_ = allow_power_off;
+  switch (pending_task_) {
+    case PendingTask::kOn:
+      return;
+    case PendingTask::kBrightnessOff:
+    case PendingTask::kPowerOff:
+      if (!allow_screen_power_off_) {
+        pending_task_ = PendingTask::kBrightnessOff;
+      } else {
+        pending_task_ = PendingTask::kPowerOff;
+      }
+      return;
+    case PendingTask::kNone:
+      if (!screen_on_) {
+        // Set screen power off if screen is already off.
+        SetScreenOff();
+      }
+      return;
+  }
+}
+
+bool ScreenPowerControllerAura::IsScreenOn() const {
+  return screen_on_;
+}
+
+void ScreenPowerControllerAura::TriggerPendingTask() {
+  switch (pending_task_) {
+    case PendingTask::kOn:
+      if (screen_on_) {
+        NOTREACHED();
+      } else if (screen_power_on_) {
+        SetScreenBrightnessOn(true);
+        pending_task_ = PendingTask::kNone;
+      } else {
+        SetScreenPowerOn();
+      }
+      return;
+    case PendingTask::kBrightnessOff:
+      if (screen_on_) {
+        SetScreenBrightnessOn(false);
+        pending_task_ = PendingTask::kNone;
+      } else if (screen_power_on_) {
+        pending_task_ = PendingTask::kNone;
+      } else {
+        SetScreenPowerOn();
+      }
+      return;
+    case PendingTask::kPowerOff:
+      if (screen_on_) {
+        SetScreenBrightnessOn(false);
+        SetScreenPowerOff();
+      } else if (screen_power_on_) {
+        SetScreenPowerOff();
+      } else {
+        NOTREACHED();
+      }
+      return;
+    case PendingTask::kNone:
+      NOTREACHED();
+  }
+}
+
+void ScreenPowerControllerAura::SetScreenBrightnessOn(bool brightness_on) {
+  delegate_->SetScreenBrightnessOn(brightness_on, kScreenOnOffDuration);
+  screen_on_ = brightness_on;
+}
+
+void ScreenPowerControllerAura::SetScreenPowerOn() {
+  delegate_->SetScreenPowerOn(
+      base::BindOnce(&ScreenPowerControllerAura::OnScreenPoweredOn,
+                     weak_factory_.GetWeakPtr()));
+}
+
+void ScreenPowerControllerAura::SetScreenPowerOff() {
+  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(&ScreenPowerControllerAura::OnDisplayOffTimeoutCompleted,
+                     weak_factory_.GetWeakPtr()),
+      kDisplayPowerOffDelay + kScreenOnOffDuration);
+}
+
+void ScreenPowerControllerAura::OnScreenPoweredOn(bool succeeded) {
+  if (!succeeded) {
+    // Fatal since the user has no other way of turning the screen on if this
+    // failed.
+    LOG(FATAL) << "Failed to power on the screen";
+    return;
+  }
+  LOG(INFO) << "Screen is powered on";
+  screen_power_on_ = true;
+
+  switch (pending_task_) {
+    case PendingTask::kOn:
+    case PendingTask::kBrightnessOff:
+      // TODO(b/161268188): This can be simplified and the delays removed if
+      // backlight timing is handled by the kernel
+      base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+          FROM_HERE,
+          base::BindOnce(
+              &ScreenPowerControllerAura::OnDisplayOnTimeoutCompleted,
+              weak_factory_.GetWeakPtr()),
+          kDisplayPowerOnDelay);
+      return;
+    case PendingTask::kPowerOff:
+      TriggerPendingTask();
+      return;
+    case PendingTask::kNone:
+      NOTREACHED();
+      return;
+  }
+}
+
+void ScreenPowerControllerAura::OnScreenPoweredOff(bool succeeded) {
+  if (!succeeded) {
+    LOG(ERROR) << "Failed to power off the screen";
+    return;
+  }
+  LOG(INFO) << "Screen is powered off";
+  screen_power_on_ = false;
+
+  switch (pending_task_) {
+    case PendingTask::kOn:
+    case PendingTask::kBrightnessOff:
+      TriggerPendingTask();
+      return;
+    case PendingTask::kPowerOff:
+      pending_task_ = PendingTask::kNone;
+      return;
+    case PendingTask::kNone:
+      NOTREACHED();
+      return;
+  }
+}
+
+void ScreenPowerControllerAura::OnDisplayOnTimeoutCompleted() {
+  switch (pending_task_) {
+    case PendingTask::kBrightnessOff:
+      pending_task_ = PendingTask::kNone;
+      return;
+    case PendingTask::kOn:
+    case PendingTask::kPowerOff:
+      TriggerPendingTask();
+      return;
+    case PendingTask::kNone:
+      NOTREACHED();
+      return;
+  }
+}
+
+void ScreenPowerControllerAura::OnDisplayOffTimeoutCompleted() {
+  switch (pending_task_) {
+    case PendingTask::kOn:
+      TriggerPendingTask();
+      return;
+    case PendingTask::kBrightnessOff:
+      pending_task_ = PendingTask::kNone;
+      return;
+    case PendingTask::kPowerOff:
+      delegate_->SetScreenPowerOff(
+          base::BindOnce(&ScreenPowerControllerAura::OnScreenPoweredOff,
+                         weak_factory_.GetWeakPtr()));
+      return;
+    case PendingTask::kNone:
+      NOTREACHED();
+      return;
+  }
+}
+
+}  // namespace chromecast

chromecast/ui/display_settings/screen_power_controller_aura.h

+// Copyright 2020 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 CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_AURA_H_
+#define CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_AURA_H_
+
+#include "base/containers/flat_map.h"
+#include "base/memory/weak_ptr.h"
+#include "chromecast/ui/display_settings/screen_power_controller.h"
+
+namespace chromecast {
+
+// This class implements the ScreenPowerController with AURA enabled. The class
+// doesn't depend on AURA but it will only be available if |use_aura| build flag
+// is true. The class wraps the logic of the following transition graph:
+//
+//          SetScreenOff() && !allow_screen_off
+//     On <==================================> Brightness Off
+//     /\             SetScreenOn()                 /\
+//     ||                                           ||
+//     ||                                           ||
+//     ||                          allow_screen_off || !allow_screen_off
+//     ||                                           ||
+//     ||                                           ||
+//     ||   SetScreenOff() && allow_screen_off      \/
+//     ++=====================================> Power Off
+//                     SetScreenOn()
+//
+//                        Screen Stage Transitions
+class ScreenPowerControllerAura : public ScreenPowerController {
+ public:
+  explicit ScreenPowerControllerAura(ScreenPowerController::Delegate* delegate);
+  ~ScreenPowerControllerAura() override;
+
+  void SetScreenOn() override;
+  void SetScreenOff() override;
+  void SetAllowScreenPowerOff(bool allow_power_off) override;
+  bool IsScreenOn() const override;
+
+ private:
+  enum PendingTask {
+    kNone = 0,
+    kOn,
+    kBrightnessOff,
+    kPowerOff,
+  };
+  void TriggerPendingTask();
+  void SetScreenBrightnessOn(bool brightness_on);
+  void SetScreenPowerOn();
+  void SetScreenPowerOff();
+  void OnScreenPoweredOn(bool succeeded);
+  void OnScreenPoweredOff(bool succeeded);
+  void OnDisplayOnTimeoutCompleted();
+  void OnDisplayOffTimeoutCompleted();
+
+  bool screen_on_;
+  bool screen_power_on_;
+  bool allow_screen_power_off_;
+  PendingTask pending_task_;
+  ScreenPowerController::Delegate* delegate_;
+
+  base::WeakPtrFactory<ScreenPowerControllerAura> weak_factory_;
+};
+
+}  // namespace chromecast
+
+#endif  // CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_AURA_H_

chromecast/ui/display_settings/screen_power_controller_aura_unittest.cc

+// Copyright 2020 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 "chromecast/ui/display_settings/screen_power_controller_aura.h"
+
+#include <memory>
+
+#include "base/callback_forward.h"
+#include "base/test/gmock_callback_support.h"
+#include "base/test/task_environment.h"
+#include "base/time/time.h"
+#include "chromecast/ui/display_settings/screen_power_controller.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromecast {
+namespace test {
+namespace {
+
+using ::testing::_;
+
+// These constants should be the same as screen_power_controller_aura.cc.
+constexpr base::TimeDelta kScreenOnOffDuration =
+    base::TimeDelta::FromMilliseconds(200);
+constexpr base::TimeDelta kDisplayPowerOnDelay =
+    base::TimeDelta::FromMilliseconds(35);
+constexpr base::TimeDelta kDisplayPowerOffDelay =
+    base::TimeDelta::FromMilliseconds(85);
+
+class MockScreenPowerControllerDelegate
+    : public ScreenPowerController::Delegate {
+ public:
+  virtual ~MockScreenPowerControllerDelegate() = default;
+
+  // ScreenPowerController::Delegate implementation:
+  MOCK_METHOD(void, SetScreenPowerOn, (PowerToggleCallback callback), (override));
+  MOCK_METHOD(void, SetScreenPowerOff, (PowerToggleCallback callback), (override));
+  MOCK_METHOD(void,
+              SetScreenBrightnessOn,
+              (bool brightness_on, base::TimeDelta duration),
+              (override));
+};
+
+class ScreenPowerControllerAuraTest : public ::testing::Test {
+ public:
+  ScreenPowerControllerAuraTest()
+      : task_env_(base::test::TaskEnvironment::TimeSource::MOCK_TIME),
+        delegate_(std::make_unique<MockScreenPowerControllerDelegate>()),
+        screen_power_controller_(
+            ScreenPowerController::Create(delegate_.get())) {}
+
+ protected:
+  void BrightnessOff() {
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(false, kScreenOnOffDuration));
+    screen_power_controller_->SetScreenOff();
+  }
+
+  void PowerOff() {
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(false, kScreenOnOffDuration));
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_))
+            .WillOnce(base::test::RunOnceCallback<0>(true));
+    screen_power_controller_->SetAllowScreenPowerOff(true);
+    screen_power_controller_->SetScreenOff();
+    task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration);
+  }
+
+  base::test::TaskEnvironment task_env_;
+  std::unique_ptr<MockScreenPowerControllerDelegate> delegate_;
+  std::unique_ptr<ScreenPowerController> screen_power_controller_;
+};
+
+}  // namespace
+
+TEST_F(ScreenPowerControllerAuraTest, PowerOnToBrightnessOff) {
+  EXPECT_CALL(*delegate_, SetScreenBrightnessOn(false, kScreenOnOffDuration));
+  EXPECT_CALL(*delegate_, SetScreenPowerOn(_)).Times(0);
+  EXPECT_CALL(*delegate_, SetScreenPowerOff(_)).Times(0);
+  screen_power_controller_->SetScreenOff();
+}
+
+TEST_F(ScreenPowerControllerAuraTest, PowerOnToPowerOff) {
+  {
+    ::testing::InSequence s;
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(false, kScreenOnOffDuration));
+    EXPECT_CALL(*delegate_, SetScreenPowerOn(_)).Times(0);
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_))
+        .WillOnce(base::test::RunOnceCallback<0>(true));
+    screen_power_controller_->SetAllowScreenPowerOff(true);
+    screen_power_controller_->SetScreenOff();
+    task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration);
+  }
+}
+
+TEST_F(ScreenPowerControllerAuraTest, BrightnessOffToPowerOff) {
+  BrightnessOff();
+
+  EXPECT_CALL(*delegate_, SetScreenBrightnessOn(_, _)).Times(0);
+  EXPECT_CALL(*delegate_, SetScreenPowerOn(_)).Times(0);
+  EXPECT_CALL(*delegate_, SetScreenPowerOff(_))
+      .WillOnce(base::test::RunOnceCallback<0>(true));
+  screen_power_controller_->SetAllowScreenPowerOff(true);
+  task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration);
+}
+
+TEST_F(ScreenPowerControllerAuraTest, BrightnessOffToBrightnessOn) {
+  BrightnessOff();
+
+  EXPECT_CALL(*delegate_, SetScreenBrightnessOn(true, kScreenOnOffDuration))
+      .Times(1);
+  EXPECT_CALL(*delegate_, SetScreenPowerOn(_)).Times(0);
+  EXPECT_CALL(*delegate_, SetScreenPowerOff(_)).Times(0);
+  screen_power_controller_->SetScreenOn();
+}
+
+TEST_F(ScreenPowerControllerAuraTest, PowerOffToBrightnessOn) {
+  PowerOff();
+
+  {
+    ::testing::InSequence s;
+    EXPECT_CALL(*delegate_, SetScreenPowerOn(_))
+        .WillOnce(base::test::RunOnceCallback<0>(true));
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_)).Times(0);
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(true, kScreenOnOffDuration))
+        .Times(1);
+    screen_power_controller_->SetScreenOn();
+    task_env_.FastForwardBy(kDisplayPowerOnDelay);
+  }
+}
+
+TEST_F(ScreenPowerControllerAuraTest, PowerOffToBrightnessOff) {
+  PowerOff();
+
+  EXPECT_CALL(*delegate_, SetScreenPowerOn(_))
+      .WillOnce(base::test::RunOnceCallback<0>(true));
+  EXPECT_CALL(*delegate_, SetScreenPowerOff(_)).Times(0);
+  EXPECT_CALL(*delegate_, SetScreenBrightnessOn(_, _)).Times(0);
+  screen_power_controller_->SetAllowScreenPowerOff(false);
+  task_env_.FastForwardBy(kDisplayPowerOnDelay);
+}
+
+TEST_F(ScreenPowerControllerAuraTest, PowerOnToPowerOffToPowerOn) {
+  {
+    ::testing::InSequence s;
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(false, kScreenOnOffDuration));
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_)).Times(0);
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(true, kScreenOnOffDuration));
+    EXPECT_CALL(*delegate_, SetScreenPowerOn(_)).Times(0);
+    screen_power_controller_->SetAllowScreenPowerOff(true);
+    screen_power_controller_->SetScreenOff();
+    screen_power_controller_->SetScreenOn();
+    task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration +
+                            kDisplayPowerOnDelay);
+  }
+}
+
+TEST_F(ScreenPowerControllerAuraTest, PowerOnToPowerOffToBrightnessOff) {
+  {
+    ::testing::InSequence s;
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(false, kScreenOnOffDuration));
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_)).Times(0);
+    EXPECT_CALL(*delegate_, SetScreenPowerOn(_)).Times(0);
+    screen_power_controller_->SetAllowScreenPowerOff(true);
+    screen_power_controller_->SetScreenOff();
+    screen_power_controller_->SetAllowScreenPowerOff(false);
+    task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration +
+                            kDisplayPowerOnDelay);
+  }
+}
+
+TEST_F(ScreenPowerControllerAuraTest, PowerOffToPowerOnToPowerOff) {
+  PowerOff();
+
+  {
+    ::testing::InSequence s;
+    EXPECT_CALL(*delegate_, SetScreenPowerOn(_))
+        .WillOnce(base::test::RunOnceCallback<0>(true));
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(_, _)).Times(0);
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_))
+        .WillOnce(base::test::RunOnceCallback<0>(true));
+    screen_power_controller_->SetScreenOn();
+    screen_power_controller_->SetScreenOff();
+    task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration +
+                            kDisplayPowerOnDelay);
+  }
+}
+
+TEST_F(ScreenPowerControllerAuraTest, DoubleTriggeredPowerOff) {
+  screen_power_controller_->SetAllowScreenPowerOff(true);
+
+  {
+    ::testing::InSequence s;
+    EXPECT_CALL(*delegate_, SetScreenPowerOn(_)).Times(0);
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(false, kScreenOnOffDuration));
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_))
+        .WillOnce(base::test::RunOnceCallback<0>(true));
+    screen_power_controller_->SetScreenOff();
+    screen_power_controller_->SetScreenOff();
+    task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration +
+                            kDisplayPowerOnDelay);
+  }
+}
+
+TEST_F(ScreenPowerControllerAuraTest, DoubleTriggeredPowerOn) {
+  PowerOff();
+
+  {
+    ::testing::InSequence s;
+    EXPECT_CALL(*delegate_, SetScreenPowerOff(_)).Times(0);
+    EXPECT_CALL(*delegate_, SetScreenPowerOn(_))
+            .WillOnce(base::test::RunOnceCallback<0>(true));
+    EXPECT_CALL(*delegate_, SetScreenBrightnessOn(true, kScreenOnOffDuration));
+    screen_power_controller_->SetScreenOn();
+    screen_power_controller_->SetScreenOn();
+    task_env_.FastForwardBy(kDisplayPowerOffDelay + kScreenOnOffDuration +
+                            kDisplayPowerOnDelay);
+  }
+}
+
+}  // namespace test
+}  // namespace chromecast

chromecast/ui/display_settings/screen_power_controller_default.cc

+// Copyright 2020 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 "chromecast/ui/display_settings/screen_power_controller_default.h"
+
+#include "base/logging.h"
+#include "base/time/time.h"
+
+namespace chromecast {
+
+namespace {
+
+constexpr base::TimeDelta kScreenOnOffDuration =
+    base::TimeDelta::FromMilliseconds(200);
+
+}
+
+std::unique_ptr<ScreenPowerController> ScreenPowerController::Create(
+    Delegate* delegate) {
+  return std::make_unique<ScreenPowerControllerDefault>(delegate);
+}
+
+ScreenPowerControllerDefault::ScreenPowerControllerDefault(
+    ScreenPowerController::Delegate* delegate)
+    : screen_on_(true), delegate_(delegate) {}
+
+ScreenPowerControllerDefault::~ScreenPowerControllerDefault() = default;
+
+void ScreenPowerControllerDefault::SetScreenOn() {
+  if (screen_on_) {
+    return;
+  }
+
+  LOG(INFO) << "Setting screen_on to true";
+  screen_on_ = true;
+  delegate_->SetScreenBrightnessOn(true, kScreenOnOffDuration);
+}
+
+void ScreenPowerControllerDefault::SetScreenOff() {
+  if (!screen_on_) {
+    return;
+  }
+  LOG(INFO) << "Setting screen_on to false";
+  screen_on_ = false;
+  delegate_->SetScreenBrightnessOn(false, kScreenOnOffDuration);
+}
+
+void ScreenPowerControllerDefault::SetAllowScreenPowerOff(
+    bool allow_power_off) {
+  LOG(WARNING)
+      << "Screen is not allowed to be powered off. Please enable AURA.";
+}
+
+}  // namespace chromecast
+
+#endif  // CHROMECAST_UI_DISPLAY_SETTINGS_BRIGHTNESS_ANIMATION_H_

chromecast/ui/display_settings/screen_power_controller_default.h

+// Copyright 2020 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 CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_DEFAULT_H_
+#define CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_DEFAULT_H_
+
+#include "base/memory/weak_ptr.h"
+#include "chromecast/ui/display_settings/screen_power_controller.h"
+
+namespace chromecast {
+
+// This class implements the ScreenPowerController's default behavior. It never
+// powers off the screen so most of the methods will do nothing other than
+// logging and invoke delegate's methods.
+class ScreenPowerControllerDefault : public ScreenPowerController {
+ public:
+  explicit ScreenPowerControllerDefault(
+      ScreenPowerController::Delegate* delegate);
+  ~ScreenPowerControllerDefault() override;
+
+  void SetScreenOn() override;
+  void SetScreenOff() override;
+  void SetAllowScreenPowerOff(bool allow_power_off) override;
+  inline bool IsScreenOn() const override { return screen_on_; }
+
+ private:
+  bool screen_on_;
+  ScreenPowerController::Delegate* delegate_;
+};
+
+}  // namespace chromecast
+
+#endif  // CHROMECAST_UI_DISPLAY_SETTINGS_SCREEN_POWER_CONTROLLER_DEFAULT_H_

chromecast/ui/display_settings_manager_impl.cc

@@ -4,7 +4,10 @@
 
 #include "chromecast/ui/display_settings_manager_impl.h"
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/logging.h"
+#include "base/time/time.h"
 #include "chromecast/graphics/cast_window_manager.h"
 #include "chromecast/ui/display_settings/brightness_animation.h"
 #include "chromecast/ui/display_settings/color_temperature_animation.h"
@@ -21,24 +24,20 @@ namespace chromecast {
 namespace {
 
 constexpr base::TimeDelta kAnimationDuration = base::TimeDelta::FromSeconds(2);
-constexpr base::TimeDelta kScreenOnOffDuration =
-    base::TimeDelta::FromMilliseconds(200);
-
-#if defined(USE_AURA)
-// These delays are needed to ensure there are no visible artifacts due to the
-// backlight turning on prior to the LCD fully initializing or vice-versa.
-// TODO(b/161140301): Make this configurable for different products
-// TODO(b/161268188): Remove these if the delays can be handled by the kernel
-constexpr base::TimeDelta kDisplayPowerOnDelay =
-    base::TimeDelta::FromMilliseconds(35);
-constexpr base::TimeDelta kDisplayPowerOffDelay =
-    base::TimeDelta::FromMilliseconds(85);
-#endif  // defined(USE_AURA)
 
 const float kMinApiBrightness = 0.0f;
 const float kMaxApiBrightness = 1.0f;
 const float kDefaultApiBrightness = kMaxApiBrightness;
 
+bool CheckDisplayStatus(const base::flat_map<int64_t, bool>& statuses) {
+  for (const auto& status : statuses) {
+    if (!status.second) {
+      return false;
+    }
+  }
+  return true;
+}
+
 }  // namespace
 
 DisplaySettingsManagerImpl::DisplaySettingsManagerImpl(
@@ -56,11 +55,7 @@ DisplaySettingsManagerImpl::DisplaySettingsManagerImpl(
       display_configurator_(nullptr),
 #endif  // defined(USE_AURA)
       brightness_(-1.0f),
-      screen_on_(true),
-#if defined(USE_AURA)
-      screen_power_on_(true),
-      allow_screen_power_off_(false),
-#endif  // defined(USE_AURA)
+      screen_power_controller_(ScreenPowerController::Create(this)),
       color_temperature_animation_(std::make_unique<ColorTemperatureAnimation>(
           window_manager_,
           display_configurator_,
@@ -110,6 +105,23 @@ void DisplaySettingsManagerImpl::AddReceiver(
   receivers_.Add(this, std::move(receiver));
 }
 
+void DisplaySettingsManagerImpl::SetScreenPowerOn(PowerToggleCallback callback) {
+  display_configurator_->EnableDisplay(
+      base::BindOnce(&CheckDisplayStatus).Then(std::move(callback)));
+}
+
+void DisplaySettingsManagerImpl::SetScreenPowerOff(PowerToggleCallback callback) {
+  display_configurator_->DisableDisplay(
+      base::BindOnce(&CheckDisplayStatus).Then(std::move(callback)));
+}
+
+void DisplaySettingsManagerImpl::SetScreenBrightnessOn(
+    bool brightness_on,
+    base::TimeDelta duration) {
+  UpdateBrightness(brightness_on ? brightness_ : 0, duration);
+  window_manager_->SetTouchInputDisabled(!brightness_on);
+}
+
 void DisplaySettingsManagerImpl::SetColorTemperature(float temperature) {
   DVLOG(4) << "Setting color temperature to " << temperature << " Kelvin.";
   color_temperature_animation_->AnimateToNewValue(temperature,
@@ -151,101 +163,28 @@ void DisplaySettingsManagerImpl::SetBrightnessSmooth(float brightness,
   brightness_ = brightness;
 
   // If the screen is off, keep the new brightness but don't apply it
-  if (!screen_on_) {
+  if (!screen_power_controller_->IsScreenOn()) {
     return;
   }
 
-  UpdateBrightness(duration);
+  UpdateBrightness(brightness_, duration);
 }
 
 void DisplaySettingsManagerImpl::ResetBrightness() {
   SetBrightness(kDefaultApiBrightness);
 }
 
-#if defined(USE_AURA)
-void DisplaySettingsManagerImpl::OnDisplayOn(
-    const base::flat_map<int64_t, bool>& statuses) {
-  for (const auto& status : statuses) {
-    bool display_success = status.second;
-    if (!display_success) {
-      // Fatal since the user has no other way of turning the screen on if this
-      // failed.
-      LOG(FATAL) << "Failed to enable screen";
-      return;
-    }
-  }
-  screen_power_on_ = true;
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE,
-      base::BindOnce(&DisplaySettingsManagerImpl::OnDisplayOnTimeoutCompleted,
-                     weak_factory_.GetWeakPtr()),
-      kDisplayPowerOnDelay);
-}
-
-void DisplaySettingsManagerImpl::OnDisplayOnTimeoutCompleted() {
-  UpdateBrightness(kScreenOnOffDuration);
-  window_manager_->SetTouchInputDisabled(false /* since screen_on = true */);
-}
-
-void DisplaySettingsManagerImpl::OnDisplayOffTimeoutCompleted() {
-  display_configurator_->DisableDisplay(
-      base::BindOnce([](const base::flat_map<int64_t, bool>& statuses) {
-        for (const auto& status : statuses) {
-          bool display_success = status.second;
-          LOG_IF(FATAL, !display_success) << "Failed to disable display";
-        }
-      }));
-  screen_power_on_ = false;
-}
-
 void DisplaySettingsManagerImpl::SetScreenOn(bool screen_on) {
-  // Allow this to run if screen_on == screen_on_ == false IF
-  // previously, the screen was turned off without powering off the screen
-  // and we want to power it off this time
-  if (screen_on == screen_on_ &&
-      !(!screen_on && allow_screen_power_off_ && screen_power_on_)) {
-    return;
-  }
-
-  LOG(INFO) << "Setting screen on to " << screen_on;
-  screen_on_ = screen_on;
-
-  // TODO(b/161268188): This can be simplified and the delays removed
-  // if backlight timing is handled by the kernel
-  if (screen_on && !screen_power_on_) {
-    display_configurator_->EnableDisplay(base::BindOnce(
-        &DisplaySettingsManagerImpl::OnDisplayOn, weak_factory_.GetWeakPtr()));
+  if (screen_on) {
+    screen_power_controller_->SetScreenOn();
   } else {
-    UpdateBrightness(kScreenOnOffDuration);
-    window_manager_->SetTouchInputDisabled(!screen_on_);
-    if (!screen_on && allow_screen_power_off_) {
-      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-          FROM_HERE,
-          base::BindOnce(
-              &DisplaySettingsManagerImpl::OnDisplayOffTimeoutCompleted,
-              weak_factory_.GetWeakPtr()),
-          kDisplayPowerOffDelay + kScreenOnOffDuration);
-    }
+    screen_power_controller_->SetScreenOff();
   }
 }
 
 void DisplaySettingsManagerImpl::SetAllowScreenPowerOff(bool allow_power_off) {
-  allow_screen_power_off_ = allow_power_off;
-}
-#else
-void DisplaySettingsManagerImpl::SetScreenOn(bool screen_on) {
-  if (screen_on == screen_on_) {
-    return;
-  }
-
-  LOG(INFO) << "Setting screen on to " << screen_on;
-  screen_on_ = screen_on;
-
-  UpdateBrightness(kScreenOnOffDuration);
-  window_manager_->SetTouchInputDisabled(!screen_on_);
+  screen_power_controller_->SetAllowScreenPowerOff(allow_power_off);
 }
-void DisplaySettingsManagerImpl::SetAllowScreenPowerOff(bool allow_power_off) {}
-#endif
 
 void DisplaySettingsManagerImpl::AddDisplaySettingsObserver(
     mojo::PendingRemote<mojom::DisplaySettingsObserver> observer) {
@@ -254,9 +193,8 @@ void DisplaySettingsManagerImpl::AddDisplaySettingsObserver(
   observers_.Add(std::move(observer_remote));
 }
 
-void DisplaySettingsManagerImpl::UpdateBrightness(base::TimeDelta duration) {
-  float brightness = screen_on_ ? brightness_ : 0;
-
+void DisplaySettingsManagerImpl::UpdateBrightness(float brightness,
+                                                  base::TimeDelta duration) {
   if (brightness_animation_)
     brightness_animation_->AnimateToNewValue(brightness, duration);
 }

chromecast/ui/display_settings_manager_impl.h

@@ -9,6 +9,7 @@
 #include <vector>
 
 #include "base/memory/weak_ptr.h"
+#include "chromecast/ui/display_settings/screen_power_controller.h"
 #include "chromecast/ui/display_settings_manager.h"
 #include "chromecast/ui/mojom/display_settings.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
@@ -32,6 +33,7 @@ class CastDisplayConfigurator;
 }
 
 class DisplaySettingsManagerImpl : public DisplaySettingsManager,
+                                   public ScreenPowerController::Delegate,
                                    public mojom::DisplaySettings {
  public:
   DisplaySettingsManagerImpl(
@@ -54,6 +56,12 @@ class DisplaySettingsManagerImpl : public DisplaySettingsManager,
   void AddReceiver(
       mojo::PendingReceiver<mojom::DisplaySettings> receiver) override;
 
+  // ScreenPowerController::Delegate implementation:
+  void SetScreenPowerOn(PowerToggleCallback callback) override;
+  void SetScreenPowerOff(PowerToggleCallback callback) override;
+  void SetScreenBrightnessOn(bool brightness_on,
+                             base::TimeDelta duration) override;
+
   // mojom::DisplaySettings implementation:
   void SetColorTemperature(float temperature) override;
   void SetColorTemperatureSmooth(float temperature,
@@ -66,16 +74,11 @@ class DisplaySettingsManagerImpl : public DisplaySettingsManager,
   void SetAllowScreenPowerOff(bool allow_power_off) override;
 
  private:
-  // mojom::DisplaySettingsObserver implementation
+  // mojom::DisplaySettingsObserver implementation:
   void AddDisplaySettingsObserver(
       mojo::PendingRemote<mojom::DisplaySettingsObserver> observer) override;
 
-  void UpdateBrightness(base::TimeDelta duration);
-#if defined(USE_AURA)
-  void OnDisplayOn(const base::flat_map<int64_t, bool>& statuses);
-  void OnDisplayOnTimeoutCompleted();
-  void OnDisplayOffTimeoutCompleted();
-#endif  // defined(USE_AURA)
+  void UpdateBrightness(float brightness, base::TimeDelta duration);
 
   CastWindowManager* const window_manager_;
   shell::CastDisplayConfigurator* const display_configurator_;
@@ -85,12 +88,8 @@ class DisplaySettingsManagerImpl : public DisplaySettingsManager,
 #endif  // defined(USE_AURA)
 
   float brightness_;
-  bool screen_on_;
-#if defined(USE_AURA)
-  bool screen_power_on_;
-  bool allow_screen_power_off_;
-#endif  // defined(USE_AURA)
 
+  std::unique_ptr<ScreenPowerController> screen_power_controller_;
   std::unique_ptr<ColorTemperatureAnimation> color_temperature_animation_;
   std::unique_ptr<BrightnessAnimation> brightness_animation_;