[webkit-reviews] review requested: [Bug 103642] Add support for generic types in arrays and sequences to the code generators : [Attachment 177263] Updated patch

[bugzilla-daemon at webkit.org]

Adam Bergkvist <adam.bergkvist at ericsson.com> has asked	for review:
Bug 103642: Add support for generic types in arrays and sequences to the code
generators
https://bugs.webkit.org/show_bug.cgi?id=103642

Attachment 177263: Updated patch
https://bugs.webkit.org/attachment.cgi?id=177263&action=review

------- Additional Comments from Adam Bergkvist <adam.bergkvist at ericsson.com>
(In reply to comment #8)
> (From update of attachment 176978 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=176978&action=review
> 
> Thanks for updating the patch. Almost looks OK.
> 
> > Source/WebCore/bindings/js/JSDOMBinding.h:416
> > +	 Vector<RefPtr<T> > toHostObjectArray(JSC::ExecState* exec,
JSC::JSValue value, T* (*toT)(JSC::JSValue value))
> 
> toRefPtrNativeArray() might be a better name, as this is just another version
of toNativeArray().

Fixed.

> > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3070
> > +	 return "Vector<" . GetNativeInnerVectorType($arrayOrSequenceType) .
">" if $arrayOrSequenceType;
> 
> Can't you use GetNativeType() ?

That was my initial approach as well (since I use it for V8), but the
JSC-version of GetNativeType doesn't behave the same as the V8 counterpart for
some types. For example, DOMString is translated to "const String&" instead of
"String".

> > Source/WebCore/bindings/v8/V8Binding.h:211
> > +	 Vector<RefPtr<T> > toHostObjectArray(v8::Handle<v8::Value> value)
> 
> toRefPtrNativeArray() might be a better name.

Fixed.

The build errors on EFL were due to a mismatch between the IDL type and the
implementation type in the Vibration API. I've added a workaround and filed a
separate bug to resolve that.