personalization: update dynamic color buttons

Listen for updates to the wallpaper or dark/light mode changing and update the color scheme buttons. BUG=b:254479725 TEST=unit_tests *ThemeProvider* TEST=ash_unittests *ColorPalette* TEST=browser_tests *PersonalizationApp*DynamicColor* Change-Id: I3738ddf55500be7b19cb55218f8923b21d25ca19 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4143923 Reviewed-by: Jeffrey Young <cowmoo@google.com> Reviewed-by: Sean Kau <skau@chromium.org> Commit-Queue: Erica Lee <ericamlee@google.com> Reviewed-by: Mustafa Emre Acer <meacer@chromium.org> Cr-Commit-Position: refs/heads/main@{#1099591}
chromium · Feb 1, 2023 · bf3f1b9 · bf3f1b9
1 parent df6356e
commit bf3f1b9
Show file tree

Hide file tree

Showing 9 changed files with 141 additions and 15 deletions.
diff --git a/ash/style/color_palette_controller.cc b/ash/style/color_palette_controller.cc
@@ -126,7 +126,7 @@ class ColorPaletteControllerImpl : public ColorPaletteController {
   }
 
   void GenerateSampleColorSchemes(
-      const std::vector<ColorScheme>& color_scheme_buttons,
+      base::span<const ColorScheme> color_scheme_buttons,
       SampleColorSchemeCallback callback) const override {
     std::vector<SampleColorScheme> samples;
     for (auto scheme : color_scheme_buttons) {

diff --git a/ash/style/color_palette_controller.h b/ash/style/color_palette_controller.h
@@ -6,6 +6,7 @@
 #define ASH_STYLE_COLOR_PALETTE_CONTROLLER_H_
 
 #include "ash/ash_export.h"
+#include "base/containers/span.h"
 #include "base/observer_list_types.h"
 #include "components/account_id/account_id.h"
 #include "components/prefs/pref_registry_simple.h"
@@ -118,7 +119,7 @@ class ASH_EXPORT ColorPaletteController {
   using SampleColorSchemeCallback =
       base::OnceCallback<void(const std::vector<ash::SampleColorScheme>&)>;
   virtual void GenerateSampleColorSchemes(
-      const std::vector<ColorScheme>& color_scheme_buttons,
+      base::span<const ColorScheme> color_scheme_buttons,
       SampleColorSchemeCallback callback) const = 0;
 };
 

diff --git a/ash/webui/personalization_app/mojom/personalization_app.mojom b/ash/webui/personalization_app/mojom/personalization_app.mojom
@@ -370,6 +370,9 @@ interface ThemeObserver {
   //Notifies the JS side about the current state of the color scheme.
   OnColorSchemeChanged(ColorScheme color_scheme);
 
+  // Notifies the JS side that the sample color schemes have changed.
+  OnSampleColorSchemesChanged(array<SampleColorScheme> sample_color_schemes);
+
   // Notifies the JS side about the current state of the static color.
   OnStaticColorChanged(skia.mojom.SkColor? color);
 };

diff --git a/ash/webui/personalization_app/mojom/personalization_app_mojom_traits.cc b/ash/webui/personalization_app/mojom/personalization_app_mojom_traits.cc
@@ -533,12 +533,12 @@ StructTraits<ash::personalization_app::mojom::SampleColorSchemeDataView,
   return sample_color_scheme.scheme;
 }
 
-// Default to false as we don't ever need to convert back to
-// `ash::ColorSchemeSample`.
 bool StructTraits<ash::personalization_app::mojom::SampleColorSchemeDataView,
                   ash::SampleColorScheme>::
     Read(ash::personalization_app::mojom::SampleColorSchemeDataView data,
          ash::SampleColorScheme* out) {
-  return false;
+  return data.ReadScheme(&out->scheme) && data.ReadPrimary(&out->primary) &&
+         data.ReadSecondary(&out->secondary) &&
+         data.ReadTertiary(&out->tertiary);
 }
 }  // namespace mojo
diff --git a/ash/webui/personalization_app/resources/js/theme/theme_observer.ts b/ash/webui/personalization_app/resources/js/theme/theme_observer.ts
@@ -4,10 +4,10 @@
 
 import {SkColor} from 'chrome://resources/mojo/skia/public/mojom/skcolor.mojom-webui.js';
 
-import {ColorScheme, ThemeObserverInterface, ThemeObserverReceiver, ThemeProviderInterface} from '../personalization_app.mojom-webui.js';
+import {ColorScheme, SampleColorScheme, ThemeObserverInterface, ThemeObserverReceiver, ThemeProviderInterface} from '../personalization_app.mojom-webui.js';
 import {PersonalizationStore} from '../personalization_store.js';
 
-import {setColorModeAutoScheduleEnabledAction, setColorSchemeAction, setDarkModeEnabledAction, setStaticColorAction} from './theme_actions.js';
+import {setColorModeAutoScheduleEnabledAction, setColorSchemeAction, setDarkModeEnabledAction, setSampleColorSchemesAction, setStaticColorAction} from './theme_actions.js';
 import {getThemeProvider} from './theme_interface_provider.js';
 
 /** @fileoverview listens for updates on color mode changes. */
@@ -56,6 +56,11 @@ export class ThemeObserver implements ThemeObserverInterface {
     store.dispatch(setColorSchemeAction(scheme));
   }
 
+  onSampleColorSchemesChanged(sampleColorSchemes: SampleColorScheme[]): void {
+    const store = PersonalizationStore.getInstance();
+    store.dispatch(setSampleColorSchemesAction(sampleColorSchemes));
+  }
+
   onStaticColorChanged(staticColor: SkColor): void {
     const store = PersonalizationStore.getInstance();
     store.dispatch(setStaticColorAction(staticColor));

diff --git a/...owser/ash/web_applications/personalization_app/personalization_app_theme_provider_impl.cc b/...owser/ash/web_applications/personalization_app/personalization_app_theme_provider_impl.cc
@@ -7,7 +7,9 @@
 #include "ash/constants/ash_features.h"
 #include "ash/constants/ash_pref_names.h"
 #include "ash/public/cpp/schedule_enums.h"
+#include "ash/shell.h"
 #include "ash/style/color_palette_controller.h"
+#include "ash/style/color_util.h"
 #include "ash/system/scheduled_feature/scheduled_feature.h"
 #include "chrome/browser/ash/web_applications/personalization_app/personalization_app_metrics.h"
 #include "chrome/browser/ash/web_applications/personalization_app/personalization_app_utils.h"
@@ -16,6 +18,15 @@
 
 namespace ash::personalization_app {
 
+// This array represents the order, number, and types of color schemes
+// represented by the color scheme buttons in the app.
+const std::array<ColorScheme, 4> kColorSchemeButtons{
+    ColorScheme::kTonalSpot,
+    ColorScheme::kNeutral,
+    ColorScheme::kVibrant,
+    ColorScheme::kExpressive,
+};
+
 PersonalizationAppThemeProviderImpl::PersonalizationAppThemeProviderImpl(
     content::WebUI* web_ui)
     : profile_(Profile::FromWebUI(web_ui)) {
@@ -77,6 +88,9 @@ void PersonalizationAppThemeProviderImpl::SetThemeObserver(
               &PersonalizationAppThemeProviderImpl::OnStaticColorChanged,
               base::Unretained(this)));
     }
+    ui::ColorProviderSourceObserver::Observe(
+        ash::ColorUtil::GetColorProviderSourceForWindow(
+            ash::Shell::GetPrimaryRootWindow()));
   }
 }
 
@@ -132,6 +146,12 @@ void PersonalizationAppThemeProviderImpl::OnStaticColorChanged() {
       color_palette_controller_->GetStaticColor(GetAccountId(profile_)));
 }
 
+void PersonalizationAppThemeProviderImpl::OnSampleColorSchemesChanged(
+    const std::vector<ash::SampleColorScheme>& sampleColorSchemes) {
+  DCHECK(theme_observer_remote_.is_bound());
+  theme_observer_remote_->OnSampleColorSchemesChanged(sampleColorSchemes);
+}
+
 bool PersonalizationAppThemeProviderImpl::IsColorModeAutoScheduleEnabled() {
   PrefService* pref_service = profile_->GetPrefs();
   DCHECK(pref_service);
@@ -192,14 +212,13 @@ void PersonalizationAppThemeProviderImpl::SetStaticColor(SkColor static_color) {
 
 void PersonalizationAppThemeProviderImpl::GenerateSampleColorSchemes(
     GenerateSampleColorSchemesCallback callback) {
-  const std::vector<ColorScheme> color_scheme_buttons = {
-      ColorScheme::kTonalSpot,
-      ColorScheme::kNeutral,
-      ColorScheme::kVibrant,
-      ColorScheme::kExpressive,
-  };
-  color_palette_controller_->GenerateSampleColorSchemes(color_scheme_buttons,
+  color_palette_controller_->GenerateSampleColorSchemes(kColorSchemeButtons,
                                                         std::move(callback));
 }
 
+void PersonalizationAppThemeProviderImpl::OnColorProviderChanged() {
+  GenerateSampleColorSchemes(base::BindOnce(
+      &PersonalizationAppThemeProviderImpl::OnSampleColorSchemesChanged,
+      weak_factory_.GetWeakPtr()));
+}
 }  // namespace ash::personalization_app
diff --git a/...rowser/ash/web_applications/personalization_app/personalization_app_theme_provider_impl.h b/...rowser/ash/web_applications/personalization_app/personalization_app_theme_provider_impl.h
@@ -15,6 +15,7 @@
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/color/color_provider_source_observer.h"
 
 class Profile;
 
@@ -26,7 +27,8 @@ namespace ash::personalization_app {
 
 class PersonalizationAppThemeProviderImpl
     : public PersonalizationAppThemeProvider,
-      ash::ColorModeObserver {
+      public ash::ColorModeObserver,
+      public ui::ColorProviderSourceObserver {
  public:
   explicit PersonalizationAppThemeProviderImpl(content::WebUI* web_ui);
 
@@ -63,6 +65,9 @@ class PersonalizationAppThemeProviderImpl
   // ash::ColorModeObserver:
   void OnColorModeChanged(bool dark_mode_enabled) override;
 
+  // ui::ColorProviderSourceObserver:
+  void OnColorProviderChanged() override;
+
   void GetColorScheme(GetColorSchemeCallback callback) override;
 
   void GetStaticColor(GetStaticColorCallback callback) override;
@@ -78,6 +83,9 @@ class PersonalizationAppThemeProviderImpl
 
   void OnColorSchemeChanged();
 
+  void OnSampleColorSchemesChanged(
+      const std::vector<ash::SampleColorScheme>& sampleColorSchemes);
+
   void OnStaticColorChanged();
 
   // Pointer to profile of user that opened personalization SWA. Not owned.
@@ -94,8 +102,17 @@ class PersonalizationAppThemeProviderImpl
   mojo::Receiver<ash::personalization_app::mojom::ThemeProvider>
       theme_receiver_{this};
 
+  // The ColorProviderSourceObserver notifies whenever the ColorProvider is
+  // updated, such as when dark light mode changes or a new wallpaper is
+  // added.
+  base::ScopedObservation<ui::ColorProviderSource,
+                          ui::ColorProviderSourceObserver>
+      color_provider_source_observer_{this};
+
   mojo::Remote<ash::personalization_app::mojom::ThemeObserver>
       theme_observer_remote_;
+
+  base::WeakPtrFactory<PersonalizationAppThemeProviderImpl> weak_factory_{this};
 };
 
 }  // namespace ash::personalization_app

diff --git a/.../web_applications/personalization_app/personalization_app_theme_provider_impl_unittest.cc b/.../web_applications/personalization_app/personalization_app_theme_provider_impl_unittest.cc
@@ -13,6 +13,7 @@
 #include "ash/style/dark_light_mode_controller_impl.h"
 #include "ash/test/ash_test_base.h"
 #include "base/run_loop.h"
+#include "base/test/bind.h"
 #include "base/test/gmock_callback_support.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/mock_callback.h"
@@ -29,6 +30,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/native_theme/native_theme.h"
 
 namespace ash::personalization_app {
 
@@ -64,6 +66,20 @@ class TestThemeObserver
     color_scheme_ = color_scheme;
   }
 
+  void OnSampleColorSchemesChanged(const std::vector<ash::SampleColorScheme>&
+                                       sample_color_schemes) override {
+    sample_color_schemes_ = sample_color_schemes;
+    on_sample_color_schemes_changed_count_ += 1;
+  }
+
+  int on_sample_color_schemes_changed_count() {
+    if (!theme_observer_receiver_.is_bound()) {
+      return 0;
+    }
+    theme_observer_receiver_.FlushForTesting();
+    return on_sample_color_schemes_changed_count_;
+  }
+
   void OnStaticColorChanged(absl::optional<::SkColor> static_color) override {
     static_color_ = static_color;
   }
@@ -113,8 +129,10 @@ class TestThemeObserver
 
   bool dark_mode_enabled_ = false;
   bool color_mode_auto_schedule_enabled_ = false;
+  int on_sample_color_schemes_changed_count_ = 0;
   ash::ColorScheme color_scheme_ = ash::ColorScheme::kTonalSpot;
   absl::optional<::SkColor> static_color_ = absl::nullopt;
+  std::vector<ash::SampleColorScheme> sample_color_schemes_;
 };
 
 }  // namespace
@@ -202,6 +220,13 @@ class PersonalizationAppThemeProviderImplTest : public ChromeAshTestBase {
     return test_theme_observer_.GetStaticColor();
   }
 
+  int GetOnSampleColorSchemesChangedCount() {
+    if (theme_provider_remote_.is_bound()) {
+      theme_provider_remote_.FlushForTesting();
+    }
+    return test_theme_observer_.on_sample_color_schemes_changed_count();
+  }
+
   const base::HistogramTester& histogram_tester() { return histogram_tester_; }
 
  private:
@@ -381,4 +406,33 @@ TEST_F(PersonalizationAppThemeProviderImplJellyTest,
   run_loop.Run();
 }
 
+TEST_F(PersonalizationAppThemeProviderImplJellyTest,
+       OnSampleColorSchemesChanged) {
+  ui::NativeTheme* theme = ui::NativeTheme::GetInstanceForNativeUi();
+  SetThemeObserver();
+  theme_provider_remote()->FlushForTesting();
+  int expected_color_schemes_changed_count =
+      GetOnSampleColorSchemesChangedCount() + 1;
+
+  // Changing the theme should trigger an update to the sample color schemes.
+  theme->NotifyOnNativeThemeUpdated();
+
+  base::RunLoop run_loop;
+  base::RepeatingTimer repeating_timer;
+  int count = 0;
+  repeating_timer.Start(FROM_HERE, base::Milliseconds(10),
+                        base::BindLambdaForTesting([&]() {
+                          count++;
+                          if (GetOnSampleColorSchemesChangedCount() >=
+                                  expected_color_schemes_changed_count ||
+                              count > 10) {
+                            repeating_timer.Stop();
+                            run_loop.Quit();
+                          }
+                        }));
+  run_loop.Run();
+  EXPECT_EQ(expected_color_schemes_changed_count,
+            GetOnSampleColorSchemesChangedCount());
+}
+
 }  // namespace ash::personalization_app
diff --git a/chrome/test/data/webui/chromeos/personalization_app/dynamic_color_element_test.ts b/chrome/test/data/webui/chromeos/personalization_app/dynamic_color_element_test.ts
@@ -139,6 +139,33 @@ suite('DynamicColorElementTest', function() {
     assertEquals(colorScheme, action.colorScheme);
   });
 
+  test('sets sample color schemes in store on changed', async () => {
+    assertDeepEquals(emptyState(), personalizationStore.data);
+    await themeProvider.whenCalled('setThemeObserver');
+    const sampleColorSchemes = [
+      ColorScheme.kTonalSpot,
+      ColorScheme.kExpressive,
+      ColorScheme.kNeutral,
+      ColorScheme.kVibrant,
+    ].map((colorScheme) => {
+      return {
+        scheme: colorScheme,
+        primary: hexColorToSkColor('#eeeeee'),
+        secondary: hexColorToSkColor('#eeeeee'),
+        tertiary: hexColorToSkColor('#eeeeee'),
+      };
+    });
+    personalizationStore.expectAction(ThemeActionName.SET_SAMPLE_COLOR_SCHEMES);
+
+    themeProvider.themeObserverRemote!.onSampleColorSchemesChanged(
+        sampleColorSchemes);
+
+    const action = await personalizationStore.waitForAction(
+                       ThemeActionName.SET_SAMPLE_COLOR_SCHEMES) as
+        SetSampleColorSchemesAction;
+    assertDeepEquals(sampleColorSchemes, action.sampleColorSchemes);
+  });
+
   test('sets static color data in store on changed', async () => {
     const staticColor = hexColorToSkColor('#123456');
     assertDeepEquals(emptyState(), personalizationStore.data);