Change default frame colors. Change custom titlebar colors on Windows.

chrome/browser/themes/theme_properties.cc

 // Copyright (c) 2013 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_properties.h"
 
 #include <memory>
 
 #include "base/macros.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/themes/browser_theme_pack.h"
 #include "chrome/grit/theme_resources.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/resources/grit/ui_resources.h"
 
 namespace {
 
 // ----------------------------------------------------------------------------
 // Defaults for properties which are stored in the browser theme pack. If you
 // change these defaults, you must increment the version number in
 // browser_theme_pack.cc.
 
 const SkColor kDefaultColorFrame = SkColorSetRGB(0xCC, 0xCC, 0xCC);
-#if defined(OS_MACOSX)
-// Used for theme fallback colors.
 const SkColor kDefaultColorFrameInactive = SkColorSetRGB(0xF6, 0xF6, 0xF6);

[Peter Kasting, 2016/12/01 06:19:04]

Shouldn't this be 0xF5?

0xCC = 204
204 + (255 - 204) * 0.8 = 244.8
245 = 0xF5

Better yet, can we kill kDefaultColorFrameInactive, kDefaultColorFrameIncognito,
and kDefaultColorFrameIncognitoInactive in favor of computing these using the
shifts??

[Bret, 2016/12/08 00:16:07]

Yes it's supposed to be F5, but I thought a tiny shift towards white was worth
eliminating a preprocessor block. I put it back tho.

As for eliminating the pre-computed colors, I'm not sure. Since I'm not really
sure how the OSX colors are determined and whether we can change them I'm
inclined to just make the minimal change for now.

[Peter Kasting, 2016/12/08 00:24:50]

Oh, my intent was to just change the OS X color too.  (I doubt that change will
be noticed by anyone...)
I'd ask bettes@ about this stuff.  Maybe we will decide to keep the current
behavior, at least for now, but I'd like to at least understand what the context
and the long-term plan is.

Perhaps we could do something like, leave Mac as returning hardcoded colors, and
have non-Mac compute these colors based on the tints.  Then in the future we can
try to remove the Mac #ifs if possible.

[Bret, 2016/12/08 02:16:59]

Ok, changed to F5.

I'll find out what we want to do for OSX. For this patch I'll make sure we don't
change anything and come back to it.

-#else
-const SkColor kDefaultColorFrameInactive = SkColorSetRGB(0xDC, 0xDC, 0xDC);
-#endif
 
 #if defined(OS_MACOSX)
 const SkColor kDefaultColorFrameIncognito =
     SkColorSetARGB(0xE6, 0x14, 0x16, 0x18);
 const SkColor kDefaultColorFrameIncognitoInactive =
     SkColorSetRGB(0x1E, 0x1E, 0x1E);

[Peter Kasting, 2016/12/01 06:19:04]

It worries me that we're leaving the Mac incognito colors unchanged, but we're
changing the tints for all platforms.  This seems inconsistent.  SHould the
tints also be different on Mac?  Or are we OK with the default incognito window
acting completely different than a themed incognito window there?

[Bret, 2016/12/08 00:16:07]

That's a good point. I added some new switches to keep the old tints on OSX.
Based on the colors that are here it already works differently between default
and themed... but again I'd rather not change it without more context.

 #else
 const SkColor kDefaultColorFrameIncognito = SkColorSetRGB(0x28, 0x2B, 0x2D);
 const SkColor kDefaultColorFrameIncognitoInactive =
-    SkColorSetRGB(0x38, 0x3B, 0x3D);
+    SkColorSetRGB(0xD4, 0xD5, 0xD5);

[Evan Stade, 2016/11/30 23:27:48]

seems a little odd that this is ever so slightly not grey (i.e. the R is off by
1)

[Bret, 2016/11/30 23:39:30]

Yes, I agree it's odd. I decided to leave it because the non-inactive color is
also slightly-not-grey and so this hints that it's a blend of that. I could make
it perfect-grey if you think it's too odd. Or I could cheat and exaggerate it
slightly to better preserve the active hue.

[Peter Kasting, 2016/12/01 06:19:04]

Let's leave it as #D4D5D5.

 #endif
 
 const SkColor kDefaultColorToolbar = SkColorSetRGB(0xF2, 0xF2, 0xF2);
 const SkColor kDefaultColorToolbarIncognito = SkColorSetRGB(0x50, 0x50, 0x50);
 
 const SkColor kDefaultDetachedBookmarkBarBackground = SK_ColorWHITE;
 const SkColor kDefaultDetachedBookmarkBarBackgroundIncognito =
     SkColorSetRGB(0x32, 0x32, 0x32);
 
 // "Toolbar" text is used for active tabs and the bookmarks bar.
@@ skipping 23 matching lines @@
 const SkColor kDefaultColorNTPHeader = SkColorSetRGB(0x96, 0x96, 0x96);
 const SkColor kDefaultColorNTPSection = SkColorSetRGB(0xE5, 0xE5, 0xE5);
 constexpr SkColor kDefaultColorNTPSectionText = SK_ColorBLACK;
 const SkColor kDefaultColorNTPSectionLink = SkColorSetRGB(0x06, 0x37, 0x74);
 constexpr SkColor kDefaultColorButtonBackground = SK_ColorTRANSPARENT;
 
 // Default tints.
 constexpr color_utils::HSL kDefaultTintButtons = {-1, -1, -1};
 constexpr color_utils::HSL kDefaultTintButtonsIncognito = {-1, -1, 0.85};
 constexpr color_utils::HSL kDefaultTintFrame = {-1, -1, -1};
-constexpr color_utils::HSL kDefaultTintFrameInactive = {-1, -1, 0.75};
+constexpr color_utils::HSL kDefaultTintFrameInactive = {-1, -1, 0.9};
 constexpr color_utils::HSL kDefaultTintFrameIncognito = {-1, 0.2, 0.35};
-constexpr color_utils::HSL kDefaultTintFrameIncognitoInactive = {-1, 0.3, 0.6};
+constexpr color_utils::HSL kDefaultTintFrameIncognitoInactive = {-1, 0.2, 0.87};
 constexpr color_utils::HSL kDefaultTintBackgroundTab = {-1, -1, 0.75};
 
 // ----------------------------------------------------------------------------
 // Defaults for properties which are not stored in the browser theme pack.
 
 constexpr SkColor kDefaultColorControlBackground = SK_ColorWHITE;
 const SkColor kDefaultDetachedBookmarkBarSeparator =
     SkColorSetRGB(0xB6, 0xB4, 0xB6);
 const SkColor kDefaultDetachedBookmarkBarSeparatorIncognito =
     SkColorSetRGB(0x28, 0x28, 0x28);
@@ skipping 219 matching lines @@
     case COLOR_FRAME_INCOGNITO:
     case COLOR_FRAME_INCOGNITO_INACTIVE:
       NOTREACHED() << "These values should be queried via their respective "
                       "non-incognito equivalents and an appropriate |otr| "
                       "value.";
       return gfx::kPlaceholderColor;
   }
 
   return gfx::kPlaceholderColor;
 }