Select dark colors for generating fallback icon if possible.

components/favicon_base/fallback_icon_style.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/favicon_base/fallback_icon_style.h"
 
 #include <algorithm>
 
 #include "ui/gfx/color_analysis.h"
 #include "ui/gfx/color_utils.h"
@@ skipping 45 matching lines @@
 }
 
 bool ValidateFallbackIconStyle(const FallbackIconStyle& style) {
   return style.font_size_ratio >= 0.0 && style.font_size_ratio <= 1.0 &&
       style.roundness >= 0.0 && style.roundness <= 1.0;
 }
 
 void SetDominantColorAsBackground(
     const scoped_refptr<base::RefCountedMemory>& bitmap_data,
     FallbackIconStyle* style) {
-  SkColor dominant_color =
-      color_utils::CalculateKMeanColorOfPNG(bitmap_data);
-  // Assumes |style.text_color| is light, and clamps luminance down to a
-  // reasonable maximum value so text is readable.
+  // Try to ensure color's luminance isn't too large so that light text is
+  // visible. Set an upper bound for the dominant color.
+  const color_utils::HSL lower_bound{-1.0, -1.0, 0.15};

[pkotwicz, 2015/10/05 14:23:27]

Is there a reason 0.15 is not a constant?

[mpawlowski, 2015/10/05 14:28:26]

Not rally, it's just the same number as the one used when calling the
"simplified" version of CalculateKMeanColor. In color_analysis.cc there are
these defaults:

const HSL kDefaultLowerHSLBound = {-1, -1, 0.15};
const HSL kDefaultUpperHSLBound = {-1, -1, 0.85};

I guess I could make them public and use them here, overwriting the last
component of the default upper bound. Do you want me to do this?

[pkotwicz, 2015/10/05 14:46:21]

I don't have much of a preference because color_analysis.cc is modified very
infrequently. I suggested putting 0.15 into a constant for consistency with
kMaxBackgroundColorLuminance. I am fine if you add a constant:
"kMinBackgroundColorLuminence" which is local to this file. I am also fine with
what you suggested.

+  const color_utils::HSL upper_bound{-1.0, -1.0, kMaxBackgroundColorLuminance};
+  color_utils::GridSampler sampler;
+  SkColor dominant_color = color_utils::CalculateKMeanColorOfPNG(
+      bitmap_data, lower_bound, upper_bound, &sampler);
+  // |CalculateKMeanColorOfPNG| will try to return a color that lies within the
+  // specified bounds if one exists in the image. If there's no such color, it
+  // will return the dominant color which may be lighter than our upper bound.
+  // Clamp luminance down to a reasonable maximum value so text is readable.
   color_utils::HSL color_hsl;
   color_utils::SkColorToHSL(dominant_color, &color_hsl);
   color_hsl.l = std::min(color_hsl.l, kMaxBackgroundColorLuminance);
   style->background_color =
       color_utils::HSLToSkColor(color_hsl, SK_AlphaOPAQUE);
 }
 
 }  // namespace favicon_base