Fixing highlight color

Fixing highlight color


BUG=118713
TEST=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128395

[Mr4D (OOO till 08-26), 2012-03-22 21:48:26]

We accidentally removed the coloring when removing GTK. I was re-adding it in
for this one case.

[Daniel Erat, 2012-03-22 22:56:13]

http://codereview.chromium.org/9826031/diff/2001/chrome/browser/renderer_pref...
File chrome/browser/renderer_preferences_util.cc (right):

http://codereview.chromium.org/9826031/diff/2001/chrome/browser/renderer_pref...
chrome/browser/renderer_preferences_util.cc:39: #else
Mind cleaning this up?  We don't need to support the TOOLKIT_USES_GTK &&
OS_CHROMEOS combination anymore, so this can just be:

#if defined(TOOLKIT_USES_GTK)
  gtk_util::UpdateGtkFontSettings(prefs);
  ThemeServiceGtk* theme_service = ThemeServiceGtk::GetFrom(profile);
  // etc.
#elif defined(OS_CHROMEOS)
  // your new stuff
#endif

  prefs->enable_referrers = ...

It might be the case that we should be using either USE_ASH or USE_AURA rather
than OS_CHROMEOS, too.  Where are the colors that you're trying to match here
defined, NativeThemeAura?

http://codereview.chromium.org/9826031/diff/2001/content/renderer/render_view...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/9826031/diff/2001/content/renderer/render_view...
content/renderer/render_view_impl.cc:4265: #if defined(OS_CHROMEOS) or
defined(TOOLKIT_USES_GTK)
i am skeptical that this compiles -- you have 'or' instead of '||'

[Ben Goodger (Google), 2012-03-22 23:04:36]

http://codereview.chromium.org/9826031/diff/2001/chrome/browser/renderer_pref...
File chrome/browser/renderer_preferences_util.cc (right):

http://codereview.chromium.org/9826031/diff/2001/chrome/browser/renderer_pref...
chrome/browser/renderer_preferences_util.cc:39: #else
USE_ASH please!

We will match OS_WIN preferences on win+aura.

http://codereview.chromium.org/9826031/diff/2001/content/renderer/render_view...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/9826031/diff/2001/content/renderer/render_view...
content/renderer/render_view_impl.cc:4265: #if defined(OS_CHROMEOS) or
defined(TOOLKIT_USES_GTK)
On 2012/03/22 22:56:13, Daniel Erat wrote:
> i am skeptical that this compiles -- you have 'or' instead of '||'

given the previous file also, USE_ASH

[Mr4D (OOO till 08-26), 2012-03-22 23:15:53]

Addressed. Thanks for reviewing!

http://codereview.chromium.org/9826031/diff/2001/chrome/browser/renderer_pref...
File chrome/browser/renderer_preferences_util.cc (right):

http://codereview.chromium.org/9826031/diff/2001/chrome/browser/renderer_pref...
chrome/browser/renderer_preferences_util.cc:39: #else
On 2012/03/22 23:04:36, Ben Goodger (Google) wrote:
> USE_ASH please!
> 
> We will match OS_WIN preferences on win+aura.

Done.

http://codereview.chromium.org/9826031/diff/2001/chrome/browser/renderer_pref...
chrome/browser/renderer_preferences_util.cc:39: #else
Sure! I was not sure if GTK is gone for good or might come back after all and as
such I was reluctant to remove entirely. Done.

http://codereview.chromium.org/9826031/diff/2001/content/renderer/render_view...
File content/renderer/render_view_impl.cc (right):

http://codereview.chromium.org/9826031/diff/2001/content/renderer/render_view...
content/renderer/render_view_impl.cc:4265: #if defined(OS_CHROMEOS) or
defined(TOOLKIT_USES_GTK)
Good one! (too much Python and JS programming for 3 years) But ... somehow it
did compile! Done.

http://codereview.chromium.org/9826031/diff/2001/content/renderer/render_view...
content/renderer/render_view_impl.cc:4265: #if defined(OS_CHROMEOS) or
defined(TOOLKIT_USES_GTK)
On 2012/03/22 23:04:36, Ben Goodger (Google) wrote:
> On 2012/03/22 22:56:13, Daniel Erat wrote:
> > i am skeptical that this compiles -- you have 'or' instead of '||'
> 
> given the previous file also, USE_ASH

Done.

[Daniel Erat, 2012-03-22 23:20:38]

lgtm

[Ben Goodger (Google), 2012-03-22 23:21:21]

LGTM2

[commit-bot: I haz the power, 2012-03-22 23:28:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/9826031/7001

[commit-bot: I haz the power, 2012-03-23 02:26:57]

Change committed as 128395