RTCPeerConnection: match createDataChannel signature in spec

RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.

The spec has been changed to remove [TreatNullAs=EmptyString]:
https://github.com/w3c/webrtc-pc/pull/1131

Review-Url: https://codereview.chromium.org/2501163002
Cr-Commit-Position: refs/heads/master@{#466606}
Committed: https://chromium.googlesource.com/chromium/src/+/517dfbd040fbab1d0175db908e38e91f96388e9f

[foolip, 2016-11-14 20:39:50]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-14 20:42:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-14 22:47:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-14 22:47:34]

Dry run: This issue passed the CQ dry run.

[foolip, 2017-04-17 13:53:38]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-17 13:53:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-17 15:28:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-17 15:28:40]

Dry run: This issue passed the CQ dry run.

[foolip, 2017-04-17 17:54:37]

Description was changed from

==========
Convert RTCPeerConnection#createDataChannel dict to IDL

BUG=460722
==========

to

==========
Convert RTCPeerConnection#createDataChannel dict to IDL
==========

[foolip, 2017-04-18 10:47:24]

Description was changed from

==========
Convert RTCPeerConnection#createDataChannel dict to IDL
==========

to

==========
RTCPeerConnection: match createDataChannel signature in spec
==========

[foolip, 2017-04-18 10:47:42]

Description was changed from

==========
RTCPeerConnection: match createDataChannel signature in spec
==========

to

==========
RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.
==========

[foolip, 2017-04-18 12:49:53]

The CQ bit was checked by foolip@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-18 12:50:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[foolip, 2017-04-18 12:51:25]

Description was changed from

==========
RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.
==========

to

==========
RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.

The spec will soon match this w.r.t label:
https://github.com/w3c/webrtc-pc/pull/1131
==========

[foolip, 2017-04-18 13:29:39]

foolip@chromium.org changed reviewers:
+ guidou@chromium.org, hbos@chromium.org

[foolip, 2017-04-18 13:29:40]

PTAL. I guess I'll wait for the spec change to be merged, unless they only do
that on a weekly basis or something.

[Guido Urdaneta, 2017-04-18 13:47:36]

lgtm

[commit-bot: I haz the power, 2017-04-18 15:03:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-18 15:03:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hbos_chromium, 2017-04-19 09:15:33]

lgtm

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html
(right):

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html:32:
// Initial id value is not defined.
The id was not specified and as such was generated by the client? Saying
"Initial id value is not defined" is confusing when the line below makes sure
channel.id is in fact defined.

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html:58:
// to false while undefined is converted to true.
(Oh man, I misread this comment not realizing you were talking about default
values. It had me confused, questioning javascript conversion rules and my very
sanity. undefined == true?! Doing console.log things to sooth my mind, minutes
fading away... I re-read it and it was clear as day.)

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html:115:
assert_throws(new TypeError, () => pc.createDataChannel('', { id: 65535 }));
Can you add a comment saying 65534 is the maximum allowed even though 65535 is
in the unsigned short range, this looked like a mistake to me until I saw the
spec.

[foolip, 2017-04-20 11:52:03]

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html
(right):

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html:32:
// Initial id value is not defined.
On 2017/04/19 09:15:33, hbos_chromium wrote:
> The id was not specified and as such was generated by the client? Saying
> "Initial id value is not defined" is confusing when the line below makes sure
> channel.id is in fact defined.

I filed https://github.com/w3c/webrtc-pc/issues/1132 for that. The problem is
that the *value* isn't defined, certainly nothing in the spec suggests 65535
which is what the value is for us.

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html:58:
// to false while undefined is converted to true.
On 2017/04/19 09:15:33, hbos_chromium wrote:
> (Oh man, I misread this comment not realizing you were talking about default
> values. It had me confused, questioning javascript conversion rules and my
very
> sanity. undefined == true?! Doing console.log things to sooth my mind, minutes
> fading away... I re-read it and it was clear as day.)

Sorry to almost have ruined your sanity :)

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html:115:
assert_throws(new TypeError, () => pc.createDataChannel('', { id: 65535 }));
On 2017/04/19 09:15:33, hbos_chromium wrote:
> Can you add a comment saying 65534 is the maximum allowed even though 65535 is
> in the unsigned short range, this looked like a mistake to me until I saw the
> spec.

Ah, will do.

[foolip, 2017-04-24 07:10:11]

Description was changed from

==========
RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.

The spec will soon match this w.r.t label:
https://github.com/w3c/webrtc-pc/pull/1131
==========

to

==========
RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.

The spec has been changed to remove [TreatNullAs=EmptyString]:
https://github.com/w3c/webrtc-pc/pull/1131
==========

[foolip, 2017-04-24 07:40:03]

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html
(right):

https://codereview.chromium.org/2501163002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/external/wpt/webrtc/RTCPeerConnection-createDataChannel.html:115:
assert_throws(new TypeError, () => pc.createDataChannel('', { id: 65535 }));
On 2017/04/20 11:52:03, foolip_UTC7 wrote:
> On 2017/04/19 09:15:33, hbos_chromium wrote:
> > Can you add a comment saying 65534 is the maximum allowed even though 65535
is
> > in the unsigned short range, this looked like a mistake to me until I saw
the
> > spec.
> 
> Ah, will do.

Wrote a comment and tested more numbers, now landing.

[foolip, 2017-04-24 07:40:10]

The CQ bit was checked by foolip@chromium.org

[foolip, 2017-04-24 07:40:10]

The patchset sent to the CQ was uploaded after l-g-t-m from guidou@chromium.org,
hbos@chromium.org
Link to the patchset: https://codereview.chromium.org/2501163002/#ps100001
(title: "expand id tests")

[commit-bot: I haz the power, 2017-04-24 07:40:27]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-24 09:14:55]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1493019610227800,
"parent_rev": "4baa4206fac22a91b3c76a429143fc061017f318", "commit_rev":
"517dfbd040fbab1d0175db908e38e91f96388e9f"}

[commit-bot: I haz the power, 2017-04-24 09:15:05]

Description was changed from

==========
RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.

The spec has been changed to remove [TreatNullAs=EmptyString]:
https://github.com/w3c/webrtc-pc/pull/1131
==========

to

==========
RTCPeerConnection: match createDataChannel signature in spec

This changes both the first argument into a non-nullable USVString and
the second argument into a RTCDataChannelInit dictionary.

The spec has been changed to remove [TreatNullAs=EmptyString]:
https://github.com/w3c/webrtc-pc/pull/1131

Review-Url: https://codereview.chromium.org/2501163002
Cr-Commit-Position: refs/heads/master@{#466606}
Committed:
https://chromium.googlesource.com/chromium/src/+/517dfbd040fbab1d0175db908e38...
==========

[commit-bot: I haz the power, 2017-04-24 09:15:07]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/517dfbd040fbab1d0175db908e38...