Let ChromiumBridge::paintTextField check if the Color object passed in is valid or not.

Let ChromiumBridge::paintTextField check if the Color object passed in is valid
or not. If not valid, just pass white to ThemeEngine.

This change was motivated by the issue
http://code.google.com/p/chromium/issues/detail?id=2756, but it has been
resolved by only the upstream change
https://bugs.webkit.org/show_bug.cgi?id=25742.

However, this verification code is still useful.

BUG=2756

[tyoshino-do-not-use, 2009-05-08 21:12:46]

Hi Dimitri,

I'd like you to do a code review.

Thanks,

[tyoshino-do-not-use, 2009-05-08 22:30:32]

On 2009/05/08 21:12:46, tyoshino wrote:
> Hi Dimitri,
> 
> I'd like you to do a code review.
> 
> Thanks,

I found better way to fix this. I've updated the description and patch. Please
take a look. Thanks.

[dglazkov, 2009-05-09 20:16:37]

Redirecting to Brett, because he's more qualified to look at this than I am.

[dglazkov, 2009-05-09 20:17:48]

LGTM, pending Brett's blessing.

[brettw, 2009-05-10 14:36:32]

Seems like both of these issues need a layout test.

http://codereview.chromium.org/113165/diff/1005/1007
File third_party/WebKit/WebCore/rendering/RenderThemeChromiumWin.cpp (right):

http://codereview.chromium.org/113165/diff/1005/1007#newcode812
Line 812: false,
Do we ever pass true here? Can we remove this parameter?

[tyoshino-do-not-use, 2009-05-11 19:56:54]

On 2009/05/10 14:36:32, brettw wrote:
> Seems like both of these issues need a layout test.
> 
> http://codereview.chromium.org/113165/diff/1005/1007
> File third_party/WebKit/WebCore/rendering/RenderThemeChromiumWin.cpp (right):
> 
> http://codereview.chromium.org/113165/diff/1005/1007#newcode812
> Line 812: false,
> Do we ever pass true here? Can we remove this parameter?

Thank you Dimitri and Brett,
Reply inlined.
I'll write layout test.

[tyoshino-do-not-use, 2009-05-11 19:57:14]

http://codereview.chromium.org/113165/diff/1005/1007
File third_party/WebKit/WebCore/rendering/RenderThemeChromiumWin.cpp (right):

http://codereview.chromium.org/113165/diff/1005/1007#newcode812
Line 812: false,
On 2009/05/10 14:36:32, brettw wrote:

> Do we ever pass true here? Can we remove this parameter?

While writing layout test, I found that most fixing this to false makes the
appearance of combo box different from other browsers.

Firefox, IE, Safari makes the background color white when background color for
the select element is specified explicitly. They don't make the option element
inherit the background color (e.g., body element). To make it consistent with
other browser's behavior, we should paint the bg white iff both background-image
and background-color are not set.

[tyoshino-do-not-use, 2009-05-13 01:15:23]

On 2009/05/11 19:57:14, tyoshino wrote:
> http://codereview.chromium.org/113165/diff/1005/1007
> File third_party/WebKit/WebCore/rendering/RenderThemeChromiumWin.cpp (right):
> 
> http://codereview.chromium.org/113165/diff/1005/1007#newcode812
> Line 812: false,
> On 2009/05/10 14:36:32, brettw wrote:
> 
> > Do we ever pass true here? Can we remove this parameter?
> 
> While writing layout test, I found that most fixing this to false makes the
> appearance of combo box different from other browsers.
> 
> Firefox, IE, Safari makes the background color white when background color for
> the select element is specified explicitly. They don't make the option element
> inherit the background color (e.g., body element). To make it consistent with
> other browser's behavior, we should paint the bg white iff both
background-image
> and background-color are not set.

I've moved the change on WebKit to Bugzilla@WebKit
https://bugs.webkit.org/show_bug.cgi?id=25742.

I added layout tests for this change. Should I request review on
webkit/data/layout_tests/chrome/fast/forms/basic-selects.html here? It looks
like Chromium specific test. I couldn't find it in WebKit tree.

Thanks,

[tyoshino-do-not-use, 2009-05-21 09:36:44]

Hi Brett,

The bug http://code.google.com/p/chromium/issues/detail?id=2756 has been
resolved by the upstream change https://bugs.webkit.org/show_bug.cgi?id=25742,
but I think this validation is still good to add. Could you take a look again at
the current patch?

Thank you.

[brettw, 2009-05-22 17:55:51]

LGTM

[tyoshino-do-not-use, 2009-05-25 04:47:51]

Thank you. Could you please check in this on behalf of me?
I'm not a committer.