[AiS] for desktop, two lines and font sytles

ui/native_theme/native_theme_win.cc

Index: ui/native_theme/native_theme_win.cc
diff --git a/ui/native_theme/native_theme_win.cc b/ui/native_theme/native_theme_win.cc
index 4429d6ba71e138521b4c631906b49c0620562c31..f6588dc2ec04b1ac9a3ea59f309aa343e142c0e1 100644
--- a/ui/native_theme/native_theme_win.cc
+++ b/ui/native_theme/native_theme_win.cc
@@ -20,6 +20,7 @@
 #include "skia/ext/platform_canvas.h"
 #include "skia/ext/skia_utils_win.h"
 #include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkColor.h"
 #include "third_party/skia/include/core/SkColorPriv.h"
 #include "third_party/skia/include/core/SkShader.h"
 #include "ui/gfx/color_utils.h"
@@ -607,8 +608,11 @@ SkColor NativeThemeWin::GetSystemColor(ColorId color_id) const {
     case kColorId_ResultsTableSelectedBackground:
       return system_colors_[COLOR_HIGHLIGHT];
     case kColorId_ResultsTableNormalText:
-    case kColorId_ResultsTableHoveredText:
       return system_colors_[COLOR_WINDOWTEXT];
+    case kColorId_ResultsTableHoveredText:
+      return color_utils::GetReadableColor(

[msw, 2015/03/18 18:03:37]

Should the other kColorId_ResultsTable*Text (Normal, Selected, etc.) entries use
GetReadableColor with their respective backgrounds?

[dschuyler, 2015/03/18 19:16:49]

I'm not sure on that.  Either way, I'd prefer to address that in a separate CL. 
Maybe it can be part of cr bug 467259.

[msw, 2015/03/18 20:20:16]

I'd assume that these colors should be consistent in their behavior... Why would
you change one and not the others in this CL? Why would the colors here be
pre-calculated against their backgrounds, but the corresponding Ids for other
themes not pre-calculated? Wouldn't that yield fundamentally
inconsistent/incorrect behavior across themes? Really, I think the
omnibox_result_view.cc code should be doing the readable color blending, not the
theme files. These theme files should just return what the text color should be
[on a white background], and the UI controls that determine the background color
should to the readable color blending, since they would know better than this
file what the actual background colors would be... Ditto for other themes.

[Peter Kasting, 2015/03/18 22:19:33]

I disagree with all of this :)

First, it would be incorrect to force the normal and selected text colors
through GetReadableColor(), because those are intended to be the user's precise
system colors.  If we use GetReadableColor(), it will either do nothing, or it
will change those colors away from system colors, which isn't what we want.

The colors that are not user-set system colors, but are hardcoded or synthesized
by us, DO need to be put through GetReadableColor() to minimize the chance of
contrast problems.  Since the user never set these colors, they won't have any
expectation for how they behave.

The GetReadableColor() calls belong here, not on the caller side, because it's
the job of the theming system to give you the actual colors you need.  Making
every consumer adjust the colors identically is extra work on the caller side
and is more error-prone.

Besides this, I intend for all this code to move back into the omnibox-specific
files anyway, at which point the question of which end does the adjusting will
be moot.

[msw, 2015/03/19 21:19:56]

Okay, that's actually reasonable.

+          system_colors_[COLOR_WINDOWTEXT],
+          GetSystemColor(kColorId_ResultsTableHoveredBackground));
     case kColorId_ResultsTableSelectedText:
       return system_colors_[COLOR_HIGHLIGHTTEXT];
     case kColorId_ResultsTableNormalDimmedText:
@@ -641,6 +645,25 @@ SkColor NativeThemeWin::GetSystemColor(ColorId color_id) const {
     case kColorId_ResultsTableSelectedDivider:
       return color_utils::AlphaBlend(system_colors_[COLOR_HIGHLIGHTTEXT],
                                      system_colors_[COLOR_HIGHLIGHT], 0x34);
+    case kColorId_ResultsTablePositiveText:
+      return color_utils::GetReadableColor(SK_ColorGREEN,

[msw, 2015/03/18 18:03:37]

ditto nits about red/green locale values and file-local constants.

[dschuyler, 2015/03/18 19:16:49]

Acknowledged.

+                                           system_colors_[COLOR_WINDOW]);
+    case kColorId_ResultsTablePositiveHoveredText:
+      return color_utils::GetReadableColor(
+          SK_ColorGREEN,
+          GetSystemColor(kColorId_ResultsTableHoveredBackground));
+    case kColorId_ResultsTablePositiveSelectedText:
+      return color_utils::GetReadableColor(SK_ColorGREEN,
+                                           system_colors_[COLOR_HIGHLIGHT]);
+    case kColorId_ResultsTableNegativeText:
+      return color_utils::GetReadableColor(SK_ColorRED,
+                                           system_colors_[COLOR_WINDOW]);
+    case kColorId_ResultsTableNegativeHoveredText:
+      return color_utils::GetReadableColor(
+          SK_ColorRED, GetSystemColor(kColorId_ResultsTableHoveredBackground));
+    case kColorId_ResultsTableNegativeSelectedText:
+      return color_utils::GetReadableColor(SK_ColorRED,
+                                           system_colors_[COLOR_HIGHLIGHT]);
   }
   NOTREACHED();
   return kInvalidColorIdColor;