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

chrome/browser/themes/theme_service_win.h

 // Copyright 2016 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.
 
 #ifndef CHROME_BROWSER_THEMES_THEME_SERVICE_WIN_H_
 #define CHROME_BROWSER_THEMES_THEME_SERVICE_WIN_H_
 
 #include "base/win/registry.h"
 #include "chrome/browser/themes/theme_service.h"
 
+// Tracks updates to the native colors on Windows 10 and calcuates the values we
+// should use (which are not always what Windows uses). None of the values here
+// are relevant to earlier versions of Windows.

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

Should this be renamed ThemeServiceWin10 then?

Not for this patch: can we also respect user-set frame colors on Win 7/8? 
Presumably the only difference is getting the right values to begin with.

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

I think the name is okay, since it's an OS-specific override of a cross-platform
base class and this is the convention.

We could get the values on 7/8, it's just a question of what we'd do with them.
For 7 it'd only be relevant to basic mode. For 8 we could custom draw the
titlebar there too but I doubt it's worth the effort.

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

Fixing Win 7 basic seems worth doing; that's used by various accessibility
folks, and technically it seems like it should be easy.

For Win 8, I wouldn't bother unless Win 8 titlebars are close enough to Win 10
that we only need minor tweaks to custom-draw them.

 class ThemeServiceWin : public ThemeService {
  public:
   ThemeServiceWin();
   ~ThemeServiceWin() override;
 
  private:
   // ThemeService:
   bool ShouldUseNativeFrame() const override;
   SkColor GetDefaultColor(int id, bool incognito) const override;
 
-  // Returns true if the window frame color is determined by the DWM, i.e. this
-  // is a native frame on Windows 10.
-  bool ShouldUseDwmFrameColor() const;
+  // Returns true if colors from DWM can be used, i.e. this is a native frame
+  // on Windows 10.
+  bool DwmColorsAllowed() const;
 
   // Callback executed when |dwm_key_| is updated.  This re-reads the active
   // frame color and updates |dwm_frame_color_|.
   void OnDwmKeyUpdated();
 
   // Registry key containing the params that determine the DWM frame color.
-  // This is only initialized on Windows 10.
   std::unique_ptr<base::win::RegKey> dwm_key_;
 
-  // The DWM frame color, if available; white otherwise.
+  // True if the frame should be colored using the DWM values here. False if the
+  // default frame colors should be used instead.
+  bool use_dwm_frame_color_;

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

Maybe instead of this bool, use a base::Optional<SkColor> for the two colors
below.

This will also let us avoid needing to compute the inactive frame color in the
.cc file when the active frame color is set but the inactive color is not, as
these two will be tested individually.

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

I don't quite understand what you mean here. Do you want to compute the inactive
frame color on the fly somewhere else? Either way we have to compute it at some
point.

I kind of like the bool value because we're using either both colors or neither
color, there's not really a case where we only want one, unless we're computing
the inactive color on the fly.

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

My intent was that this class is only used to report user-set colors, and that
computing the inactive frame color from the active one be done elsewhere.

The motivation is that we already need to do that computation for theme images,
and I'm hoping to also do it for the default (unthemed) colors, to replace most
of the hardcoded colors (see comments in theme_properties.cc).  So I'm looking
to have all this happen in one spot, basically.

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

Oh okay I see what you mean now. I don't think the object hierarchy will do
exactly what you want. ThemeProperties is the last link in the chain, making it
hard to pass the ThemeServiceWin values "backwards" to ThemeProperties. So if
you want to eliminate the pre-computed defaults we'd have to do the inactive
color calculation in two places.

[Peter Kasting, 2016/12/08 04:59:47]

Hmm.  I know we'd probably have to write some similar code in ThemeService and
ThemeProperties (to use the maybe-themed tints/colors and base colors,
respectively), but it seems like ultimately both would boil down to something
like:

  return color_utils::HSLShift(frame_color, GetTint(inactive_tint, incognito));

...which seems like a better place to be than doing a direct AlphaBlend() with
0x33 in the code here.

(Without trying to implement this, it's not clear to me how much further this
can be simplified.  A single helper function that both ThemeProperties and
ThemeService can call?)

I have said my piece on how I think this should work, I will leave it to you to
decide what should be implemented in this CL, what should be implemented as
followups, and what's not worth worrying about.

+
+  // The frame color when active.
   SkColor dwm_frame_color_;
 
+  // The frame color when inactive.
+  SkColor dwm_inactive_frame_color_;
+
   // The DWM accent border color, if available; white otherwise.
   SkColor dwm_accent_border_color_;
 
   DISALLOW_COPY_AND_ASSIGN(ThemeServiceWin);
 };
 
 #endif  // CHROME_BROWSER_THEMES_THEME_SERVICE_WIN_H_