Extracting more than one wallpaper prominent color

components/wallpaper/wallpaper_color_calculator.cc

Index: components/wallpaper/wallpaper_color_calculator.cc
diff --git a/components/wallpaper/wallpaper_color_calculator.cc b/components/wallpaper/wallpaper_color_calculator.cc
index 499fdd544da2872e05e56088757c6bd60fc01db6..37c932ecd6257d94ec0b48a880835b7e385a8002 100644
--- a/components/wallpaper/wallpaper_color_calculator.cc
+++ b/components/wallpaper/wallpaper_color_calculator.cc
@@ -12,6 +12,10 @@
 #include "base/task_runner.h"
 #include "base/task_runner_util.h"
 #include "components/wallpaper/wallpaper_color_calculator_observer.h"
+#include "components/wallpaper/wallpaper_color_extraction_result.h"
+
+using LumaRange = color_utils::LumaRange;
+using SaturationRange = color_utils::SaturationRange;
 
 namespace wallpaper {
 
@@ -23,24 +27,75 @@ namespace {
 // thread would actually take longer.
 const int kMaxPixelsForSynchronousCalculation = 100;
 
-// Wrapper for color_utils::CalculateProminentColorOfBitmap() that records
+// Wrapper for color_utils::CalculateProminentColorsOfBitmap() that records
 // wallpaper specific metrics.
 //
 // NOTE: |image| is intentionally a copy to ensure it exists for the duration of
 // the calculation.
-SkColor CalculateWallpaperColor(const gfx::ImageSkia image,
-                                color_utils::LumaRange luma,
-                                color_utils::SaturationRange saturation) {
+std::vector<SkColor> CalculateWallpaperColor(
+    const gfx::ImageSkia image,
+    const color_utils::ColorProfiles& color_profiles) {
   base::TimeTicks start_time = base::TimeTicks::Now();
-  const SkColor prominent_color = color_utils::CalculateProminentColorOfBitmap(
-      *image.bitmap(), luma, saturation);
+  const std::vector<SkColor> prominent_colors =
+      color_utils::CalculateProminentColorsOfBitmap(*image.bitmap(),
+                                                    color_profiles);
 
   UMA_HISTOGRAM_TIMES("Ash.Wallpaper.ColorExtraction.Durations",
                       base::TimeTicks::Now() - start_time);
-  UMA_HISTOGRAM_BOOLEAN("Ash.Wallpaper.ColorExtractionResult",
-                        prominent_color != SK_ColorTRANSPARENT);
+  for (size_t i = 0; i < color_profiles.size(); ++i) {
+    bool is_result_transparent = prominent_colors[i] == SK_ColorTRANSPARENT;
+    if (color_profiles[i].saturation == SaturationRange::VIBRANT) {
+      switch (color_profiles[i].luma) {
+        case LumaRange::DARK:
+          UMA_HISTOGRAM_ENUMERATION("Ash.Wallpaper.ColorExtractionResult",
+                                    is_result_transparent
+                                        ? RESULT_DARK_VIBRANT_TRANSPARENT
+                                        : RESULT_DARK_VIBRANT_OPAQUE,
+                                    NUM_COLOR_EXTRACTION_RESULTS);
+          break;
+        case LumaRange::NORMAL:
+          UMA_HISTOGRAM_ENUMERATION("Ash.Wallpaper.ColorExtractionResult",
+                                    is_result_transparent
+                                        ? RESULT_NORMAL_VIBRANT_TRANSPARENT
+                                        : RESULT_NORMAL_VIBRANT_OPAQUE,
+                                    NUM_COLOR_EXTRACTION_RESULTS);
+          break;
+        case LumaRange::LIGHT:
+          UMA_HISTOGRAM_ENUMERATION("Ash.Wallpaper.ColorExtractionResult",
+                                    is_result_transparent
+                                        ? RESULT_LIGHT_VIBRANT_TRANSPARENT
+                                        : RESULT_LIGHT_VIBRANT_OPAQUE,
+                                    NUM_COLOR_EXTRACTION_RESULTS);
+          break;
+      }
+    } else {
+      switch (color_profiles[i].luma) {
+        case LumaRange::DARK:
+          UMA_HISTOGRAM_ENUMERATION("Ash.Wallpaper.ColorExtractionResult",
+                                    is_result_transparent
+                                        ? RESULT_DARK_MUTED_TRANSPARENT
+                                        : RESULT_DARK_MUTED_OPAQUE,
+                                    NUM_COLOR_EXTRACTION_RESULTS);
+          break;
+        case LumaRange::NORMAL:
+          UMA_HISTOGRAM_ENUMERATION("Ash.Wallpaper.ColorExtractionResult",
+                                    is_result_transparent
+                                        ? RESULT_NORMAL_MUTED_TRANSPARENT
+                                        : RESULT_NORMAL_MUTED_OPAQUE,
+                                    NUM_COLOR_EXTRACTION_RESULTS);
+          break;
+        case LumaRange::LIGHT:
+          UMA_HISTOGRAM_ENUMERATION("Ash.Wallpaper.ColorExtractionResult",
+                                    is_result_transparent
+                                        ? RESULT_LIGHT_MUTED_TRANSPARENT
+                                        : RESULT_LIGHT_MUTED_OPAQUE,
+                                    NUM_COLOR_EXTRACTION_RESULTS);

[Ilya Sherman, 2017/06/20 23:00:00]

Please move the histogram macro invocation out of the switch-stmt; just set a
result value in the switch.  That both makes for shorter, easier-to-read code;
and it also helps the compiler avoid duplicating the work that the macro does.

[Qiang(Joe) Xu, 2017/06/21 01:10:52]

Done.

+          break;
+      }
+    }
+  }
 
-  return prominent_color;
+  return prominent_colors;
 }
 
 bool ShouldCalculateSync(const gfx::ImageSkia& image) {
@@ -51,16 +106,17 @@ bool ShouldCalculateSync(const gfx::ImageSkia& image) {
 
 WallpaperColorCalculator::WallpaperColorCalculator(
     const gfx::ImageSkia& image,
-    color_utils::LumaRange luma,
-    color_utils::SaturationRange saturation,
+    const color_utils::ColorProfiles& color_profiles,
     scoped_refptr<base::TaskRunner> task_runner)
     : image_(image),
-      luma_(luma),
-      saturation_(saturation),
+      color_profiles_(color_profiles),
       task_runner_(std::move(task_runner)),
-      weak_ptr_factory_(this) {}
+      weak_ptr_factory_(this) {
+  prominent_colors_ =
+      std::vector<SkColor>(color_profiles_.size(), SK_ColorTRANSPARENT);
+}
 
-WallpaperColorCalculator::~WallpaperColorCalculator() {}
+WallpaperColorCalculator::~WallpaperColorCalculator() = default;
 
 void WallpaperColorCalculator::AddObserver(
     WallpaperColorCalculatorObserver* observer) {
@@ -74,25 +130,26 @@ void WallpaperColorCalculator::RemoveObserver(
 
 bool WallpaperColorCalculator::StartCalculation() {
   if (ShouldCalculateSync(image_)) {
-    const SkColor prominent_color =
-        CalculateWallpaperColor(image_, luma_, saturation_);
-    NotifyCalculationComplete(prominent_color);
+    const std::vector<SkColor> prominent_colors =
+        CalculateWallpaperColor(image_, color_profiles_);
+    NotifyCalculationComplete(prominent_colors);
     return true;
   }
 
   image_.MakeThreadSafe();
   if (base::PostTaskAndReplyWithResult(
           task_runner_.get(), FROM_HERE,
-          base::Bind(&CalculateWallpaperColor, image_, luma_, saturation_),
+          base::Bind(&CalculateWallpaperColor, image_, color_profiles_),
           base::Bind(&WallpaperColorCalculator::OnAsyncCalculationComplete,
                      weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now()))) {
     return true;
   }
 
   LOG(WARNING) << "PostSequencedWorkerTask failed. "
-               << "Wallpaper promiment color may not be calculated.";
+               << "Wallpaper prominent colors may not be calculated.";
 
-  prominent_color_ = SK_ColorTRANSPARENT;
+  prominent_colors_ =
+      std::vector<SkColor>(color_profiles_.size(), SK_ColorTRANSPARENT);
   return false;
 }
 
@@ -103,15 +160,15 @@ void WallpaperColorCalculator::SetTaskRunnerForTest(
 
 void WallpaperColorCalculator::OnAsyncCalculationComplete(
     base::TimeTicks async_start_time,
-    SkColor prominent_color) {
+    const std::vector<SkColor>& prominent_colors) {
   UMA_HISTOGRAM_TIMES("Ash.Wallpaper.ColorExtraction.UserDelay",
                       base::TimeTicks::Now() - async_start_time);
-  NotifyCalculationComplete(prominent_color);
+  NotifyCalculationComplete(prominent_colors);
 }
 
 void WallpaperColorCalculator::NotifyCalculationComplete(
-    SkColor prominent_color) {
-  prominent_color_ = prominent_color;
+    const std::vector<SkColor>& prominent_colors) {
+  prominent_colors_ = prominent_colors;
   for (auto& observer : observers_)
     observer.OnColorCalculationComplete();