Revert "ambient: support multiple displays"

This reverts commit 6d681c88.

Reason for revert: Causing test failures on MSAN builds:

https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/21346

Original change's description:
> ambient: support multiple displays
>
> Run ambient mode UI on each connected display.
> RootWindowController construction handles adding ambient widgets
> to new displays while ambient mode is running, and destructing them
> on display removed.
>
> BUG=b:139953389
>
> Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
> Change-Id: Ic57c33491098b66c7c0e90ef1f3d12c71f1fc303
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2516968
> Commit-Queue: Jeffrey Young <cowmoo@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Tao Wu <wutao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#828329}

TBR=oshima@chromium.org,xiaohuic@chromium.org,wutao@chromium.org,cowmoo@chromium.org

Change-Id: I255333ed722549fdc9d0e945c65cf3f742b1cabc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:139953389
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2545752Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#828486}

ash/ambient/ambient_controller.cc

@@ -7,7 +7,6 @@
 #include <memory>
 #include <string>
 #include <utility>
-#include <vector>
 
 #include "ash/ambient/model/ambient_backend_model_observer.h"
 #include "ash/ambient/ui/ambient_container_view.h"
@@ -23,7 +22,6 @@
 #include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
 #include "ash/public/cpp/assistant/controller/assistant_ui_controller.h"
 #include "ash/public/cpp/shell_window_ids.h"
-#include "ash/root_window_controller.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "ash/system/power/power_status.h"
@@ -46,6 +44,7 @@
 #include "ui/aura/client/aura_constants.h"
 #include "ui/base/ui_base_types.h"
 #include "ui/base/user_activity/user_activity_detector.h"
+#include "ui/events/types/event_type.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/wm/core/cursor_manager.h"
@@ -77,6 +76,11 @@ std::unique_ptr<AmbientBackendController> CreateAmbientBackendController() {
 #endif  // BUILDFLAG(ENABLE_CROS_AMBIENT_MODE_BACKEND)
 }
 
+aura::Window* GetWidgetContainer() {
+  return Shell::GetContainer(Shell::GetPrimaryRootWindow(),
+                             kShellWindowId_AmbientModeContainer);
+}
+
 // Returns the name of the ambient widget.
 std::string GetWidgetName() {
   if (ambient::util::IsShowing(LockScreen::ScreenType::kLock))
@@ -186,7 +190,10 @@ AmbientController::AmbientController(
       fingerprint_observer_receiver_.BindNewPipeAndPassRemote());
 }
 
-AmbientController::~AmbientController() = default;
+AmbientController::~AmbientController() {
+  if (container_view_)
+    container_view_->GetWidget()->CloseNow();
+}
 
 void AmbientController::OnAmbientUiVisibilityChanged(
     AmbientUiVisibility visibility) {
@@ -221,7 +228,7 @@ void AmbientController::OnAmbientUiVisibilityChanged(
       break;
     case AmbientUiVisibility::kHidden:
     case AmbientUiVisibility::kClosed:
-      CloseAllWidgets(/*immediately=*/false);
+      CloseWidget(/*immediately=*/false);
 
       // TODO(wutao): This will clear the image cache currently. It will not
       // work with `kHidden` if the token has expired and ambient mode is shown
@@ -279,6 +286,7 @@ void AmbientController::OnAmbientUiVisibilityChanged(
 
 void AmbientController::OnAutoShowTimeOut() {
   DCHECK(IsUiHidden(ambient_ui_model_.ui_visibility()));
+  DCHECK(!container_view_);
 
   // Show ambient screen after time out.
   ambient_ui_model_.SetUiVisibility(AmbientUiVisibility::kShown);
@@ -380,6 +388,48 @@ 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.
+    //   2. Clicking tablet power button.
+    //   3. Close lid.
+    // Need to specially close the widget immediately here to be able to close
+    // the UI before device goes to suspend. Otherwise when opening lid after
+    // lid closed, there may be a flash of the old window before previous
+    // closing finished.
+    CloseWidget(/*immediately=*/true);
+    CloseUi();
+    return;
+  }
+
+  // change.percent() > kMinBrightness
+  if (!is_screen_off_)
+    return;
+  is_screen_off_ = false;
+
+  // Reset image failures to allow retrying ambient mode because screen has
+  // turned back on.
+  GetAmbientBackendModel()->ResetImageFailures();
+
+  // If screen is back on, turn on ambient mode for lock screen.
+  if (LockScreen::HasInstance())
+    ShowHiddenUi();
+}
+
 void AmbientController::ScreenIdleStateChanged(
     const power_manager::ScreenIdleState& idle_state) {
   DVLOG(1) << "ScreenIdleStateChanged: dimmed(" << idle_state.dimmed()
@@ -388,48 +438,25 @@ void AmbientController::ScreenIdleStateChanged(
   if (!IsAmbientModeEnabled())
     return;
 
-  if (idle_state.off()) {
-    DVLOG(1) << "Screen is off, close ambient mode.";
-
-    CloseAllWidgets(/*immediately=*/true);
-    CloseUi();
+  // "off" state should already be handled by the screen brightness handler.
+  if (idle_state.off())
     return;
-  }
 
-  if (idle_state.dimmed()) {
-    // Do not show the UI if lockscreen is active. The inactivity monitor should
-    // have activated ambient mode.
-    if (LockScreen::HasInstance())
-      return;
-
-    // Do not show UI if loading images was unsuccessful.
-    if (GetAmbientBackendModel()->ImageLoadingFailed()) {
-      VLOG(1) << "Skipping ambient mode activation due to prior failure";
-      return;
-    }
+  if (!idle_state.dimmed())
+    return;
 
-    ShowUi();
+  // Do not show the UI if lockscreen is active. The inactivity monitor should
+  // have activated ambient mode.
+  if (LockScreen::HasInstance())
     return;
-  }
 
-  if (LockScreen::HasInstance() &&
-      ambient_ui_model_.ui_visibility() == AmbientUiVisibility::kClosed) {
-    // Restart hidden ui if the screen is back on and lockscreen is shown.
-    ShowHiddenUi();
+  // Do not show UI if loading images was unsuccessful.
+  if (GetAmbientBackendModel()->ImageLoadingFailed()) {
+    VLOG(1) << "Skipping ambient mode activation due to prior failure";
+    return;
   }
-}
 
-void AmbientController::SuspendImminent(
-    power_manager::SuspendImminent::Reason reason) {
-  // If about to suspend, turn everything off. This covers:
-  //   1. Clicking power button.
-  //   2. Close lid.
-  // Need to specially close the widget immediately here to be able to close
-  // the UI before device goes to suspend. Otherwise when opening lid after
-  // lid closed, there may be a flash of the old window before previous
-  // closing finished.
-  CloseAllWidgets(/*immediately=*/true);
-  CloseUi();
+  ShowUi();
 }
 
 void AmbientController::OnAuthScanDone(
@@ -480,7 +507,6 @@ void AmbientController::CloseUi() {
   DVLOG(1) << __func__;
 
   ambient_ui_model_.SetUiVisibility(AmbientUiVisibility::kClosed);
-  GetAmbientBackendModel()->ResetImageFailures();
 }
 
 void AmbientController::ToggleInSessionUi() {
@@ -491,7 +517,7 @@ void AmbientController::ToggleInSessionUi() {
 }
 
 bool AmbientController::IsShown() const {
-  return ambient_ui_model_.ui_visibility() == AmbientUiVisibility::kShown;
+  return container_view_ && container_view_->IsDrawn();
 }
 
 void AmbientController::AcquireWakeLock() {
@@ -532,11 +558,16 @@ void AmbientController::ReleaseWakeLock() {
   delayed_lock_timer_.Stop();
 }
 
-void AmbientController::CloseAllWidgets(bool immediately) {
-  for (auto* root_window_controller :
-       RootWindowController::root_window_controllers()) {
-    root_window_controller->CloseAmbientWidget(immediately);
-  }
+void AmbientController::CloseWidget(bool immediately) {
+  if (!container_view_)
+    return;
+
+  if (immediately)
+    container_view_->GetWidget()->CloseNow();
+  else
+    container_view_->GetWidget()->Close();
+
+  container_view_ = nullptr;
 }
 
 void AmbientController::OnLockScreenInactivityTimeoutPrefChanged() {
@@ -600,7 +631,7 @@ AmbientBackendModel* AmbientController::GetAmbientBackendModel() {
 }
 
 void AmbientController::OnImagesReady() {
-  CreateAndShowWidgets();
+  CreateAndShowWidget();
 }
 
 void AmbientController::OnImagesFailed() {
@@ -608,24 +639,27 @@ void AmbientController::OnImagesFailed() {
   CloseUi();
 }
 
-std::unique_ptr<views::Widget> AmbientController::CreateWidget(
-    aura::Window* container) {
-  auto container_view = std::make_unique<AmbientContainerView>(&delegate_);
-  auto* widget_delegate = new AmbientWidgetDelegate();
-  widget_delegate->SetInitiallyFocusedView(container_view.get());
+std::unique_ptr<AmbientContainerView> AmbientController::CreateContainerView() {
+  DCHECK(!container_view_);
+
+  auto container = std::make_unique<AmbientContainerView>(&delegate_);
+  container_view_ = container.get();
+  return container;
+}
+
+void AmbientController::CreateAndShowWidget() {
+  DCHECK(!container_view_);
 
   views::Widget::InitParams params;
   params.type = views::Widget::InitParams::TYPE_WINDOW_FRAMELESS;
   params.name = GetWidgetName();
   params.show_state = ui::SHOW_STATE_FULLSCREEN;
-  params.parent = container;
-  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
-  params.delegate = widget_delegate;
-  params.visible_on_all_workspaces = true;
+  params.parent = GetWidgetContainer();
+  params.delegate = new AmbientWidgetDelegate();
 
-  auto widget = std::make_unique<views::Widget>();
+  views::Widget* widget = new views::Widget;
   widget->Init(std::move(params));
-  widget->SetContentsView(std::move(container_view));
+  widget->SetContentsView(CreateContainerView());
 
   widget->SetVisibilityAnimationTransition(
       views::Widget::VisibilityTransition::ANIMATE_BOTH);
@@ -635,16 +669,11 @@ std::unique_ptr<views::Widget> AmbientController::CreateWidget(
 
   widget->Show();
 
-  return widget;
-}
-
-void AmbientController::CreateAndShowWidgets() {
   // Hide cursor.
   Shell::Get()->cursor_manager()->HideCursor();
-  for (auto* root_window_controller :
-       RootWindowController::root_window_controllers()) {
-    root_window_controller->CreateAmbientWidget();
-  }
+
+  // Requests keyboard focus for |container_view_| to receive keyboard events.
+  container_view_->RequestFocus();
 }
 
 void AmbientController::StartRefreshingImages() {

ash/ambient/ambient_controller.h

@@ -6,7 +6,6 @@
 #define ASH_AMBIENT_AMBIENT_CONTROLLER_H_
 
 #include <memory>
-#include <vector>
 
 #include "ash/ambient/ambient_access_token_controller.h"
 #include "ash/ambient/ambient_photo_controller.h"
@@ -71,9 +70,10 @@ class ASH_EXPORT AmbientController
   void OnPowerStatusChanged() override;
 
   // chromeos::PowerManagerClient::Observer:
+  void ScreenBrightnessChanged(
+      const power_manager::BacklightBrightnessChange& change) override;
   void ScreenIdleStateChanged(
       const power_manager::ScreenIdleState& idle_state) override;
-  void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
 
   // fingerprint::mojom::FingerprintObserver:
   void OnAuthScanDone(
@@ -100,17 +100,13 @@ class ASH_EXPORT AmbientController
 
   void ToggleInSessionUi();
 
-  // Returns true if ambient mode containers are visible or are being
-  // constructed.
+  // Returns true if the |container_view_| is currently visible.
   bool IsShown() const;
 
   void RequestAccessToken(
       AmbientAccessTokenController::AccessTokenCallback callback,
       bool may_refresh_token_on_lock = false);
 
-  // Creates a widget and |AmbientContainerView| for the container.
-  std::unique_ptr<views::Widget> CreateWidget(aura::Window* container);
-
   AmbientBackendModel* GetAmbientBackendModel();
 
   AmbientBackendController* ambient_backend_controller() {
@@ -133,9 +129,14 @@ class ASH_EXPORT AmbientController
   void OnImagesReady() override;
   void OnImagesFailed() override;
 
-  // Creates and shows a full-screen widget for each root window to show the
-  // ambient UI.
-  void CreateAndShowWidgets();
+  // Initializes the |container_view_|. Called in |CreateWidget()| to create the
+  // contents view.
+  std::unique_ptr<AmbientContainerView> CreateContainerView();
+
+  // TODO(meilinw): reuses the lock-screen widget: b/156531168, b/157175030.
+  // Creates and shows a full-screen widget responsible for showing
+  // the ambient UI.
+  void CreateAndShowWidget();
 
   void StartRefreshingImages();
   void StopRefreshingImages();
@@ -154,17 +155,24 @@ class ASH_EXPORT AmbientController
   // Release |wake_lock_|. Unbalanced release call will have no effect.
   void ReleaseWakeLock();
 
-  void CloseAllWidgets(bool immediately);
+  void CloseWidget(bool immediately);
 
   // Invoked when the Ambient mode prefs state changes.
   void OnLockScreenInactivityTimeoutPrefChanged();
   void OnLockScreenBackgroundTimeoutPrefChanged();
   void OnPhotoRefreshIntervalPrefChanged();
 
+  AmbientContainerView* get_container_view_for_testing() {
+    return container_view_;
+  }
+
   AmbientAccessTokenController* access_token_controller_for_testing() {
     return &access_token_controller_;
   }
 
+  // Owned by |RootView| of its parent widget.
+  AmbientContainerView* container_view_ = nullptr;
+
   AmbientViewDelegateImpl delegate_{this};
   AmbientUiModel ambient_ui_model_;
 
@@ -192,6 +200,8 @@ class ASH_EXPORT AmbientController
   ScopedObserver<ui::UserActivityDetector, ui::UserActivityObserver>
       user_activity_observer_{this};
 
+  bool is_screen_off_ = false;
+
   // Observes user profile prefs for ambient.
   std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
 

ash/ambient/test/ambient_ash_test_base.cc

@@ -20,13 +20,11 @@
 #include "ash/public/cpp/ambient/ambient_prefs.h"
 #include "ash/public/cpp/ambient/ambient_ui_model.h"
 #include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
-#include "ash/root_window_controller.h"
 #include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "base/callback.h"
 #include "base/files/file_util.h"
 #include "base/memory/ptr_util.h"
-#include "base/notreached.h"
 #include "base/run_loop.h"
 #include "base/sequenced_task_runner.h"
 #include "base/threading/scoped_blocking_call.h"
@@ -35,11 +33,9 @@
 #include "chromeos/dbus/power/fake_power_manager_client.h"
 #include "chromeos/dbus/power/power_manager_client.h"
 #include "chromeos/dbus/power_manager/idle.pb.h"
-#include "ui/display/screen.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_unittest_util.h"
 #include "ui/views/controls/label.h"
-#include "ui/views/widget/widget.h"
 
 namespace ash {
 namespace {
@@ -216,46 +212,36 @@ void AmbientAshTestBase::SetScreenIdleStateAndWait(bool is_screen_dimmed,
   base::RunLoop().RunUntilIdle();
 }
 
-std::vector<views::View*>
-AmbientAshTestBase::GetMediaStringViewTextContainers() {
-  std::vector<views::View*> result;
-  for (auto* view : GetMediaStringViews())
-    result.push_back(view->media_text_container_for_testing());
-  return result;
+void AmbientAshTestBase::SetScreenBrightnessAndWait(double percent) {
+  power_manager::BacklightBrightnessChange change;
+  change.set_percent(percent);
+
+  chromeos::FakePowerManagerClient::Get()->SendScreenBrightnessChanged(change);
+  base::RunLoop().RunUntilIdle();
 }
 
 views::View* AmbientAshTestBase::GetMediaStringViewTextContainer() {
   return GetMediaStringView()->media_text_container_for_testing();
 }
 
-std::vector<views::Label*> AmbientAshTestBase::GetMediaStringViewTextLabels() {
-  std::vector<views::Label*> result;
-  for (auto* view : GetMediaStringViews())
-    result.push_back(view->media_text_label_for_testing());
-  return result;
-}
-
 views::Label* AmbientAshTestBase::GetMediaStringViewTextLabel() {
   return GetMediaStringView()->media_text_label_for_testing();
 }
 
 void AmbientAshTestBase::SimulateMediaMetadataChanged(
     media_session::MediaMetadata metadata) {
-  for (auto* view : GetMediaStringViews())
-    view->MediaSessionMetadataChanged(metadata);
+  GetMediaStringView()->MediaSessionMetadataChanged(metadata);
 }
 
 void AmbientAshTestBase::SimulateMediaPlaybackStateChanged(
     media_session::mojom::MediaPlaybackState state) {
-  for (auto* media_string_view : GetMediaStringViews()) {
-    // Creates media session info.
-    media_session::mojom::MediaSessionInfoPtr session_info(
-        media_session::mojom::MediaSessionInfo::New());
-    session_info->playback_state = state;
-
-    // Simulate media session info changed.
-    media_string_view->MediaSessionInfoChanged(std::move(session_info));
-  }
+  // Creates media session info.
+  media_session::mojom::MediaSessionInfoPtr session_info(
+      media_session::mojom::MediaSessionInfo::New());
+  session_info->playback_state = state;
+
+  // Simulate media session info changed.
+  GetMediaStringView()->MediaSessionInfoChanged(std::move(session_info));
 }
 
 void AmbientAshTestBase::SetPhotoViewImageSize(int width, int height) {
@@ -265,36 +251,17 @@ void AmbientAshTestBase::SetPhotoViewImageSize(int width, int height) {
   image_decoder->SetImageSize(width, height);
 }
 
-std::vector<AmbientBackgroundImageView*>
-AmbientAshTestBase::GetAmbientBackgroundImageViews() {
-  std::vector<AmbientBackgroundImageView*> result;
-  for (auto* view : GetContainerViews()) {
-    auto* background_image_view =
-        view->GetViewByID(AmbientViewID::kAmbientBackgroundImageView);
-    result.push_back(
-        static_cast<AmbientBackgroundImageView*>(background_image_view));
-  }
-  return result;
-}
-
 AmbientBackgroundImageView*
 AmbientAshTestBase::GetAmbientBackgroundImageView() {
-  return static_cast<AmbientBackgroundImageView*>(
-      GetContainerView()->GetViewByID(kAmbientBackgroundImageView));
-}
-
-std::vector<MediaStringView*> AmbientAshTestBase::GetMediaStringViews() {
-  std::vector<MediaStringView*> result;
-  for (auto* view : GetContainerViews()) {
-    auto* media_string_view = view->GetViewByID(kAmbientMediaStringView);
-    result.push_back(static_cast<MediaStringView*>(media_string_view));
-  }
-  return result;
+  DCHECK(container_view());
+  return static_cast<AmbientBackgroundImageView*>(container_view()->GetViewByID(
+      AmbientViewID::kAmbientBackgroundImageView));
 }
 
 MediaStringView* AmbientAshTestBase::GetMediaStringView() {
+  DCHECK(container_view());
   return static_cast<MediaStringView*>(
-      GetContainerView()->GetViewByID(kAmbientMediaStringView));
+      container_view()->GetViewByID(AmbientViewID::kAmbientMediaStringView));
 }
 
 void AmbientAshTestBase::FastForwardToLockScreenTimeout() {
@@ -400,30 +367,8 @@ AmbientPhotoController* AmbientAshTestBase::photo_controller() {
   return ambient_controller()->ambient_photo_controller();
 }
 
-std::vector<AmbientContainerView*> AmbientAshTestBase::GetContainerViews() {
-  std::vector<AmbientContainerView*> result;
-  for (auto* ctrl : RootWindowController::root_window_controllers()) {
-    auto* widget = ctrl->ambient_widget_for_testing();
-    if (widget) {
-      auto* view = widget->GetContentsView();
-      DCHECK(view && view->GetID() == kAmbientContainerView);
-      result.push_back(static_cast<AmbientContainerView*>(view));
-    }
-  }
-  return result;
-}
-
-AmbientContainerView* AmbientAshTestBase::GetContainerView() {
-  auto* widget =
-      Shell::GetPrimaryRootWindowController()->ambient_widget_for_testing();
-
-  if (widget) {
-    auto* container_view = widget->GetContentsView();
-    DCHECK(container_view && container_view->GetID() == kAmbientContainerView);
-    return static_cast<AmbientContainerView*>(container_view);
-  }
-
-  return nullptr;
+AmbientContainerView* AmbientAshTestBase::container_view() {
+  return ambient_controller()->get_container_view_for_testing();
 }
 
 AmbientAccessTokenController* AmbientAshTestBase::token_controller() {

ash/ambient/test/ambient_ash_test_base.h

@@ -7,7 +7,6 @@
 
 #include <memory>
 #include <string>
-#include <vector>
 
 #include "ash/ambient/ambient_access_token_controller.h"
 #include "ash/ambient/ambient_controller.h"
@@ -15,7 +14,6 @@
 #include "ash/ambient/ui/ambient_background_image_view.h"
 #include "ash/test/ash_test_base.h"
 #include "services/media_session/public/mojom/media_session.mojom.h"
-#include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace views {
@@ -66,16 +64,8 @@ class AmbientAshTestBase : public AshTestBase {
   void SimulateSystemSuspendAndWait(
       power_manager::SuspendImminent::Reason reason);
 
-  // Return all media string view text containers. There is one per display.
-  std::vector<views::View*> GetMediaStringViewTextContainers();
-  // Return the media string view text container for the ambient mode container
-  // on the default display.
   views::View* GetMediaStringViewTextContainer();
 
-  // Return all media string view text labels. There is one per display.
-  std::vector<views::Label*> GetMediaStringViewTextLabels();
-  // Return the media string view text label for the ambient mode container on
-  // the default display.
   views::Label* GetMediaStringViewTextLabel();
 
   // Simulates the system starting to resume.
@@ -86,6 +76,9 @@ class AmbientAshTestBase : public AshTestBase {
   // Wait until the event has been processed.
   void SetScreenIdleStateAndWait(bool is_screen_dimmed, bool is_off);
 
+  // Simulates a screen brightness changed event.
+  void SetScreenBrightnessAndWait(double percent);
+
   void SimulateMediaMetadataChanged(media_session::MediaMetadata metadata);
 
   void SimulateMediaPlaybackStateChanged(
@@ -127,24 +120,17 @@ class AmbientAshTestBase : public AshTestBase {
 
   base::TimeDelta GetRefreshTokenDelay();
 
-  // Returns the ambient image view for each display.
-  std::vector<AmbientBackgroundImageView*> GetAmbientBackgroundImageViews();
-  // Returns the AmbientBackgroundImageView for the default display.
   AmbientBackgroundImageView* GetAmbientBackgroundImageView();
 
-  // Returns the media string views for displaying ongoing media info.
-  std::vector<MediaStringView*> GetMediaStringViews();
-  // Returns the media string view for the default display.
+  // Returns the media string view for displaying ongoing media info.
   MediaStringView* GetMediaStringView();
 
   AmbientController* ambient_controller();
 
   AmbientPhotoController* photo_controller();
 
-  // Returns the top-level views which contains all the ambient components.
-  std::vector<AmbientContainerView*> GetContainerViews();
-  // Returns the top level ambient container view for the primary root window.
-  AmbientContainerView* GetContainerView();
+  // Returns the top-level view which contains all the ambient components.
+  AmbientContainerView* container_view();
 
   AmbientAccessTokenController* token_controller();
 

ash/ambient/ui/ambient_container_view_unittest.cc

@@ -18,18 +18,15 @@ using AmbientContainerViewTest = AmbientAshTestBase;
 // Tests that AmbientContainerView window should be fullscreen.
 TEST_F(AmbientContainerViewTest, WindowFullscreenSize) {
   ShowAmbientScreen();
-  for (const auto* container : GetContainerViews()) {
-    const views::Widget* widget = container->GetWidget();
-
-    gfx::Rect root_window_bounds =
-        display::Screen::GetScreen()
-            ->GetDisplayNearestWindow(
-                widget->GetNativeWindow()->GetRootWindow())
-            .bounds();
-    gfx::Rect container_window_bounds =
-        widget->GetNativeWindow()->GetBoundsInScreen();
-    EXPECT_EQ(root_window_bounds, container_window_bounds);
-  }
+  views::Widget* widget = container_view()->GetWidget();
+
+  gfx::Rect root_window_bounds =
+      display::Screen::GetScreen()
+          ->GetDisplayNearestWindow(widget->GetNativeWindow()->GetRootWindow())
+          .bounds();
+  gfx::Rect container_window_bounds =
+      widget->GetNativeWindow()->GetBoundsInScreen();
+  EXPECT_EQ(root_window_bounds, container_window_bounds);
 
   // Clean up.
   CloseAmbientScreen();

ash/ambient/ui/glanceable_info_view.cc

@@ -74,7 +74,7 @@ GlanceableInfoView::GlanceableInfoView(AmbientViewDelegate* delegate)
   DCHECK(delegate);
   SetID(AmbientViewID::kAmbientGlanceableInfoView);
   auto* backend_model = delegate_->GetAmbientBackendModel();
-  scoped_backend_model_observer_.Observe(backend_model);
+  backend_model->AddObserver(this);
 
   InitLayout();
 
@@ -84,7 +84,9 @@ GlanceableInfoView::GlanceableInfoView(AmbientViewDelegate* delegate)
   }
 }
 
-GlanceableInfoView::~GlanceableInfoView() = default;
+GlanceableInfoView::~GlanceableInfoView() {
+  delegate_->GetAmbientBackendModel()->RemoveObserver(this);
+}
 
 const char* GlanceableInfoView::GetClassName() const {
   return "GlanceableInfoView";

ash/ambient/ui/glanceable_info_view.h

@@ -5,9 +5,7 @@
 #ifndef ASH_AMBIENT_UI_GLANCEABLE_INFO_VIEW_H_
 #define ASH_AMBIENT_UI_GLANCEABLE_INFO_VIEW_H_
 
-#include "ash/ambient/model/ambient_backend_model.h"
 #include "ash/ambient/model/ambient_backend_model_observer.h"
-#include "base/scoped_observation.h"
 #include "ui/views/view.h"
 
 namespace views {
@@ -55,9 +53,6 @@ class GlanceableInfoView : public views::View,
 
   // Owned by |AmbientController|.
   AmbientViewDelegate* const delegate_ = nullptr;
-
-  base::ScopedObservation<AmbientBackendModel, AmbientBackendModelObserver>
-      scoped_backend_model_observer_{this};
 };
 
 }  // namespace ash

ash/ambient/ui/media_string_view_unittest.cc

@@ -269,8 +269,7 @@ TEST_F(MediaStringViewTest, HasNoMaskLayerWithShortText) {
       media_session::mojom::MediaPlaybackState::kPlaying);
   SimulateMediaMetadataChanged(metadata);
   // Force re-layout.
-  for (auto* view : GetContainerViews())
-    view->Layout();
+  container_view()->Layout();
 
   EXPECT_LT(GetMediaStringViewTextLabel()->GetPreferredSize().width(),
             kMediaStringMaxWidthDip);
@@ -289,8 +288,7 @@ TEST_F(MediaStringViewTest, HasMaskLayerWithLongText) {
       media_session::mojom::MediaPlaybackState::kPlaying);
   SimulateMediaMetadataChanged(metadata);
   // Force re-layout.
-  for (auto* view : GetContainerViews())
-    view->Layout();
+  container_view()->Layout();
 
   EXPECT_GT(GetMediaStringViewTextLabel()->GetPreferredSize().width(),
             kMediaStringMaxWidthDip);
@@ -309,8 +307,7 @@ TEST_F(MediaStringViewTest, MaskLayerShouldUpdate) {
       media_session::mojom::MediaPlaybackState::kPlaying);
   SimulateMediaMetadataChanged(metadata);
   // Force re-layout.
-  for (auto* view : GetContainerViews())
-    view->Layout();
+  container_view()->Layout();
 
   EXPECT_LT(GetMediaStringViewTextLabel()->GetPreferredSize().width(),
             kMediaStringMaxWidthDip);
@@ -322,8 +319,7 @@ TEST_F(MediaStringViewTest, MaskLayerShouldUpdate) {
 
   SimulateMediaMetadataChanged(metadata);
   // Force re-layout.
-  for (auto* view : GetContainerViews())
-    view->Layout();
+  container_view()->Layout();
 
   EXPECT_GT(GetMediaStringViewTextLabel()->GetPreferredSize().width(),
             kMediaStringMaxWidthDip);
@@ -335,8 +331,7 @@ TEST_F(MediaStringViewTest, MaskLayerShouldUpdate) {
 
   SimulateMediaMetadataChanged(metadata);
   // Force re-layout.
-  for (auto* view : GetContainerViews())
-    view->Layout();
+  container_view()->Layout();
 
   EXPECT_LT(GetMediaStringViewTextLabel()->GetPreferredSize().width(),
             kMediaStringMaxWidthDip);

ash/ambient/ui/photo_view.cc

@@ -44,7 +44,9 @@ PhotoView::PhotoView(AmbientViewDelegate* delegate) : delegate_(delegate) {
   Init();
 }
 
-PhotoView::~PhotoView() = default;
+PhotoView::~PhotoView() {
+  delegate_->GetAmbientBackendModel()->RemoveObserver(this);
+}
 
 const char* PhotoView::GetClassName() const {
   return "PhotoView";
@@ -80,7 +82,7 @@ void PhotoView::Init() {
   image_views_[1]->layer()->SetOpacity(0.0f);
 
   auto* model = delegate_->GetAmbientBackendModel();
-  scoped_backend_model_observer_.Observe(model);
+  model->AddObserver(this);
 
   UpdateImage(model->GetCurrentImage());
 }

ash/ambient/ui/photo_view.h

@@ -7,12 +7,10 @@
 
 #include <memory>
 
-#include "ash/ambient/model/ambient_backend_model.h"
 #include "ash/ambient/model/ambient_backend_model_observer.h"
 #include "ash/ambient/ui/ambient_background_image_view.h"
 #include "ash/ash_export.h"
 #include "base/macros.h"
-#include "base/scoped_observation.h"
 #include "ui/compositor/layer_animation_observer.h"
 #include "ui/views/view.h"
 
@@ -68,9 +66,6 @@ class ASH_EXPORT PhotoView : public views::View,
 
   // The index of |image_views_| to update the next image.
   int image_index_ = 0;
-
-  base::ScopedObservation<AmbientBackendModel, AmbientBackendModelObserver>
-      scoped_backend_model_observer_{this};
 };
 
 }  // namespace ash

ash/root_window_controller.cc

@@ -12,7 +12,6 @@
 #include "ash/accessibility/accessibility_panel_layout_manager.h"
 #include "ash/accessibility/touch_exploration_controller.h"
 #include "ash/accessibility/touch_exploration_manager.h"
-#include "ash/ambient/ambient_controller.h"
 #include "ash/app_menu/app_menu_model_adapter.h"
 #include "ash/focus_cycler.h"
 #include "ash/high_contrast/high_contrast_controller.h"
@@ -97,7 +96,6 @@
 #include "ui/views/controls/menu/menu_runner.h"
 #include "ui/views/view_model.h"
 #include "ui/views/view_model_utils.h"
-#include "ui/views/widget/widget.h"
 #include "ui/wm/core/capture_controller.h"
 #include "ui/wm/core/coordinate_conversion.h"
 #include "ui/wm/core/visibility_controller.h"
@@ -651,7 +649,6 @@ void RootWindowController::Shutdown() {
 
   touch_exploration_manager_.reset();
   wallpaper_widget_controller_.reset();
-  CloseAmbientWidget(/*immediately=*/true);
 
   CloseChildWindows();
   aura::Window* root_window = GetRootWindow();
@@ -818,27 +815,6 @@ void RootWindowController::UpdateAfterLoginStatusChange(LoginStatus status) {
     status_area_widget->UpdateAfterLoginStatusChange(status);
 }
 
-void RootWindowController::CreateAmbientWidget() {
-  DCHECK(!ambient_widget_);
-
-  auto* ambient_controller = Shell::Get()->ambient_controller();
-  if (ambient_controller && ambient_controller->IsShown()) {
-    ambient_widget_ = ambient_controller->CreateWidget(
-        GetRootWindow()->GetChildById(kShellWindowId_AmbientModeContainer));
-  }
-}
-
-void RootWindowController::CloseAmbientWidget(bool immediately) {
-  if (ambient_widget_) {
-    if (immediately)
-      ambient_widget_->CloseNow();
-    else
-      ambient_widget_->CloseWithReason(views::Widget::ClosedReason::kLostFocus);
-  }
-
-  ambient_widget_.reset();
-}
-
 AccessibilityPanelLayoutManager*
 RootWindowController::GetAccessibilityPanelLayoutManagerForTest() {
   return GetAccessibilityPanelLayoutManager();
@@ -907,8 +883,6 @@ void RootWindowController::Init(RootWindowType root_window_type) {
       Shell::Get()->session_controller()->IsUserSessionBlocked());
   root_window_layout_manager_->OnWindowResized();
 
-  CreateAmbientWidget();
-
   // Explicitly update the desks controller before notifying the ShellObservers.
   // This is to make sure the desks' states are correct before clients are
   // updated.

ash/root_window_controller.h

@@ -30,8 +30,7 @@ class WindowTreeHost;
 
 namespace views {
 class MenuRunner;
-class Widget;
-}  // namespace views
+}
 
 namespace wm {
 class ScopedCaptureClient;
@@ -223,11 +222,6 @@ class ASH_EXPORT RootWindowController {
   // Called when the login status changes after login (such as lock/unlock).
   void UpdateAfterLoginStatusChange(LoginStatus status);
 
-  void CreateAmbientWidget();
-  void CloseAmbientWidget(bool immediately);
-
-  views::Widget* ambient_widget_for_testing() { return ambient_widget_.get(); }
-
   // Returns accessibility panel layout manager for this root window.
   AccessibilityPanelLayoutManager* GetAccessibilityPanelLayoutManagerForTest();
 
@@ -311,8 +305,6 @@ class ASH_EXPORT RootWindowController {
   std::unique_ptr<LockScreenActionBackgroundController>
       lock_screen_action_background_controller_;
 
-  std::unique_ptr<views::Widget> ambient_widget_;
-
   // Whether child windows have been closed during shutdown. Exists to avoid
   // calling related cleanup code more than once.
   bool did_close_child_windows_ = false;