From 3881bedee2d430b6e73f2e52db9e6e337e47a067 Mon Sep 17 00:00:00 2001 From: Andreas Pehrson Date: Thu, 9 Feb 2023 15:33:03 +0000 Subject: [PATCH] Bug 1813491 - Simplify promises and stats shuffling in MediaTransportHandler::GetIceStats. r=bwc Differential Revision: https://phabricator.services.mozilla.com/D168731 --- dom/media/webrtc/PMediaTransport.ipdl | 4 +- dom/media/webrtc/WebrtcGlobal.h | 31 ---------- .../webrtc/jsapi/MediaTransportHandler.cpp | 40 ++++-------- .../webrtc/jsapi/MediaTransportHandlerIPC.cpp | 62 +++++++------------ .../webrtc/jsapi/MediaTransportParent.cpp | 7 +-- 5 files changed, 39 insertions(+), 105 deletions(-) diff --git a/dom/media/webrtc/PMediaTransport.ipdl b/dom/media/webrtc/PMediaTransport.ipdl index cf6aacf62496..b754d34ad2a9 100644 --- a/dom/media/webrtc/PMediaTransport.ipdl +++ b/dom/media/webrtc/PMediaTransport.ipdl @@ -14,7 +14,7 @@ using mozilla::StringVector from "mozilla/media/webrtc/WebrtcIPCTraits.h"; using mozilla::CandidateInfo from "mozilla/media/webrtc/WebrtcIPCTraits.h"; using mozilla::DtlsDigestList from "mozilla/media/webrtc/WebrtcIPCTraits.h"; using std::string from "string"; -using struct mozilla::dom::NotReallyMovableButLetsPretendItIsRTCStatsCollection from "mozilla/dom/RTCStatsReportBinding.h"; +using struct mozilla::dom::RTCStatsCollection from "mozilla/dom/RTCStatsReportBinding.h"; using WebrtcGlobalLog from "mozilla/media/webrtc/WebrtcGlobal.h"; using mozilla::dom::RTCIceServer from "mozilla/dom/RTCConfigurationBinding.h"; using mozilla::dom::RTCIceTransportPolicy from "mozilla/dom/RTCConfigurationBinding.h"; @@ -87,7 +87,7 @@ parent: async UpdateNetworkState(bool online); - async GetIceStats(string transportId, double now) returns (NotReallyMovableButLetsPretendItIsRTCStatsCollection stats); + async GetIceStats(string transportId, double now) returns (UniquePtr stats); child: async OnCandidate(string transportId, CandidateInfo candidateInfo); diff --git a/dom/media/webrtc/WebrtcGlobal.h b/dom/media/webrtc/WebrtcGlobal.h index 16a7b4531837..0dec8c03335b 100644 --- a/dom/media/webrtc/WebrtcGlobal.h +++ b/dom/media/webrtc/WebrtcGlobal.h @@ -19,22 +19,6 @@ typedef mozilla::dom::Sequence WebrtcGlobalLog; namespace mozilla { namespace dom { -// webidl dictionaries don't have move semantics, which is something that ipdl -// needs for async returns. So, we create a "moveable" subclass that just -// copies. _Really_ lame, but it gets the job done. -struct NotReallyMovableButLetsPretendItIsRTCStatsCollection - : public RTCStatsCollection { - NotReallyMovableButLetsPretendItIsRTCStatsCollection() = default; - explicit NotReallyMovableButLetsPretendItIsRTCStatsCollection( - RTCStatsCollection&& aStats) { - RTCStatsCollection::operator=(aStats); - } - explicit NotReallyMovableButLetsPretendItIsRTCStatsCollection( - const RTCStatsCollection& aStats) { - RTCStatsCollection::operator=(aStats); - } -}; - // Calls aFunction with all public members of aStats. // Typical usage would have aFunction take a parameter pack. // To avoid inconsistencies, this should be the only explicit list of the @@ -94,21 +78,6 @@ struct ParamTraits mozilla::dom::RTCIceCandidateType::Host, mozilla::dom::RTCIceCandidateType::EndGuard_> {}; -template <> -struct ParamTraits< - mozilla::dom::NotReallyMovableButLetsPretendItIsRTCStatsCollection> { - typedef mozilla::dom::NotReallyMovableButLetsPretendItIsRTCStatsCollection - paramType; - static void Write(MessageWriter* aWriter, const paramType& aParam) { - WriteParam(aWriter, - static_cast(aParam)); - } - static bool Read(MessageReader* aReader, paramType* aResult) { - return ReadParam(aReader, - static_cast(aResult)); - } -}; - template <> struct ParamTraits : public ContiguousEnumSerializer< diff --git a/dom/media/webrtc/jsapi/MediaTransportHandler.cpp b/dom/media/webrtc/jsapi/MediaTransportHandler.cpp index b60bea31bec6..f5abb708bc42 100644 --- a/dom/media/webrtc/jsapi/MediaTransportHandler.cpp +++ b/dom/media/webrtc/jsapi/MediaTransportHandler.cpp @@ -1275,35 +1275,17 @@ RefPtr MediaTransportHandlerSTS::GetIceStats( const std::string& aTransportId, DOMHighResTimeStamp aNow) { MOZ_RELEASE_ASSERT(mInitPromise); - return mInitPromise - ->Then( - mStsThread, __func__, - [=, self = RefPtr(this)]() { - UniquePtr stats( - new dom::RTCStatsCollection); - if (mIceCtx) { - for (const auto& stream : mIceCtx->GetStreams()) { - if (aTransportId.empty() || aTransportId == stream->GetId()) { - GetIceStats(*stream, aNow, stats.get()); - } - } - } - return dom::RTCStatsPromise::CreateAndResolve(std::move(stats), - __func__); - }) - ->Then(mStsThread, __func__, - [](dom::RTCStatsPromise::ResolveOrRejectValue&& aValue) { - // Eat errors! Caller is using MozPromise::All, and we don't - // want the whole thing to fail if this fails. - if (aValue.IsResolve()) { - return dom::RTCStatsPromise::CreateAndResolve( - std::move(aValue.ResolveValue()), __func__); - } - UniquePtr empty( - new dom::RTCStatsCollection); - return dom::RTCStatsPromise::CreateAndResolve(std::move(empty), - __func__); - }); + return mInitPromise->Then(mStsThread, __func__, [=, self = RefPtr(this)]() { + UniquePtr stats(new dom::RTCStatsCollection); + if (mIceCtx) { + for (const auto& stream : mIceCtx->GetStreams()) { + if (aTransportId.empty() || aTransportId == stream->GetId()) { + GetIceStats(*stream, aNow, stats.get()); + } + } + } + return dom::RTCStatsPromise::CreateAndResolve(std::move(stats), __func__); + }); } RefPtr diff --git a/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp b/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp index 0848080a8f34..d5930c93d94f 100644 --- a/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp +++ b/dom/media/webrtc/jsapi/MediaTransportHandlerIPC.cpp @@ -324,47 +324,33 @@ void MediaTransportHandlerIPC::UpdateNetworkState(bool aOnline) { RefPtr MediaTransportHandlerIPC::GetIceStats( const std::string& aTransportId, DOMHighResTimeStamp aNow) { + using IPCPromise = dom::PMediaTransportChild::GetIceStatsPromise; return mInitPromise - ->Then( - mCallbackThread, __func__, - [aTransportId, aNow, this, - self = RefPtr(this)](bool /*dummy*/) { - if (!mChild) { - return dom::RTCStatsPromise::CreateAndReject(NS_ERROR_FAILURE, - __func__); - } - RefPtr promise = - mChild->SendGetIceStats(aTransportId, aNow) - ->Then( - mCallbackThread, __func__, - [](const dom::RTCStatsCollection& aStats) { - UniquePtr stats( - new dom::RTCStatsCollection(aStats)); - return dom::RTCStatsPromise::CreateAndResolve( - std::move(stats), __func__); - }, - [](ipc::ResponseRejectReason aReason) { - return dom::RTCStatsPromise::CreateAndReject( - NS_ERROR_FAILURE, __func__); - }); - return promise; - }, - [](const nsCString& aError) { - return dom::RTCStatsPromise::CreateAndReject(NS_ERROR_FAILURE, - __func__); - }) ->Then(mCallbackThread, __func__, - [](dom::RTCStatsPromise::ResolveOrRejectValue&& aValue) { - // Eat errors! Caller is using MozPromise::All, and we don't - // want the whole thing to fail if this fails. - if (aValue.IsResolve()) { - return dom::RTCStatsPromise::CreateAndResolve( - std::move(aValue.ResolveValue()), __func__); + [aTransportId, aNow, this, self = RefPtr(this)]( + const InitPromise::ResolveOrRejectValue& aValue) { + if (aValue.IsReject()) { + return IPCPromise::CreateAndResolve( + MakeUnique(), + "MediaTransportHandlerIPC::GetIceStats_1"); } - UniquePtr empty( - new dom::RTCStatsCollection); - return dom::RTCStatsPromise::CreateAndResolve(std::move(empty), - __func__); + if (!mChild) { + return IPCPromise::CreateAndResolve( + MakeUnique(), + "MediaTransportHandlerIPC::GetIceStats_1"); + } + return mChild->SendGetIceStats(aTransportId, aNow); + }) + ->Then(mCallbackThread, __func__, + [](IPCPromise::ResolveOrRejectValue&& aValue) { + if (aValue.IsReject()) { + return dom::RTCStatsPromise::CreateAndResolve( + MakeUnique(), + "MediaTransportHandlerIPC::GetIceStats_2"); + } + return dom::RTCStatsPromise::CreateAndResolve( + std::move(aValue.ResolveValue()), + "MediaTransportHandlerIPC::GetIceStats_2"); }); } diff --git a/dom/media/webrtc/jsapi/MediaTransportParent.cpp b/dom/media/webrtc/jsapi/MediaTransportParent.cpp index 12a3f1da66fe..d0f90df8c5fe 100644 --- a/dom/media/webrtc/jsapi/MediaTransportParent.cpp +++ b/dom/media/webrtc/jsapi/MediaTransportParent.cpp @@ -226,12 +226,9 @@ mozilla::ipc::IPCResult MediaTransportParent::RecvGetIceStats( [aResolve = std::move(aResolve)]( dom::RTCStatsPromise::ResolveOrRejectValue&& aResult) { if (aResult.IsResolve()) { - aResolve( - dom::NotReallyMovableButLetsPretendItIsRTCStatsCollection( - *aResult.ResolveValue())); + aResolve(aResult.ResolveValue()); } else { - dom::NotReallyMovableButLetsPretendItIsRTCStatsCollection empty; - aResolve(empty); + aResolve(MakeUnique()); } });