Dynamically compute tab/frame separator color.

For reference, this color is overlaid atop the frame/toolbar when outlining tabs
and for outlines/shadows when drawing the new tab button.  It's either white or
black with a varying (but usually 0x40) alpha value.

Computing this is surprisingly complicated, beause we're trying to contrast with
two different colors simultaneously (tab and frame), and we also want to set our
magic numbers so as to achieve the colors from the design specs in the default
theme.

After quite a bit of thought, I elected to use a mechanism that defaults to
moving the frame away from the tab luminance; that is, if the tab is "brighter"
than the frame, we make the separator darken the frame, and if the tab is
darker, we try to lighten the frame.  If the frame is already so dark or light
that the result has too low of contrast with the frame color, we reverse
direction.  ("Too low" here is a lot lower than I'd like, but any higher and
we'd end up using light separators for the default theme in incognito mode,
which I think looks good but Sebastien dislikes.)  In the case where we reversed
direction, we have to worry that the result of all this won't contrast enough
with the tab; in that case, we push up the alpha value so the result contrasts
enough with the tab as well.  This last computation is the most expensive
because of how I chose to make it behave, but I think the behavior I selected
(too complicated to explain here, see code/comments for details) will feel more
consistent than any of the simpler methods I considered.

Because computing the separator color can be expensive (every call to
GetRelativeLuminance() can involve floating-point exponentiation among other
things, and in the worst case the color computation computes the desired color
via a 7-iteration loop), I elected to cache the computed value in a map.  This
might be a case of premature optimization, but in debugging it looked like this
color could be requested frequently, and I didn't want to risk performance
problems.

Even this choice presented options.  I used a simple map that I never prune
entries from; at worst, we'll add up to 2 entries per distinct theme the user
switches to while running, which didn't seem too bad.  I considered instead
using base::MRUCache, which would let me cap the size.  I also considered just
keeping a couple member structs containing the relevant information for the
normal and incognito color schemes, but this generally seemed like it ended up
as more code than the other routes.  None of these options seems wildly better
or worse than the others; I'm willing to change course in the face of violent
opinion :)

Finally, because the alpha value of the separator can now vary, I converted the
code in tab_strip.cc that set it to fixed values to instead use scaling
multipliers.  These will compute the same values as before when the separator
has its default (0x40) alpha, but in the case where we've computed some larger
alpha, scaling proportionally seemed like the best thing to do.  I used a
saturated_cast in one place where I wasn't sure the result was guaranteed to
stay <= 255.

BUG=585470
TEST=See bug comment 0

Review URL: https://codereview.chromium.org/1785613004

Cr-Commit-Position: refs/heads/master@{#381617}

chrome/browser/themes/theme_service.cc

@@ -425,12 +425,28 @@ SkColor ThemeService::GetDefaultColor(int id, bool incognito) const {
       return SkColorSetA(
           GetColor(ThemeProperties::COLOR_TOOLBAR_BUTTON_ICON, incognito),
           0x33);
+    case ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR: {
+      const SkColor tab_color =
+          GetColor(ThemeProperties::COLOR_TOOLBAR, incognito);
+      const SkColor frame_color =
+          GetColor(ThemeProperties::COLOR_FRAME, incognito);
+      const SeparatorColorKey key(tab_color, frame_color);
+      auto i = separator_color_cache_.find(key);
+      if (i != separator_color_cache_.end())
+        return i->second;
+      const SkColor separator_color = GetSeparatorColor(tab_color, frame_color);
+      separator_color_cache_[key] = separator_color;
+      return separator_color;
+    }
     case ThemeProperties::COLOR_BACKGROUND_TAB: {
       // The tints here serve a different purpose than TINT_BACKGROUND_TAB.
       // That tint is used to create background tab images for custom themes by
       // lightening the frame images.  The tints here create solid colors for
-      // background tabs by darkening the foreground tab (toolbar) color.
-      const color_utils::HSL kTint = {-1, -1, 0.4296875};
+      // background tabs by darkening the foreground tab (toolbar) color.  These
+      // values are chosen to turn the default normal and incognito MD frame
+      // colors (0xf2f2f2 and 0x505050) into 0xd0d0d0 and 0x373737,
+      // respectively.
+      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),
@@ -591,6 +607,62 @@ bool ThemeService::HasCustomImage(int id) const {
          theme_supplier_->HasCustomImage(id);
 }
 
+// static
+SkColor ThemeService::GetSeparatorColor(SkColor tab_color,
+                                        SkColor frame_color) {
+  // We use this alpha value for the separator if possible.
+  const SkAlpha kAlpha = 0x40;
+
+  // In most cases, if the tab is lighter than the frame, we darken the
+  // frame; if the tab is darker than the frame, we lighten the frame.
+  // However, if the frame is already very dark or very light, respectively,
+  // this won't contrast sufficiently with the frame color, so we'll need to
+  // reverse when we're lightening and darkening.
+  const double tab_luminance = color_utils::GetRelativeLuminance(tab_color);
+  const double frame_luminance = color_utils::GetRelativeLuminance(frame_color);
+  const bool lighten = tab_luminance < frame_luminance;
+  SkColor separator_color = lighten ? SK_ColorWHITE : SK_ColorBLACK;
+  double separator_luminance = color_utils::GetRelativeLuminance(
+      color_utils::AlphaBlend(separator_color, frame_color, kAlpha));
+  // The minimum contrast ratio here is just under the ~1.1469 in the default MD
+  // incognito theme.  We want the separator to still darken the frame in that
+  // theme, but that's about as low of contrast as we're willing to accept.
+  const double kMinContrastRatio = 1.1465;
+  if (color_utils::GetContrastRatio(separator_luminance, frame_luminance) >=
+      kMinContrastRatio)
+    return SkColorSetA(separator_color, kAlpha);
+
+  // We need to reverse whether we're darkening or lightening.  We know the new
+  // separator color will contrast with the frame; check whether it also
+  // contrasts at least as well with the tab.
+  separator_color = color_utils::InvertColor(separator_color);
+  separator_luminance = color_utils::GetRelativeLuminance(
+      color_utils::AlphaBlend(separator_color, frame_color, kAlpha));
+  if (color_utils::GetContrastRatio(separator_luminance, tab_luminance) >=
+      color_utils::GetContrastRatio(separator_luminance, frame_luminance))
+    return SkColorSetA(separator_color, kAlpha);
+
+  // The reversed separator doesn't contrast enough with the tab.  Compute the
+  // resulting luminance from adjusting the tab color, instead of the frame
+  // color, by the separator color.
+  const double target_luminance = color_utils::GetRelativeLuminance(
+      color_utils::AlphaBlend(separator_color, tab_color, kAlpha));
+
+  // Now try to compute an alpha for the separator such that, when blended with
+  // the frame, it results in the above luminance.  Because the luminance
+  // computation is not easily invertible, we use a binary search over the
+  // possible range of alpha values.
+  SkAlpha alpha = 128;
+  for (int delta = lighten ? 64 : -64; delta != 0; delta /= 2) {
+    const double luminance = color_utils::GetRelativeLuminance(
+        color_utils::AlphaBlend(separator_color, frame_color, alpha));
+    if (luminance == target_luminance)
+      break;
+    alpha += (luminance < target_luminance) ? -delta : delta;
+  }
+  return SkColorSetA(separator_color, alpha);
+}
+
 gfx::ImageSkia* ThemeService::GetImageSkiaNamed(int id, bool incognito) const {
   gfx::Image image = GetImageNamed(id, incognito);
   if (image.IsEmpty())

chrome/browser/themes/theme_service.h

@@ -215,6 +215,20 @@ class ThemeService : public base::NonThreadSafe,
   friend class BrowserThemeProvider;
   friend class theme_service_internal::ThemeServiceTest;
 
+  // Key for cache of separator colors; pair is <tab color, frame color>.
+  using SeparatorColorKey = std::pair<SkColor, SkColor>;
+  using SeparatorColorCache = std::map<SeparatorColorKey, SkColor>;
+
+  // Computes the "toolbar top separator" color.  This color is drawn atop the
+  // frame to separate it from tabs, the toolbar, and the new tab button, as
+  // well as atop background tabs to separate them from other tabs or the
+  // toolbar.  We use semitransparent black or white so as to darken or lighten
+  // the frame, with the goal of contrasting with both the frame color and the
+  // active tab (i.e. toolbar) color.  (It's too difficult to try to find colors
+  // that will contrast with both of these as well as the background tab color,
+  // and contrasting with the foreground tab is the most important).
+  static SkColor GetSeparatorColor(SkColor tab_color, SkColor frame_color);
+
   // These methods provide the implementation for ui::ThemeProvider (exposed
   // via BrowserThemeProvider).
   gfx::ImageSkia* GetImageSkiaNamed(int id, bool incognito) const;
@@ -295,6 +309,10 @@ class ThemeService : public base::NonThreadSafe,
   // The number of infobars currently displayed.
   int number_of_infobars_;
 
+  // A cache of already-computed values for COLOR_TOOLBAR_TOP_SEPARATOR, which
+  // can be expensive to compute.
+  mutable SeparatorColorCache separator_color_cache_;
+
   content::NotificationRegistrar registrar_;
 
   scoped_ptr<ThemeSyncableService> theme_syncable_service_;

chrome/browser/themes/theme_service_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/themes/theme_service.h"
 
+#include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
 #include "base/path_service.h"
@@ -29,6 +30,8 @@
 #include "extensions/common/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/material_design/material_design_controller.h"
+#include "ui/base/test/material_design_controller_test_api.h"
+#include "ui/base/ui_base_switches.h"
 
 #if defined(ENABLE_SUPERVISED_USERS)
 #include "chrome/browser/supervised_user/supervised_user_service.h"
@@ -45,6 +48,17 @@ class ThemeServiceTest : public extensions::ExtensionServiceTestBase {
                        registry_(NULL) {}
   ~ThemeServiceTest() override {}
 
+  void SetUp() override {
+    extensions::ExtensionServiceTestBase::SetUp();
+    extensions::ExtensionServiceTestBase::ExtensionServiceInitParams params =
+        CreateDefaultInitParams();
+    params.profile_is_supervised = is_supervised_;
+    InitializeExtensionService(params);
+    service_->Init();
+    registry_ = ExtensionRegistry::Get(profile_.get());
+    ASSERT_TRUE(registry_);
+  }
+
   // Moves a minimal theme to |temp_dir_path| and unpacks it from that
   // directory.
   std::string LoadUnpackedThemeAt(const base::FilePath& temp_dir) {
@@ -92,21 +106,27 @@ class ThemeServiceTest : public extensions::ExtensionServiceTestBase {
     base::MessageLoop::current()->RunUntilIdle();
   }
 
-  void SetUp() override {
-    extensions::ExtensionServiceTestBase::SetUp();
-    extensions::ExtensionServiceTestBase::ExtensionServiceInitParams params =
-        CreateDefaultInitParams();
-    params.profile_is_supervised = is_supervised_;
-    InitializeExtensionService(params);
-    service_->Init();
-    registry_ = ExtensionRegistry::Get(profile_.get());
-    ASSERT_TRUE(registry_);
-  }
-
   const CustomThemeSupplier* get_theme_supplier(ThemeService* theme_service) {
     return theme_service->get_theme_supplier();
   }
 
+  // Alpha blends a non-opaque foreground color against an opaque background.
+  // This is not the same as color_utils::AlphaBlend() since it gets the opacity
+  // from the foreground color and then does not blend the two colors' alpha
+  // values together.
+  static SkColor AlphaBlend(SkColor foreground, SkColor background) {
+    return color_utils::AlphaBlend(SkColorSetA(foreground, SK_AlphaOPAQUE),
+                                   background, SkColorGetA(foreground));
+  }
+
+  // Returns the separator color as the opaque result of blending it atop the
+  // frame color (which is the color we use when calculating the contrast of the
+  // separator with the tab and frame colors).
+  static SkColor GetSeparatorColor(SkColor tab_color, SkColor frame_color) {
+    return AlphaBlend(ThemeService::GetSeparatorColor(tab_color, frame_color),
+                      frame_color);
+  }
+
  protected:
   bool is_supervised_;
   ExtensionRegistry* registry_;
@@ -376,4 +396,113 @@ TEST_F(ThemeServiceSupervisedUserTest, SupervisedUserThemeReplacesNativeTheme) {
 #endif // defined(OS_LINUX) && !defined(OS_CHROMEOS)
 #endif // defined(ENABLE_SUPERVISED_USERS)
 
+#if !defined(OS_MACOSX)  // Mac uses different colors than other platforms.
+// Simple class to run tests in material design mode.
+class ThemeServiceMaterialDesignTest : public ThemeServiceTest {
+ public:
+  void SetUp() override {
+    ThemeServiceTest::SetUp();
+    ui::test::MaterialDesignControllerTestAPI::SetMode(
+        ui::MaterialDesignController::MATERIAL_NORMAL);
+  }
+
+  void TearDown() override {
+    ThemeServiceTest::TearDown();
+    ui::test::MaterialDesignControllerTestAPI::UninitializeMode();
+  }
+};
+
+// Check that the function which computes the separator color behaves as
+// expected for a variety of inputs.  We run in material design mode so we can
+// use the material normal and incognito color combinations, which differ from
+// each other in ways that are interesting to test.
+TEST_F(ThemeServiceMaterialDesignTest, SeparatorColor) {
+  // Ensure Windows 10 machines use the built-in default colors rather than the
+  // current system native colors.
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      switches::kDisableDwmComposition);
+
+  // Check that the TOOLBAR_TOP_SEPARATOR color is the same whether we ask the
+  // theme provider or compute it manually.
+  const ui::ThemeProvider& theme_provider =
+      ThemeService::GetThemeProviderForProfile(profile_.get());
+  SkColor frame_color = theme_provider.GetColor(ThemeProperties::COLOR_FRAME);
+  SkColor theme_color = AlphaBlend(
+      theme_provider.GetColor(ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR),
+      frame_color);
+  SkColor tab_color = theme_provider.GetColor(ThemeProperties::COLOR_TOOLBAR);
+  SkColor separator_color = GetSeparatorColor(tab_color, frame_color);
+  EXPECT_EQ(theme_color, separator_color);
+
+  // For the default theme, the separator should darken the frame.
+  double frame_luminance = color_utils::GetRelativeLuminance(frame_color);
+  EXPECT_LT(color_utils::GetRelativeLuminance(separator_color),
+            frame_luminance);
+
+  // If we reverse the colors, the separator should darken the "frame" (which
+  // in this case is actually the tab color), since otherwise the contrast with
+  // the "frame" would be too minimal.  It should also be darker than the "tab"
+  // (frame color) since otherwise the contrast the contrast with the "tab
+  // color" would be too minimal.
+  separator_color = GetSeparatorColor(frame_color, tab_color);
+  double tab_luminance = color_utils::GetRelativeLuminance(tab_color);
+  double separator_luminance =
+      color_utils::GetRelativeLuminance(separator_color);
+  EXPECT_LT(separator_luminance, tab_luminance);
+  EXPECT_LT(separator_luminance, frame_luminance);
+
+  // When the frame color is black, the separator should lighten the frame, but
+  // it should still be darker than the tab color.
+  separator_color = GetSeparatorColor(tab_color, SK_ColorBLACK);
+  separator_luminance = color_utils::GetRelativeLuminance(separator_color);
+  EXPECT_GT(separator_luminance, 0);
+  EXPECT_LT(separator_luminance, tab_luminance);
+
+  // When the frame color is white, the separator should darken the frame; it
+  // should also be lighter than the tab color since otherwise the contrast with
+  // the tab would be too minimal.
+  separator_color = GetSeparatorColor(tab_color, SK_ColorWHITE);
+  separator_luminance = color_utils::GetRelativeLuminance(separator_color);
+  EXPECT_LT(separator_luminance, 1);
+  EXPECT_LT(separator_luminance, tab_luminance);
+
+  // Now make similar checks as above but for the incognito theme.
+  const ui::ThemeProvider& otr_provider =
+      ThemeService::GetThemeProviderForProfile(
+          profile_->GetOffTheRecordProfile());
+  frame_color = otr_provider.GetColor(ThemeProperties::COLOR_FRAME);
+  theme_color = AlphaBlend(
+      otr_provider.GetColor(ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR),
+      frame_color);
+  tab_color = otr_provider.GetColor(ThemeProperties::COLOR_TOOLBAR);
+  separator_color = GetSeparatorColor(tab_color, frame_color);
+  EXPECT_EQ(theme_color, separator_color);
+
+  // For the default incognito theme, the separator should darken the frame.
+  EXPECT_LT(color_utils::GetRelativeLuminance(separator_color),
+            color_utils::GetRelativeLuminance(frame_color));
+
+  // And if we reverse the colors, the separator should lighten the "frame"
+  // (tab color).
+  separator_color = GetSeparatorColor(frame_color, tab_color);
+  tab_luminance = color_utils::GetRelativeLuminance(tab_color);
+  EXPECT_GT(color_utils::GetRelativeLuminance(separator_color), tab_luminance);
+
+  // When the frame color is black, the separator should lighten the frame; it
+  // should also be lighter than the tab color since otherwise the contrast with
+  // the tab would be too minimal.
+  separator_color = GetSeparatorColor(tab_color, SK_ColorBLACK);
+  separator_luminance = color_utils::GetRelativeLuminance(separator_color);
+  EXPECT_GT(separator_luminance, 0);
+  EXPECT_GT(separator_luminance, tab_luminance);
+
+  // When the frame color is white, the separator should darken the frame, but
+  // it should still be lighter than the tab color.
+  separator_color = GetSeparatorColor(tab_color, SK_ColorWHITE);
+  separator_luminance = color_utils::GetRelativeLuminance(separator_color);
+  EXPECT_LT(separator_luminance, 1);
+  EXPECT_GT(separator_luminance, tab_luminance);
+}
+#endif  // !defined(OS_MACOSX)
+
 }; // namespace theme_service_internal

chrome/browser/ui/views/frame/opaque_browser_frame_view.cc

@@ -747,6 +747,8 @@ void OpaqueBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) const {
     // MD the client edge images start at the top of the toolbar.
     y += md ? toolbar_bounds.y() : toolbar_bounds.bottom();
   } else {
+    // Note that windows without tabstrips are never themed, so we always use
+    // the default colors in this section.
     toolbar_color = ThemeProperties::GetDefaultColor(
         ThemeProperties::COLOR_TOOLBAR, incognito);
 
@@ -757,7 +759,8 @@ void OpaqueBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) const {
 
       // Shadow.
       BrowserView::Paint1pxHorizontalLine(
-          canvas, tp->GetColor(ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR),
+          canvas, ThemeProperties::GetDefaultColor(
+                      ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR, incognito),
           client_bounds, true);
     } else {
       // Ensure the client edge rects are drawn to the top of the location bar.

chrome/browser/ui/views/tabs/tab_strip.cc

@@ -418,10 +418,15 @@ void NewTabButton::OnPaint(gfx::Canvas* canvas) {
     paint.setAntiAlias(true);
     const SkColor stroke_color =
         tp->GetColor(ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR);
+    const float alpha = SkColorGetA(stroke_color);
+    const SkAlpha shadow_alpha =
+        base::saturated_cast<SkAlpha>(std::round(2.1875f * alpha));
     skia::RefPtr<SkDrawLooper> stroke_looper =
-        CreateShadowDrawLooper(SkColorSetA(stroke_color, 0x8C));
+        CreateShadowDrawLooper(SkColorSetA(stroke_color, shadow_alpha));
     paint.setLooper(stroke_looper.get());
-    paint.setColor(SkColorSetA(stroke_color, pressed ? 0x38 : 0x27));
+    const SkAlpha path_alpha = static_cast<SkAlpha>(
+        std::round((pressed ? 0.875f : 0.609375f) * alpha));
+    paint.setColor(SkColorSetA(stroke_color, path_alpha));
     canvas->DrawPath(stroke, paint);
   } else {
     // Fill.
@@ -567,8 +572,10 @@ void NewTabButton::PaintFill(bool pressed,
       }
       const SkColor stroke_color = GetThemeProvider()->GetColor(
           ThemeProperties::COLOR_TOOLBAR_TOP_SEPARATOR);
+      const SkAlpha alpha = static_cast<SkAlpha>(
+          std::round(SkColorGetA(stroke_color) * 0.59375f));
       skia::RefPtr<SkDrawLooper> looper =
-          CreateShadowDrawLooper(SkColorSetA(stroke_color, 0x26));
+          CreateShadowDrawLooper(SkColorSetA(stroke_color, alpha));
       paint.setLooper(looper.get());
       canvas->DrawPath(fill, paint);
     }