[webkit-dev] js binding: function argument type checking

[Kenneth Russell]

For what it's worth, I think this change should be made for all of the
DOM bindings, not just those for WebGL. The IDL code generators'
support for overloaded methods already generates TypeError when an
incoming argument doesn't implement any of the interfaces required by
the overloaded variants. The new behavior will be closer to the rules
specified by Web IDL in
http://dev.w3.org/2006/webapi/WebIDL/#es-interface and also, as I
understand it, closer to what Firefox implements.

It would be possible to add support for another extended attribute to
the code generators and annotate all of the methods in
WebGLRenderingContext.idl, but I really think the default behavior
should be changed.

-Ken

On Thu, Aug 12, 2010 at 1:15 PM, Mo, Zhenyao <zhenyao at gmail.com> wrote:
> Hardly.  Right now we already do the type checking in the generated
> toType(argument) functions.  Instead of casting to null, we throw a
> TypeError, which adds no extra cost if the type is correct.
>
> Besides, where I looked, after toType(argument) call, exception is
> checked.  Only that currently toType(argument) is not generating
> exceptions.
>
> Mo
>
> On Thu, Aug 12, 2010 at 9:20 AM, Sam Weinig <sam.weinig at gmail.com> wrote:
>>
>>
>> On Wed, Aug 11, 2010 at 10:58 PM, Darin Fisher <darin at chromium.org> wrote:
>>>
>>> On Wed, Aug 11, 2010 at 10:37 PM, Sam Weinig <sam.weinig at gmail.com> wrote:
>>>>
>>>> On Wed, Aug 11, 2010 at 10:29 PM, Cedric Vivier <cedricv at neonux.com>
>>>> wrote:
>>>>>
>>>>> On Thu, Aug 12, 2010 at 13:26, Sam Weinig <sam.weinig at gmail.com> wrote:
>>>>>>
>>>>>> For this specific case, it seems like you could easily check for a null
>>>>>> WebGLProgram* in WebGLRenderingContext::useProgram and set the
>>>>>> ExceptionCode.
>>>>>
>>>>> Nope, null is a valid argument, it bounds to the initial program, which
>>>>> means nothing will be drawn with WebGL.
>>>>> Certainly not the expected behavior when one pass the wrong type to the
>>>>> argument like Zhenyao pointed out, therefore throwing TypeError really makes
>>>>> sense here (and elsewhere with WebGL API).
>>>>
>>>> Ok, in that case, it seems like you need to do it in the bindings for
>>>> this. I would prefer not making a sweeping change at this time, and instead
>>>> keeping the changes just to places where the extra checking is necessary due
>>>> to ambiguity (as in this useProgram case).
>>>> -Sam
>>>
>>> Out of curiosity, if we have the ability for code to be auto generated
>>> from the IDL, why not use it universally?  I'm trying to guess to understand
>>> your preference :)
>>> -Darin
>>
>> My concern with doing it universally is the performance cost of doing the
>> check twice in many places (once in the bindings and once in the
>> implementation with a null check). We could certainly re-evaluate the way we
>> do these type checks, potentially even converting the existing null checks
>> in the implementation to asserts, but I think that discussion shouldn't be
>> conflated with this bug fix.
>> -Sam
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>
>>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>