Bug 215992 – EnabledBySetting extended attributes on a partial interface do not get merged with EnabledBySetting extended attributes on properties of that partial interface

• Status: RESOLVED
• Resolution: FIXED
• Component: Bindings

[Sam Weinig]

While investigating https://bugs.webkit.org/show_bug.cgi?id=215981, one failure was in the generation of NavigatorMediaDevices.idl which currently looks like:

[
    Conditional=MEDIA_STREAM,
    EnabledAtRuntime=MediaDevices,
] partial interface Navigator {
    [SameObject, SecureContext, ContextAllowsMediaDevices] readonly attribute MediaDevices mediaDevices;
    [Custom, EnabledBySetting=LegacyGetUserMedia] undefined getUserMedia(object constraints, object? successCallback, object? errorCallback);
};

One thing to note here is that the extended attributes on the partial interface declaration itself (Conditional=MEDIA_STREAM and EnabledAtRuntime=MediaDevices) are really shorthand for mapping those extended attributes to all the member properties.

In the mentioned bug, I want to change the partial interface wide "EnabledAtRuntime=MediaDevices" to "EnabledBySetting=MediaDevices", which maps the EnabledBySetting to each member of the partial interface. The issue is that the code generator currently overwrites "EnabledBySetting=LegacyGetUserMedia" on the getUserMedia operation, but really it would make more sense to merge them.

This seems to be a general problem for many potentially mergeable extended attributes, we already kind of merge "Conditional" extended attributes, but only for operations for some reason.

[Sam Weinig]

Created attachment 407579 [details]
Patch

[Darin Adler]

Comment on attachment 407579 [details]
Patch

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

> Source/WebCore/bindings/scripts/test/JS/JSTestEnabledBySettingSupplemental.cpp:5
> +    JSTestEnabledBySettingSupplemental.cpp at every build. This file must not be tried to compile.

Not new or part of this patch, but just noticed:

1) We might want to reword "This file must not be tried to compile." since that's not English.

2) If we don’t want people to compile something normally we’d put #error in it.

[EWS]

Committed r266339: <https://trac.webkit.org/changeset/266339>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407579 [details].

[Radar WebKit Bug Importer]

<rdar://problem/68045887>

[Sam Weinig]

(In reply to Darin Adler from comment #2)
> Comment on attachment 407579 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407579&action=review
> 
> > Source/WebCore/bindings/scripts/test/JS/JSTestEnabledBySettingSupplemental.cpp:5
> > +    JSTestEnabledBySettingSupplemental.cpp at every build. This file must not be tried to compile.
> 
> Not new or part of this patch, but just noticed:
> 
> 1) We might want to reword "This file must not be tried to compile." since
> that's not English.
> 
> 2) If we don’t want people to compile something normally we’d put #error in
> it.

Will do this with https://bugs.webkit.org/show_bug.cgi?id=215995.