Select dark colors for generating fallback icon if possible.

Select dark colors for generating fallback icon if possible.

When trying to establish the background color for the fallback icon,
we select the dominant color of a site's favicon and clamp its luminosity
so that we don't end up with a very light background and white text.

If the favicon has a lot of white and a bit of blue, the algorithm
returns white which we clamp down to grey. Instead, we should have used
the blue. We can force the algorithm to return blue by specifying the
upper bound of the dominant color we'd like to receive.

We still need to clamp the result because the source image may actually
not have any dark colors, in which case the algorithm will ignore the
upper bound.

BUG=539771
Committed: https://crrev.com/a6da3b567a399861eeced15fe989a6b5acbab91c
Cr-Commit-Position: refs/heads/master@{#352798}

[mpawlowski, 2015-10-05 07:22:25]

mpawlowski@opera.com changed reviewers:
+ pkotwicz@chromium.org, sky@chromium.org, stevenjb@chromium.org

[pkotwicz, 2015-10-05 14:23:28]

LGTM assuming that you have tested this.

I can send you the favicons of the top 100 US mobile sites if you want to check
the before and after results

https://codereview.chromium.org/1382343002/diff/1/components/favicon_base/fal...
File components/favicon_base/fallback_icon_style.cc (right):

https://codereview.chromium.org/1382343002/diff/1/components/favicon_base/fal...
components/favicon_base/fallback_icon_style.cc:68: const color_utils::HSL
lower_bound{-1.0, -1.0, 0.15};
Is there a reason 0.15 is not a constant?

[mpawlowski, 2015-10-05 14:27:50]

On 2015/10/05 14:23:28, pkotwicz wrote:
> LGTM assuming that you have tested this.
> 
> I can send you the favicons of the top 100 US mobile sites if you want to
check
> the before and after results

That would be cool, you can send them to my mail. I've tested this only on a
couple of sites.

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

https://codereview.chromium.org/1382343002/diff/1/components/favicon_base/fal...
File components/favicon_base/fallback_icon_style.cc (right):

https://codereview.chromium.org/1382343002/diff/1/components/favicon_base/fal...
components/favicon_base/fallback_icon_style.cc:68: const color_utils::HSL
lower_bound{-1.0, -1.0, 0.15};
On 2015/10/05 14:23:27, pkotwicz wrote:
> Is there a reason 0.15 is not a constant?

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]

Can you add a Bug Id to this CL?

https://codereview.chromium.org/1382343002/diff/1/components/favicon_base/fal...
File components/favicon_base/fallback_icon_style.cc (right):

https://codereview.chromium.org/1382343002/diff/1/components/favicon_base/fal...
components/favicon_base/fallback_icon_style.cc:68: const color_utils::HSL
lower_bound{-1.0, -1.0, 0.15};
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.

[mpawlowski, 2015-10-07 07:24:18]

The CQ bit was checked by mpawlowski@opera.com

[mpawlowski, 2015-10-07 07:24:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkotwicz@chromium.org
Link to the patchset: https://codereview.chromium.org/1382343002/#ps20001
(title: "Constant for min background luminance")

[commit-bot: I haz the power, 2015-10-07 07:24:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1382343002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1382343002/20001

[commit-bot: I haz the power, 2015-10-07 08:13:38]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-07 08:14:37]

Patchset 2 (id:??) landed as
https://crrev.com/a6da3b567a399861eeced15fe989a6b5acbab91c
Cr-Commit-Position: refs/heads/master@{#352798}