[Webkit-unassigned] [Bug 57760] Factoring the creation of 'FunctionOnly' callbacks in V8

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 4 11:40:02 PDT 2011


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





--- Comment #4 from Leandro GraciĆ” Gil <leandrogracia at chromium.org>  2011-04-04 11:40:03 PST ---
(From update of attachment 88069)
View in context: https://bugs.webkit.org/attachment.cgi?id=88069&action=review

>> Source/WebCore/bindings/v8/V8Utilities.h:58
>> +    void throwTypeMismatchException();
> 
> Now that the call-sites have been unified, there's probably no need for this to be a function - you can just call throwError() directly.

Including V8Proxy.h in the header generates a dependency cycle. Keeping it in the cpp file avoids this.

>> Source/WebCore/bindings/v8/V8Utilities.h:61
>> +    PassRefPtr<V8CallbackType> createFunctionOnlyCallback(v8::Local<v8::Value> value, bool& succeeded, bool allowUndefined, bool allowNull)
> 
> It's usual to have the out param last. Also, an enum would be much easier to read than a pair of bools for allowUndefined and allowNull.

Fixed. Using bitmasks and a typedef to unsigned as argument type, mimicking UpdateLayerPositionsFlag in RenderLayer.h.

>> Source/WebCore/bindings/v8/custom/V8GeolocationCustom.cpp:-68
>> -    // Argument is optional (hence undefined is allowed), and null is allowed.
> 
> Would be good to preserve this comment at the call-site of createFunctionOnlyCallback().

Fixed.

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


More information about the webkit-unassigned mailing list