ambient: fix not stopping when display idle

* Previously display idle event will restart ambient mode. Fixed the
  unittest that should have caught this.
* Also added some DVLOG for future debugging.

Bug: None
Change-Id: I49cc798f4d975a3449e506b835469bd740b4308f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354945
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798191}

ash/ambient/ambient_controller.cc

@@ -300,13 +300,18 @@ void AmbientController::OnPowerStatusChanged() {
 
 void AmbientController::ScreenBrightnessChanged(
     const power_manager::BacklightBrightnessChange& change) {
+  DVLOG(1) << "ScreenBrightnessChanged: "
+           << (change.has_percent() ? change.percent() : -1);
+
   if (!change.has_percent())
     return;
 
   constexpr double kMinBrightness = 0.01;
   if (change.percent() < kMinBrightness) {
     if (is_screen_off_)
       return;
+
+    DVLOG(1) << "Screen is off, close ambient mode.";
     is_screen_off_ = true;
     // If screen is off, turn everything off. This covers:
     //   1. Manually turn screen off.
@@ -332,9 +337,16 @@ void AmbientController::ScreenBrightnessChanged(
 
 void AmbientController::ScreenIdleStateChanged(
     const power_manager::ScreenIdleState& idle_state) {
+  DVLOG(1) << "ScreenIdleStateChanged: dimmed(" << idle_state.dimmed()
+           << ") off(" << idle_state.off() << ")";
+
   if (!IsAmbientModeEnabled())
     return;
 
+  // "off" state should already be handled by the screen brightness handler.
+  if (idle_state.off())
+    return;
+
   if (!idle_state.dimmed())
     return;
 
@@ -361,6 +373,8 @@ void AmbientController::RemoveAmbientViewDelegateObserver(
 }
 
 void AmbientController::ShowUi(AmbientUiMode mode) {
+  DVLOG(1) << "ShowUi: " << mode;
+
   // TODO(meilinw): move the eligibility check to the idle entry point once
   // implemented: b/149246117.
   if (!IsAmbientModeEnabled()) {

ash/ambient/ambient_controller_unittest.cc

@@ -352,11 +352,13 @@ TEST_F(AmbientControllerTest, ShouldHideAmbientScreenWhenDisplayIsOff) {
   // Should dismiss ambient mode screen.
   SetScreenBrightnessAndWait(/*percent=*/0);
   SetScreenIdleStateAndWait(/*dimmed=*/true, /*off=*/true);
+  FastForwardToNextImage();
   EXPECT_FALSE(ambient_controller()->IsShown());
 
   // Screen back on again, should not have ambient screen.
   SetScreenBrightnessAndWait(/*percent=*/50);
   SetScreenIdleStateAndWait(/*dimmed=*/false, /*off=*/false);
+  FastForwardToNextImage();
   EXPECT_FALSE(ambient_controller()->IsShown());
 }
 
@@ -378,6 +380,8 @@ TEST_F(AmbientControllerTest,
   // Should dismiss ambient mode screen.
   SetScreenBrightnessAndWait(/*percent=*/0);
   SetScreenIdleStateAndWait(/*dimmed=*/true, /*off=*/true);
+  FastForwardToInactivity();
+  FastForwardToNextImage();
   EXPECT_FALSE(ambient_controller()->IsShown());
 
   // Screen back on again, should not have ambient screen, but still has lock

ash/public/cpp/ambient/ambient_ui_model.cc

@@ -56,4 +56,16 @@ void AmbientUiModel::NotifyAmbientUiVisibilityChanged() {
     observer.OnAmbientUiVisibilityChanged(ui_visibility_);
 }
 
+std::ostream& operator<<(std::ostream& out, AmbientUiMode mode) {
+  switch (mode) {
+    case AmbientUiMode::kLockScreenUi:
+      out << "kLockScreenUi";
+      break;
+    case AmbientUiMode::kInSessionUi:
+      out << "kInSessionUi";
+      break;
+  }
+  return out;
+}
+
 }  // namespace ash

ash/public/cpp/ambient/ambient_ui_model.h

@@ -65,6 +65,9 @@ class ASH_PUBLIC_EXPORT AmbientUiModel {
   base::ObserverList<AmbientUiModelObserver> observers_;
 };
 
+ASH_PUBLIC_EXPORT std::ostream& operator<<(std::ostream& out,
+                                           AmbientUiMode mode);
+
 }  // namespace ash
 
 #endif  // ASH_PUBLIC_CPP_AMBIENT_AMBIENT_UI_MODEL_H_