Summary: | Add initial DOM support for Media Capabilities | ||
---|---|---|---|
Product: | WebKit | Reporter: | Jer Noble <jer.noble> |
Component: | Media | Assignee: | 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
Jer Noble
2017-12-20 16:25:51 PST
Created attachment 329964 [details]
Patch
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 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 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
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 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
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 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
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). Created attachment 330003 [details]
Patch
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.
(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. Created attachment 330008 [details]
Patch
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.
Created attachment 330011 [details]
Patch
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 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 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
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 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
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 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
Created attachment 330031 [details]
Patch
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 on attachment 330031 [details] Patch Clearing flags on attachment: 330031 Committed r226228: <https://trac.webkit.org/changeset/226228> All reviewed patches have been landed. Closing bug. |