Bug 181064

Summary: Add initial DOM support for Media Capabilities
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: MediaAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric.carlson, ews-watchlist, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews102 for mac-elcapitan
none
Archive of layout-test-results from ews104 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews114 for mac-elcapitan
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews102 for mac-elcapitan
none
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
none
Archive of layout-test-results from ews112 for mac-elcapitan
none
Patch none

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 EWS Watchlist 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 EWS Watchlist 2017-12-20 17:31:54 PST Comment hidden (obsolete)
Comment 5 EWS Watchlist 2017-12-20 17:31:55 PST Comment hidden (obsolete)
Comment 6 EWS Watchlist 2017-12-20 17:49:37 PST Comment hidden (obsolete)
Comment 7 EWS Watchlist 2017-12-20 17:49:38 PST Comment hidden (obsolete)
Comment 8 EWS Watchlist 2017-12-20 18:05:38 PST Comment hidden (obsolete)
Comment 9 EWS Watchlist 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 EWS Watchlist 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 EWS Watchlist 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 EWS Watchlist 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 EWS Watchlist 2017-12-20 23:53:47 PST Comment hidden (obsolete)
Comment 19 EWS Watchlist 2017-12-20 23:53:48 PST Comment hidden (obsolete)
Comment 20 EWS Watchlist 2017-12-21 00:06:19 PST Comment hidden (obsolete)
Comment 21 EWS Watchlist 2017-12-21 00:06:20 PST Comment hidden (obsolete)
Comment 22 EWS Watchlist 2017-12-21 00:16:29 PST Comment hidden (obsolete)
Comment 23 EWS Watchlist 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 EWS Watchlist 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.