⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 282444 in webkit


Ignore:
Timestamp:
Sep 15, 2021, 1:19:01 AM (5 years ago)
Author:
youenn@apple.com
Message:

Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription 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:
Location:
trunk/Source
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/ThirdParty/libwebrtc/ChangeLog

    r282197 r282444  
     12021-09-15  Youenn Fablet  <youenn@apple.com>
     2
     3        Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription variants
     4        https://bugs.webkit.org/show_bug.cgi?id=230262
     5
     6        Reviewed by Darin Adler.
     7
     8        * Configurations/libwebrtc.iOS.exp:
     9        * Configurations/libwebrtc.iOSsim.exp:
     10        * Configurations/libwebrtc.mac.exp:
     11
    1122021-09-09  Youenn Fablet  <youenn@apple.com>
    213

  • trunk/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOS.exp

    r282197 r282444  
    2626__ZN6webrtc19RTCCertificateStats5kTypeE
    2727__ZN6webrtc19RTCDataChannelStats5kTypeE
    28 __ZN6webrtc24CreateSessionDescriptionERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEES8_PNS_13SdpParseErrorE
     28__ZN6webrtc24CreateSessionDescriptionENS_7SdpTypeERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS_13SdpParseErrorE
    2929__ZN6webrtc24RTCIceCandidatePairStats5kTypeE
    3030__ZN6webrtc24RTCInboundRTPStreamStats5kTypeE

  • trunk/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.iOSsim.exp

    r282197 r282444  
    2626__ZN6webrtc19RTCCertificateStats5kTypeE
    2727__ZN6webrtc19RTCDataChannelStats5kTypeE
    28 __ZN6webrtc24CreateSessionDescriptionERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEES8_PNS_13SdpParseErrorE
     28__ZN6webrtc24CreateSessionDescriptionENS_7SdpTypeERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS_13SdpParseErrorE
    2929__ZN6webrtc24RTCIceCandidatePairStats5kTypeE
    3030__ZN6webrtc24RTCInboundRTPStreamStats5kTypeE

  • trunk/Source/ThirdParty/libwebrtc/Configurations/libwebrtc.mac.exp

    r282197 r282444  
    2626__ZN6webrtc19RTCCertificateStats5kTypeE
    2727__ZN6webrtc19RTCDataChannelStats5kTypeE
    28 __ZN6webrtc24CreateSessionDescriptionERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEES8_PNS_13SdpParseErrorE
     28__ZN6webrtc24CreateSessionDescriptionENS_7SdpTypeERKNSt3__112basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEPNS_13SdpParseErrorE
    2929__ZN6webrtc24RTCIceCandidatePairStats5kTypeE
    3030__ZN6webrtc24RTCInboundRTPStreamStats5kTypeE

  • trunk/Source/WebCore/ChangeLog

    r282443 r282444  
     12021-09-15  Youenn Fablet  <youenn@apple.com>
     2
     3        Migrate to libwebrtc non-racy setRemoteDescription/setLocalDescription variants
     4        https://bugs.webkit.org/show_bug.cgi?id=230262
     5
     6        Reviewed by Darin Adler.
     7
     8        We were using the old versions of libwebrtc SetLocalDescription/SetRemoteDescription.
     9        As per the header file, these versions are potentially racy.
     10        This patch migrates to new versions of the same API which in addition take more modern parameters instead of raw pointers.
     11        We also modernize a bit the code by using methods manipulating unique_ptr instead of raw pointers.
     12        Drive-by fix: Add support for getting back to new for ICE gathering state. This was missing and without it,
     13        imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-explicit-rollback-iceGatheringState.html would time out.
     14        Update mock to implement the new method versions.
     15
     16        Covered by existing tests.
     17
     18        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
     19        (WebCore::sessionDescriptionType):
     20        (WebCore::LibWebRTCMediaEndpoint::doSetLocalDescription):
     21        (WebCore::LibWebRTCMediaEndpoint::doSetRemoteDescription):
     22        (WebCore::LibWebRTCMediaEndpoint::OnIceGatheringChange):
     23        * Modules/mediastream/libwebrtc/LibWebRTCObservers.h:
     24        * testing/MockLibWebRTCPeerConnection.cpp:
     25        (WebCore::MockLibWebRTCPeerConnection::SetLocalDescription):
     26        (WebCore::MockLibWebRTCPeerConnection::SetRemoteDescription):
     27        * testing/MockLibWebRTCPeerConnection.h:
     28
    1292021-09-15  Said Abou-Hallawa  <said@apple.com>
    230

  • trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp

    r282362 r282444  
    128128}
    129129
    130 static inline const char* sessionDescriptionType(RTCSdpType sdpType)
     130static inline webrtc::SdpType sessionDescriptionType(RTCSdpType sdpType)
    131131{
    132132    switch (sdpType) {
    133133    case RTCSdpType::Offer:
    134         return "offer";
     134        return webrtc::SdpType::kOffer;
    135135    case RTCSdpType::Pranswer:
    136         return "pranswer";
     136        return webrtc::SdpType::kPrAnswer;
    137137    case RTCSdpType::Answer:
    138         return "answer";
     138        return webrtc::SdpType::kAnswer;
    139139    case RTCSdpType::Rollback:
    140         return "rollback";
    141     }
    142 
    143     ASSERT_NOT_REACHED();
    144     return "";
     140        return webrtc::SdpType::kRollback;
     141    }
    145142}
    146143
    … …  
    155152
    156153    webrtc::SdpParseError error;
    157     std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description->type()), description->sdp().utf8().data(), &error));
     154    auto sessionDescription = webrtc::CreateSessionDescription(sessionDescriptionType(description->type()), description->sdp().utf8().data(), &error);
    158155
    159156    if (!sessionDescription) {
    … …  
    168165    }
    169166
    170     m_backend->SetLocalDescription(&m_setLocalSessionDescriptionObserver, sessionDescription.release());
     167    m_backend->SetLocalDescription(WTFMove(sessionDescription), &m_setLocalSessionDescriptionObserver);
    171168}
    172169
    … …  
    176173
    177174    webrtc::SdpParseError error;
    178     std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error));
     175    auto sessionDescription = webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error);
    179176    if (!sessionDescription) {
    180177        m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { SyntaxError, fromStdString(error.description) });
    181178        return;
    182179    }
    183     m_backend->SetRemoteDescription(&m_setRemoteSessionDescriptionObserver, sessionDescription.release());
     180
     181    m_backend->SetRemoteDescription(WTFMove(sessionDescription), &m_setRemoteSessionDescriptionObserver);
    184182
    185183    startLoggingStats();
    … …  
    539537        else if (state == webrtc::PeerConnectionInterface::kIceGatheringGathering)
    540538            protectedThis->m_peerConnectionBackend.connection().updateIceGatheringState(RTCIceGatheringState::Gathering);
     539        else if (state == webrtc::PeerConnectionInterface::kIceGatheringNew)
     540            protectedThis->m_peerConnectionBackend.connection().updateIceGatheringState(RTCIceGatheringState::New);
    541541    });
    542542}

  • trunk/Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCObservers.h

    r267799 r282444  
    6060
    6161template<typename Endpoint>
    62 class SetLocalSessionDescriptionObserver final : public webrtc::SetSessionDescriptionObserver {
     62class SetLocalSessionDescriptionObserver final : public webrtc::SetLocalDescriptionObserverInterface {
    6363public:
    6464    explicit SetLocalSessionDescriptionObserver(Endpoint &endpoint)
    … …  
    6767    }
    6868
    69     void OnSuccess() final { m_endpoint.setLocalSessionDescriptionSucceeded(); }
    70     void OnFailure(webrtc::RTCError error) final { m_endpoint.setLocalSessionDescriptionFailed(toExceptionCode(error.type()), error.message()); }
    71 
    7269    void AddRef() const { m_endpoint.AddRef(); }
    7370    rtc::RefCountReleaseStatus Release() const { return m_endpoint.Release(); }
    7471
    7572private:
     73    void OnSetLocalDescriptionComplete(webrtc::RTCError error) final
     74    {
     75        if (!error.ok()) {
     76            m_endpoint.setLocalSessionDescriptionFailed(toExceptionCode(error.type()), error.message());
     77            return;
     78        }
     79        m_endpoint.setLocalSessionDescriptionSucceeded();
     80    }
     81
    7682    Endpoint& m_endpoint;
    7783};
    7884
    7985template<typename Endpoint>
    80 class SetRemoteSessionDescriptionObserver final : public webrtc::SetSessionDescriptionObserver {
     86class SetRemoteSessionDescriptionObserver final : public webrtc::SetRemoteDescriptionObserverInterface {
    8187public:
    8288    explicit SetRemoteSessionDescriptionObserver(Endpoint &endpoint)
    … …  
    8591    }
    8692
    87     void OnSuccess() final { m_endpoint.setRemoteSessionDescriptionSucceeded(); }
    88     void OnFailure(webrtc::RTCError error) final { m_endpoint.setRemoteSessionDescriptionFailed(toExceptionCode(error.type()), error.message()); }
    89 
    9093    void AddRef() const { m_endpoint.AddRef(); }
    9194    rtc::RefCountReleaseStatus Release() const { return m_endpoint.Release(); }
    9295
    9396private:
     97    void OnSetRemoteDescriptionComplete(webrtc::RTCError error) final
     98    {
     99        if (!error.ok()) {
     100            m_endpoint.setRemoteSessionDescriptionFailed(toExceptionCode(error.type()), error.message());
     101            return;
     102        }
     103        m_endpoint.setRemoteSessionDescriptionSucceeded();
     104    }
     105
    94106    Endpoint& m_endpoint;
    95107};

  • trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.cpp

    r281985 r282444  
    206206
    207207private:
    208     void SetLocalDescription(webrtc::SetSessionDescriptionObserver* observer, webrtc::SessionDescriptionInterface* sessionDescription) final
     208    void SetLocalDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface> observer) final
    209209    {
    210         std::unique_ptr<webrtc::SessionDescriptionInterface> toBeFreed(sessionDescription);
    211210        releaseInNetworkThread(*this, *observer);
    212211    }
    … …  
    261260}
    262261
    263 void MockLibWebRTCPeerConnection::SetLocalDescription(webrtc::SetSessionDescriptionObserver* observer, webrtc::SessionDescriptionInterface* sessionDescription)
     262void MockLibWebRTCPeerConnection::SetLocalDescription(std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription, rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface> observer)
    264263{
    265264    bool isCorrectState = true;
    … …  
    283282    if (!isCorrectState) {
    284283        LibWebRTCProvider::callOnWebRTCSignalingThread([observer] {
    285             observer->OnFailure(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
     284            observer->OnSetLocalDescriptionComplete(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
    286285        });
    287286        return;
    288287    }
    289288
    290     std::unique_ptr<webrtc::SessionDescriptionInterface> toBeFreed(sessionDescription);
    291289    LibWebRTCProvider::callOnWebRTCSignalingThread([this, observer] {
    292         observer->OnSuccess();
     290        observer->OnSetLocalDescriptionComplete(webrtc::RTCError::OK());
    293291        gotLocalDescription();
    294292    });
    295293}
    296294
    297 void MockLibWebRTCPeerConnection::SetRemoteDescription(webrtc::SetSessionDescriptionObserver* observer, webrtc::SessionDescriptionInterface* sessionDescription)
     295void MockLibWebRTCPeerConnection::SetRemoteDescription(std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription, rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface> observer)
    298296{
    299297    bool isCorrectState = true;
    … …  
    317315    if (!isCorrectState) {
    318316        LibWebRTCProvider::callOnWebRTCSignalingThread([observer] {
    319             observer->OnFailure(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
     317            observer->OnSetRemoteDescriptionComplete(webrtc::RTCError(webrtc::RTCErrorType::INVALID_STATE));
    320318        });
    321319        return;
    322320    }
    323321
    324     std::unique_ptr<webrtc::SessionDescriptionInterface> toBeFreed(sessionDescription);
    325322    LibWebRTCProvider::callOnWebRTCSignalingThread([observer] {
    326         observer->OnSuccess();
     323        observer->OnSetRemoteDescriptionComplete(webrtc::RTCError::OK());
    327324    });
    328325    ASSERT(sessionDescription);

  • trunk/Source/WebCore/testing/MockLibWebRTCPeerConnection.h

    r281985 r282444  
    300300
    301301protected:
    302     void SetRemoteDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) final;
    303     void SetRemoteDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface>) override { }
     302    void SetRemoteDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) final { ASSERT_NOT_REACHED(); }
     303    void SetRemoteDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetRemoteDescriptionObserverInterface>) override;
    304304    bool RemoveIceCandidates(const std::vector<cricket::Candidate>&) override { return true; }
    305305    rtc::scoped_refptr<webrtc::DtlsTransportInterface> LookupDtlsTransportByMid(const std::string&) override { return { }; }
    … …  
    315315    webrtc::RTCError SetBitrate(const webrtc::BitrateSettings&) final { return { }; }
    316316
    317     void SetLocalDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) override;
     317    void SetLocalDescription(webrtc::SetSessionDescriptionObserver*, webrtc::SessionDescriptionInterface*) final { ASSERT_NOT_REACHED(); };
     318    void SetLocalDescription(std::unique_ptr<webrtc::SessionDescriptionInterface>, rtc::scoped_refptr<webrtc::SetLocalDescriptionObserverInterface>) override;
    318319    bool GetStats(webrtc::StatsObserver*, webrtc::MediaStreamTrackInterface*, StatsOutputLevel) override { return false; }
    319320    void CreateOffer(webrtc::CreateSessionDescriptionObserver*, const webrtc::PeerConnectionInterface::RTCOfferAnswerOptions&) override;
Note: See TracChangeset for help on using the changeset viewer.