[webkit-reviews] review granted: [Bug 40273] Canvas: "currentColor" should inherit the canvas element's color : [Attachment 70040] Proposed patch v5

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Andreas Kling <kling at webkit.org>'s
request for review:
Bug 40273: Canvas: "currentColor" should inherit the canvas element's color
https://bugs.webkit.org/show_bug.cgi?id=40273

Attachment 70040: Proposed patch v5
https://bugs.webkit.org/attachment.cgi?id=70040&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=70040&action=review

> WebCore/html/canvas/CanvasStyle.cpp:81
> +    if (parseResult == ParseFailed)
> +	   return false;
> +    if (parseResult == ParsedCurrentColor)
> +	   parsedColor = currentColor(canvas);
> +    return true;

This should use a switch statement, because then the compiler will check to be
sure we cover all values of ColorParseResult.

> WebCore/html/canvas/CanvasStyle.cpp:135
> +    if (parseResult == ParseFailed)
>	   return 0;
> +    if (parseResult == ParsedCurrentColor)
> +	   return adoptRef(new CanvasStyle(CurrentColor));
>      return adoptRef(new CanvasStyle(rgba));

This should use a switch statement, because then the compiler will check to be
sure we cover all values of ColorParseResult.

> WebCore/html/canvas/CanvasStyle.h:53
> +	   float overrideAlpha() const { return m_overrideAlpha; }

This should assert that m_type is CurrentColorWithOverrideAlpha.

> WebCore/html/canvas/CanvasStyle.h:55
>	   String color() const { return Color(m_rgba).serialized(); }

This should assert that m_type is RGBA.