Bug 197476 – Hide MediaCapabilities.encodingInfo() when the platform does not support it.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Hide MediaCapabilities.encodingInfo() when the platform does not support it.

[Jer Noble]

Created attachment 368712 [details]
Patch

[EWS Watchlist]

Attachment 368712 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.cpp:77:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.cpp:82:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 368712 [details]
Patch

Attachment 368712 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12055054

New failing tests:
media/mediacapabilities/mediacapabilities-types.html

[EWS Watchlist]

Created attachment 368733 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4

[Eric Carlson]

Comment on attachment 368712 [details]
Patch

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

> Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.cpp:78
> +bool MediaEngineConfigurationFactory::hasDecodingConfigurationFactory()
> +{
> +    return mockEnabled() || WTF::anyOf(factories(), [] (auto& factory) { return factory.createDecodingConfiguration; });
> +}

This isn't used, should it be?

[Jer Noble]

(In reply to Eric Carlson from comment #5)
> Comment on attachment 368712 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=368712&action=review
> 
> > Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.cpp:78
> > +bool MediaEngineConfigurationFactory::hasDecodingConfigurationFactory()
> > +{
> > +    return mockEnabled() || WTF::anyOf(factories(), [] (auto& factory) { return factory.createDecodingConfiguration; });
> > +}
> 
> This isn't used, should it be?

I guess it should be, just in case there's any port who wants to do encodingInfo() and not decodingInfo().

[Jer Noble]

Created attachment 369798 [details]
Patch

[EWS Watchlist]

Attachment 369798 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.cpp:77:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.cpp:82:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 2 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Comment on attachment 369798 [details]
Patch

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

> LayoutTests/ChangeLog:13
> +        * platform/ios/media/mediacapabilities/mediacapabilities-encodingInfo-undefined-expected.txt: Added.
> +        * platform/ios/media/mediacapabilities/mediacapabilities-encodingInfo-undefined.html: Added.
> +        * platform/ios/media/mediacapabilities/mediacapabilities-types-expected.txt: Added.
> +        * platform/mac/media/mediacapabilities/mediacapabilities-encodingInfo-undefined-expected.txt: Added.
> +        * platform/mac/media/mediacapabilities/mediacapabilities-encodingInfo-undefined.html: Added.
> +        * platform/mac/media/mediacapabilities/mediacapabilities-types-expected.txt: Added.

Do we really need to add duplicate tests and results, couldn't you add one copy of each in media/mediacapabilities/ and skip the tests on platforms where they aren't supported (yet)?

[Jer Noble]

(In reply to Eric Carlson from comment #9)
> Comment on attachment 369798 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=369798&action=review
> 
> > LayoutTests/ChangeLog:13
> > +        * platform/ios/media/mediacapabilities/mediacapabilities-encodingInfo-undefined-expected.txt: Added.
> > +        * platform/ios/media/mediacapabilities/mediacapabilities-encodingInfo-undefined.html: Added.
> > +        * platform/ios/media/mediacapabilities/mediacapabilities-types-expected.txt: Added.
> > +        * platform/mac/media/mediacapabilities/mediacapabilities-encodingInfo-undefined-expected.txt: Added.
> > +        * platform/mac/media/mediacapabilities/mediacapabilities-encodingInfo-undefined.html: Added.
> > +        * platform/mac/media/mediacapabilities/mediacapabilities-types-expected.txt: Added.
> 
> Do we really need to add duplicate tests and results, couldn't you add one
> copy of each in media/mediacapabilities/ and skip the tests on platforms
> where they aren't supported (yet)?

We really do; the outliers for the "types" tests are iOS and macOS ports, but the layout test results don't cascade together anywhere, so duplicate results are needed there. Same deal  with the "encodingInfo-undefined" tests; those are necessarily macOS and iOS-only tests.

[WebKit Commit Bot]

Comment on attachment 369798 [details]
Patch

Rejecting attachment 369798 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 369798, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=369798&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=197476&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 369798 from bug 197476.
Fetching: https://bugs.webkit.org/attachment.cgi?id=369798
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Eric Carlson']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 16 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/Modules/mediacapabilities/MediaCapabilities.idl
patching file Source/WebCore/WebCore.xcodeproj/project.pbxproj
Hunk #1 succeeded at 13431 (offset 2 lines).
Hunk #2 succeeded at 20618 (offset 2 lines).
patching file Source/WebCore/bindings/js/JSMediaCapabilitiesCustom.h
patching file Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Hunk #1 succeeded at 1710 (offset 1 line).
Hunk #2 succeeded at 3746 (offset 1 line).
patching file Source/WebCore/bindings/scripts/IDLAttributes.json
patching file Source/WebCore/bindings/scripts/preprocess-idls.pl
Hunk #1 FAILED at 283.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/bindings/scripts/preprocess-idls.pl.rej
patching file Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.cpp
patching file Source/WebCore/platform/mediacapabilities/MediaEngineConfigurationFactory.h
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/ios/media/mediacapabilities/mediacapabilities-encodingInfo-undefined-expected.txt
patching file LayoutTests/platform/ios/media/mediacapabilities/mediacapabilities-encodingInfo-undefined.html
patching file LayoutTests/platform/ios/media/mediacapabilities/mediacapabilities-types-expected.txt
patching file LayoutTests/platform/mac/media/mediacapabilities/mediacapabilities-encodingInfo-undefined-expected.txt
patching file LayoutTests/platform/mac/media/mediacapabilities/mediacapabilities-encodingInfo-undefined.html
patching file LayoutTests/platform/mac/media/mediacapabilities/mediacapabilities-types-expected.txt

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Eric Carlson']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/12200461

[Jon Lee]

rdar://problem/45180484

[Jer Noble]

Created attachment 370319 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 370319 [details]
Patch for landing

Rejecting attachment 370319 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 370319, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/12246946

[Jon Lee]

Comment on attachment 370319 [details]
Patch for landing

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

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1714
> +        || $context->extendedAttributes->{CustomEnabled};

No bindings gen test?

[Jer Noble]

Created attachment 370427 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 370427 [details]
Patch for landing

Clearing flags on attachment: 370427

Committed r245636: <https://trac.webkit.org/changeset/245636>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.