GTK: Use GTK+ theme selection colors and plumb them into webkit.

GTK: Use GTK+ theme selection colors and plumb them into webkit. [Chromium side of patch]

BUG=25831
TEST=none

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

[Elliot Glaysher, 2010-01-19 22:37:36]

After test driving this patch, chrome linux with a GTK theme without selection
colors looks weird.

I assume that I'll have to make a full webkit checkout and move the two attached
patches with a ChangeLog entry or something?

[Evan Martin, 2010-01-19 22:44:35]

LGTM

I suggest merging your two patches into one for ease of review.  On the WebKit
side you'll need a layout test to cover this as well.

If you get everything in order, patch your stuff into your WebKit tree (without
doing any git commits or anything) and then run
  ./WebKitTools/Scripts/webkit-patch upload
it *should* automatically walk you through the ChangeLog-editing, bug-creating,
and uploading for review process.

http://codereview.chromium.org/554004/diff/1/2
File chrome/browser/gtk/gtk_theme_provider.cc (right):

http://codereview.chromium.org/554004/diff/1/2#newcode627
chrome/browser/gtk/gtk_theme_provider.cc:627: inactive_selection_fg_color_ =
SkColorSetRGB(50, 50, 50);
Where do these colors come from?

[Evan Stade, 2010-01-19 22:55:43]

lgtm2

[Elliot Glaysher, 2010-01-19 23:44:45]

http://codereview.chromium.org/554004/diff/1/2
File chrome/browser/gtk/gtk_theme_provider.cc (right):

http://codereview.chromium.org/554004/diff/1/2#newcode627
chrome/browser/gtk/gtk_theme_provider.cc:627: inactive_selection_fg_color_ =
SkColorSetRGB(50, 50, 50);
On 2010/01/19 22:44:35, Evan Martin wrote:
> Where do these colors come from?

These are the current values from RenderThemeChromiumSkia.cpp; the same values
in Webkit::Color form are now the default colors in RenderThemeChromiumLinux.

[Elliot Glaysher, 2010-01-20 00:09:00]

On 2010/01/19 22:44:35, Evan Martin wrote:
> On the WebKit
> side you'll need a layout test to cover this as well.

I am confused about how I do this. For example, in Markus' scrollbar patch
(http://trac.webkit.org/changeset/51827), the ChangeLog mentions that testing is
done in our test_shell. I don't see any changes to test_shell that do that,
though.

Are you asking that I somehow expose setSelectionColors() to javascript and then
do an image test showing off the changed color?

[Evan Martin, 2010-01-20 00:17:26]

On Tue, Jan 19, 2010 at 4:09 PM,  <erg@chromium.org> wrote:
> I am confused about how I do this. For example, in Markus' scrollbar patch
> (http://trac.webkit.org/changeset/51827), the ChangeLog mentions that
> testing is
> done in our test_shell. I don't see any changes to test_shell that do that,
> though.

Oh yeah, hrm.  Maybe you can sneak it in the same way, but it'd
probably be preferable to write a test if it isn't too hard.  Not sure
if that's true though.  And the test won't cover the new API added
unless you call it from test shell.

> Are you asking that I somehow expose setSelectionColors() to javascript and
> then
> do an image test showing off the changed color?

That is typically how they work.  LayoutTestController is an object
available to layout tests.
http://webkit.org/quality/testwriting.html

[Elliot Glaysher, 2010-01-20 23:11:33]

Updated with layout tests on this side, and uploaded webkit side of patch to
https://bugs.webkit.org/show_bug.cgi?id=33921

[dglazkov, 2010-03-19 23:42:57]

Dudes, you landed the test on chromium side! :)