[Webkit-unassigned] [Bug 70117] Expose HTMLCanvasElement supportsContext

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=70117


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #203954|review?                     |review+
               Flag|                            |




--- Comment #67 from Darin Adler <darin at apple.com>  2013-06-06 13:40:34 PST ---
(From update of attachment 203954)
View in context: https://bugs.webkit.org/attachment.cgi?id=203954&action=review

Much better. I didn’t carefully review the entire test, but it looks like it has pretty good coverage now. Still a few small things we could improve.

> Source/WebCore/bindings/js/JSHTMLCanvasElementCustom.cpp:84
> +        if (!get3DContextAttributes(exec, attrs))
> +            return jsUndefined();

The get3DContextAttributes function doesn’t really need a return value. Instead it can be:

    get3DContextAttributes(exec, attrs);
    if (exec->hadException())
        return jsUndefined();

In fact, I prefer this idiom because it makes it more clear why we are returning undefined and matches what we do in other cases.

> Source/WebCore/html/HTMLCanvasElement.h:97
> +    static bool is2dType(const String& type) { return type == "2d"; }
> +    static bool is3dType(const String& type)

Not sure these should really be inline functions. It clutters the header and I’m not sure there’s a significant performance boost.

> Source/WebCore/html/HTMLCanvasElement.h:100
> +        return (type == "webkit-3d") || (type == "experimental-webgl");

No need for the parentheses in this expression.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.