Try to improve toolbar separator coloration for themed browser windows.

chrome/browser/themes/theme_service.cc

 // Copyright (c) 2012 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 "chrome/browser/themes/theme_service.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 
@@ skipping 459 matching lines @@
       const color_utils::HSL kTint = {-1, -1, 0.42975};
       const color_utils::HSL kTintIncognito = {-1, -1, 0.34375};
       return color_utils::HSLShift(
           GetColor(ThemeProperties::COLOR_TOOLBAR, incognito),
           incognito ? kTintIncognito : kTint);
     }
     case ThemeProperties::COLOR_BOOKMARK_BAR_INSTRUCTIONS_TEXT:
       if (UsingDefaultTheme())
         break;
       return GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT, incognito);
+    case ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_SEPARATOR: {
+      // Same as the normal toolbar separator, but with increased luminance.

[Peter Kasting, 2017/01/17 20:51:25]

It seems kind of random that we increase the luminance here as a way of making
this separator less noticeable.  I assume that's because the original author
here was thinking of a white NTP background.  Shouldn't we instead blend towards
whatever the NTP background color actually is?

[Evan Stade, 2017/01/18 00:16:59]

In the dark case (e.g. incognito) it is actually more noticeable. I think for
incognito if we blended towards the NTP background it would effectively be
invisible.

[Peter Kasting, 2017/01/18 00:40:16]

Which seems like a bug.  Why would we want a stronger separation between the
detached bar bottom and the area below it than between the toolbar and the
detached bar?  And why would we want it to be less noticeable in regular, but
more in incognito?

What is the computed toolbar separator color in incognito with your changes? 
This would help answer the question of how visible it would be if we blend
toward the NTP background color.

[Evan Stade, 2017/01/24 00:46:08]

In practice, it doesn't look like a bug. You can test it easily enough because
this is what Chrome already does.
The bottom separator is essentially invisible in incognito as it is because the
contrast to the ntp bg is very low. This is per Sebastien's design. If we blend
towards the NTP bg color it is even lower in contrast and the floating bookmark
bar will essentially not have a separator.

If we're talking about the default theme here, this patch intentionally leaves
the current behavior intact. I don't think there's a bug here.

[Peter Kasting, 2017/01/24 21:43:33]

Staring at the current behavior more closely, I think blending the toolbar
separator lighter happens to work for normal and incognito windows happens to
work, but only coincidentally; and my suggestion of blending towards the NTP
background color is wrong too.  The problem is that both of these start from the
toolbar separator color, but that isn't guaranteed to have any relationship to
what we want.

In the default theme, the detached bookmark bar and the NTP background are
always both the same color: white for normal windows, dark grey for incognito. 
What we want in a separator is something that will contrast with both of those.

Today, lightening the toolbar separator color produces a medium grey, which is
darker than the default window's white and lighter than the incognito window's
dark grey, and thus is visible in both cases.  But there's nothing guaranteeing
this will still be true with arbitrary theme colors.

This is basically the same problem we had to solve with
ThemeService::GetSeparatorColor(): given two arbitrary colors, find a third that
contrasts with both.  I wonder if we could just call GetSeparatorColor(), or
maybe tweak the kAlpha value it uses (i.e. pass that as a param).

+      // Either double it or halve the distance to 1.0, whichever is less of a
+      // difference.
+      color_utils::HSL hsl;
+      color_utils::SkColorToHSL(
+          GetColor(ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR, incognito),
+          &hsl);
+      hsl.l = std::min((hsl.l + 1) / 2, hsl.l * 2);
+      return color_utils::HSLToSkColor(hsl, SK_AlphaOPAQUE);
+    }
     case ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_BACKGROUND:
       if (UsingDefaultTheme())
         break;
       return GetColor(ThemeProperties::COLOR_TOOLBAR, incognito);
-    case ThemeProperties::COLOR_DETACHED_BOOKMARK_BAR_SEPARATOR:
+    case ThemeProperties::COLOR_TOOLBAR_BOTTOM_SEPARATOR:
       if (UsingDefaultTheme())
         break;
-      // Use 50% of bookmark text color as separator color.
-      return SkColorSetA(
-          GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT, incognito), 128);
+      // Use 25% of bookmark text color as separator color.
+      return color_utils::BlendTowardOppositeLuma(
+          GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT, incognito), 0xBF);

[Peter Kasting, 2017/01/17 20:51:25]

Doesn't blending toward the opposite luma here assume the bookmark bar
background is (whatever the opposite luma is)?

That is, it seems like maybe you instead want 25% bookmark bar text, 75%
bookmark bar background?  (In which case you might have to raise the fraction
back up from 25% since the difference won't be so large)

[Evan Stade, 2017/01/18 00:16:59]

I think it's a safe assumption that blending towards the opposite luma will make
the separator a less dramatic version of the text color. This would only not be
true if the bookmark text was something unreadable like black on black, right?
Whether we want to tinge the separator color with the bookmark bar bg color is a
matter of judgment I suppose, but the text doesn't do that so I don't know why
the separator should.

[Peter Kasting, 2017/01/18 00:40:16]

The effect of the previous alpha-blend was, basically, to blend the text color
toward the background color.  You said you were trying to make this fully
opaque; I'm looking for a way to duplicate the old effect more closely in that
world.  My worry about blending toward opposite luma is that the level of
contrast you get is much less predictable than with "blend toward background
color".

[Evan Stade, 2017/01/24 00:46:08]

I'm not sure I follow because there's no way to predict the contrast of two
colors a theme author has chosen, but I can change this to use AlphaBlend. When
I do that, it looks better across the handful of tested themes if I lower the
foreground percent even further.

[Peter Kasting, 2017/01/24 21:43:33]

I just meant, if we're trying to guarantee the separator contrasts less than the
bookmark text, then blending toward background will achieve that, but blending
toward opposite luma could contrast more.
I wonder if this, too, should be using GetSeparatorColor() somehow.  This would
likely change the behavior for the default incognito theme (which I'd be OK with
since the black separator used there today seems invisible to me).

     case ThemeProperties::COLOR_NTP_SECTION_HEADER_TEXT:
       return IncreaseLightness(GetColor(kNtpText, incognito), 0.30);
     case ThemeProperties::COLOR_NTP_SECTION_HEADER_TEXT_HOVER:
       return GetColor(kNtpText, incognito);
     case ThemeProperties::COLOR_NTP_SECTION_HEADER_RULE:
       return IncreaseLightness(GetColor(kNtpText, incognito), 0.70);
     case ThemeProperties::COLOR_NTP_SECTION_HEADER_RULE_LIGHT:
       return IncreaseLightness(GetColor(kNtpText, incognito), 0.86);
     case ThemeProperties::COLOR_NTP_TEXT_LIGHT:
       return IncreaseLightness(GetColor(kNtpText, incognito), 0.40);
@@ skipping 375 matching lines @@
 
 #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
 bool ThemeService::IsSupervisedUser() const {
   return profile_->IsSupervised();
 }
 
 void ThemeService::SetSupervisedUserTheme() {
   SetCustomDefaultTheme(new SupervisedUserTheme);
 }
 #endif