Title: [214421] trunk
Revision
214421
Author
[email protected]
Date
2017-03-27 11:13:19 -0700 (Mon, 27 Mar 2017)

Log Message

Tighten RTCDatachannel creation and parameter getters
https://bugs.webkit.org/show_bug.cgi?id=170081

Patch by Youenn Fablet <[email protected]> on 2017-03-27
Reviewed by Eric Carlson.

Source/WebCore:

Covered by updated tests.

Adding some parameter checks when creating data channels.
Making some getters nullable as per the spec.

* Modules/mediastream/RTCDataChannel.h:
* Modules/mediastream/RTCDataChannel.idl:
* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::createDataChannel):
* Modules/mediastream/RTCPeerConnection.idl:
* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::LibWebRTCMediaEndpoint::createDataChannel):
(WebCore::LibWebRTCMediaEndpoint::addDataChannel):
* platform/mediastream/RTCDataChannelHandler.h:
(): Deleted.

LayoutTests:

* webrtc/datachannel/basic-expected.txt:
* webrtc/datachannel/basic.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (214420 => 214421)


--- trunk/LayoutTests/ChangeLog	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/LayoutTests/ChangeLog	2017-03-27 18:13:19 UTC (rev 214421)
@@ -1,5 +1,15 @@
 2017-03-27  Youenn Fablet  <[email protected]>
 
+        Tighten RTCDatachannel creation and parameter getters
+        https://bugs.webkit.org/show_bug.cgi?id=170081
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/datachannel/basic-expected.txt:
+        * webrtc/datachannel/basic.html:
+
+2017-03-27  Youenn Fablet  <[email protected]>
+
         Add support for RTCRtpReceiver/RTCRtpSender getParameters
         https://bugs.webkit.org/show_bug.cgi?id=170057
 

Modified: trunk/LayoutTests/webrtc/datachannel/basic-expected.txt (214420 => 214421)


--- trunk/LayoutTests/webrtc/datachannel/basic-expected.txt	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/LayoutTests/webrtc/datachannel/basic-expected.txt	2017-03-27 18:13:19 UTC (rev 214421)
@@ -3,4 +3,15 @@
 PASS Basic data channel exchange from receiver to offerer 
 PASS Basic data channel exchange from offerer to receiver using UDP only 
 PASS Basic data channel exchange from offerer to receiver using TCP only 
+PASS Basic data channel exchange from offerer to receiver 
+PASS Creating data channel with very long label 
+PASS Creating data channel after closing the connection 
+PASS Wrong data channel init: long protocol 
+PASS Wrong data channel init: long id 
+PASS Wrong data channel init: maxPacketLifeTime and maxRetransmits 
+PASS Right data channel init: ordered init 
+PASS Right data channel init: unordered init 
+PASS Right data channel init: default ordered init with id 
+PASS Right data channel init: no init 
+PASS Right data channel init: null 
 

Modified: trunk/LayoutTests/webrtc/datachannel/basic.html (214420 => 214421)


--- trunk/LayoutTests/webrtc/datachannel/basic.html	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/LayoutTests/webrtc/datachannel/basic.html	2017-03-27 18:13:19 UTC (rev 214421)
@@ -118,6 +118,107 @@
     });
 }, "Basic data channel exchange from offerer to receiver using TCP only");
 
+promise_test((test) => {
+    counter = 0;
+    return new Promise((resolve, reject) => {
+        if (window.internals)
+            internals.useMockRTCPeerConnectionFactory("TwoRealPeerConnections");
+
+        var checkDataChannelOptions = (channel, init) => {
+            assert_equals(channel.ordered, init.ordered, "ordered");
+            assert_equals(channel.maxPacketLifeTime, init.maxPacketLifeTime, "maxPacketLifeTime");
+            assert_equals(channel.maxRetransmitTime, init.maxRetransmitTime, "maxRetransmitTime");
+            assert_equals(channel.maxRetransmits, init.maxRetransmits, "maxRetransmits");
+            assert_equals(channel.protocol, init.protocol, "protocol");
+            assert_equals(channel.negotiated, init.negotiated, "negotiated");
+            assert_equals(channel.id, init.id, "id");
+        };
+
+        finishTest = resolve;
+        createConnections((localConnection) => {
+            var init = { ordered: true, maxPacketLifeTime: 10, maxRetransmitTime: 11, protocol: "whatever", negotiated: false, id: "id" };
+            localChannel = localConnection.createDataChannel('sendDataChannel', init);
+            localChannel._onopen_ = () => { sendMessages(localChannel) };
+        }, (remoteConnection) => {
+            remoteConnection._ondatachannel_ = (event) => {
+                remoteChannel = event.channel;
+                remoteChannel._onmessage_ = receiveMessages;
+            };
+        });
+    });
+}, "Basic data channel exchange from offerer to receiver");
+
+var longString = "abcdefgh";
+for (var cptr = 0; cptr < 16; ++cptr)
+    longString += longString;
+
+test(() => {
+    if (window.internals)
+        internals.useMockRTCPeerConnectionFactory("OneRealPeerConnections");
+    var pc = new RTCPeerConnection();
+    assert_throws(new TypeError, () => { pc.createDataChannel(longString); });
+}, "Creating data channel with very long label");
+
+test(() => {
+    if (window.internals)
+        internals.useMockRTCPeerConnectionFactory("OneRealPeerConnections");
+    var pc = new RTCPeerConnection();
+    pc.close();
+    assert_throws("InvalidStateError", () => { pc.createDataChannel('sendDataChannel'); });
+}, "Creating data channel after closing the connection");
+
+function testWrongDataChannelInit(init, title)
+{
+    return test(() => {
+        if (window.internals)
+            internals.useMockRTCPeerConnectionFactory("OneRealPeerConnections");
+        var pc = new RTCPeerConnection();
+        assert_throws(new TypeError, () => { pc.createDataChannel('sendDataChannel', init); });
+    }, "Wrong data channel init: " + title);
+}
+
+function testRightDataChannelInit(init, title)
+{
+    return test(() => {
+        if (window.internals)
+            internals.useMockRTCPeerConnectionFactory("OneRealPeerConnections");
+        var pc = new RTCPeerConnection();
+        channel = pc.createDataChannel('sendDataChannel', init);
+
+        if (!init)
+            init = { };
+        if (init.ordered === undefined)
+            init.ordered = true;
+        if (init.maxPacketLifeTime === undefined)
+            init.maxPacketLifeTime = null;
+        if (init.maxRetransmits === undefined)
+            init.maxRetransmits = null;
+        if (init.protocol === undefined)
+            init.protocol = "";
+        if (init.negotiated === undefined)
+            init.negotiated = false;
+        if (init.id === undefined)
+            init.id = null;
+
+        assert_equals(channel.ordered, init.ordered, "ordered");
+        assert_equals(channel.maxPacketLifeTime, init.maxPacketLifeTime, "maxPacketLifeTime");
+        assert_equals(channel.maxRetransmitTime, init.maxRetransmitTime, "maxRetransmitTime");
+        assert_equals(channel.maxRetransmits, init.maxRetransmits, "maxRetransmits");
+        assert_equals(channel.protocol, init.protocol, "protocol");
+        assert_equals(channel.negotiated, init.negotiated, "negotiated");
+        assert_equals(channel.id, init.id, "id");
+     }, "Right data channel init: " + title);
+}
+
+testWrongDataChannelInit({negotiated: false, protocol: longString}, "long protocol");
+testWrongDataChannelInit({id: 65535}, "long id");
+testWrongDataChannelInit({maxPacketLifeTime: 1, maxRetransmits: 1}, "maxPacketLifeTime and maxRetransmits");
+
+testRightDataChannelInit({ordered: true, maxRetransmit: 11, protocol: "whatever", negotiated: false, id: 1 }, "ordered init");
+testRightDataChannelInit({ordered: false, maxPacketLifeTime: 10, protocol: "whatever", negotiated: false, id: 2 }, "unordered init");
+testRightDataChannelInit({protocol: "whatever", negotiated: false, id: 123 }, "default ordered init with id");
+testRightDataChannelInit(undefined, "no init");
+testRightDataChannelInit(null, "null");
     </script>
   </body>
 </html>

Modified: trunk/Source/WebCore/ChangeLog (214420 => 214421)


--- trunk/Source/WebCore/ChangeLog	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/Source/WebCore/ChangeLog	2017-03-27 18:13:19 UTC (rev 214421)
@@ -1,5 +1,28 @@
 2017-03-27  Youenn Fablet  <[email protected]>
 
+        Tighten RTCDatachannel creation and parameter getters
+        https://bugs.webkit.org/show_bug.cgi?id=170081
+
+        Reviewed by Eric Carlson.
+
+        Covered by updated tests.
+
+        Adding some parameter checks when creating data channels.
+        Making some getters nullable as per the spec.
+
+        * Modules/mediastream/RTCDataChannel.h:
+        * Modules/mediastream/RTCDataChannel.idl:
+        * Modules/mediastream/RTCPeerConnection.cpp:
+        (WebCore::RTCPeerConnection::createDataChannel):
+        * Modules/mediastream/RTCPeerConnection.idl:
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+        (WebCore::LibWebRTCMediaEndpoint::createDataChannel):
+        (WebCore::LibWebRTCMediaEndpoint::addDataChannel):
+        * platform/mediastream/RTCDataChannelHandler.h:
+        (): Deleted.
+
+2017-03-27  Youenn Fablet  <[email protected]>
+
         Add support for RTCRtpReceiver/RTCRtpSender getParameters
         https://bugs.webkit.org/show_bug.cgi?id=170057
 

Modified: trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.h (214420 => 214421)


--- trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.h	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.h	2017-03-27 18:13:19 UTC (rev 214421)
@@ -48,12 +48,12 @@
 public:
     static Ref<RTCDataChannel> create(ScriptExecutionContext&, std::unique_ptr<RTCDataChannelHandler>&&, String&&, RTCDataChannelInit&&);
 
-    bool ordered() const { return m_options.ordered; }
-    unsigned short maxRetransmitTime() const { return m_options.maxRetransmitTime; }
-    unsigned short maxRetransmits() const { return m_options.maxRetransmits; }
+    bool ordered() const { return *m_options.ordered; }
+    std::optional<unsigned short> maxPacketLifeTime() const { return m_options.maxPacketLifeTime; }
+    std::optional<unsigned short> maxRetransmits() const { return m_options.maxRetransmits; }
     String protocol() const { return m_options.protocol; }
-    bool negotiated() const { return m_options.negotiated; };
-    unsigned short id() const { return m_options.id; };
+    bool negotiated() const { return *m_options.negotiated; };
+    std::optional<unsigned short> id() const { return m_options.id; };
 
     String label() const { return m_label; }
     const AtomicString& readyState() const;

Modified: trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.idl (214420 => 214421)


--- trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.idl	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.idl	2017-03-27 18:13:19 UTC (rev 214421)
@@ -28,11 +28,11 @@
 ] interface RTCDataChannel : EventTarget {
     readonly attribute DOMString label;
     readonly attribute boolean ordered;
-    readonly attribute unsigned short maxRetransmitTime;
-    readonly attribute unsigned short maxRetransmits;
+    readonly attribute unsigned short? maxPacketLifeTime;
+    readonly attribute unsigned short? maxRetransmits;
     readonly attribute DOMString protocol;
     readonly attribute boolean negotiated;
-    readonly attribute unsigned short id;
+    readonly attribute unsigned short? id;
     readonly attribute DOMString readyState;
     readonly attribute unsigned long bufferedAmount;
 

Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp (214420 => 214421)


--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2017-03-27 18:13:19 UTC (rev 214421)
@@ -332,7 +332,15 @@
     if (m_signalingState == RTCSignalingState::Closed)
         return Exception { INVALID_STATE_ERR };
 
-    // FIXME: Check options
+    if (options.negotiated && !options.negotiated.value() && (label.length() > 65535 || options.protocol.length() > 65535))
+        return Exception { TypeError };
+
+    if (options.maxPacketLifeTime && options.maxRetransmits)
+        return Exception { TypeError };
+
+    if (options.id && options.id.value() > 65534)
+        return Exception { TypeError };
+    
     auto channelHandler = m_backend->createDataChannelHandler(label, options);
     if (!channelHandler)
         return Exception { NOT_SUPPORTED_ERR };

Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.idl (214420 => 214421)


--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.idl	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.idl	2017-03-27 18:13:19 UTC (rev 214421)
@@ -38,8 +38,7 @@
     EnabledAtRuntime=PeerConnection
 ] dictionary RTCDataChannelInit {
     boolean ordered = true;
-    // FIXME 169644: rename to maxPacketLifeTime;
-    unsigned short maxRetransmitTime;
+    unsigned short maxPacketLifeTime;
     unsigned short maxRetransmits;
     USVString protocol = "";
     boolean negotiated = false;

Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp (214420 => 214421)


--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp	2017-03-27 18:13:19 UTC (rev 214421)
@@ -562,12 +562,17 @@
 std::unique_ptr<RTCDataChannelHandler> LibWebRTCMediaEndpoint::createDataChannel(const String& label, const RTCDataChannelInit& options)
 {
     webrtc::DataChannelInit init;
-    init.ordered = options.ordered;
-    init.maxRetransmitTime = options.maxRetransmitTime;
-    init.maxRetransmits = options.maxRetransmits;
+    if (options.ordered)
+        init.ordered = *options.ordered;
+    if (options.maxPacketLifeTime)
+        init.maxRetransmitTime = *options.maxPacketLifeTime;
+    if (options.maxRetransmits)
+        init.maxRetransmits = *options.maxRetransmits;
     init.protocol = options.protocol.utf8().data();
-    init.negotiated = options.negotiated;
-    init.id = options.id;
+    if (options.negotiated)
+        init.negotiated = *options.negotiated;
+    if (options.id)
+        init.id = *options.id;
 
     return std::make_unique<LibWebRTCDataChannelHandler>(m_backend->CreateDataChannel(label.utf8().data(), &init));
 }
@@ -579,7 +584,7 @@
 
     RTCDataChannelInit init;
     init.ordered = dataChannel->ordered();
-    init.maxRetransmitTime = dataChannel->maxRetransmitTime();
+    init.maxPacketLifeTime = dataChannel->maxRetransmitTime();
     init.maxRetransmits = dataChannel->maxRetransmits();
     init.protocol = String(protocol.data(), protocol.size());
     init.negotiated = dataChannel->negotiated();

Modified: trunk/Source/WebCore/platform/mediastream/RTCDataChannelHandler.h (214420 => 214421)


--- trunk/Source/WebCore/platform/mediastream/RTCDataChannelHandler.h	2017-03-27 18:10:45 UTC (rev 214420)
+++ trunk/Source/WebCore/platform/mediastream/RTCDataChannelHandler.h	2017-03-27 18:13:19 UTC (rev 214421)
@@ -26,17 +26,18 @@
 
 #if ENABLE(WEB_RTC)
 
+#include <wtf/Optional.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebCore {
 
 struct RTCDataChannelInit {
-    bool ordered { true };
-    int maxRetransmitTime { -1 };
-    int maxRetransmits { -1 };
+    std::optional<bool> ordered;
+    std::optional<unsigned short> maxPacketLifeTime;
+    std::optional<unsigned short> maxRetransmits;
     String protocol;
-    bool negotiated { false };
-    int id { -1 };
+    std::optional<bool> negotiated;
+    std::optional<unsigned short> id;
 };
 
 class RTCDataChannelHandlerClient;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to