- Revision
- 282638
- Author
- [email protected]
- Date
- 2021-09-17 00:07:07 -0700 (Fri, 17 Sep 2021)
Log Message
Make sure to use event queue when settling RTCPeerConnection.addIceCandidate promise
https://bugs.webkit.org/show_bug.cgi?id=230346
Reviewed by Eric Carlson.
Before the patch, we were resolving the promise in a callOnMainThread lambda.
We now do like for other methods: hop to main thread, then queue a task in event loop to resolve the promise.
Covered by existing tests.
* Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::addIceCandidate):
* Modules/mediastream/PeerConnectionBackend.h:
* Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::addIceCandidate):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (282637 => 282638)
--- trunk/Source/WebCore/ChangeLog 2021-09-17 07:02:29 UTC (rev 282637)
+++ trunk/Source/WebCore/ChangeLog 2021-09-17 07:07:07 UTC (rev 282638)
@@ -1,5 +1,22 @@
2021-09-17 Youenn Fablet <[email protected]>
+ Make sure to use event queue when settling RTCPeerConnection.addIceCandidate promise
+ https://bugs.webkit.org/show_bug.cgi?id=230346
+
+ Reviewed by Eric Carlson.
+
+ Before the patch, we were resolving the promise in a callOnMainThread lambda.
+ We now do like for other methods: hop to main thread, then queue a task in event loop to resolve the promise.
+ Covered by existing tests.
+
+ * Modules/mediastream/PeerConnectionBackend.cpp:
+ (WebCore::PeerConnectionBackend::addIceCandidate):
+ * Modules/mediastream/PeerConnectionBackend.h:
+ * Modules/mediastream/RTCPeerConnection.cpp:
+ (WebCore::RTCPeerConnection::addIceCandidate):
+
+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
Modified: trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp (282637 => 282638)
--- trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp 2021-09-17 07:02:29 UTC (rev 282637)
+++ trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp 2021-09-17 07:07:07 UTC (rev 282638)
@@ -294,40 +294,39 @@
return false;
}
-void PeerConnectionBackend::addIceCandidate(RTCIceCandidate* iceCandidate, DOMPromiseDeferred<void>&& promise)
+void PeerConnectionBackend::addIceCandidate(RTCIceCandidate* iceCandidate, Function<void(ExceptionOr<void>&&)>&& callback)
{
ASSERT(!m_peerConnection.isClosed());
if (!iceCandidate) {
- endOfIceCandidates(WTFMove(promise));
+ callback({ });
return;
}
- // FIXME: As per https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-addicecandidate(), this check should be done before enqueuing the task.
- if (iceCandidate->sdpMid().isNull() && !iceCandidate->sdpMLineIndex()) {
- promise.reject(Exception { TypeError, "Trying to add a candidate that is missing both sdpMid and sdpMLineIndex"_s });
- return;
- }
-
if (shouldIgnoreIceCandidate(*iceCandidate)) {
- promise.resolve();
+ callback({ });
return;
}
- doAddIceCandidate(*iceCandidate, [weakThis = makeWeakPtr(this), promise = WTFMove(promise)](auto&& result) mutable {
- ASSERT(isMainThread());
- if (!weakThis || weakThis->m_peerConnection.isClosed())
+ doAddIceCandidate(*iceCandidate, [weakThis = makeWeakPtr(this), callback = WTFMove(callback)](auto&& result) mutable {
+ if (!weakThis)
return;
- if (result.hasException()) {
- RELEASE_LOG_ERROR(WebRTC, "Adding ice candidate failed %d", result.exception().code());
- promise.reject(result.releaseException());
- return;
- }
+ auto& peerConnection = weakThis->m_peerConnection;
+ peerConnection.queueTaskKeepingObjectAlive(peerConnection, TaskSource::Networking, [&peerConnection, callback = WTFMove(callback), result = WTFMove(result)]() mutable {
+ if (peerConnection.isClosed())
+ return;
- if (auto descriptions = result.releaseReturnValue())
- weakThis->m_peerConnection.updateDescriptions(WTFMove(*descriptions));
- promise.resolve();
+ if (result.hasException()) {
+ RELEASE_LOG_ERROR(WebRTC, "Adding ice candidate failed %d", result.exception().code());
+ callback(result.releaseException());
+ return;
+ }
+
+ if (auto descriptions = result.releaseReturnValue())
+ peerConnection.updateDescriptions(WTFMove(*descriptions));
+ callback({ });
+ });
});
}
@@ -386,11 +385,6 @@
m_peerConnection.updateIceGatheringState(RTCIceGatheringState::Complete);
}
-void PeerConnectionBackend::endOfIceCandidates(DOMPromiseDeferred<void>&& promise)
-{
- promise.resolve();
-}
-
void PeerConnectionBackend::stop()
{
m_offerAnswerCallback = nullptr;
Modified: trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h (282637 => 282638)
--- trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h 2021-09-17 07:02:29 UTC (rev 282637)
+++ trunk/Source/WebCore/Modules/mediastream/PeerConnectionBackend.h 2021-09-17 07:07:07 UTC (rev 282638)
@@ -98,7 +98,7 @@
void createAnswer(RTCAnswerOptions&&, CreateCallback&&);
void setLocalDescription(const RTCSessionDescription*, Function<void(ExceptionOr<void>&&)>&&);
void setRemoteDescription(const RTCSessionDescription&, Function<void(ExceptionOr<void>&&)>&&);
- void addIceCandidate(RTCIceCandidate*, DOMPromiseDeferred<void>&&);
+ void addIceCandidate(RTCIceCandidate*, Function<void(ExceptionOr<void>&&)>&&);
virtual std::unique_ptr<RTCDataChannelHandler> createDataChannelHandler(const String&, const RTCDataChannelInit&) = 0;
@@ -229,7 +229,6 @@
virtual void doSetLocalDescription(const RTCSessionDescription*) = 0;
virtual void doSetRemoteDescription(const RTCSessionDescription&) = 0;
virtual void doAddIceCandidate(RTCIceCandidate&, AddIceCandidateCallback&&) = 0;
- virtual void endOfIceCandidates(DOMPromiseDeferred<void>&&);
virtual void doStop() = 0;
protected:
Modified: trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp (282637 => 282638)
--- trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp 2021-09-17 07:02:29 UTC (rev 282637)
+++ trunk/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp 2021-09-17 07:07:07 UTC (rev 282638)
@@ -346,17 +346,27 @@
});
}
+ ALWAYS_LOG(LOGIDENTIFIER, "Received ice candidate:\n", candidate ? candidate->candidate() : "null");
+
if (exception) {
promise->reject(*exception);
return;
}
+ if (candidate && candidate->sdpMid().isNull() && !candidate->sdpMLineIndex()) {
+ promise->reject(Exception { TypeError, "Trying to add a candidate that is missing both sdpMid and sdpMLineIndex"_s });
+ return;
+ }
+
if (isClosed())
return;
- ALWAYS_LOG(LOGIDENTIFIER, "Received ice candidate:\n", candidate ? candidate->candidate() : "null");
chainOperation(WTFMove(promise), [this, candidate = WTFMove(candidate)](auto&& promise) mutable {
- m_backend->addIceCandidate(candidate.get(), WTFMove(promise));
+ m_backend->addIceCandidate(candidate.get(), [protectedThis = makeRef(*this), promise = DOMPromiseDeferred<void>(WTFMove(promise))](auto&& result) mutable {
+ if (protectedThis->isClosed())
+ return;
+ promise.settle(WTFMove(result));
+ });
});
}
