Extracting more than one wallpaper prominent color

ash/wallpaper/wallpaper_controller.h

Index: ash/wallpaper/wallpaper_controller.h
diff --git a/ash/wallpaper/wallpaper_controller.h b/ash/wallpaper/wallpaper_controller.h
index 3b35434854a9a85db05a6520a81f56c4bd6e4aef..895e517a05c465567178fa9c2b64ff79e1b9b0cd 100644
--- a/ash/wallpaper/wallpaper_controller.h
+++ b/ash/wallpaper/wallpaper_controller.h
@@ -20,13 +20,15 @@
 #include "components/wallpaper/wallpaper_resizer_observer.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "ui/compositor/compositor_lock.h"
-#include "ui/gfx/color_analysis.h"
-#include "ui/gfx/image/image_skia.h"
 
 namespace base {
 class SequencedTaskRunner;
 }
 
+namespace color_utils {
+struct ColorProfile;
+}
+
 namespace wallpaper {
 class WallpaperColorCalculator;
 class WallpaperResizer;
@@ -39,7 +41,7 @@ class WallpaperControllerObserver;
 // Controls the desktop background wallpaper:
 //   - Sets a wallpaper image and layout;
 //   - Handles display change (add/remove display, configuration change etc);
-//   - Calculates prominent color for shelf;
+//   - Calculates prominent colors.
 //   - Move wallpaper to locked container(s) when session state is not ACTIVE to
 //     hide the user desktop and move it to unlocked container when session
 //     state is ACTIVE;
@@ -54,9 +56,20 @@ class ASH_EXPORT WallpaperController
  public:
   enum WallpaperMode { WALLPAPER_NONE, WALLPAPER_IMAGE };
 
-  // The value assigned to |prominent_color_| if extraction fails or the feature
-  // is disabled (e.g. command line, lock/login screens).
-  static constexpr SkColor kInvalidColor = SK_ColorTRANSPARENT;
+  // This enum is used to get the corresponding prominent color from the
+  // calculation results of |color_calculator_|.
+  enum ColorProfileIndex {
+    COLOR_PROFILE_INDEX_DARK_VIBRANT = 0,
+    COLOR_PROFILE_INDEX_NORMAL_VIBRANT,
+    COLOR_PROFILE_INDEX_LIGHT_VIBRANT,
+    COLOR_PROFILE_INDEX_DARK_MUTED,
+    COLOR_PROFILE_INDEX_NORMAL_MUTED,
+    COLOR_PROFILE_INDEX_LIGHT_MUTED,
+  };
+
+  // The value assigned if extraction fails or the feature is disabled (e.g.
+  // command line, lock/login screens).
+  static const SkColor kInvalidColor;

[xdai1, 2017/06/21 16:29:30]

I think we prefer constexpr now?

And for static const integral or enum type, it can be declared and defined in
the class.

[Qiang(Joe) Xu, 2017/06/21 22:35:26]

as discussed offline with xiyuan, constexpr by default doesn't generate storage.
But vector constructor I am using is taking reference:

explicit vector (size_type n, const value_type& val = value_type(),
                 const allocator_type& alloc = allocator_type());

So no change here.

 
   WallpaperController();
   ~WallpaperController() override;
@@ -68,7 +81,12 @@ class ASH_EXPORT WallpaperController
   void AddObserver(WallpaperControllerObserver* observer);
   void RemoveObserver(WallpaperControllerObserver* observer);
 
-  SkColor prominent_color() const { return prominent_color_; }
+  std::vector<SkColor> prominent_colors() const { return prominent_colors_; }
+
+  // Returns the corresponding color profile index based on the color profiles
+  // passed to the |color_calculator_|.
+  WallpaperController::ColorProfileIndex GetColorProfileIndex(

[bruthig, 2017/06/21 15:54:17]

nit:Do WallpaperController clients really need to know about an index and that
the prominent colors are stored in a vector?  Forcing clients to index into a
vector doesn't seem very user friendly to me.
Consider replacing with:

SkColor GetProminentColor(color_utils::ColorProfile) const;

(Sorry I didn't comment on this in earlier patches)

[Qiang(Joe) Xu, 2017/06/21 22:35:27]

Done.

[bruthig, 2017/06/22 15:31:09]

Much thanks =D

+      color_utils::ColorProfile color_profile) const;
 
   // Provides current image on the wallpaper, or empty gfx::ImageSkia if there
   // is no image, e.g. wallpaper is none.
@@ -152,10 +170,10 @@ class ASH_EXPORT WallpaperController
   // wallpaper cahce or not.
   void UpdateWallpaper(bool clear_cache);
 
-  // Sets |prominent_color_| and notifies the observers if there is a change.
-  void SetProminentColor(SkColor color);
+  // Sets |prominent_colors_| and notifies the observers if there is a change.
+  void SetProminentColors(const std::vector<SkColor>& prominent_colors);
 
-  // Calculates a prominent color based on the wallpaper image and notifies
+  // Calculates prominent colors based on the wallpaper image and notifies
   // |observers_| of the value, either synchronously or asynchronously. In some
   // cases the wallpaper image will not actually be processed (e.g. user isn't
   // logged in, feature isn't enabled).
@@ -197,9 +215,12 @@ class ASH_EXPORT WallpaperController
   // Asynchronous task to extract colors from the wallpaper.
   std::unique_ptr<wallpaper::WallpaperColorCalculator> color_calculator_;
 
-  // The prominent color extracted from the current wallpaper.
+  // The prominent colors extracted from the current wallpaper.
   // kInvalidColor is used by default or if extracting colors fails.
-  SkColor prominent_color_;
+  std::vector<SkColor> prominent_colors_;
+
+  // Caches the color profiles that need to do wallpaper color extracting.
+  const std::vector<color_utils::ColorProfile> color_profiles_;
 
   gfx::Size current_max_display_size_;