Currently IDLParser.pm is not able to parse sequence<T> if it is one methods argument.
Created attachment 134554 [details] proposed patch Attaching proposed patch to get comments. This is to add support for parser to parse arguments of type sequence<T>. Though currently no function Source Code passes sequence<T> as argument but we can consider this. Please let me know your review comments on this. Thanks.
Comment on attachment 134554 [details] proposed patch The patch looks OK.
Comment on attachment 134554 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=134554&action=review Thanks haraken for quick review. > Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp:697 > { Just doubtful here "WebDOMsequence<ScriptProfile>& sequenceArg" is this desired? Or may be this can be fixed when we actually start using sequence<T> as argument.
Comment on attachment 134554 [details] proposed patch Marking for review.
Comment on attachment 134554 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=134554&action=review >> Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp:697 >> { > > Just doubtful here "WebDOMsequence<ScriptProfile>& sequenceArg" is this desired? Or may be this can be fixed when we actually start using sequence<T> as argument. Oh, good catch! We are not planning to support sequence<T> in ObjC, GObject and CPP, for now. Then shall we skip generating code for sequence<T> in those three bindings? (See SkipFunction() in CodeGeneratorGObject.pm, ShouldSkipType() in CodeGeneratorCPP.pm. You might need to add something like it to CodeGeneratorObjC.pm.)
(In reply to comment #5) > (From update of attachment 134554 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134554&action=review > > >> Source/WebCore/bindings/scripts/test/CPP/WebDOMTestObj.cpp:697 > >> { > > > > Just doubtful here "WebDOMsequence<ScriptProfile>& sequenceArg" is this desired? Or may be this can be fixed when we actually start using sequence<T> as argument. > > Oh, good catch! We are not planning to support sequence<T> in ObjC, GObject and CPP, for now. Then shall we skip generating code for sequence<T> in those three bindings? (See SkipFunction() in CodeGeneratorGObject.pm, ShouldSkipType() in CodeGeneratorCPP.pm. You might need to add something like it to CodeGeneratorObjC.pm.) Sure I will update patch :)
Is there an example of a feature that would use this functionality?
(In reply to comment #7) > Is there an example of a feature that would use this functionality? As of now we don't have any plan to use this functionality but we can consider the using sequence<T> in implementations like http://trac.webkit.org/browser/trunk/Source/WebCore/dom/MessagePort.idl?rev=106737#L40 where it passes Array.
Created attachment 134636 [details] updated_patch
I would have thought in-parameters would use T[] rather than Sequence<T>, but I'm not sure I fully understand the subtle differences between the two.
Comment on attachment 134636 [details] updated_patch View in context: https://bugs.webkit.org/attachment.cgi?id=134636&action=review > Source/WebCore/bindings/scripts/CodeGeneratorCPP.pm:472 > + if ($codeGenerator->GetArrayType($param->type)) { > + next TOP; > + } Scattering this kind of code around is a bit dirty. Shall we put the code in ShouldSkipType($function)? e.g. sub ShouldSkipType() { ...; foreach my $param (@{$function->parameters}) { return 1 if $codeGenerator->GetArrayType($param->type); } ...; } Same comments for all "next TOP" in this patch. > Source/WebCore/bindings/scripts/CodeGeneratorObjC.pm:848 > foreach my $function (@{$dataNode->functions}) { You can put something like ShouldSkipType() here. Maybe you need to prepare ShouldSkipType() in CodeGeneratorObjC.pm. > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3606 > + AddToImplIncludes($arrayType); This should be "${arrayType}.h"
(In reply to comment #10) > I would have thought in-parameters would use T[] rather than Sequence<T>, but I'm not sure I fully understand the subtle differences between the two. vineet: Would you confirm the spec around this issue?
(In reply to comment #12) > (In reply to comment #10) > > I would have thought in-parameters would use T[] rather than Sequence<T>, but I'm not sure I fully understand the subtle differences between the two. > > vineet: Would you confirm the spec around this issue? While going through spec. for MessagePort http://dev.w3.org/html5/postmsg/#dom-messageport-postmessage or http://www.w3.org/TR/html5/comms.html#messageportarray The signature of postMessage() is like : void postMessage(any message, optional sequence<Transferable> transfer); Does that means in-parameter can use sequence<T> ?
(In reply to comment #11) > (From update of attachment 134636 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134636&action=review > Scattering this kind of code around is a bit dirty. Shall we put the code in ShouldSkipType($function)? e.g. > > You can put something like ShouldSkipType() here. Maybe you need to prepare ShouldSkipType() in CodeGeneratorObjC.pm. Added SkipFuntion and SkipAttribute. The reason we could not keep common implementation as SkipType is foreach my $param (@{$function->parameters}) Attributes doesn't have call to "parameters" so I though of having separate functions is it oke? > This should be "${arrayType}.h" Done.
Created attachment 134756 [details] Patch
Comment on attachment 134756 [details] Patch Thanks for the patch!
(In reply to comment #13) > While going through spec. for MessagePort http://dev.w3.org/html5/postmsg/#dom-messageport-postmessage or http://www.w3.org/TR/html5/comms.html#messageportarray > > The signature of postMessage() is like : > void postMessage(any message, optional sequence<Transferable> transfer); > > Does that means in-parameter can use sequence<T> ? Thanks for the clarification. I think the spec supports your patch. (In reply to comment #14) > Added SkipFuntion and SkipAttribute. > The reason we could not keep common implementation as SkipType is > foreach my $param (@{$function->parameters}) > > Attributes doesn't have call to "parameters" so I though of having separate functions is it oke? Yes, it is a desirable change.
Comment on attachment 134756 [details] Patch (In reply to comment #17) > (In reply to comment #13) > > While going through spec. for MessagePort http://dev.w3.org/html5/postmsg/#dom-messageport-postmessage or http://www.w3.org/TR/html5/comms.html#messageportarray > > > > The signature of postMessage() is like : > > void postMessage(any message, optional sequence<Transferable> transfer); > > > > Does that means in-parameter can use sequence<T> ? > > Thanks for the clarification. I think the spec supports your patch. Thanks haraken.. :) Should I take this as next <TO_DO> task..?
(In reply to comment #18) > Thanks haraken.. :) Should I take this as next <TO_DO> task..? We are always happy to kill custom bindings:) (unless it messes code generators)
Comment on attachment 134756 [details] Patch Clearing flags on attachment: 134756 Committed r112653: <http://trac.webkit.org/changeset/112653>
All reviewed patches have been landed. Closing bug.