Bug 27133 – [Chromium] Upstream RGBColor

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Nate Chapin]

See summary.

[Nate Chapin]

Created attachment 32538 [details]
patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 32538 [details]
patch

> Index: WebCore/bindings/v8/RGBColor.cpp
...
> +    unsigned int value = (m_rgbColor >> 16) & 0xFF;
> +    return CSSPrimitiveValue::create(value, CSSPrimitiveValue::CSS_NUMBER);

nit: i think the prevailing style in webkit is to just use "unsigned" instead of
"unsigned int"

otherwise, R=me

[Sam Weinig]

What is v8 specific about this new file?  It seems like it should be in css/.

[Darin Fisher (:fishd, Google)]

We discussed this on IRC.  Moving this file to WebCore/css is a good idea since it may make sense in the future for the JSC bindings to use the same file.

[Darin Fisher (:fishd, Google)]

Comment on attachment 32538 [details]
patch

[Nate Chapin]

Created attachment 32562 [details]
patch2 - moved to WebCore/css/

Moved to css/ and renamed "unsigned int" to "unsigned".

That was all that was required, right?

[Eric Seidel (no email)]

Comment on attachment 32562 [details]
patch2 - moved to WebCore/css/

This could use Color.h for data storage and then the code for accessing red/green/blue would be simpler (just m_color.red()), but maybe that's overkill?

Otherwise this looks fine.

[Sam Weinig]

Comment on attachment 32562 [details]
patch2 - moved to WebCore/css/

> +
> +    class RGBColor : public RefCounted<RGBColor> {
> +    public:
> +        // TODO(ager): Make constructor private once codegenerator changes
> +        // have landed upstream.

We prefer FIXME, instead of TODO(name).

[Nate Chapin]

Landed as r45872.