Bug 181064 – Add initial DOM support for Media Capabilities

• Component: Media
• Status: RESOLVED FIXED

[Jer Noble]

Add initial DOM support for Media Capabilities

[Jer Noble]

Created attachment 329964 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/36167620>

[EWS Watchlist]

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.

[EWS Watchlist]

Comment on attachment 329964 [details]
Patch

Attachment 329964 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5782205

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 329974 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 329964 [details]
Patch

Attachment 329964 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5782337

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 329981 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 329964 [details]
Patch

Attachment 329964 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5782355

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 329983 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Eric Carlson]

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).

[Jer Noble]

Created attachment 330003 [details]
Patch

[EWS Watchlist]

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.

[Jer Noble]

(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.

[Jer Noble]

Created attachment 330008 [details]
Patch

[EWS Watchlist]

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.

[Jer Noble]

Created attachment 330011 [details]
Patch

[EWS Watchlist]

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.

[EWS Watchlist]

Comment on attachment 330011 [details]
Patch

Attachment 330011 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5786178

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 330013 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 330011 [details]
Patch

Attachment 330011 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5786200

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 330014 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 330011 [details]
Patch

Attachment 330011 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5786191

New failing tests:
fast/dom/navigator-detached-no-crash.html

[EWS Watchlist]

Created attachment 330016 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Jer Noble]

Created attachment 330031 [details]
Patch

[EWS Watchlist]

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.

[WebKit Commit Bot]

Comment on attachment 330031 [details]
Patch

Clearing flags on attachment: 330031

Committed r226228: <https://trac.webkit.org/changeset/226228>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.