Bug 167590 - RTCPeerConnection methods can take dictionaries as input
Summary: RTCPeerConnection methods can take dictionaries as input
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-30 08:58 PST by youenn fablet
Modified: 2017-01-31 09:54 PST (History)
6 users (show)

See Also:


Attachments
Patch (22.76 KB, patch)
2017-01-30 09:09 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (935.93 KB, application/zip)
2017-01-30 10:14 PST, Build Bot
no flags Details
Patch (27.99 KB, patch)
2017-01-30 17:08 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (27.98 KB, patch)
2017-01-31 09:15 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-01-30 08:58:16 PST
Currently they take objects.
This is the case for setLocalDescription, setRemoteDescription, addIceCandidate
Comment 1 youenn fablet 2017-01-30 09:09:42 PST
Created attachment 300115 [details]
Patch
Comment 2 Build Bot 2017-01-30 10:14:05 PST
Comment on attachment 300115 [details]
Patch

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

New failing tests:
fast/mediastream/RTCPeerConnection-js-built-ins-check-this.html
Comment 3 Build Bot 2017-01-30 10:14:09 PST
Created attachment 300120 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 4 Sam Weinig 2017-01-30 11:46:29 PST
Comment on attachment 300115 [details]
Patch

It seems really unfortunate that all this has to be written manually. Can this be converted to use WebIDL where you would be able to get this for free?
Comment 5 youenn fablet 2017-01-30 11:59:16 PST
Ideally, we would like to remove the callback legacy API.

If the callback legacy API was removed, the binding generator should do (and does probably already) handle that.

I prefer doing this handling in JS than adding the complexity in the binding generator.
It would be easier to remove it in the future if we can.
Comment 6 youenn fablet 2017-01-30 17:08:51 PST
Created attachment 300163 [details]
Patch
Comment 7 Alex Christensen 2017-01-30 21:06:01 PST
Comment on attachment 300163 [details]
Patch

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

> Source/WebCore/Modules/mediastream/RTCPeerConnectionInternals.js:78
> +        else if (objectInfo.maybeDictionary) {

What if this condition is false?
Comment 8 youenn fablet 2017-01-31 08:37:53 PST
Comment on attachment 300163 [details]
Patch

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

>> Source/WebCore/Modules/mediastream/RTCPeerConnectionInternals.js:78
>> +        else if (objectInfo.maybeDictionary) {
> 
> What if this condition is false?

Then objectArgOk is false and we reject the promise.
Comment 9 WebKit Commit Bot 2017-01-31 08:58:44 PST
Comment on attachment 300163 [details]
Patch

Rejecting attachment 300163 [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', 'apply-attachment', '--no-update', '--non-interactive', 300163, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
hing file LayoutTests/fast/mediastream/RTCPeerConnection-setRemoteDescription-offer-expected.txt
patching file LayoutTests/fast/mediastream/RTCPeerConnection-setRemoteDescription-offer.html
patching file LayoutTests/webrtc/rtcpeerconnection-error-messages-expected.txt
patching file LayoutTests/webrtc/rtcpeerconnection-error-messages.html

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

Full output: http://webkit-queues.webkit.org/results/2979940
Comment 10 youenn fablet 2017-01-31 09:15:15 PST
Created attachment 300223 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2017-01-31 09:53:58 PST
Comment on attachment 300223 [details]
Patch for landing

Clearing flags on attachment: 300223

Committed r211436: <http://trac.webkit.org/changeset/211436>
Comment 12 WebKit Commit Bot 2017-01-31 09:54:03 PST
All reviewed patches have been landed.  Closing bug.