[Webkit-unassigned] [Bug 57770] Factoring the creation of 'FunctionOnly' callbacks in JavaScriptCore

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Darin Adler <darin at apple.com>  2011-04-06 07:22:14 PST ---
(From update of attachment 88272)
View in context: https://bugs.webkit.org/attachment.cgi?id=88272&action=review

>>> Source/WebCore/bindings/js/CallbackFunction.h:30
>>> +#include <wtf/Forward.h>
>> 
>> What is wtf/Forward.h needed for? I don’t see anything here that would benefit from including that.
> 
> The return value of the template is a PassRefPtr object.

Yes, but including Forward.h is insufficient to compile the definition of a function that returns a PassRefPtr. It’s enough to compile a declaration, but not a definition.

The code compiles because JSFunction.h ends up including PassRefPtr.h and your include of Forward.h has no effect.

>>>> Source/WebCore/bindings/js/CallbackFunction.h:35
>>>> +    CallbackAllowFunction = 0,
>>> 
>>> Should probably be named CallbackAllowFunctionOnly. It’s misleading to have it be named AllowFunction when a function will be allowed even if you don’t pass this value!
>> 
>> I already thought that, but then it makes no sense to write something valid like CallbackAllowFunctionOnly | CallbackAllowNull. Do you think it should be better to have CallbackAllowFunctionOnly since that value is only used in the exclusive cases and ignore it in other situations?
> 
> Yes, I'd thought about this too. I see problems with both names, so don't feel strongly either way.

Yes, I think the real problem is more fundamental. Since these constants are designed to or together it makes no sense to have one with the value 0; you can’t or that with something else in a meaningful way. We’d be better off with no name for the value 0, and with the flags defaulting to 0. Then no caller has to pass 0 explicitly.

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