For example: in RTCPeerConnection.idl: [StrictTypeChecking, RaisesException] RTCRtpSender addTrack(MediaStreamTrack track, MediaStream... streams); If MediaStream is only mentioned at the line above (as a variadic arg), JSMediaStream.cpp won't be included in JSRTCPeerConnection.cpp.
Created attachment 288947 [details] Patch
I have not added new binding test in the uploaded patch since seems covered by TestObj.idl test. Is it necessary to add a specific test for this ?
Thanks for picking up this bug. There's a FIXME related to this bug at [1] where an extra include is used as a workaround. [1] https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h#L41
(In reply to comment #3) > Thanks for picking up this bug. > > There's a FIXME related to this bug at [1] where an extra include is used as > a workaround. > > [1] > https://trac.webkit.org/browser/trunk/Source/WebCore/Modules/mediastream/ > RTCPeerConnection.h#L41 Yes, I removed this workaround in the uploaded patch.
Comment on attachment 288947 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288947&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4010 > + AddVariadicToImplIncludes($type, $function->signature->extendedAttributes->{"Conditional"}); Can we write instead AddToImplIncludes(...) unless ...? That would remove the need for a dedicated AddVariadicToImplIncludes, which is not really about variadic so should probably be renamed.
Comment on attachment 288947 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288947&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4010 >> + AddVariadicToImplIncludes($type, $function->signature->extendedAttributes->{"Conditional"}); > > Can we write instead AddToImplIncludes(...) unless ...? > That would remove the need for a dedicated AddVariadicToImplIncludes, which is not really about variadic so should probably be renamed. I think there is already a AddIncludesForTypeInImpl() for this purpose. I also don't think we need a new subroutine that is specific to variadic.
> Comment on attachment 288947 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=288947&action=review > > >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4010 > >> + AddVariadicToImplIncludes($type, $function->signature->extendedAttributes->{"Conditional"}); > > > > Can we write instead AddToImplIncludes(...) unless ...? > > That would remove the need for a dedicated AddVariadicToImplIncludes, which is not really about variadic so should probably be renamed. > > I think there is already a AddIncludesForTypeInImpl() for this purpose. I > also don't think we need a new subroutine that is specific to variadic. Thanks for the reviews. Actually, I added a new function for variadic since AddIncludesForTypeInImpl() is not supporting conditional attributes. In addition, isCallback argument of AddIncludesForTypeInImpl() must be set to 1 to include the binding header even if the parameter is not a callback. So, is it OK to use AddToImplIncludes() as proposed by Youenn?
(In reply to comment #7) > > Comment on attachment 288947 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=288947&action=review > > > > >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:4010 > > >> + AddVariadicToImplIncludes($type, $function->signature->extendedAttributes->{"Conditional"}); > > > > > > Can we write instead AddToImplIncludes(...) unless ...? > > > That would remove the need for a dedicated AddVariadicToImplIncludes, which is not really about variadic so should probably be renamed. > > > > I think there is already a AddIncludesForTypeInImpl() for this purpose. I > > also don't think we need a new subroutine that is specific to variadic. > Thanks for the reviews. > > Actually, I added a new function for variadic since > AddIncludesForTypeInImpl() is not supporting conditional attributes. In > addition, isCallback argument of AddIncludesForTypeInImpl() must be set to 1 > to include the binding header even if the parameter is not a callback. > > So, is it OK to use AddToImplIncludes() as proposed by Youenn? Fine by me as long as it works.
Created attachment 289222 [details] Patch
Comment on attachment 289222 [details] Patch Clearing flags on attachment: 289222 Committed r206094: <http://trac.webkit.org/changeset/206094>
All reviewed patches have been landed. Closing bug.