Bug 181064 - Add initial DOM support for Media Capabilities
Summary: Add initial DOM support for Media Capabilities
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media Elements (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-20 16:25 PST by Jer Noble
Modified: 2017-12-21 10:28 PST (History)
5 users (show)

See Also:


Attachments
Patch (95.15 KB, patch)
2017-12-20 16:32 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-elcapitan (2.26 MB, application/zip)
2017-12-20 17:31 PST, Build Bot
no flags Details
Archive of layout-test-results from ews104 for mac-elcapitan-wk2 (2.55 MB, application/zip)
2017-12-20 17:49 PST, Build Bot
no flags Details
Archive of layout-test-results from ews114 for mac-elcapitan (2.90 MB, application/zip)
2017-12-20 18:05 PST, Build Bot
no flags Details
Patch (97.24 KB, patch)
2017-12-20 21:13 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (103.41 KB, patch)
2017-12-20 22:18 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Patch (103.77 KB, patch)
2017-12-20 22:46 PST, Jer Noble
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-elcapitan (2.15 MB, application/zip)
2017-12-20 23:53 PST, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (2.54 MB, application/zip)
2017-12-21 00:06 PST, Build Bot
no flags Details
Archive of layout-test-results from ews112 for mac-elcapitan (3.00 MB, application/zip)
2017-12-21 00:16 PST, Build Bot
no flags Details
Patch (108.36 KB, patch)
2017-12-21 08:31 PST, Jer Noble
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2017-12-20 16:25:51 PST
Add initial DOM support for Media Capabilities
Comment 1 Jer Noble 2017-12-20 16:32:39 PST
Created attachment 329964 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2017-12-20 16:34:10 PST
<rdar://problem/36167620>
Comment 3 Build Bot 2017-12-20 16:36:44 PST
Attachment 329964 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:31:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:32:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 48 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Build Bot 2017-12-20 17:31:54 PST Comment hidden (obsolete)
Comment 5 Build Bot 2017-12-20 17:31:55 PST Comment hidden (obsolete)
Comment 6 Build Bot 2017-12-20 17:49:37 PST Comment hidden (obsolete)
Comment 7 Build Bot 2017-12-20 17:49:38 PST Comment hidden (obsolete)
Comment 8 Build Bot 2017-12-20 18:05:38 PST Comment hidden (obsolete)
Comment 9 Build Bot 2017-12-20 18:05:44 PST Comment hidden (obsolete)
Comment 10 Eric Carlson 2017-12-20 20:47:35 PST
Comment on attachment 329964 [details]
Patch

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

> Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:66
> +    // value describing a single media codec. Otherwise, it eMUST contain no parameters.

Nit: "eMUST"

> Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:97
> +    // 1. If configurationâs contentType is not a valid video MIME type, return false and abort these steps.

Nit: might as well cite the spec here as you did in isValidVideoConfiguration

> Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:136
> +
> +    m_taskQueue.enqueueTask([configuration = WTFMove(configuration), promise = WTFMove(promise)] () mutable {

// 4. Let p be a new promise.
// 5. In parallel, run the create a MediaCapabilitiesInfo algorithm with configuration and resolve p with its result.
// 6. Return p.

> Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:144
> +{
> +    if (!isValidMediaConfiguration(configuration)) {

Citation and steps as in decodingInfo?

> Source/WebKit/UIProcess/API/C/WKPreferences.cpp:1942
> +
> +void WKPreferencesSetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef, bool enabled)
> +{
> +    toImpl(preferencesRef)->setMediaCapabilitiesEnabled(enabled);
> +}
> +
> +bool WKPreferencesGetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef)
> +{
> +    return toImpl(preferencesRef)->mediaCapabilitiesEnabled();
> +}
> +

Do we really need to add this to the C API?

> Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:308
> +WK_EXPORT bool WKPreferencesGetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef);
> +WK_EXPORT void WKPreferencesSetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef, bool enabled);

Ditto.

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:684
> +        @NO, WebKitMediaCapabilitiesEnabledPreferenceKey,

Do we really need to add this to WK1? If this is just for testing, you can add a function to InternalSettings to set the runtime flag (eg. InternalSettings::setScreenCaptureEnabled).
Comment 11 Jer Noble 2017-12-20 21:13:25 PST
Created attachment 330003 [details]
Patch
Comment 12 Build Bot 2017-12-20 21:16:20 PST
Attachment 330003 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:31:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:32:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 50 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Jer Noble 2017-12-20 21:41:03 PST
(In reply to Eric Carlson from comment #10)
> Comment on attachment 329964 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329964&action=review
> 
> > Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:66
> > +    // value describing a single media codec. Otherwise, it eMUST contain no parameters.
> 
> Nit: "eMUST"

Fixed;

> > Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:97
> > +    // 1. If configurationâs contentType is not a valid video MIME type, return false and abort these steps.
> 
> Nit: might as well cite the spec here as you did in isValidVideoConfiguration

Ok.

> > Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:136
> > +
> > +    m_taskQueue.enqueueTask([configuration = WTFMove(configuration), promise = WTFMove(promise)] () mutable {
> 
> // 4. Let p be a new promise.
> // 5. In parallel, run the create a MediaCapabilitiesInfo algorithm with
> configuration and resolve p with its result.
> // 6. Return p.

Ok.

> > Source/WebCore/Modules/mediacapabilities/MediaCapabilities.cpp:144
> > +{
> > +    if (!isValidMediaConfiguration(configuration)) {
> 
> Citation and steps as in decodingInfo?

Added.

> > Source/WebKit/UIProcess/API/C/WKPreferences.cpp:1942
> > +
> > +void WKPreferencesSetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef, bool enabled)
> > +{
> > +    toImpl(preferencesRef)->setMediaCapabilitiesEnabled(enabled);
> > +}
> > +
> > +bool WKPreferencesGetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef)
> > +{
> > +    return toImpl(preferencesRef)->mediaCapabilitiesEnabled();
> > +}
> > +
> 
> Do we really need to add this to the C API?

Yep. Because, amazingly, WebKitTestRunner still uses the C preferences API. :(

> > Source/WebKit/UIProcess/API/C/WKPreferencesRef.h:308
> > +WK_EXPORT bool WKPreferencesGetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef);
> > +WK_EXPORT void WKPreferencesSetMediaCapabilitiesEnabled(WKPreferencesRef preferencesRef, bool enabled);
> 
> Ditto.
> 
> > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:684
> > +        @NO, WebKitMediaCapabilitiesEnabledPreferenceKey,
> 
> Do we really need to add this to WK1? If this is just for testing, you can
> add a function to InternalSettings to set the runtime flag (eg.
> InternalSettings::setScreenCaptureEnabled).

I guess we could, but that's not how any of the other preferences work. Adding it only to InternalSettings would mean we'd have to enable it for individual tests in the test itself. This API will eventually be enabled by default (on Cocoa ports, at least), so adding "internal" enable doesn't seem correct here.
Comment 14 Jer Noble 2017-12-20 22:18:48 PST
Created attachment 330008 [details]
Patch
Comment 15 Build Bot 2017-12-20 22:20:47 PST
Attachment 330008 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:31:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:32:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 50 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 Jer Noble 2017-12-20 22:46:55 PST
Created attachment 330011 [details]
Patch
Comment 17 Build Bot 2017-12-20 22:48:58 PST
Attachment 330011 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:31:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:32:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 50 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Build Bot 2017-12-20 23:53:47 PST Comment hidden (obsolete)
Comment 19 Build Bot 2017-12-20 23:53:48 PST Comment hidden (obsolete)
Comment 20 Build Bot 2017-12-21 00:06:19 PST Comment hidden (obsolete)
Comment 21 Build Bot 2017-12-21 00:06:20 PST Comment hidden (obsolete)
Comment 22 Build Bot 2017-12-21 00:16:29 PST Comment hidden (obsolete)
Comment 23 Build Bot 2017-12-21 00:16:31 PST Comment hidden (obsolete)
Comment 24 Jer Noble 2017-12-21 08:31:50 PST
Created attachment 330031 [details]
Patch
Comment 25 Build Bot 2017-12-21 08:34:29 PST
Attachment 330031 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:31:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/mediacapabilities/ScreenColorGamut.h:32:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 2 in 55 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 WebKit Commit Bot 2017-12-21 10:28:27 PST
Comment on attachment 330031 [details]
Patch

Clearing flags on attachment: 330031

Committed r226228: <https://trac.webkit.org/changeset/226228>
Comment 27 WebKit Commit Bot 2017-12-21 10:28:29 PST
All reviewed patches have been landed.  Closing bug.