[Webkit-unassigned] [Bug 103642] Add support for generic types in arrays and sequences to the code generators

[bugzilla-daemon at webkit.org]

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


Adam Bergkvist <adam.bergkvist at ericsson.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #176740|0                           |1
        is obsolete|                            |
 Attachment #176978|                            |review?
               Flag|                            |




--- Comment #6 from Adam Bergkvist <adam.bergkvist at ericsson.com>  2012-11-30 09:04:44 PST ---
Created an attachment (id=176978)
 --> (https://bugs.webkit.org/attachment.cgi?id=176978&action=review)
Updated patch

Thank you for reviewing.

(In reply to comment #5)
> (From update of attachment 176740 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176740&action=review
> 
> The approach looks reasonable. r-ing due to duplicated code in toHostObjectArray(). I hope you can use toNativeArray().
> 
> I guess you need to make some change to CodeGenerator{GObject,CPP,ObjC}.pm to fix build errors.

The build errors are due to a "sequence<String>" in an IDL file (the change to "sequence<DOMString>" got lost in a last minute rebase). This is fixed now.
> 
> > Source/WebCore/bindings/js/JSDOMBinding.h:416
> > +    template <class T, class JST>
> > +    Vector<RefPtr<T> > toHostObjectArray(JSC::ExecState* exec, JSC::JSValue value, T* (*toT)(JSC::JSValue value))
> 
> Can you avoid adding this method by using NativeValueTraits<T> ? See toNativeArray() and NativeValueTraits<T> in JSDOMBinding.h. toNativeArray() is doing the same thing as this method.
>

We could solve this by adding a "struct NativeValueTraits<RefPtr<[Type]> >" for every host object type that we need to put in an array or sequence, but then we would have to update the code generators every time a new type is needed.

The alternative would be to make a NativeValueTraits solution for the general RefPtr<T> type, but I think it will be hard to add that kind of support to the existing toNativeArray() which only deals with primitive types and strings. For example, besides the type T, the binding type (JST and V8T in this patch) is needed to deal with host objects.

I've kept toHostObjectArray() for now.

> > Source/WebCore/bindings/js/JSDOMBinding.h:421
> > +            return result;
> 
> Nit: 'return Vector<T>()' would be clearer.

Fixed (for V8Binding.h as well)

> 
> > Source/WebCore/bindings/js/JSDOMBinding.h:430
> > +                return result;
> 
> Maybe this should be 'return Vector<T>()' ?

You're right. Fixed (for V8Binding.h as well)

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