Title: [282637] trunk
Revision
282637
Author
[email protected]
Date
2021-09-17 00:02:29 -0700 (Fri, 17 Sep 2021)

Log Message

Compute RTCPeerConnection.connectionState as per https://w3c.github.io/webrtc-pc/#rtcpeerconnectionstate-enum
https://bugs.webkit.org/show_bug.cgi?id=230341

Reviewed by Eric Carlson.

LayoutTests/imported/w3c:

* web-platform-tests/webrtc/RTCPeerConnection-connectionState.https-expected.txt:

Source/WebCore:

We should compute connection states according ICE and DTLS transport state, as per spec.
Given we compute the state from ICE and DTLS states, we now make sure to update connection state whenever DTLS state changes.
Make also sure to not fire events in case peer connection is closed, as per spec.

Covered by existing and rebased tests.

* Modules/mediastream/RTCDtlsTransport.cpp:
(WebCore::RTCDtlsTransport::onStateChanged):
* Modules/mediastream/RTCIceTransport.h:
(WebCore::RTCIceTransport::connection const):
* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::close):
(WebCore::RTCPeerConnection::computeConnectionState):
(WebCore::RTCPeerConnection::processIceTransportStateChange):
* Modules/mediastream/RTCPeerConnection.h:

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (282636 => 282637)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-09-17 06:46:59 UTC (rev 282636)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-09-17 07:02:29 UTC (rev 282637)
@@ -1,3 +1,12 @@
+2021-09-17  Youenn Fablet  <[email protected]>
+
+        Compute RTCPeerConnection.connectionState as per https://w3c.github.io/webrtc-pc/#rtcpeerconnectionstate-enum
+        https://bugs.webkit.org/show_bug.cgi?id=230341
+
+        Reviewed by Eric Carlson.
+
+        * web-platform-tests/webrtc/RTCPeerConnection-connectionState.https-expected.txt:
+
 2021-09-16  Youenn Fablet  <[email protected]>
 
         WPT webrtc RTCPeerConnection-setLocalDescription-offer.html and RTCPeerConnection-setLocalDescription-answer.html are flaky due to always changing failing assertion

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-connectionState.https-expected.txt (282636 => 282637)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-connectionState.https-expected.txt	2021-09-17 06:46:59 UTC (rev 282636)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-connectionState.https-expected.txt	2021-09-17 07:02:29 UTC (rev 282637)
@@ -2,8 +2,8 @@
 PASS Initial connectionState should be new
 PASS Closing the connection should set connectionState to closed
 PASS connection with one data channel should eventually have connected connection state
-FAIL connection with one data channel should eventually have transports in connected state assert_equals: Expect DTLS transport to be in connected state expected "connected" but got "connecting"
-FAIL connectionState remains new when not adding remote ice candidates assert_equals: expected "new" but got "checking"
+PASS connection with one data channel should eventually have transports in connected state
+PASS connectionState remains new when not adding remote ice candidates
 PASS connectionState transitions to connected via connecting
 PASS Closing a PeerConnection should not fire connectionstatechange event
 

Modified: trunk/Source/WebCore/ChangeLog (282636 => 282637)


--- trunk/Source/WebCore/ChangeLog	2021-09-17 06:46:59 UTC (rev 282636)
+++ trunk/Source/WebCore/ChangeLog	2021-09-17 07:02:29 UTC (rev 282637)
@@ -1,3 +1,26 @@
+2021-09-17  Youenn Fablet  <[email protected]>
+
+        Compute RTCPeerConnection.connectionState as per https://w3c.github.io/webrtc-pc/#rtcpeerconnectionstate-enum
+        https://bugs.webkit.org/show_bug.cgi?id=230341
+
+        Reviewed by Eric Carlson.
+
+        We should compute connection states according ICE and DTLS transport state, as per spec.
+        Given we compute the state from ICE and DTLS states, we now make sure to update connection state whenever DTLS state changes.
+        Make also sure to not fire events in case peer connection is closed, as per spec.
+
+        Covered by existing and rebased tests.
+
+        * Modules/mediastream/RTCDtlsTransport.cpp:
+        (WebCore::RTCDtlsTransport::onStateChanged):
+        * Modules/mediastream/RTCIceTransport.h:
+        (WebCore::RTCIceTransport::connection const):
+        * Modules/mediastream/RTCPeerConnection.cpp:
+        (WebCore::RTCPeerConnection::close):
+        (WebCore::RTCPeerConnection::computeConnectionState):
+        (WebCore::RTCPeerConnection::processIceTransportStateChange):
+        * Modules/mediastream/RTCPeerConnection.h:
+
 2021-09-16  Cameron McCormack  <[email protected]>
 
         Preserve canvas color space when producing JPEGs from toDataURL/toBlob

Modified: trunk/Source/WebCore/Modules/mediastream/RTCDtlsTransport.cpp (282636 => 282637)


--- trunk/Source/WebCore/Modules/mediastream/RTCDtlsTransport.cpp	2021-09-17 06:46:59 UTC (rev 282636)
+++ trunk/Source/WebCore/Modules/mediastream/RTCDtlsTransport.cpp	2021-09-17 07:02:29 UTC (rev 282637)
@@ -34,6 +34,7 @@
 #include "NotImplemented.h"
 #include "RTCDtlsTransportBackend.h"
 #include "RTCIceTransport.h"
+#include "RTCPeerConnection.h"
 #include "ScriptExecutionContext.h"
 #include <wtf/IsoMallocInlines.h>
 
@@ -81,6 +82,8 @@
 
         if (m_state != state) {
             m_state = state;
+            if (auto* connection = m_iceTransport->connection())
+                connection->updateConnectionState();
             dispatchEvent(Event::create(eventNames().statechangeEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
         }
     });

Modified: trunk/Source/WebCore/Modules/mediastream/RTCIceTransport.h (282636 => 282637)


--- trunk/Source/WebCore/Modules/mediastream/RTCIceTransport.h	2021-09-17 06:46:59 UTC (rev 282636)
+++ trunk/Source/WebCore/Modules/mediastream/RTCIceTransport.h	2021-09-17 07:02:29 UTC (rev 282637)
@@ -38,6 +38,7 @@
 #include "RTCIceGatheringState.h"
 #include "RTCIceTransportBackend.h"
 #include "RTCIceTransportState.h"
+#include "RTCPeerConnection.h"
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
 
@@ -58,6 +59,7 @@
     RTCIceGatheringState gatheringState() const { return m_gatheringState; }
 
     const RTCIceTransportBackend& backend() const { return m_backend.get(); }
+    RTCPeerConnection* connection() const { return m_connection.get(); }
 
     using RefCounted<RTCIceTransport>::ref;
     using RefCounted<RTCIceTransport>::deref;

Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp (282636 => 282637)


--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2021-09-17 06:46:59 UTC (rev 282636)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp	2021-09-17 07:02:29 UTC (rev 282637)
@@ -561,7 +561,6 @@
     if (!doClose())
         return;
 
-    updateConnectionState();
     ASSERT(isClosed());
     m_backend->close();
 }
@@ -675,21 +674,44 @@
     });
 }
 
+// https://w3c.github.io/webrtc-pc/#rtcpeerconnectionstate-enum
 RTCPeerConnectionState RTCPeerConnection::computeConnectionState()
 {
-    if (m_iceConnectionState == RTCIceConnectionState::Closed)
+    if (isClosed())
         return RTCPeerConnectionState::Closed;
-    if (m_iceConnectionState == RTCIceConnectionState::Disconnected)
+
+    auto iceTransports = m_iceTransports;
+    iceTransports.removeAllMatching([&](auto& iceTransport) {
+        if (m_sctpTransport && &m_sctpTransport->transport().iceTransport() == iceTransport.ptr())
+            return false;
+        return allOf(m_transceiverSet.list(), [&iceTransport](auto& transceiver) {
+            return !isIceTransportUsedByTransceiver(iceTransport.get(), *transceiver);
+        });
+    });
+
+    auto dtlsTransports = m_dtlsTransports;
+    dtlsTransports.removeAllMatching([&](auto& dtlsTransport) {
+        if (m_sctpTransport && &m_sctpTransport->transport() == dtlsTransport.ptr())
+            return false;
+        return allOf(m_transceiverSet.list(), [&dtlsTransport](auto& transceiver) {
+            return transceiver->sender().transport() != dtlsTransport.ptr();
+        });
+    });
+
+    if (anyOf(iceTransports, [](auto& transport) { return transport->state() == RTCIceTransportState::Failed; }) || anyOf(dtlsTransports, [](auto& transport) { return transport->state() == RTCDtlsTransportState::Failed; }))
+        return RTCPeerConnectionState::Failed;
+
+    if (anyOf(iceTransports, [](auto& transport) { return transport->state() == RTCIceTransportState::Disconnected; }))
         return RTCPeerConnectionState::Disconnected;
-    if (m_iceConnectionState == RTCIceConnectionState::Failed)
-        return RTCPeerConnectionState::Failed;
-    if (m_iceConnectionState == RTCIceConnectionState::New && m_iceGatheringState == RTCIceGatheringState::New)
+
+    if (allOf(iceTransports, [](auto& transport) { return transport->state() == RTCIceTransportState::New || transport->state() == RTCIceTransportState::Closed; }) && allOf(dtlsTransports, [](auto& transport) { return transport->state() == RTCDtlsTransportState::New || transport->state() == RTCDtlsTransportState::Closed; }))
         return RTCPeerConnectionState::New;
-    if (m_iceConnectionState == RTCIceConnectionState::Checking || m_iceGatheringState == RTCIceGatheringState::Gathering)
+
+    if (anyOf(iceTransports, [](auto& transport) { return transport->state() == RTCIceTransportState::New || transport->state() == RTCIceTransportState::Checking; }) || anyOf(dtlsTransports, [](auto& transport) { return transport->state() == RTCDtlsTransportState::New || transport->state() == RTCDtlsTransportState::Connecting; }))
         return RTCPeerConnectionState::Connecting;
-    if ((m_iceConnectionState == RTCIceConnectionState::Completed || m_iceConnectionState == RTCIceConnectionState::Connected) && m_iceGatheringState == RTCIceGatheringState::Complete)
-        return RTCPeerConnectionState::Connected;
-    return m_connectionState;
+
+    ASSERT(allOf(iceTransports, [](auto& transport) { return transport->state() == RTCIceTransportState::Connected || transport->state() == RTCIceTransportState::Completed || transport->state() == RTCIceTransportState::Closed; }) && allOf(dtlsTransports, [](auto& transport) { return transport->state() == RTCDtlsTransportState::Connected || transport->state() == RTCDtlsTransportState::Closed; }));
+    return RTCPeerConnectionState::Connected;
 }
 
 void RTCPeerConnection::updateConnectionState()
@@ -753,9 +775,9 @@
     m_connectionState = newConnectionState;
 
     iceTransport.dispatchEvent(Event::create(eventNames().statechangeEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
-    if (iceConnectionStateChanged)
+    if (iceConnectionStateChanged && !isClosed())
         dispatchEvent(Event::create(eventNames().iceconnectionstatechangeEvent, Event::CanBubble::No, Event::IsCancelable::No));
-    if (connectionStateChanged)
+    if (connectionStateChanged && !isClosed())
         dispatchEvent(Event::create(eventNames().connectionstatechangeEvent, Event::CanBubble::No, Event::IsCancelable::No));
 }
 

Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h (282636 => 282637)


--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h	2021-09-17 06:46:59 UTC (rev 282636)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h	2021-09-17 07:02:29 UTC (rev 282637)
@@ -171,9 +171,9 @@
     WEBCORE_EXPORT void emulatePlatformEvent(const String& action);
 
     // API used by PeerConnectionBackend and relatives
-    void setSignalingState(RTCSignalingState);
     void updateIceGatheringState(RTCIceGatheringState);
     void updateIceConnectionState(RTCIceConnectionState);
+    void updateConnectionState();
 
     void updateNegotiationNeededFlag(std::optional<uint32_t>);
 
@@ -227,7 +227,6 @@
     void resume() final;
     bool virtualHasPendingActivity() const final;
 
-    void updateConnectionState();
     bool updateIceConnectionStateFromIceTransports();
     RTCIceConnectionState computeIceConnectionStateFromIceTransports();
     RTCPeerConnectionState computeConnectionState();
@@ -247,6 +246,8 @@
     RefPtr<RTCDtlsTransport> getOrCreateDtlsTransport(std::unique_ptr<RTCDtlsTransportBackend>&&);
     void updateTransceiverTransports();
 
+    void setSignalingState(RTCSignalingState);
+
     bool m_isStopped { false };
     RTCSignalingState m_signalingState { RTCSignalingState::Stable };
     RTCIceGatheringState m_iceGatheringState { RTCIceGatheringState::New };
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to