- Revision
- 283307
- Author
- you...@apple.com
- Date
- 2021-09-30 04:20:49 -0700 (Thu, 30 Sep 2021)
Log Message
Layout Test imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-iceConnectionState.https.html is a flaky failure
https://bugs.webkit.org/show_bug.cgi?id=203256
<rdar://problem/56506063>
Reviewed by Eric Carlson.
Source/WebCore:
Test was flaky for a few reasons:
- Setting local/remote descriptions may change the ICE transports in use so we need to update the ice connection state when setting local/remote descriptions.
- We start observing ICE transport backend state asynchronously and we might miss the checking state which can happen very quickly. Synthesize it if needed.
- We were using the ICE connection state value from the backend as well as computing it from our RTCIceTransport objects. We now fully compute it from RTCIceTransport objects.
This ensures we have consistent state between RTCIceTransport and RTCPeerConnection objects with regards to ICE state.
Covered by no longer flaky test.
* Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::setLocalDescriptionSucceeded):
(WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::updateIceGatheringState):
(WebCore::RTCPeerConnection::updateIceConnectionState):
(WebCore::RTCPeerConnection::computeIceConnectionStateFromIceTransports):
* Modules/mediastream/RTCPeerConnection.h:
* Modules/mediastream/libwebrtc/LibWebRTCIceTransportBackend.cpp:
(WebCore::LibWebRTCIceTransportBackendObserver::start):
LayoutTests:
* TestExpectations:
Mark webrtc/connection-state.html as flaky as it is probably too restrictive and should be reworked.
* platform/mac-wk2/TestExpectations:
Unflake test.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (283306 => 283307)
--- trunk/LayoutTests/ChangeLog 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/LayoutTests/ChangeLog 2021-09-30 11:20:49 UTC (rev 283307)
@@ -1,3 +1,16 @@
+2021-09-30 Youenn Fablet <you...@apple.com>
+
+ Layout Test imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-iceConnectionState.https.html is a flaky failure
+ https://bugs.webkit.org/show_bug.cgi?id=203256
+ <rdar://problem/56506063>
+
+ Reviewed by Eric Carlson.
+
+ * TestExpectations:
+ Mark webrtc/connection-state.html as flaky as it is probably too restrictive and should be reworked.
+ * platform/mac-wk2/TestExpectations:
+ Unflake test.
+
2021-09-29 Youenn Fablet <you...@apple.com>
Import WPT push api tests
Modified: trunk/LayoutTests/TestExpectations (283306 => 283307)
--- trunk/LayoutTests/TestExpectations 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/LayoutTests/TestExpectations 2021-09-30 11:20:49 UTC (rev 283307)
@@ -2073,6 +2073,8 @@
webrtc/h264-baseline.html [ Slow ]
webrtc/h264-high.html [ Slow ]
+webkit.org/b/207798 webrtc/connection-state.html [ Pass Failure ]
+
imported/w3c/web-platform-tests/webrtc/RTCRtpReceiver-getSynchronizationSources.https.html [ Pass Failure Slow ]
webkit.org/b/214197 imported/w3c/web-platform-tests/webrtc/RTCRtpTransceiver.https.html [ Pass Failure Crash ]
imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-iceGatheringState.html [ Pass Failure ]
Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (283306 => 283307)
--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2021-09-30 11:20:49 UTC (rev 283307)
@@ -987,9 +987,6 @@
webkit.org/b/199089 [ Debug ] plugins/window-open.html [ Skip ]
-# <rdar://problem/56506063> Layout Test imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-iceConnectionState.https.html is a flaky failure (203256)
-webkit.org/b/203256 [ Debug ] imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-iceConnectionState.https.html [ Pass Failure ]
-
webkit.org/b/205808 fast/text/international/unicode-bidi-other-neutrals.html [ Pass Failure ]
webkit.org/b/206864 http/tests/workers/service/basic-timeout.https.html [ Pass Failure ]
@@ -1033,8 +1030,6 @@
webkit.org/b/207796 [ Release ] fast/box-shadow/hidpi-box-shadow.html [ Pass ImageOnlyFailure ]
-webkit.org/b/207798 webrtc/connection-state.html [ Pass Failure ]
-
webkit.org/b/207938 http/wpt/crypto/derive-hmac-key-crash.any.html [ Pass Crash ]
webkit.org/b/207962 accessibility/mac/aria-menu-item-selected-notification.html [ Slow ]
Modified: trunk/Source/WebCore/ChangeLog (283306 => 283307)
--- trunk/Source/WebCore/ChangeLog 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/Source/WebCore/ChangeLog 2021-09-30 11:20:49 UTC (rev 283307)
@@ -1,3 +1,30 @@
+2021-09-30 Youenn Fablet <you...@apple.com>
+
+ Layout Test imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-iceConnectionState.https.html is a flaky failure
+ https://bugs.webkit.org/show_bug.cgi?id=203256
+ <rdar://problem/56506063>
+
+ Reviewed by Eric Carlson.
+
+ Test was flaky for a few reasons:
+ - Setting local/remote descriptions may change the ICE transports in use so we need to update the ice connection state when setting local/remote descriptions.
+ - We start observing ICE transport backend state asynchronously and we might miss the checking state which can happen very quickly. Synthesize it if needed.
+ - We were using the ICE connection state value from the backend as well as computing it from our RTCIceTransport objects. We now fully compute it from RTCIceTransport objects.
+ This ensures we have consistent state between RTCIceTransport and RTCPeerConnection objects with regards to ICE state.
+
+ Covered by no longer flaky test.
+
+ * Modules/mediastream/PeerConnectionBackend.cpp:
+ (WebCore::PeerConnectionBackend::setLocalDescriptionSucceeded):
+ (WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
+ * Modules/mediastream/RTCPeerConnection.cpp:
+ (WebCore::RTCPeerConnection::updateIceGatheringState):
+ (WebCore::RTCPeerConnection::updateIceConnectionState):
+ (WebCore::RTCPeerConnection::computeIceConnectionStateFromIceTransports):
+ * Modules/mediastream/RTCPeerConnection.h:
+ * Modules/mediastream/libwebrtc/LibWebRTCIceTransportBackend.cpp:
+ (WebCore::LibWebRTCIceTransportBackendObserver::start):
+
2021-09-30 Carlos Garcia Campos <cgar...@igalia.com>
Unreviewed GTK build fix after r283304
Modified: trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp (283306 => 283307)
--- trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp 2021-09-30 11:20:49 UTC (rev 283307)
@@ -177,7 +177,7 @@
m_peerConnection.updateDescriptions(WTFMove(*descriptionStates));
m_peerConnection.updateTransceiversAfterSuccessfulLocalDescription();
m_peerConnection.updateSctpBackend(WTFMove(sctpBackend));
-
+ m_peerConnection.processIceTransportChanges();
callback({ });
});
}
@@ -235,6 +235,7 @@
m_peerConnection.updateTransceiversAfterSuccessfulRemoteDescription();
m_peerConnection.updateSctpBackend(WTFMove(sctpBackend));
+ m_peerConnection.processIceTransportChanges();
callback({ });
});
}
Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp (283306 => 283307)
--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp 2021-09-30 11:20:49 UTC (rev 283307)
@@ -670,13 +670,14 @@
});
}
-void RTCPeerConnection::updateIceConnectionState(RTCIceConnectionState newState)
+void RTCPeerConnection::updateIceConnectionState(RTCIceConnectionState)
{
- ALWAYS_LOG(LOGIDENTIFIER, newState);
-
- queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this, newState] {
- if (isClosed() || m_iceConnectionState == newState)
+ queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this] {
+ if (isClosed())
return;
+ auto newState = computeIceConnectionStateFromIceTransports();
+ if (m_iceConnectionState == newState)
+ return;
m_iceConnectionState = newState;
dispatchEvent(Event::create(eventNames().iceconnectionstatechangeEvent, Event::CanBubble::No, Event::IsCancelable::No));
@@ -791,6 +792,16 @@
dispatchEvent(Event::create(eventNames().connectionstatechangeEvent, Event::CanBubble::No, Event::IsCancelable::No));
}
+void RTCPeerConnection::processIceTransportChanges()
+{
+ auto newIceConnectionState = computeIceConnectionStateFromIceTransports();
+ bool iceConnectionStateChanged = m_iceConnectionState != newIceConnectionState;
+ m_iceConnectionState = newIceConnectionState;
+
+ if (iceConnectionStateChanged && !isClosed())
+ dispatchEvent(Event::create(eventNames().iceconnectionstatechangeEvent, Event::CanBubble::No, Event::IsCancelable::No));
+}
+
void RTCPeerConnection::updateNegotiationNeededFlag(std::optional<uint32_t> eventId)
{
queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this, eventId]() mutable {
Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h (283306 => 283307)
--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.h 2021-09-30 11:20:49 UTC (rev 283307)
@@ -192,6 +192,7 @@
void updateSctpBackend(std::unique_ptr<RTCSctpTransportBackend>&&);
void processIceTransportStateChange(RTCIceTransport&);
+ void processIceTransportChanges();
RTCSctpTransport* sctp() { return m_sctpTransport.get(); }
Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCIceTransportBackend.cpp (283306 => 283307)
--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCIceTransportBackend.cpp 2021-09-30 08:42:16 UTC (rev 283306)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCIceTransportBackend.cpp 2021-09-30 11:20:49 UTC (rev 283307)
@@ -108,7 +108,15 @@
return;
internal->SignalIceTransportStateChanged.connect(this, &LibWebRTCIceTransportBackendObserver::onIceTransportStateChanged);
internal->SignalGatheringState.connect(this, &LibWebRTCIceTransportBackendObserver::onGatheringStateChanged);
- callOnMainThread([protectedThis = Ref { *this }, transportState = internal->GetIceTransportState(), gatheringState = internal->gathering_state()] {
+ auto transportState = internal->GetIceTransportState();
+ // We start observing a bit late and might miss the checking state. Synthesize it as needed.
+ if (transportState > webrtc::IceTransportState::kChecking && transportState != webrtc::IceTransportState::kClosed) {
+ callOnMainThread([protectedThis = Ref { *this }] {
+ if (protectedThis->m_client)
+ protectedThis->m_client->onStateChanged(RTCIceTransportState::Checking);
+ });
+ }
+ callOnMainThread([protectedThis = Ref { *this }, transportState, gatheringState = internal->gathering_state()] {
if (!protectedThis->m_client)
return;
protectedThis->m_client->onStateChanged(toRTCIceTransportState(transportState));
