Bug 160606 – Implement constrainable type ConstrainDoubleRange

• Component: Media
• Status: RESOLVED CONFIGURATION CHANGED

[George Ruan]

https://w3c.github.io/mediacapture-main/#idl-def-constraindoublerange

[Radar WebKit Bug Importer]

<rdar://problem/27725318>

[George Ruan]

Created attachment 285449 [details]
Patch

[Chris Dumez]

Comment on attachment 285449 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285449&action=review

> Source/WebCore/Modules/mediastream/ConstrainDoubleRange.idl:27
> +Conditional=MEDIA_STREAM,

Should be indented.

> Source/WebCore/Modules/mediastream/ConstrainDoubleRange.idl:28
> +] dictionary ConstrainDoubleRange : DoubleRange {

Can you check the generated bindings, I doubt we support dictionary inheritance currently.

> Source/WebCore/Modules/mediastream/DoubleRange.idl:28
> +] dictionary DoubleRange {

I am not aware that we support dictionaries defined in their own IDL file. AFAIK, the dictionary needs to be defined in the IDL of the interface where it is used (e.g. MediaDevices.idl).

> Source/WebCore/Modules/mediastream/MediaDevices.h:58
> +struct ConstrainDoubleRange : DoubleRange {

Shouldn't this be public inheritance?

[Chris Dumez]

Comment on attachment 285449 [details]
Patch

I don't think it makes sense to add IDL files for which we do not generate any code.

[Chris Dumez]

(In reply to comment #4)
> Comment on attachment 285449 [details]
> Patch
> 
> I don't think it makes sense to add IDL files for which we do not generate
> any code.

FYI, Source/WebCore/DerivedSources.make is where we add the IDL files so that we actually generate code for them. However, in your case, you probably don't need additional IDL files if you move those dictionaries to the existing IDL file where they are used.

[George Ruan]

Created attachment 285457 [details]
Patch

[Chris Dumez]

Comment on attachment 285457 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285457&action=review

> Source/WebCore/ChangeLog:10
> +        patch.

While this patch seems correct, I am not sure how helpful this is. This adds support for a ConstrainDoubleRange dictionary. However, this type is not used directly.
I looked at the specification and there is no API taking a ConstrainDoubleRange. However, there is a MediaTrackConstraintSet dictionary with a member of type ConstrainDouble.

ContrainDouble is defined as so:
typedef (double or ConstrainDoubleRange) ConstrainDouble;

However, since we don't support union types, you'll either need to add support for union types first or more likely use the following:
typedef any ConstrainDouble;

And do the conversion of this member on native side. In which case the ConstrainDoubleRange type you're adding would not end up being used.