Bug 165943 - Remove all custom bindings from media streams, using dictionaries instead
Summary: Remove all custom bindings from media streams, using dictionaries instead
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-15 22:22 PST by Darin Adler
Modified: 2016-12-16 22:43 PST (History)
4 users (show)

See Also:


Attachments
Patch (71.21 KB, patch)
2016-12-15 22:33 PST, Darin Adler
sam: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (1.07 MB, application/zip)
2016-12-15 23:39 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2016-12-15 22:22:07 PST
Remove all custom bindings from media streams, using dictionaries instead
Comment 1 Darin Adler 2016-12-15 22:33:53 PST
Created attachment 297300 [details]
Patch
Comment 2 Build Bot 2016-12-15 23:39:00 PST
Comment on attachment 297300 [details]
Patch

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

New failing tests:
fast/mediastream/MediaDevices-getSupportedConstraints.html
fast/mediastream/MediaStreamTrack-getSettings.html
fast/mediastream/MediaStreamTrack-getCapabilities.html
Comment 3 Build Bot 2016-12-15 23:39:04 PST
Created attachment 297304 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 4 Sam Weinig 2016-12-16 09:33:32 PST
Looks like the failures in the tests are due to two problems:

1) The generated dictionary -> JS conversion adds the objects in alphabetical order, so when the tests iterate, they produce the same results in a different order. 
    MediaStreamTrack-getSettings.html and MediaStreamTrack-getCapabilities.html just need to be rebased.

2) The old version of getSupportedConstraints() only added values to the returned object if the constraint was supported. Given the definition of the MediaTrackSupportedConstraints dictionary (https://w3c.github.io/mediacapture-main/#idl-def-mediatracksupportedconstraints), that was wrong, and your new code is right. 
    MediaDevices-getSupportedConstraints.html needs to be changed to be ok with a few of the properties being unsupported.
Comment 5 Sam Weinig 2016-12-16 09:34:55 PST
Comment on attachment 297300 [details]
Patch

Other than the test changes needed, change looks great. r=me.
Comment 6 Darin Adler 2016-12-16 13:25:40 PST
Thanks for the review and the help understanding the status of the tests.

Looking forward to landing this and then removing WebCore::Dictionary and WebCore::ArrayValue. By the way, I just noticed that I don’t think any of this code was actually uses WebCore::Dictionary or WebCore::ArrayValue, but the patch does remove what probably is the last include of "Dictionary.h" outside those classes.
Comment 7 Darin Adler 2016-12-16 22:43:18 PST
Committed r209959: <http://trac.webkit.org/changeset/209959>