Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (282824 => 282825)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-09-21 17:05:59 UTC (rev 282825)
@@ -1,5 +1,15 @@
2021-09-21 Youenn Fablet <[email protected]>
+ imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close.html is flaky
+ https://bugs.webkit.org/show_bug.cgi?id=230399
+
+ Reviewed by Eric Carlson.
+
+ * web-platform-tests/webrtc/RTCDataChannel-close-expected.txt:
+ * web-platform-tests/webrtc/RTCDataChannel-close.html:
+
+2021-09-21 Youenn Fablet <[email protected]>
+
RTCPeerConnection perfect negotiation tests are flaky
https://bugs.webkit.org/show_bug.cgi?id=230344
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close-expected.txt (282824 => 282825)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close-expected.txt 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close-expected.txt 2021-09-21 17:05:59 UTC (rev 282825)
@@ -3,8 +3,10 @@
PASS Close datachannel causes closing and close event to be called
PASS Close peerconnection causes close event and error to be called on datachannel
PASS Close peerconnection after datachannel close causes no events
+PASS Close peerconnection causes close event and error on many channels, datachannel
PASS Close negotiated datachannel causes onclosing and onclose to be called
PASS Close negotiated datachannel causes closing and close event to be called
PASS Close peerconnection causes close event and error to be called on negotiated datachannel
PASS Close peerconnection after negotiated datachannel close causes no events
+PASS Close peerconnection causes close event and error on many channels, negotiated datachannel
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close.html (282824 => 282825)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close.html 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close.html 2021-09-21 17:05:59 UTC (rev 282825)
@@ -45,7 +45,7 @@
assert_true(closingSeen, 'Closing event was seen');
}, `Close ${mode} causes closing and close event to be called`);
- promise_test(async t => {
+ promise_test(async t => {
const pc1 = new RTCPeerConnection();
t.add_cleanup(() => pc1.close());
const [channel1, channel2] = await createDataChannelPair(t, options, pc1);
@@ -100,5 +100,81 @@
pc1.close();
await new Promise(resolve => t.step_timeout(resolve, 10));
}, `Close peerconnection after ${mode} close causes no events`);
+
+ promise_test(async t => {
+ const pc1 = new RTCPeerConnection();
+ t.add_cleanup(() => pc1.close());
+ const pc2 = new RTCPeerConnection();
+ t.add_cleanup(() => pc2.close());
+ pc1.createDataChannel('not-counted', options);
+ const tokenDataChannel = new Promise(resolve => {
+ pc2._ondatachannel_ = resolve;
+ });
+ exchangeIceCandidates(pc1, pc2);
+ await exchangeOfferAnswer(pc1, pc2);
+ if (!options.negotiated) {
+ await tokenDataChannel;
+ }
+ let closeExpectedCount = 0;
+ let errorExpectedCount = 0;
+ let resolveCountIsZero;
+ let waitForCountIsZero = new Promise(resolve => {
+ resolveCountIsZero = resolve;
+ });
+ for (let i = 1; i <= 10; i++) {
+ if ('id' in options) {
+ options.id = i;
+ }
+ pc1.createDataChannel('', options);
+ if (options.negotiated) {
+ const channel = pc2.createDataChannel('', options);
+ channel.addEventListener('error', t.step_func(event => {
+ assert_true(event instanceof RTCErrorEvent, 'error event ' + event);
+ errorExpectedCount -= 1;
+ }));
+ channel.addEventListener('close', t.step_func(event => {
+ closeExpectedCount -= 1;
+ if (closeExpectedCount == 0) {
+ resolveCountIsZero();
+ }
+ }));
+ } else {
+ await new Promise(resolve => {
+ pc2._ondatachannel_ = ({channel}) => {
+ channel.addEventListener('error', t.step_func(event => {
+ assert_true(event instanceof RTCErrorEvent);
+ errorExpectedCount -= 1;
+ }));
+ channel.addEventListener('close', t.step_func(event => {
+ closeExpectedCount -= 1;
+ if (closeExpectedCount == 0) {
+ resolveCountIsZero();
+ }
+ }));
+ resolve();
+ }
+ });
+ }
+ ++closeExpectedCount;
+ ++errorExpectedCount;
+ }
+ assert_equals(closeExpectedCount, 10);
+ // We have to wait until SCTP is connected before we close, otherwise
+ // there will be no signal.
+ // The state is not available under Plan B, and unreliable on negotiated
+ // channels.
+ // TODO(bugs.webrtc.org/12259): Remove dependency on "negotiated"
+ if (pc1.sctp && !options.negotiated) {
+ waitForState(pc1.sctp, 'connected');
+ } else {
+ // Under plan B, we don't have a dtls transport to wait on, so just
+ // wait a bit.
+ await new Promise(resolve => t.step_timeout(resolve, 100));
+ }
+ pc1.close();
+ await waitForCountIsZero;
+ assert_equals(closeExpectedCount, 0);
+ assert_equals(errorExpectedCount, 0);
+ }, `Close peerconnection causes close event and error on many channels, ${mode}`);
}
</script>
Modified: trunk/Source/WebCore/ChangeLog (282824 => 282825)
--- trunk/Source/WebCore/ChangeLog 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/Source/WebCore/ChangeLog 2021-09-21 17:05:59 UTC (rev 282825)
@@ -1,3 +1,38 @@
+2021-09-21 Youenn Fablet <[email protected]>
+
+ imported/w3c/web-platform-tests/webrtc/RTCDataChannel-close.html is flaky
+ https://bugs.webkit.org/show_bug.cgi?id=230399
+
+ Reviewed by Eric Carlson.
+
+ We fix a number of flaky issues:
+ - RTCDataChannel::didChangeReadyState should queue a task and inside the task update the state + dispatch event.
+ Previously, we were updating the state, then queueing a task to dispatch the event.
+ - We were creating the data channel at the time we were creating the data channel handler.
+ In that case, we might have missed some state changes.
+ We now create the data channel handler as soon as possible, then queue a task where we create the data channel and fire the channel event.
+ - We check the backend data channel state as soon as creating the data channel handler, instead of when creating the data channel for the same reason.
+ - When we buffer states, we need to store whether an error occurs to properly fire the error event as needed.
+ - Similarly to readyState, we need to queue a task to update bufferedAmount and, if needed, synchronously fire the bufferedAmountLow event.
+
+ Covered by updated RTCDataChannel-close.html.
+
+ * Modules/mediastream/PeerConnectionBackend.cpp:
+ (WebCore::PeerConnectionBackend::newDataChannel):
+ * Modules/mediastream/PeerConnectionBackend.h:
+ * Modules/mediastream/RTCDataChannel.cpp:
+ (WebCore::RTCDataChannel::didChangeReadyState):
+ (WebCore::RTCDataChannel::bufferedAmountIsDecreasing):
+ * Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp:
+ (WebCore::LibWebRTCDataChannelHandler::LibWebRTCDataChannelHandler):
+ (WebCore::LibWebRTCDataChannelHandler::dataChannelInit const):
+ (WebCore::LibWebRTCDataChannelHandler::label const):
+ (WebCore::LibWebRTCDataChannelHandler::setClient):
+ (WebCore::LibWebRTCDataChannelHandler::checkState):
+ * Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h:
+ * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+ (WebCore::LibWebRTCMediaEndpoint::OnDataChannel):
+
2021-09-21 Alan Bujtas <[email protected]>
[LFC][IFC] Ceil the descent value when adjusting the layout bounds for an inline box
Modified: trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp (282824 => 282825)
--- trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp 2021-09-21 17:05:59 UTC (rev 282825)
@@ -38,6 +38,7 @@
#include "LibWebRTCCertificateGenerator.h"
#include "Logging.h"
#include "Page.h"
+#include "RTCDataChannelEvent.h"
#include "RTCDtlsTransport.h"
#include "RTCIceCandidate.h"
#include "RTCPeerConnection.h"
@@ -375,6 +376,17 @@
});
}
+void PeerConnectionBackend::newDataChannel(UniqueRef<RTCDataChannelHandler>&& channelHandler, String&& label, RTCDataChannelInit&& channelInit)
+{
+ m_peerConnection.queueTaskKeepingObjectAlive(m_peerConnection, TaskSource::Networking, [connection = makeRef(m_peerConnection), label = WTFMove(label), channelHandler = WTFMove(channelHandler), channelInit = WTFMove(channelInit)]() mutable {
+ if (connection->isClosed())
+ return;
+
+ auto channel = RTCDataChannel::create(*connection->document(), channelHandler.moveToUniquePtr(), WTFMove(label), WTFMove(channelInit));
+ connection->dispatchEvent(RTCDataChannelEvent::create(eventNames().datachannelEvent, Event::CanBubble::No, Event::IsCancelable::No, WTFMove(channel)));
+ });
+}
+
void PeerConnectionBackend::doneGatheringCandidates()
{
ASSERT(isMainThread());
Modified: trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h (282824 => 282825)
--- trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h 2021-09-21 17:05:59 UTC (rev 282825)
@@ -137,6 +137,8 @@
};
void newICECandidate(String&& sdp, String&& mid, unsigned short sdpMLineIndex, String&& serverURL, std::optional<DescriptionStates>&&);
+ void newDataChannel(UniqueRef<RTCDataChannelHandler>&&, String&&, RTCDataChannelInit&&);
+
virtual void disableICECandidateFiltering();
void enableICECandidateFiltering();
Modified: trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.cpp (282824 => 282825)
--- trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.cpp 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/Source/WebCore/Modules/mediastream/RTCDataChannel.cpp 2021-09-21 17:05:59 UTC (rev 282825)
@@ -194,29 +194,31 @@
void RTCDataChannel::didChangeReadyState(RTCDataChannelState newState)
{
- if (m_stopped || m_readyState == RTCDataChannelState::Closed || m_readyState == newState)
- return;
+ queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this, newState] {
+ if (m_stopped || m_readyState == RTCDataChannelState::Closed || m_readyState == newState)
+ return;
- if (m_readyState == RTCDataChannelState::Closing && newState == RTCDataChannelState::Open)
- return;
+ if (m_readyState == RTCDataChannelState::Closing && newState == RTCDataChannelState::Open)
+ return;
- m_readyState = newState;
+ m_readyState = newState;
- switch (m_readyState) {
- case RTCDataChannelState::Connecting:
- ASSERT_NOT_REACHED();
- break;
- case RTCDataChannelState::Open:
- scheduleDispatchEvent(Event::create(eventNames().openEvent, Event::CanBubble::No, Event::IsCancelable::No));
- break;
- case RTCDataChannelState::Closing:
- scheduleDispatchEvent(Event::create(eventNames().closingEvent, Event::CanBubble::No, Event::IsCancelable::No));
- break;
- case RTCDataChannelState::Closed:
- scheduleDispatchEvent(Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No));
- m_stopped = true;
- break;
- }
+ switch (m_readyState) {
+ case RTCDataChannelState::Connecting:
+ ASSERT_NOT_REACHED();
+ break;
+ case RTCDataChannelState::Open:
+ dispatchEvent(Event::create(eventNames().openEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ break;
+ case RTCDataChannelState::Closing:
+ dispatchEvent(Event::create(eventNames().closingEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ break;
+ case RTCDataChannelState::Closed:
+ dispatchEvent(Event::create(eventNames().closeEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ m_stopped = true;
+ break;
+ }
+ });
}
void RTCDataChannel::didReceiveStringData(const String& text)
@@ -244,10 +246,12 @@
void RTCDataChannel::bufferedAmountIsDecreasing(size_t amount)
{
- auto previousBufferedAmount = m_bufferedAmount;
- m_bufferedAmount -= amount;
- if (previousBufferedAmount > m_bufferedAmountLowThreshold && m_bufferedAmount <= m_bufferedAmountLowThreshold)
- scheduleDispatchEvent(Event::create(eventNames().bufferedamountlowEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ queueTaskKeepingObjectAlive(*this, TaskSource::Networking, [this, amount] {
+ auto previousBufferedAmount = m_bufferedAmount;
+ m_bufferedAmount -= amount;
+ if (previousBufferedAmount > m_bufferedAmountLowThreshold && m_bufferedAmount <= m_bufferedAmountLowThreshold)
+ dispatchEvent(Event::create(eventNames().bufferedamountlowEvent, Event::CanBubble::No, Event::IsCancelable::No));
+ });
}
void RTCDataChannel::stop()
Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp (282824 => 282825)
--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.cpp 2021-09-21 17:05:59 UTC (rev 282825)
@@ -30,7 +30,6 @@
#include "EventNames.h"
#include "LibWebRTCUtils.h"
#include "RTCDataChannel.h"
-#include "RTCDataChannelEvent.h"
#include "RTCError.h"
#include <wtf/MainThread.h>
@@ -54,30 +53,11 @@
return init;
}
-Ref<RTCDataChannelEvent> LibWebRTCDataChannelHandler::channelEvent(Document& document, rtc::scoped_refptr<webrtc::DataChannelInterface>&& dataChannel)
-{
- auto protocol = dataChannel->protocol();
- auto label = dataChannel->label();
-
- RTCDataChannelInit init;
- init.ordered = dataChannel->ordered();
- init.maxPacketLifeTime = dataChannel->maxRetransmitTime();
- init.maxRetransmits = dataChannel->maxRetransmits();
- init.protocol = fromStdString(protocol);
- init.negotiated = dataChannel->negotiated();
- init.id = dataChannel->id();
- init.priority = toRTCPriorityType(dataChannel->priority());
-
- auto handler = makeUnique<LibWebRTCDataChannelHandler>(WTFMove(dataChannel));
- auto channel = RTCDataChannel::create(document, WTFMove(handler), fromStdString(label), WTFMove(init));
-
- return RTCDataChannelEvent::create(eventNames().datachannelEvent, Event::CanBubble::No, Event::IsCancelable::No, WTFMove(channel));
-}
-
LibWebRTCDataChannelHandler::LibWebRTCDataChannelHandler(rtc::scoped_refptr<webrtc::DataChannelInterface>&& channel)
: m_channel(WTFMove(channel))
{
ASSERT(m_channel);
+ checkState();
m_channel->RegisterObserver(this);
}
@@ -86,26 +66,48 @@
m_channel->UnregisterObserver();
}
+RTCDataChannelInit LibWebRTCDataChannelHandler::dataChannelInit() const
+{
+ auto protocol = m_channel->protocol();
+ auto label = m_channel->label();
+
+ RTCDataChannelInit init;
+ init.ordered = m_channel->ordered();
+ init.maxPacketLifeTime = m_channel->maxRetransmitTime();
+ init.maxRetransmits = m_channel->maxRetransmits();
+ init.protocol = fromStdString(protocol);
+ init.negotiated = m_channel->negotiated();
+ init.id = m_channel->id();
+ init.priority = toRTCPriorityType(m_channel->priority());
+ return init;
+}
+
+String LibWebRTCDataChannelHandler::label() const
+{
+ return fromStdString(m_channel->label());
+}
+
void LibWebRTCDataChannelHandler::setClient(RTCDataChannelHandlerClient& client, ScriptExecutionContextIdentifier contextIdentifier)
{
- {
- Locker locker { m_clientLock };
- ASSERT(!m_client);
- m_client = &client;
- m_contextIdentifier = contextIdentifier;
+ Locker locker { m_clientLock };
+ ASSERT(!m_client);
+ m_client = &client;
+ m_contextIdentifier = contextIdentifier;
- for (auto& message : m_bufferedMessages) {
- switchOn(message, [&](Ref<SharedBuffer>& data) {
- client.didReceiveRawData(data->data(), data->size());
- }, [&](String& text) {
- client.didReceiveStringData(text);
- }, [&](RTCDataChannelState state) {
- client.didChangeReadyState(state);
- });
- }
- m_bufferedMessages.clear();
+ for (auto& message : m_bufferedMessages) {
+ switchOn(message, [&](Ref<SharedBuffer>& data) {
+ client.didReceiveRawData(data->data(), data->size());
+ }, [&](String& text) {
+ client.didReceiveStringData(text);
+ }, [&](StateChange stateChange) {
+ if (stateChange.error) {
+ if (auto rtcError = toRTCError(*stateChange.error))
+ client.didDetectError(rtcError.releaseNonNull());
+ }
+ client.didChangeReadyState(stateChange.state);
+ });
}
- checkState();
+ m_bufferedMessages.clear();
}
bool LibWebRTCDataChannelHandler::sendStringData(const CString& utf8Text)
@@ -150,13 +152,15 @@
Locker locker { m_clientLock };
if (!m_client) {
- m_bufferedMessages.append(state);
+ m_bufferedMessages.append(StateChange { state, WTFMove(error) });
return;
}
postTask([protectedClient = makeRef(*m_client), state, error = WTFMove(error)] {
if (error && !error->ok()) {
- if (auto rtcError = toRTCError(*error))
- protectedClient->didDetectError(rtcError.releaseNonNull());
+ auto rtcError = toRTCError(*error);
+ if (!rtcError)
+ rtcError = RTCError::create(RTCError::Init { RTCErrorDetailType::DataChannelFailure, { }, { }, { }, { } }, String { });
+ protectedClient->didDetectError(rtcError.releaseNonNull());
}
protectedClient->didChangeReadyState(state);
});
Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h (282824 => 282825)
--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCDataChannelHandler.h 2021-09-21 17:05:59 UTC (rev 282825)
@@ -56,8 +56,10 @@
explicit LibWebRTCDataChannelHandler(rtc::scoped_refptr<webrtc::DataChannelInterface>&&);
~LibWebRTCDataChannelHandler();
+ RTCDataChannelInit dataChannelInit() const;
+ String label() const;
+
static webrtc::DataChannelInit fromRTCDataChannelInit(const RTCDataChannelInit&);
- static Ref<RTCDataChannelEvent> channelEvent(Document&, rtc::scoped_refptr<webrtc::DataChannelInterface>&&);
private:
// RTCDataChannelHandler API
@@ -73,7 +75,11 @@
void checkState();
- using Message = Variant<RTCDataChannelState, String, Ref<SharedBuffer>>;
+ struct StateChange {
+ RTCDataChannelState state;
+ std::optional<webrtc::RTCError> error;
+ };
+ using Message = Variant<StateChange, String, Ref<SharedBuffer>>;
using PendingMessages = Vector<Message>;
void storeMessage(PendingMessages&, const webrtc::DataBuffer&);
void processMessage(const webrtc::DataBuffer&);
@@ -85,7 +91,7 @@
Lock m_clientLock;
RTCDataChannelHandlerClient* m_client WTF_GUARDED_BY_LOCK(m_clientLock) { nullptr };
ScriptExecutionContextIdentifier m_contextIdentifier;
- PendingMessages m_bufferedMessages;
+ PendingMessages m_bufferedMessages WTF_GUARDED_BY_LOCK(m_clientLock);
};
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp (282824 => 282825)
--- trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp 2021-09-21 16:47:36 UTC (rev 282824)
+++ trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp 2021-09-21 17:05:59 UTC (rev 282825)
@@ -457,14 +457,12 @@
void LibWebRTCMediaEndpoint::OnDataChannel(rtc::scoped_refptr<webrtc::DataChannelInterface> dataChannel)
{
callOnMainThread([protectedThis = Ref { *this }, dataChannel = WTFMove(dataChannel)]() mutable {
- auto& connection = protectedThis->m_peerConnectionBackend.connection();
- connection.queueTaskKeepingObjectAlive(connection, TaskSource::Networking, [protectedThis = WTFMove(protectedThis), dataChannel = WTFMove(dataChannel)]() mutable {
- if (protectedThis->isStopped())
- return;
-
- auto& connection = protectedThis->m_peerConnectionBackend.connection();
- connection.dispatchEvent(LibWebRTCDataChannelHandler::channelEvent(*connection.document(), WTFMove(dataChannel)));
- });
+ if (protectedThis->isStopped())
+ return;
+ auto channelHandler = makeUniqueRef<LibWebRTCDataChannelHandler>(WTFMove(dataChannel));
+ auto label = channelHandler->label();
+ auto dataChannelInit = channelHandler->dataChannelInit();
+ protectedThis->m_peerConnectionBackend.newDataChannel(WTFMove(channelHandler), WTFMove(label), WTFMove(dataChannelInit));
});
}
