Revert "Reland "[ambient] Notify the JS side when screen saver is clo…

…sed"" This reverts commit bed8604. Reason for revert: Causing Linux Chromium OS ASan LSan Tests (1) to fail. Original change's description: > Reland "[ambient] Notify the JS side when screen saver is closed" > > This is a reland of commit 90408e4 > > Original change's description: > > [ambient] Notify the JS side when screen saver is closed > > > > When the screensaver is closed for any reason, we notify the JS side. As a result the UI is updated. > > > > Bug: b/259716397 > > Change-Id: I2a78a0cb7be8dbf7e19cc741295ce29e3aebbcfb > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4021018 > > Reviewed-by: Alex Gough <ajgo@chromium.org> > > Commit-Queue: Ilkin Safarli <safarli@google.com> > > Reviewed-by: Yuki Awano <yawano@google.com> > > Reviewed-by: Tao Wu <wutao@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#1074824} > > Bug: b/259716397 > Change-Id: Iddae022254bf494eab65e7c633eca231b02d57cc > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4053420 > Reviewed-by: Yuki Awano <yawano@google.com> > Commit-Queue: Ilkin Safarli <safarli@google.com> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> > Reviewed-by: Alex Gough <ajgo@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1076375} Bug: b/259716397 Change-Id: Ifdeed629d6c9aea6bb6dbb72468fc6dd787fea89 Fixed: 1394315 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4060301 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Giovanni Ortuno Urquidi <ortuno@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Owners-Override: Giovanni Ortuno Urquidi <ortuno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1076686}
chromium · Nov 29, 2022 · a596f88 · a596f88
1 parent 9db2fed
commit a596f88
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 53 deletions.
diff --git a/ash/public/cpp/ambient/ambient_ui_model.h b/ash/public/cpp/ambient/ambient_ui_model.h
@@ -16,9 +16,9 @@ namespace ash {
 enum class AmbientUiVisibility {
   kShown,    // Screen saver is shown.
   kPreview,  // Same as kShown, but do not lock screen or acquire wake lock.
-             // kPreview state is used to show a preview of the screen saver.
-             // Users should be able to exit from the preview mode directly.
-             // Hence, no need to lock the screen or acquire wake lock.
+             // kPreview state is used to show a preview of the screensaver.
+             // Users should be able to exit from the preview mode directly
+             // into. Hence, no need to lock the screen or acquire wake lock.
   kHidden,   // Screen saver is closed; start inactivity timer to restart it.
   kClosed,   // Screen saver is closed; all observers and timers are cancelled.
 };

diff --git a/ash/webui/personalization_app/mojom/personalization_app.mojom b/ash/webui/personalization_app/mojom/personalization_app.mojom
@@ -529,9 +529,6 @@ interface AmbientObserver {
   // Notifies the JS side about the previews of the selected Google Photos
   // albums.
   OnGooglePhotosAlbumsPreviewsFetched(array<url.mojom.Url> previews);
-
-  // Nofities the JS side when the screen saver is closed.
-  OnScreenSaverClosed();
 };
 
 // Provides APIs to expose Ambient mode settings.

diff --git a/ash/webui/personalization_app/resources/js/ambient/ambient_observer.ts b/ash/webui/personalization_app/resources/js/ambient/ambient_observer.ts
@@ -91,6 +91,4 @@ export class AmbientObserver implements AmbientObserverInterface {
     const store = PersonalizationStore.getInstance();
     store.dispatch(setGooglePhotosAlbumsPreviewsAction(previews));
   }
-
-  onScreenSaverClosed() {}
 }
diff --git a/...ser/ash/web_applications/personalization_app/personalization_app_ambient_provider_impl.cc b/...ser/ash/web_applications/personalization_app/personalization_app_ambient_provider_impl.cc
@@ -81,8 +81,6 @@ PersonalizationAppAmbientProviderImpl::PersonalizationAppAmbientProviderImpl(
       base::BindRepeating(
           &PersonalizationAppAmbientProviderImpl::OnAnimationThemeChanged,
           base::Unretained(this)));
-  ambient_ui_model_observer_.Observe(
-      Shell::Get()->ambient_controller()->ambient_ui_model());
 }
 
 PersonalizationAppAmbientProviderImpl::
@@ -578,12 +576,4 @@ void PersonalizationAppAmbientProviderImpl::StartScreenSaverPreview() {
   Shell::Get()->ambient_controller()->StartScreenSaverPreview();
 }
 
-void PersonalizationAppAmbientProviderImpl::OnAmbientUiVisibilityChanged(
-    AmbientUiVisibility visibility) {
-  if (ambient_observer_remote_.is_bound() &&
-      visibility == AmbientUiVisibility::kClosed) {
-    ambient_observer_remote_->OnScreenSaverClosed();
-  }
-}
-
 }  // namespace ash::personalization_app
diff --git a/...wser/ash/web_applications/personalization_app/personalization_app_ambient_provider_impl.h b/...wser/ash/web_applications/personalization_app/personalization_app_ambient_provider_impl.h
@@ -5,10 +5,8 @@
 #ifndef CHROME_BROWSER_ASH_WEB_APPLICATIONS_PERSONALIZATION_APP_PERSONALIZATION_APP_AMBIENT_PROVIDER_IMPL_H_
 #define CHROME_BROWSER_ASH_WEB_APPLICATIONS_PERSONALIZATION_APP_PERSONALIZATION_APP_AMBIENT_PROVIDER_IMPL_H_
 
-#include "base/scoped_observation.h"
-
 #include "ash/constants/ambient_animation_theme.h"
-#include "ash/public/cpp/ambient/ambient_ui_model.h"
+#include "ash/public/cpp/ambient/ambient_backend_controller.h"
 #include "ash/public/cpp/ambient/common/ambient_settings.h"
 #include "ash/webui/personalization_app/mojom/personalization_app.mojom.h"
 #include "ash/webui/personalization_app/personalization_app_ambient_provider.h"
@@ -28,8 +26,7 @@ class Profile;
 namespace ash::personalization_app {
 
 class PersonalizationAppAmbientProviderImpl
-    : public PersonalizationAppAmbientProvider,
-      public AmbientUiModelObserver {
+    : public PersonalizationAppAmbientProvider {
  public:
   explicit PersonalizationAppAmbientProviderImpl(content::WebUI* web_ui);
 
@@ -44,9 +41,6 @@ class PersonalizationAppAmbientProviderImpl
       mojo::PendingReceiver<ash::personalization_app::mojom::AmbientProvider>
           receiver) override;
 
-  // AmbientUiModelObserver:
-  void OnAmbientUiVisibilityChanged(AmbientUiVisibility visibility) override;
-
   // ash::personalization_app::mojom:AmbientProvider:
   void IsAmbientModeEnabled(IsAmbientModeEnabledCallback callback) override;
   void SetAmbientObserver(
@@ -107,8 +101,7 @@ class PersonalizationAppAmbientProviderImpl
 
   void FetchGooglePhotosAlbumsPreviews(
       const std::vector<std::string>& album_ids);
-  void OnGooglePhotosAlbumsPreviewsFetched(
-      const std::vector<GURL>& preview_urls);
+  void OnGooglePhotosAlbumsPreviewsFetched(const std::vector<GURL>& preview_urls);
 
   ash::PersonalAlbum* FindPersonalAlbumById(const std::string& album_id);
 
@@ -160,9 +153,6 @@ class PersonalizationAppAmbientProviderImpl
   // A flag to record if the user has seen the ambient mode page.
   bool page_viewed_ = false;
 
-  base::ScopedObservation<AmbientUiModel, AmbientUiModelObserver>
-      ambient_ui_model_observer_{this};
-
   base::WeakPtrFactory<PersonalizationAppAmbientProviderImpl>
       write_weak_factory_{this};
   base::WeakPtrFactory<PersonalizationAppAmbientProviderImpl>

diff --git a/...eb_applications/personalization_app/personalization_app_ambient_provider_impl_unittest.cc b/...eb_applications/personalization_app/personalization_app_ambient_provider_impl_unittest.cc
@@ -6,14 +6,12 @@
 
 #include <memory>
 #include <vector>
-#include "ash/ambient/ambient_controller.h"
+
 #include "ash/ambient/test/ambient_ash_test_helper.h"
 #include "ash/constants/ambient_animation_theme.h"
 #include "ash/public/cpp/ambient/ambient_prefs.h"
 #include "ash/public/cpp/ambient/common/ambient_settings.h"
 #include "ash/public/cpp/ambient/fake_ambient_backend_controller_impl.h"
-#include "ash/shell.h"
-#include "ash/test/ash_test_base.h"
 #include "ash/webui/personalization_app/mojom/personalization_app.mojom.h"
 #include "base/callback_helpers.h"
 #include "base/ranges/algorithm.h"
@@ -70,8 +68,6 @@ class TestAmbientObserver
     previews_ = std::move(previews);
   }
 
-  void OnScreenSaverClosed() override { screen_saver_closed_ = true; }
-
   mojo::PendingRemote<ash::personalization_app::mojom::AmbientObserver>
   pending_remote() {
     if (ambient_observer_receiver_.is_bound()) {
@@ -116,8 +112,6 @@ class TestAmbientObserver
       ambient_observer_receiver_{this};
 
   bool ambient_mode_enabled_ = false;
-  bool screen_saver_closed_ = false;
-
   ash::AmbientAnimationTheme animation_theme_ =
       ash::AmbientAnimationTheme::kSlideshow;
   ash::AmbientModeTopicSource topic_source_ =
@@ -130,13 +124,10 @@ class TestAmbientObserver
 
 }  // namespace
 
-class PersonalizationAppAmbientProviderImplTest : public ash::AshTestBase {
+class PersonalizationAppAmbientProviderImplTest : public testing::Test {
  public:
   PersonalizationAppAmbientProviderImplTest()
-      : ash::AshTestBase(std::unique_ptr<base::test::TaskEnvironment>(
-            std::make_unique<content::BrowserTaskEnvironment>(
-                base::test::TaskEnvironment::TimeSource::MOCK_TIME))),
-        profile_manager_(TestingBrowserProcess::GetGlobal()) {}
+      : profile_manager_(TestingBrowserProcess::GetGlobal()) {}
   PersonalizationAppAmbientProviderImplTest(
       const PersonalizationAppAmbientProviderImplTest&) = delete;
   PersonalizationAppAmbientProviderImplTest& operator=(
@@ -146,8 +137,6 @@ class PersonalizationAppAmbientProviderImplTest : public ash::AshTestBase {
  protected:
   // testing::Test:
   void SetUp() override {
-    ash::AshTestBase::SetUp();
-
     ASSERT_TRUE(profile_manager_.SetUp());
     profile_ = profile_manager_.CreateTestingProfile(kFakeTestEmail);
 
@@ -162,17 +151,12 @@ class PersonalizationAppAmbientProviderImplTest : public ash::AshTestBase {
         ambient_provider_remote_.BindNewPipeAndPassReceiver());
 
     SetEnabledPref(true);
-    GetAmbientAshTestHelper()->ambient_client().SetAutomaticalyIssueToken(true);
-
-    Shell::Get()->ambient_controller()->set_backend_controller_for_testing(
-        nullptr);
-
     fake_backend_controller_ =
         std::make_unique<ash::FakeAmbientBackendControllerImpl>();
+    ambient_ash_test_helper_ = std::make_unique<ash::AmbientAshTestHelper>();
+    ambient_ash_test_helper_->ambient_client().SetAutomaticalyIssueToken(true);
   }
 
-  void TearDown() override { ash::AshTestBase::TearDown(); }
-
   TestingProfile* profile() { return profile_; }
 
   mojo::Remote<ash::personalization_app::mojom::AmbientProvider>&
@@ -304,7 +288,7 @@ class PersonalizationAppAmbientProviderImplTest : public ash::AshTestBase {
   }
 
   void FastForwardBy(base::TimeDelta time) {
-    task_environment()->FastForwardBy(time);
+    task_environment_.FastForwardBy(time);
   }
 
   bool IsFetchSettingsPendingAtBackend() const {
@@ -327,6 +311,8 @@ class PersonalizationAppAmbientProviderImplTest : public ash::AshTestBase {
   }
 
  private:
+  content::BrowserTaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
   TestingProfileManager profile_manager_;
   content::TestWebUI web_ui_;
   std::unique_ptr<content::WebContents> web_contents_;