Adds DataChannelInit param to createDataChannel.

Adds DataChannelInit param to createDataChannel.
It depends on the Blink side of the change: https://codereview.chromium.org/16025003/
The spec for the change is here: http://dev.w3.org/2011/webrtc/editor/webrtc.html#idl-def-RTCDataChannelInit

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=204081

[jiayl, 2013-05-30 17:31:15]

Could you take a look?

[jiayl, 2013-06-03 17:59:21]

PTAL.

[Ami GONE FROM CHROMIUM, 2013-06-03 18:27:31]

Might be worth pointing at the spec change that added this in the CL
description.

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.cc:580:
WebKit::WebRTCDataChannelHandler* RTCPeerConnectionHandler::createDataChannel(
nit: keep order in .h same as in .cc

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.cc:582: return
createDataChannel(label, false);
s/false/ordered && maxRetransmitTime == 0 && maxRetransmits == 0/ ?

(per http://dev.w3.org/2011/webrtc/editor/webrtc.html#rtcdatachannel: 
A RTCDataChannel can be configured to operate in different reliability modes. A
reliable channel ensures that the data is delivered at the other peer through
retransmissions. An unreliable channel is configured to either limit the number
of retransmissions (maxRetransmits) or set a time during which retransmissions
are allowed (maxRetransmitTime). These properties can not be used simultaneously
and an attempt to do so will result in an error. Not setting any of these
properties results in a reliable channel.
)

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.h (right):

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.h:133: const
WebKit::WebRTCDataChannelInit& init) OVERRIDE;
FYI if you remove OVERRIDE you can land CLs a bit more freely (don't have to
wait for as many rolls from chromium into blink and from blink into chromium).
Just remember to add back any OVERRIDEs that apply when you're done.

(chromium has a clang plugin that checks that all virtual methods specify
OVERRIDE, but there's an exception for WebKit-implementing things for exactly
this reason)

[Ami GONE FROM CHROMIUM, 2013-06-03 18:27:56]

Oh, and LGTM % nit & you doing what you like about the other comments.

[jiayl, 2013-06-03 18:52:58]

Thanks for the review! Another patch is up.

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.cc (right):

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.cc:580:
WebKit::WebRTCDataChannelHandler* RTCPeerConnectionHandler::createDataChannel(
On 2013/06/03 18:27:31, Ami Fischman wrote:
> nit: keep order in .h same as in .cc

Done.

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.cc:582: return
createDataChannel(label, false);
Libjingle rejects to create reliable RTP data channels and this will break
existing demos. I added a TODO to update this to the correct logic once
Libjingle can handle it.

On 2013/06/03 18:27:31, Ami Fischman wrote:
> s/false/ordered && maxRetransmitTime == 0 && maxRetransmits == 0/ ?
> 
> (per http://dev.w3.org/2011/webrtc/editor/webrtc.html#rtcdatachannel: 
> A RTCDataChannel can be configured to operate in different reliability modes.
A
> reliable channel ensures that the data is delivered at the other peer through
> retransmissions. An unreliable channel is configured to either limit the
number
> of retransmissions (maxRetransmits) or set a time during which retransmissions
> are allowed (maxRetransmitTime). These properties can not be used
simultaneously
> and an attempt to do so will result in an error. Not setting any of these
> properties results in a reliable channel.
> )

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.h (right):

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.h:133: const
WebKit::WebRTCDataChannelInit& init) OVERRIDE;
Hmm, but I still depend on the definition of DataChannelInit in the same Blink
CL.
On 2013/06/03 18:27:31, Ami Fischman wrote:
> FYI if you remove OVERRIDE you can land CLs a bit more freely (don't have to
> wait for as many rolls from chromium into blink and from blink into chromium).
> Just remember to add back any OVERRIDEs that apply when you're done.
> 
> (chromium has a clang plugin that checks that all virtual methods specify
> OVERRIDE, but there's an exception for WebKit-implementing things for exactly
> this reason)

[Ami GONE FROM CHROMIUM, 2013-06-03 19:32:19]

still LGTM

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.h (right):

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.h:133: const
WebKit::WebRTCDataChannelInit& init) OVERRIDE;
On 2013/06/03 18:52:58, jiayl wrote:
> Hmm, but I still depend on the definition of DataChannelInit in the same Blink
CL.

True; this would just allow you to remove the old function in the same blink CL
that converts to using the new function.

[jiayl, 2013-06-03 19:54:51]

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
File content/renderer/media/rtc_peer_connection_handler.h (right):

https://codereview.chromium.org/16232004/diff/1/content/renderer/media/rtc_pe...
content/renderer/media/rtc_peer_connection_handler.h:133: const
WebKit::WebRTCDataChannelInit& init) OVERRIDE;
I see. Makes sense!
On 2013/06/03 19:32:19, Ami Fischman wrote:
> On 2013/06/03 18:52:58, jiayl wrote:
> > Hmm, but I still depend on the definition of DataChannelInit in the same
Blink
> CL.
> 
> True; this would just allow you to remove the old function in the same blink
CL
> that converts to using the new function.

[commit-bot: I haz the power, 2013-06-04 16:45:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jiayl@chromium.org/16232004/8003

[commit-bot: I haz the power, 2013-06-04 18:01:14]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-06-04 18:14:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jiayl@chromium.org/16232004/8003

[commit-bot: I haz the power, 2013-06-04 23:54:03]

Change committed as 204081