Skip to content
Permalink
Browse files
Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescriptio…
…n variants

https://bugs.webkit.org/show_bug.cgi?id=230262

Reviewed by Darin Adler.

Source/ThirdParty/libwebrtc:

* Configurations/libwebrtc.iOS.exp:
* Configurations/libwebrtc.iOSsim.exp:
* Configurations/libwebrtc.mac.exp:

Source/WebCore:

We were using the old versions of libwebrtc SetLocalDescription/SetRemoteDescription.
As per the header file, these versions are potentially racy.
This patch migrates to new versions of the same API which in addition take more modern parameters instead of raw pointers.
We also modernize a bit the code by using methods manipulating unique_ptr instead of raw pointers.
Drive-by fix: Add support for getting back to new for ICE gathering state. This was missing and without it,
imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-explicit-rollback-iceGatheringState.html would time out.
Update mock to implement the new method versions.

Covered by existing tests.

* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::sessionDescriptionType):
(WebCore::LibWebRTCMediaEndpoint::doSetLocalDescription):
(WebCore::LibWebRTCMediaEndpoint::doSetRemoteDescription):
(WebCore::LibWebRTCMediaEndpoint::OnIceGatheringChange):
* Modules/mediastream/libwebrtc/LibWebRTCObservers.h:
* testing/MockLibWebRTCPeerConnection.cpp:
(WebCore::MockLibWebRTCPeerConnection::SetLocalDescription):
(WebCore::MockLibWebRTCPeerConnection::SetRemoteDescription):
* testing/MockLibWebRTCPeerConnection.h:


Canonical link: https://commits.webkit.org/241697@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282444 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
youennf committed Sep 15, 2021
1 parent 6537833 commit dabfb9ed21f56b0197299fe0fad4408f8bc9bfd8
Showing 9 changed files with 85 additions and 36 deletions.
@@ -1,3 +1,14 @@
2021-09-15 Youenn Fablet <youenn@apple.com>

Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription variants
https://bugs.webkit.org/show_bug.cgi?id=230262

Reviewed by Darin Adler.

* Configurations/libwebrtc.iOS.exp:
* Configurations/libwebrtc.iOSsim.exp:
* Configurations/libwebrtc.mac.exp:

2021-09-09 Youenn Fablet <youenn@apple.com>

Add support for RTCSctpTransport
@@ -25,7 +25,7 @@ __ZN6webrtc16ConvertVideoTypeENS_9VideoTypeE
__ZN6webrtc18CreateIceCandidateERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEEiS8_PNS_13SdpParseErrorE
__ZN6webrtc19RTCCertificateStats5kTypeE
__ZN6webrtc19RTCDataChannelStats5kTypeE
__ZN6webrtc24CreateSessionDescriptionERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEES8_PNS_13SdpParseErrorE
__ZN6webrtc24CreateSessionDescriptionENS_7SdpTypeERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS_13SdpParseErrorE
__ZN6webrtc24RTCIceCandidatePairStats5kTypeE
__ZN6webrtc24RTCInboundRTPStreamStats5kTypeE
__ZN6webrtc24RTCMediaStreamTrackStats5kTypeE
@@ -25,7 +25,7 @@ __ZN6webrtc16ConvertVideoTypeENS_9VideoTypeE
__ZN6webrtc18CreateIceCandidateERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEEiS8_PNS_13SdpParseErrorE
__ZN6webrtc19RTCCertificateStats5kTypeE
__ZN6webrtc19RTCDataChannelStats5kTypeE
__ZN6webrtc24CreateSessionDescriptionERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEES8_PNS_13SdpParseErrorE
__ZN6webrtc24CreateSessionDescriptionENS_7SdpTypeERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS_13SdpParseErrorE
__ZN6webrtc24RTCIceCandidatePairStats5kTypeE
__ZN6webrtc24RTCInboundRTPStreamStats5kTypeE
__ZN6webrtc24RTCMediaStreamTrackStats5kTypeE
@@ -25,7 +25,7 @@ __ZN6webrtc16ConvertVideoTypeENS_9VideoTypeE
__ZN6webrtc18CreateIceCandidateERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEEiS8_PNS_13SdpParseErrorE
__ZN6webrtc19RTCCertificateStats5kTypeE
__ZN6webrtc19RTCDataChannelStats5kTypeE
__ZN6webrtc24CreateSessionDescriptionERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEES8_PNS_13SdpParseErrorE
__ZN6webrtc24CreateSessionDescriptionENS_7SdpTypeERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS_13SdpParseErrorE
__ZN6webrtc24RTCIceCandidatePairStats5kTypeE
__ZN6webrtc24RTCInboundRTPStreamStats5kTypeE
__ZN6webrtc24RTCMediaStreamTrackStats5kTypeE
@@ -1,3 +1,31 @@
2021-09-15 Youenn Fablet <youenn@apple.com>

Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription variants
https://bugs.webkit.org/show_bug.cgi?id=230262

Reviewed by Darin Adler.

We were using the old versions of libwebrtc SetLocalDescription/SetRemoteDescription.
As per the header file, these versions are potentially racy.
This patch migrates to new versions of the same API which in addition take more modern parameters instead of raw pointers.
We also modernize a bit the code by using methods manipulating unique_ptr instead of raw pointers.
Drive-by fix: Add support for getting back to new for ICE gathering state. This was missing and without it,
imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-explicit-rollback-iceGatheringState.html would time out.
Update mock to implement the new method versions.

Covered by existing tests.

* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::sessionDescriptionType):
(WebCore::LibWebRTCMediaEndpoint::doSetLocalDescription):
(WebCore::LibWebRTCMediaEndpoint::doSetRemoteDescription):
(WebCore::LibWebRTCMediaEndpoint::OnIceGatheringChange):
* Modules/mediastream/libwebrtc/LibWebRTCObservers.h:
* testing/MockLibWebRTCPeerConnection.cpp:
(WebCore::MockLibWebRTCPeerConnection::SetLocalDescription):
(WebCore::MockLibWebRTCPeerConnection::SetRemoteDescription):
* testing/MockLibWebRTCPeerConnection.h:

2021-09-15 Said Abou-Hallawa <said@apple.com>

Linear gradient sometimes is drawn incorrectly and sometimes hangs
@@ -127,21 +127,18 @@ bool LibWebRTCMediaEndpoint::isNegotiationNeeded(uint32_t eventId) const
return m_backend ? m_backend->ShouldFireNegotiationNeededEvent(eventId) : false;
}

static inline const char* sessionDescriptionType(RTCSdpType sdpType)
static inline webrtc::SdpType sessionDescriptionType(RTCSdpType sdpType)
{
switch (sdpType) {
case RTCSdpType::Offer:
return "offer";
return webrtc::SdpType::kOffer;
case RTCSdpType::Pranswer:
return "pranswer";
return webrtc::SdpType::kPrAnswer;
case RTCSdpType::Answer:
return "answer";
return webrtc::SdpType::kAnswer;
case RTCSdpType::Rollback:
return "rollback";
return webrtc::SdpType::kRollback;
}

ASSERT_NOT_REACHED();
return "";
}

void LibWebRTCMediaEndpoint::doSetLocalDescription(const RTCSessionDescription* description)
@@ -154,7 +151,7 @@ void LibWebRTCMediaEndpoint::doSetLocalDescription(const RTCSessionDescription*
}

webrtc::SdpParseError error;
std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description->type()), description->sdp().utf8().data(), &error));
auto sessionDescription = webrtc::CreateSessionDescription(sessionDescriptionType(description->type()), description->sdp().utf8().data(), &error);

if (!sessionDescription) {
m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, fromStdString(error.description) });
@@ -167,20 +164,21 @@ void LibWebRTCMediaEndpoint::doSetLocalDescription(const RTCSessionDescription*
return;
}

m_backend->SetLocalDescription(&m_setLocalSessionDescriptionObserver, sessionDescription.release());
m_backend->SetLocalDescription(WTFMove(sessionDescription), &m_setLocalSessionDescriptionObserver);
}

void LibWebRTCMediaEndpoint::doSetRemoteDescription(const RTCSessionDescription& description)
{
ASSERT(m_backend);

webrtc::SdpParseError error;
std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error));
auto sessionDescription = webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error);
if (!sessionDescription) {
m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { SyntaxError, fromStdString(error.description) });
return;
}
m_backend->SetRemoteDescription(&m_setRemoteSessionDescriptionObserver, sessionDescription.release());

m_backend->SetRemoteDescription(WTFMove(sessionDescription), &m_setRemoteSessionDescriptionObserver);

startLoggingStats();
}
@@ -538,6 +536,8 @@ void LibWebRTCMediaEndpoint::OnIceGatheringChange(webrtc::PeerConnectionInterfac
protectedThis->m_peerConnectionBackend.doneGatheringCandidates();
else if (state == webrtc::PeerConnectionInterface::kIceGatheringGathering)
protectedThis->m_peerConnectionBackend.connection().updateIceGatheringState(RTCIceGatheringState::Gathering);
else if (state == webrtc::PeerConnectionInterface::kIceGatheringNew)
protectedThis->m_peerConnectionBackend.connection().updateIceGatheringState(RTCIceGatheringState::New);
});
}

@@ -59,38 +59,50 @@ class CreateSessionDescriptionObserver final : public webrtc::CreateSessionDescr
};

template<typename Endpoint>
class SetLocalSessionDescriptionObserver final : public webrtc::SetSessionDescriptionObserver {
class SetLocalSessionDescriptionObserver final : public webrtc::SetLocalDescriptionObserverInterface {
public:
explicit SetLocalSessionDescriptionObserver(Endpoint &endpoint)
: m_endpoint(endpoint)
{
}

void OnSuccess() final { m_endpoint.setLocalSessionDescriptionSucceeded(); }
void OnFailure(webrtc::RTCError error) final { m_endpoint.setLocalSessionDescriptionFailed(toExceptionCode(error.type()), error.message()); }

void AddRef() const { m_endpoint.AddRef(); }
rtc::RefCountReleaseStatus Release() const { return m_endpoint.Release(); }

private:
void OnSetLocalDescriptionComplete(webrtc::RTCError error) final
{
if (!error.ok()) {
m_endpoint.setLocalSessionDescriptionFailed(toExceptionCode(error.type()), error.message());
return;
}
m_endpoint.setLocalSessionDescriptionSucceeded();
}

Endpoint& m_endpoint;
};

template<typename Endpoint>
class SetRemoteSessionDescriptionObserver final : public webrtc::SetSessionDescriptionObserver {
class SetRemoteSessionDescriptionObserver final : public webrtc::SetRemoteDescriptionObserverInterface {
public:
explicit SetRemoteSessionDescriptionObserver(Endpoint &endpoint)
: m_endpoint(endpoint)
{
}

void OnSuccess() final { m_endpoint.setRemoteSessionDescriptionSucceeded(); }
void OnFailure(webrtc::RTCError error) final { m_endpoint.setRemoteSessionDescriptionFailed(toExceptionCode(error.type()), error.message()); }

void AddRef() const { m_endpoint.AddRef(); }
rtc::RefCountReleaseStatus Release() const { return m_endpoint.Release(); }

private:
void OnSetRemoteDescriptionComplete(webrtc::RTCError error) final
{
if (!error.ok()) {
m_endpoint.setRemoteSessionDescriptionFailed(toExceptionCode(error.type()), error.message());
return;
}
m_endpoint.setRemoteSessionDescriptionSucceeded();
}

Endpoint& m_endpoint;
};

@@ -205,9 +205,8 @@ class MockLibWebRTCPeerConnectionReleasedInNetworkThreadWhileSettingDescription
virtual ~MockLibWebRTCPeerConnectionReleasedInNetworkThreadWhileSettingDescription() = default;

private:
void SetLocalDescription(webrtc::SetSessionDescriptionObserver* observer, webrtc::SessionDescriptionInterface* sessionDescription) final
void SetLocalDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface> observer) final
{
std::unique_ptr<webrtc::SessionDescriptionInterface> toBeFreed(sessionDescription);
releaseInNetworkThread(*this, *observer);
}
};
@@ -260,7 +259,7 @@ rtc::scoped_refptr<webrtc::MediaStreamInterface> MockLibWebRTCPeerConnectionFact
return new rtc::RefCountedObject<webrtc::MediaStream>(label);
}

void MockLibWebRTCPeerConnection::SetLocalDescription(webrtc::SetSessionDescriptionObserver* observer, webrtc::SessionDescriptionInterface* sessionDescription)
void MockLibWebRTCPeerConnection::SetLocalDescription(std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription, rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface> observer)
{
bool isCorrectState = true;
switch (m_signalingState) {
@@ -282,19 +281,18 @@ void MockLibWebRTCPeerConnection::SetLocalDescription(webrtc::SetSessionDescript
}
if (!isCorrectState) {
LibWebRTCProvider::callOnWebRTCSignalingThread([observer] {
observer->OnFailure(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
observer->OnSetLocalDescriptionComplete(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
});
return;
}

std::unique_ptr<webrtc::SessionDescriptionInterface> toBeFreed(sessionDescription);
LibWebRTCProvider::callOnWebRTCSignalingThread([this, observer] {
observer->OnSuccess();
observer->OnSetLocalDescriptionComplete(webrtc::RTCError::OK());
gotLocalDescription();
});
}

void MockLibWebRTCPeerConnection::SetRemoteDescription(webrtc::SetSessionDescriptionObserver* observer, webrtc::SessionDescriptionInterface* sessionDescription)
void MockLibWebRTCPeerConnection::SetRemoteDescription(std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription, rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface> observer)
{
bool isCorrectState = true;
switch (m_signalingState) {
@@ -316,14 +314,13 @@ void MockLibWebRTCPeerConnection::SetRemoteDescription(webrtc::SetSessionDescrip
}
if (!isCorrectState) {
LibWebRTCProvider::callOnWebRTCSignalingThread([observer] {
observer->OnFailure(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
observer->OnSetRemoteDescriptionComplete(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
});
return;
}

std::unique_ptr<webrtc::SessionDescriptionInterface> toBeFreed(sessionDescription);
LibWebRTCProvider::callOnWebRTCSignalingThread([observer] {
observer->OnSuccess();
observer->OnSetRemoteDescriptionComplete(webrtc::RTCError::OK());
});
ASSERT(sessionDescription);
if (sessionDescription->type() == "offer") {
@@ -299,8 +299,8 @@ class MockLibWebRTCPeerConnection : public webrtc::PeerConnectionInterface {
std::vector<rtc::scoped_refptr<webrtc::RtpTransceiverInterface>> GetTransceivers() const final;

protected:
void SetRemoteDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) final;
void SetRemoteDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface>) override { }
void SetRemoteDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) final { ASSERT_NOT_REACHED(); }
void SetRemoteDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface>) override;
bool RemoveIceCandidates(const std::vector<cricket::Candidate>&) override { return true; }
rtc::scoped_refptr<webrtc::DtlsTransportInterface> LookupDtlsTransportByMid(const std::string&) override { return { }; }
rtc::scoped_refptr<webrtc::SctpTransportInterface> GetSctpTransport() const override { return { }; }
@@ -314,7 +314,8 @@ class MockLibWebRTCPeerConnection : public webrtc::PeerConnectionInterface {
bool RemoveTrack(webrtc::RtpSenderInterface*) final;
webrtc::RTCError SetBitrate(const webrtc::BitrateSettings&) final { return { }; }

void SetLocalDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) override;
void SetLocalDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) final { ASSERT_NOT_REACHED(); };
void SetLocalDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface>) override;
bool GetStats(webrtc::StatsObserver*, webrtc::MediaStreamTrackInterface*, StatsOutputLevel) override { return false; }
void CreateOffer(webrtc::CreateSessionDescriptionObserver*, const webrtc::PeerConnectionInterface::RTCOfferAnswerOptions&) override;

0 comments on commit dabfb9e

Please sign in to comment.