Use native theme colors for textfields; etc.

chrome/browser/ui/views/location_bar/location_bar_view.cc

Index: chrome/browser/ui/views/location_bar/location_bar_view.cc
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index ee59f1810df80d6c187de363f216dcd40eb08c02..7f73c710b61951dd1b2cc5a5055ae4bf2078e5e1 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -120,12 +120,6 @@ const int kDesktopScriptBadgeEdgeItemPadding = kDesktopScriptBadgeItemPadding;
 const int kTouchItemPadding = 8;
 const int kTouchEdgeItemPadding = kTouchItemPadding;
 
-#if defined(OS_CHROMEOS)
-const SkColor kOmniboxBackgroundColor = SkColorSetARGB(0, 255, 255, 255);
-#else
-const SkColor kOmniboxBackgroundColor = SkColorSetARGB(255, 255, 255, 255);
-#endif
-
 }  // namespace
 
 // static
@@ -332,26 +326,24 @@ bool LocationBarView::IsInitialized() const {
 
 SkColor LocationBarView::GetColor(ToolbarModel::SecurityLevel security_level,
                                   ColorKind kind) const {
-#if defined(OS_WIN)
-  if (GetNativeTheme() == ui::NativeThemeWin::instance()) {
-    switch (kind) {
-      case BACKGROUND:
-        return color_utils::GetSysSkColor(COLOR_WINDOW);
-      case TEXT:
-        return color_utils::GetSysSkColor(COLOR_WINDOWTEXT);
-      case SELECTED_TEXT:
-        return color_utils::GetSysSkColor(COLOR_HIGHLIGHTTEXT);
-      default:
-        // Other cases are handled below.
-        break;
-    }
-  }
-#endif
+  const ui::NativeTheme* native_theme = GetNativeTheme();
   switch (kind) {
-    // TODO(beng): source from theme provider.
-    case BACKGROUND:    return kOmniboxBackgroundColor;
-    case TEXT:          return SK_ColorBLACK;
-    case SELECTED_TEXT: return SK_ColorWHITE;
+    case BACKGROUND:
+#if defined(OS_CHROMEOS)
+      // Chrome OS requires a transparent omnibox background color.

[varunjain, 2012/12/06 18:36:58]

I agree that native_theme should not have a separate color for omnibox. I think
the omnibox popup (nothing else seems to be getting affected) should not be
using textfieldBackgroundColor. It should probably use the background color of a
popup. There is none defined in native_theme, closest seems to be
kColorId_MenuBackgroundColor

[msw, 2012/12/06 18:54:46]

That might be feasible for a follow-up change. It may make more sense to offer
control over the presence of the solid color background used in
NativeTextfieldViews.

A problem with either approach (and the current code) is that DEEMPHASIZED_TEXT
and SECURITY_TEXT calculate colors using the BACKGROUND color, which is actually
the unspecified color within the images, not the transparent white supplied
here.

[sky, 2012/12/06 20:43:57]

Doesn't the transparent background apply to OS_WIN when native theme ==
nativethemeaura?

[msw, 2012/12/07 02:04:01]

Hmm, not afaik; I think this is okay as-is. The old kOmniboxBackgroundColor
definition was transparent white for CrOS and opaque white for Win. My new code
preserves the local transparent white for CrOS and uses the equivalent
NativeThemeAura value for kColorId_TextfieldDefaultBackground
(kTextfieldDefaultBackground == SK_ColorWHITE) on Aura.

[sky, 2012/12/07 16:35:12]

I'm still confused. OS_CHROMEOS and OS_WIN when the theme == NativeThemeAura
should get the same colors. If you're saying native_theme->GetSystemColor for
!OS_CHROMEOS does the right thing, why do we need the ifdef at all?

[msw, 2012/12/07 18:22:28]

This change preserves the existing behavior, which contradicts your assertion.
Prior to my change, OS_CHROMEOS and OS_WIN when the theme == NativeThemeAura get
different colors. They'd both wind up outside the #if block and return
kOmniboxBackgroundColor, defined as transparent (0, 255, 255, 255) for CrOS and
normal (255, 255, 255, 255) for Win (see left side line 123). I can change/test
that, but I'm not sure that's our intent. Switching to transparent white here
breaks the non-Aura Win omnibox result dropdown, not sure if it would also break
Aura Win. Sorry for the trouble :(

[sky, 2012/12/07 18:58:02]

Ah, ok. I see it now.

+      return SkColorSetARGB(0, 255, 255, 255);
+#else
+      return native_theme->GetSystemColor(
+          ui::NativeTheme::kColorId_TextfieldDefaultBackground);
+#endif
+
+    case TEXT:
+      return native_theme->GetSystemColor(
+          ui::NativeTheme::kColorId_TextfieldDefaultColor);
+
+    case SELECTED_TEXT:
+      return native_theme->GetSystemColor(
+          ui::NativeTheme::kColorId_TextfieldSelectionColor);
 
     case DEEMPHASIZED_TEXT:
       return color_utils::AlphaBlend(