DataChannels created asynchronously never open and are unusable
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Dec 2018 03:43:38 +0000 (03:43 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Dec 2018 03:43:38 +0000 (03:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192566

Reviewed by Eric Carlson.

Source/WebCore:

For every new data channel (remote or local), we should check the underlying backend state.
This allows firing events if needed.
We were not always doing that which was prohibiting sending some open
events for data channels created after the SCTP connection is set up.

Covered by updated test.

* Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp:
(WebCore::LibWebRTCDataChannelHandler::channelEvent):
(WebCore::LibWebRTCDataChannelHandler::setClient):
(WebCore::LibWebRTCDataChannelHandler::OnStateChange):
(WebCore::LibWebRTCDataChannelHandler::checkState):
* Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h:

LayoutTests:

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239064 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webrtc/datachannel/basic-expected.txt
LayoutTests/webrtc/datachannel/basic.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h

index 73e014f..fecbcaf 100644 (file)
@@ -1,3 +1,13 @@
+2018-12-10  Youenn Fablet  <youenn@apple.com>
+
+        DataChannels created asynchronously never open and are unusable
+        https://bugs.webkit.org/show_bug.cgi?id=192566
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/datachannel/basic-expected.txt:
+        * webrtc/datachannel/basic.html:
+
 2018-12-10  Rob Buis  <rbuis@igalia.com>
 
         XMLHttpRequest removes spaces from content-types before processing
index 0eb226e..b8ebe43 100644 (file)
@@ -3,4 +3,5 @@ PASS Basic data channel exchange from offerer to receiver
 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 
+PASS Create a second channel asynchronously and send messages 
 
index fa315d0..fb9f4db 100644 (file)
@@ -126,6 +126,40 @@ promise_test((test) => {
     });
 }, "Basic data channel exchange from offerer to receiver");
 
+promise_test(async (test) => {
+    await new Promise((resolve, reject) => {
+        finishTest = resolve;
+        createConnections((localConnection) => {
+            localChannel = localConnection.createDataChannel('sendDataChannel');
+        }, (remoteConnection) => {
+            remoteConnection.ondatachannel = resolve;
+        });
+        setTimeout(() => { reject("Test step 1 timed out"); }, 5000);
+    });
+    await waitFor(50);
+    let waitForLocalChannelOpening = new Promise((resolve) => {
+        localChannel = localConnection.createDataChannel('sendDataChannel2');
+        localChannel.onopen = resolve;
+    });
+
+    let waitForRemoteChannel = new Promise((resolve) => {
+        remoteConnection.ondatachannel = (event) => {
+            remoteChannel = event.channel;
+            resolve();
+        };
+    });
+
+    await Promise.all([waitForLocalChannelOpening, waitForRemoteChannel]);
+
+    counter = 0;
+    await new Promise((resolve, reject) => {
+        finishTest = resolve;
+        remoteChannel.onmessage = receiveMessages;
+        sendMessages(localChannel);
+        setTimeout(() => { reject("Test step 2 timed out"); }, 5000);
+    });
+}, "Create a second channel asynchronously and send messages");
+
     </script>
   </body>
 </html>
index ce61833..a7fb202 100644 (file)
@@ -1,3 +1,24 @@
+2018-12-10  Youenn Fablet  <youenn@apple.com>
+
+        DataChannels created asynchronously never open and are unusable
+        https://bugs.webkit.org/show_bug.cgi?id=192566
+
+        Reviewed by Eric Carlson.
+
+        For every new data channel (remote or local), we should check the underlying backend state.
+        This allows firing events if needed.
+        We were not always doing that which was prohibiting sending some open
+        events for data channels created after the SCTP connection is set up.
+
+        Covered by updated test.
+
+        * Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp:
+        (WebCore::LibWebRTCDataChannelHandler::channelEvent):
+        (WebCore::LibWebRTCDataChannelHandler::setClient):
+        (WebCore::LibWebRTCDataChannelHandler::OnStateChange):
+        (WebCore::LibWebRTCDataChannelHandler::checkState):
+        * Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h:
+
 2018-12-10  Ryosuke Niwa  <rniwa@webkit.org>
 
         Use WeakPtr to refer to VTTCue in VTTCueBox
index bcfe2a4..aaa4000 100644 (file)
@@ -69,19 +69,9 @@ Ref<RTCDataChannelEvent> LibWebRTCDataChannelHandler::channelEvent(ScriptExecuti
     init.negotiated = dataChannel->negotiated();
     init.id = dataChannel->id();
 
-    bool isOpened = dataChannel->state() == webrtc::DataChannelInterface::kOpen;
-
     auto handler =  std::make_unique<LibWebRTCDataChannelHandler>(WTFMove(dataChannel));
     auto channel = RTCDataChannel::create(context, WTFMove(handler), fromStdString(label), WTFMove(init));
 
-    if (isOpened) {
-        callOnMainThread([channel = channel.copyRef()] {
-            // FIXME: We should be able to write channel->didChangeReadyState(...)
-            RTCDataChannelHandlerClient& client = channel.get();
-            client.didChangeReadyState(RTCDataChannelState::Open);
-        });
-    }
-
     return RTCDataChannelEvent::create(eventNames().datachannelEvent, Event::CanBubble::No, Event::IsCancelable::No, WTFMove(channel));
 }
 
@@ -96,6 +86,7 @@ void LibWebRTCDataChannelHandler::setClient(RTCDataChannelHandlerClient& client)
     ASSERT(!m_client);
     m_client = &client;
     m_channel->RegisterObserver(this);
+    checkState();
 }
 
 bool LibWebRTCDataChannelHandler::sendStringData(const String& text)
@@ -122,7 +113,11 @@ void LibWebRTCDataChannelHandler::OnStateChange()
 {
     if (!m_client)
         return;
+    checkState();
+}
 
+void LibWebRTCDataChannelHandler::checkState()
+{
     RTCDataChannelState state;
     switch (m_channel->state()) {
     case webrtc::DataChannelInterface::kConnecting:
index 00552bf..e801019 100644 (file)
@@ -57,6 +57,7 @@ public:
 private:
     // RTCDataChannelHandler API
     void setClient(RTCDataChannelHandlerClient&) final;
+    void checkState();
     bool sendStringData(const String&) final;
     bool sendRawData(const char*, size_t) final;
     void close() final;