Removes the old createDataChannel and starts to use the new one with WebRTCDataChannelInit

Removes the old createDataChannel and starts to use the new one with WebRTCDataChannelInit.
This depends on the Chrome change https://codereview.chromium.org/16232004/

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=151876

[jiayl, 2013-06-04 00:44:32]

Could you take a look?
dglazkov: public/platform, source/core
tommyw: source/modules/mediastream

[Tommy Widenflycht, 2013-06-04 08:02:56]

This patch doesn't contain the definition of the WebRTCDataChannelInit, but it
seems to be a simple struct. I have not seen that pattern before but it makes
the code simpler. I'm adding abarth for reviewer since he is very familiar with
this code base and also know about the new WebKit API patterns.

https://codereview.chromium.org/16305017/diff/4001/Source/modules/mediastream...
File Source/modules/mediastream/RTCPeerConnection.cpp (right):

https://codereview.chromium.org/16305017/diff/4001/Source/modules/mediastream...
Source/modules/mediastream/RTCPeerConnection.cpp:64: 
Nit: extra empty line

https://codereview.chromium.org/16305017/diff/4001/Source/modules/mediastream...
Source/modules/mediastream/RTCPeerConnection.cpp:455: WTF::String buffer;
The WTF:: is not needed. And btw maybe call the variable protocol or
protocolString.

https://codereview.chromium.org/16305017/diff/4001/Tools/DumpRenderTree/chrom...
File
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.cpp
(right):

https://codereview.chromium.org/16305017/diff/4001/Tools/DumpRenderTree/chrom...
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.cpp:63:
, m_reliable(true)
Remove this line since it is initialized in the body

https://codereview.chromium.org/16305017/diff/4001/Tools/DumpRenderTree/chrom...
File Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.h
(right):

https://codereview.chromium.org/16305017/diff/4001/Tools/DumpRenderTree/chrom...
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.h:34: 
Remove the empty lines.

[abarth-chromium, 2013-06-04 08:08:02]

This CL seems to be missing WebRTCDataChannelInit but otherwise looks to be on a
good track.

https://codereview.chromium.org/16305017/diff/4001/Source/modules/mediastream...
File Source/modules/mediastream/RTCPeerConnection.cpp (right):

https://codereview.chromium.org/16305017/diff/4001/Source/modules/mediastream...
Source/modules/mediastream/RTCPeerConnection.cpp:449:
WebKit::WebRTCDataChannelInit init = { 0, true, true, -1, -1, "" };
Should WebRTCDataChannelInit have a default constructor that sets up these
values?

https://codereview.chromium.org/16305017/diff/4001/Tools/DumpRenderTree/chrom...
File
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCPeerConnectionHandler.cpp
(right):

https://codereview.chromium.org/16305017/diff/4001/Tools/DumpRenderTree/chrom...
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCPeerConnectionHandler.cpp:165:
WebRTCDataChannelInit init = { 0, true, true, -1, -1, "" };
The default constructor would help here too.

[jiayl, 2013-06-04 16:58:47]

PTAL. All comments are resolved in the new patch.
Note that WebRTCDataChannelInit was added in
https://codereview.chromium.org/16025003/

[Tommy Widenflycht, 2013-06-04 17:13:01]

lgtm

https://codereview.chromium.org/16305017/diff/14001/public/platform/WebRTCPee...
File public/platform/WebRTCPeerConnectionHandler.h (right):

https://codereview.chromium.org/16305017/diff/14001/public/platform/WebRTCPee...
public/platform/WebRTCPeerConnectionHandler.h:50: struct WebRTCDataChannelInit {
Optional: I would be happy if this lived in its own header file. It makes it
much easier to find and is highly recommended in Blink.

[abarth-chromium, 2013-06-04 17:22:48]

LGTM2

https://codereview.chromium.org/16305017/diff/14001/Tools/DumpRenderTree/chro...
File
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.cpp
(right):

https://codereview.chromium.org/16305017/diff/14001/Tools/DumpRenderTree/chro...
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.cpp:33:

You've got an extra blank line here.

https://codereview.chromium.org/16305017/diff/14001/public/platform/WebRTCPee...
File public/platform/WebRTCPeerConnectionHandler.h (right):

https://codereview.chromium.org/16305017/diff/14001/public/platform/WebRTCPee...
public/platform/WebRTCPeerConnectionHandler.h:50: struct WebRTCDataChannelInit {
On 2013/06/04 17:13:01, Tommy Widenflycht wrote:
> Optional: I would be happy if this lived in its own header file. It makes it
> much easier to find and is highly recommended in Blink.

+1

[jiayl, 2013-06-04 17:41:27]

Thanks! Comments resolved.

https://codereview.chromium.org/16305017/diff/14001/Tools/DumpRenderTree/chro...
File
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.cpp
(right):

https://codereview.chromium.org/16305017/diff/14001/Tools/DumpRenderTree/chro...
Tools/DumpRenderTree/chromium/TestRunner/src/MockWebRTCDataChannelHandler.cpp:33:

On 2013/06/04 17:22:48, abarth wrote:
> You've got an extra blank line here.

Done.

https://codereview.chromium.org/16305017/diff/14001/public/platform/WebRTCPee...
File public/platform/WebRTCPeerConnectionHandler.h (right):

https://codereview.chromium.org/16305017/diff/14001/public/platform/WebRTCPee...
public/platform/WebRTCPeerConnectionHandler.h:50: struct WebRTCDataChannelInit {
On 2013/06/04 17:22:48, abarth wrote:
> On 2013/06/04 17:13:01, Tommy Widenflycht wrote:
> > Optional: I would be happy if this lived in its own header file. It makes it
> > much easier to find and is highly recommended in Blink.
> 
> +1

Done.

[commit-bot: I haz the power, 2013-06-05 16:49:42]

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

[commit-bot: I haz the power, 2013-06-05 16:49:47]

Failed to apply patch for public/platform/WebRTCDataChannelInit.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         public/platform/WebRTCDataChannelInit.h
  Copied Source/WebKit/chromium/public/WebDOMResourceProgressEvent.h ->
public/platform/WebRTCDataChannelInit.h
  patching file public/platform/WebRTCDataChannelInit.h
  Hunk #1 FAILED at 28.
  1 out of 1 hunk FAILED -- saving rejects to file
public/platform/WebRTCDataChannelInit.h.rej

Patch:   NR 
Source/WebKit/chromium/public/WebDOMResourceProgressEvent.h->public/platform/WebRTCDataChannelInit.h
Index: public/platform/WebRTCDataChannelInit.h
diff --git a/Source/WebKit/chromium/public/WebDOMResourceProgressEvent.h
b/public/platform/WebRTCDataChannelInit.h
similarity index 78%
copy from Source/WebKit/chromium/public/WebDOMResourceProgressEvent.h
copy to public/platform/WebRTCDataChannelInit.h
index
b65d93c80e006d6e99c837e840eb38cb3a71055d..5b013783521486f6b4327dd3f561debb7fde2881
100644
--- a/Source/WebKit/chromium/public/WebDOMResourceProgressEvent.h
+++ b/public/platform/WebRTCDataChannelInit.h
@@ -28,21 +28,30 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef WebDOMResourceProgressEvent_h
-#define WebDOMResourceProgressEvent_h
+#ifndef WebRTCDataChannelInit_h
+#define WebRTCDataChannelInit_h
 
-#include "WebDOMProgressEvent.h"
+#include "WebString.h"
 
 namespace WebKit {
 
-class WebString;
-
-class WebDOMResourceProgressEvent : public WebDOMProgressEvent {
+struct WebRTCDataChannelInit {
 public:
-    WebDOMResourceProgressEvent() { }
-    WEBKIT_EXPORT WebDOMResourceProgressEvent(const WebString& type, bool
lengthIsComputable, unsigned long long loaded, unsigned long long total, const
WebString& url);
+    WebRTCDataChannelInit()
+        : id(0)
+        , ordered(true)
+        , negotiated(true)
+        , maxRetransmitTime(-1)
+        , maxRetransmits(-1) { }
+
+    unsigned short id;
+    bool ordered;
+    bool negotiated;
+    int maxRetransmitTime;
+    int maxRetransmits;
+    WebString protocol;
 };
 
 } // namespace WebKit
 
-#endif
+#endif // WebRTCDataChannelInit_h

[commit-bot: I haz the power, 2013-06-05 16:58:18]

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

[jiayl, 2013-06-05 17:40:27]

The new patch changed id to int to pass down the unset state. Also, ignore
values outside range of "unsigned short" for id, maxRetransmits and
maxRetransmitTime.

[commit-bot: I haz the power, 2013-06-05 17:40:46]

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

[commit-bot: I haz the power, 2013-06-05 18:15:08]

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

[commit-bot: I haz the power, 2013-06-05 19:16:19]

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

[Ami GONE FROM CHROMIUM, 2013-06-05 20:18:28]

drive-by FYI

https://codereview.chromium.org/16305017/diff/50001/public/platform/WebRTCDat...
File public/platform/WebRTCDataChannelInit.h (right):

https://codereview.chromium.org/16305017/diff/50001/public/platform/WebRTCDat...
public/platform/WebRTCDataChannelInit.h:51: int maxRetransmits;
IWBN for this to match the order in talk/app/webrtc/datachannelinterface.h
(there, maxRetransmits comes before maxRetransmitTime)

It seems natural to me to order both according to the WebIDL but maybe I'm
missing something:
http://dev.w3.org/2011/webrtc/editor/webrtc.html#idl-def-RTCDataChannelInit

[jiayl, 2013-06-05 21:25:56]

On 2013/06/05 20:18:28, Ami Fischman wrote:
> drive-by FYI
> 
>
https://codereview.chromium.org/16305017/diff/50001/public/platform/WebRTCDat...
> File public/platform/WebRTCDataChannelInit.h (right):
> 
>
https://codereview.chromium.org/16305017/diff/50001/public/platform/WebRTCDat...
> public/platform/WebRTCDataChannelInit.h:51: int maxRetransmits;
> IWBN for this to match the order in talk/app/webrtc/datachannelinterface.h
> (there, maxRetransmits comes before maxRetransmitTime)
> 
> It seems natural to me to order both according to the WebIDL but maybe I'm
> missing something:
> http://dev.w3.org/2011/webrtc/editor/webrtc.html#idl-def-RTCDataChannelInit

Initially I reordered the libjingle struct to avoid padding inside the struct,
before id was changed to int. 
I changed the webkit struct following the spec order now.

[commit-bot: I haz the power, 2013-06-05 21:26:08]

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

[commit-bot: I haz the power, 2013-06-05 21:34:36]

Retried try job too often on blink_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=blink_pres...

[commit-bot: I haz the power, 2013-06-05 21:37:17]

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

[commit-bot: I haz the power, 2013-06-06 02:13:00]

Change committed as 151876