Commit e0ea7be7 authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

Switch RTCRtpSenderImpl away from base::Bind

This CL changes this class and its inner ones to use
the CrossThreadBind variants.

It also delays the conversion to std types
RTCRtpSenderImpl::SetStreams until they are actually needed
(ie before calling libwebrtc). Basically Vector<String> plays
nicely with CrossThreadCopier, whereas std::vector<std::string>
does not.


BUG=787254
R=haraken@chromium.org

Change-Id: Ie747d0c58af1426f18c067460383bb210703d756
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984507Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#728381}
parent 46c6dbf8
...@@ -7,13 +7,30 @@ ...@@ -7,13 +7,30 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include "base/bind.h"
#include "base/logging.h" #include "base/logging.h"
#include "third_party/blink/renderer/platform/peerconnection/rtc_dtmf_sender_handler.h" #include "third_party/blink/renderer/platform/peerconnection/rtc_dtmf_sender_handler.h"
#include "third_party/blink/renderer/platform/peerconnection/rtc_stats.h" #include "third_party/blink/renderer/platform/peerconnection/rtc_stats.h"
#include "third_party/blink/renderer/platform/peerconnection/rtc_void_request.h" #include "third_party/blink/renderer/platform/peerconnection/rtc_void_request.h"
#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
#include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h" #include "third_party/blink/renderer/platform/wtf/thread_safe_ref_counted.h"
namespace WTF {
template <>
struct CrossThreadCopier<webrtc::RtpParameters>
: public CrossThreadCopierPassThrough<webrtc::RtpParameters> {
STATIC_ONLY(CrossThreadCopier);
};
template <>
struct CrossThreadCopier<webrtc::RTCError>
: public CrossThreadCopierPassThrough<webrtc::RTCError> {
STATIC_ONLY(CrossThreadCopier);
};
} // namespace WTF
namespace blink { namespace blink {
namespace { namespace {
...@@ -201,12 +218,13 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal ...@@ -201,12 +218,13 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal
track_ref = track_map_->GetOrCreateLocalTrackAdapter(with_track); track_ref = track_map_->GetOrCreateLocalTrackAdapter(with_track);
webrtc_track = track_ref->webrtc_track(); webrtc_track = track_ref->webrtc_track();
} }
signaling_task_runner_->PostTask( PostCrossThreadTask(
FROM_HERE, *signaling_task_runner_.get(), FROM_HERE,
base::BindOnce(&RTCRtpSenderImpl::RTCRtpSenderInternal:: CrossThreadBindOnce(&RTCRtpSenderImpl::RTCRtpSenderInternal::
ReplaceTrackOnSignalingThread, ReplaceTrackOnSignalingThread,
this, std::move(track_ref), WrapRefCounted(this), std::move(track_ref),
base::Unretained(webrtc_track), std::move(callback))); CrossThreadUnretained(webrtc_track),
CrossThreadBindOnce(std::move(callback))));
} }
std::unique_ptr<blink::RtcDtmfSenderHandler> GetDtmfSender() const { std::unique_ptr<blink::RtcDtmfSenderHandler> GetDtmfSender() const {
...@@ -250,20 +268,22 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal ...@@ -250,20 +268,22 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal
encoding.scale_resolution_down_by; encoding.scale_resolution_down_by;
} }
signaling_task_runner_->PostTask( PostCrossThreadTask(
FROM_HERE, *signaling_task_runner_.get(), FROM_HERE,
base::BindOnce(&RTCRtpSenderImpl::RTCRtpSenderInternal:: CrossThreadBindOnce(&RTCRtpSenderImpl::RTCRtpSenderInternal::
SetParametersOnSignalingThread, SetParametersOnSignalingThread,
this, std::move(new_parameters), std::move(callback))); WrapRefCounted(this), std::move(new_parameters),
CrossThreadBindOnce(std::move(callback))));
} }
void GetStats(RTCStatsReportCallback callback, void GetStats(RTCStatsReportCallback callback,
const Vector<webrtc::NonStandardGroupId>& exposed_group_ids) { const Vector<webrtc::NonStandardGroupId>& exposed_group_ids) {
signaling_task_runner_->PostTask( PostCrossThreadTask(
FROM_HERE, *signaling_task_runner_.get(), FROM_HERE,
base::BindOnce( CrossThreadBindOnce(
&RTCRtpSenderImpl::RTCRtpSenderInternal::GetStatsOnSignalingThread, &RTCRtpSenderImpl::RTCRtpSenderInternal::GetStatsOnSignalingThread,
this, std::move(callback), exposed_group_ids)); WrapRefCounted(this), CrossThreadBindOnce(std::move(callback)),
exposed_group_ids));
} }
bool RemoveFromPeerConnection(webrtc::PeerConnectionInterface* pc) { bool RemoveFromPeerConnection(webrtc::PeerConnectionInterface* pc) {
...@@ -278,12 +298,13 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal ...@@ -278,12 +298,13 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal
return true; return true;
} }
void SetStreams(std::vector<std::string> stream_ids) { void SetStreams(const Vector<String>& stream_ids) {
DCHECK(main_task_runner_->BelongsToCurrentThread()); DCHECK(main_task_runner_->BelongsToCurrentThread());
signaling_task_runner_->PostTask( PostCrossThreadTask(
FROM_HERE, base::BindOnce(&RTCRtpSenderImpl::RTCRtpSenderInternal:: *signaling_task_runner_.get(), FROM_HERE,
SetStreamsOnSignalingThread, CrossThreadBindOnce(&RTCRtpSenderImpl::RTCRtpSenderInternal::
this, std::move(stream_ids))); SetStreamsOnSignalingThread,
WrapRefCounted(this), stream_ids));
} }
private: private:
...@@ -299,61 +320,69 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal ...@@ -299,61 +320,69 @@ class RTCRtpSenderImpl::RTCRtpSenderInternal
// |webrtc_track| is passed as an argument because |track_ref->webrtc_track()| // |webrtc_track| is passed as an argument because |track_ref->webrtc_track()|
// cannot be accessed on the signaling thread. https://crbug.com/756436 // cannot be accessed on the signaling thread. https://crbug.com/756436
void ReplaceTrackOnSignalingThread( void ReplaceTrackOnSignalingThread(
std::unique_ptr<blink::WebRtcMediaStreamTrackAdapterMap::AdapterRef> std::unique_ptr<WebRtcMediaStreamTrackAdapterMap::AdapterRef> track_ref,
track_ref,
webrtc::MediaStreamTrackInterface* webrtc_track, webrtc::MediaStreamTrackInterface* webrtc_track,
base::OnceCallback<void(bool)> callback) { CrossThreadOnceFunction<void(bool)> callback) {
DCHECK(signaling_task_runner_->BelongsToCurrentThread()); DCHECK(signaling_task_runner_->BelongsToCurrentThread());
bool result = webrtc_sender_->SetTrack(webrtc_track); bool result = webrtc_sender_->SetTrack(webrtc_track);
main_task_runner_->PostTask( PostCrossThreadTask(
FROM_HERE, *main_task_runner_.get(), FROM_HERE,
base::BindOnce( CrossThreadBindOnce(
&RTCRtpSenderImpl::RTCRtpSenderInternal::ReplaceTrackCallback, this, &RTCRtpSenderImpl::RTCRtpSenderInternal::ReplaceTrackCallback,
result, std::move(track_ref), std::move(callback))); WrapRefCounted(this), result, std::move(track_ref),
std::move(callback)));
} }
void ReplaceTrackCallback( void ReplaceTrackCallback(
bool result, bool result,
std::unique_ptr<blink::WebRtcMediaStreamTrackAdapterMap::AdapterRef> std::unique_ptr<blink::WebRtcMediaStreamTrackAdapterMap::AdapterRef>
track_ref, track_ref,
base::OnceCallback<void(bool)> callback) { CrossThreadOnceFunction<void(bool)> callback) {
DCHECK(main_task_runner_->BelongsToCurrentThread()); DCHECK(main_task_runner_->BelongsToCurrentThread());
if (result) if (result)
state_.set_track_ref(std::move(track_ref)); state_.set_track_ref(std::move(track_ref));
std::move(callback).Run(result); std::move(callback).Run(result);
} }
using RTCStatsReportCallbackInternal =
CrossThreadOnceFunction<void(std::unique_ptr<RTCStatsReportPlatform>)>;
void GetStatsOnSignalingThread( void GetStatsOnSignalingThread(
RTCStatsReportCallback callback, RTCStatsReportCallbackInternal callback,
const Vector<webrtc::NonStandardGroupId>& exposed_group_ids) { const Vector<webrtc::NonStandardGroupId>& exposed_group_ids) {
native_peer_connection_->GetStats( native_peer_connection_->GetStats(
webrtc_sender_.get(), webrtc_sender_.get(),
CreateRTCStatsCollectorCallback(main_task_runner_, std::move(callback), CreateRTCStatsCollectorCallback(
exposed_group_ids)); main_task_runner_, ConvertToBaseOnceCallback(std::move(callback)),
exposed_group_ids));
} }
void SetParametersOnSignalingThread( void SetParametersOnSignalingThread(
webrtc::RtpParameters parameters, webrtc::RtpParameters parameters,
base::OnceCallback<void(webrtc::RTCError)> callback) { CrossThreadOnceFunction<void(webrtc::RTCError)> callback) {
DCHECK(signaling_task_runner_->BelongsToCurrentThread()); DCHECK(signaling_task_runner_->BelongsToCurrentThread());
webrtc::RTCError result = webrtc_sender_->SetParameters(parameters); webrtc::RTCError result = webrtc_sender_->SetParameters(parameters);
main_task_runner_->PostTask( PostCrossThreadTask(
FROM_HERE, *main_task_runner_.get(), FROM_HERE,
base::BindOnce( CrossThreadBindOnce(
&RTCRtpSenderImpl::RTCRtpSenderInternal::SetParametersCallback, &RTCRtpSenderImpl::RTCRtpSenderInternal::SetParametersCallback,
this, std::move(result), std::move(callback))); WrapRefCounted(this), std::move(result), std::move(callback)));
} }
void SetParametersCallback( void SetParametersCallback(
webrtc::RTCError result, webrtc::RTCError result,
base::OnceCallback<void(webrtc::RTCError)> callback) { CrossThreadOnceFunction<void(webrtc::RTCError)> callback) {
DCHECK(main_task_runner_->BelongsToCurrentThread()); DCHECK(main_task_runner_->BelongsToCurrentThread());
std::move(callback).Run(std::move(result)); std::move(callback).Run(std::move(result));
} }
void SetStreamsOnSignalingThread(std::vector<std::string> stream_ids) { void SetStreamsOnSignalingThread(const Vector<String>& stream_ids) {
DCHECK(signaling_task_runner_->BelongsToCurrentThread()); DCHECK(signaling_task_runner_->BelongsToCurrentThread());
webrtc_sender_->SetStreams(stream_ids); std::vector<std::string> ids;
for (auto stream_id : stream_ids)
ids.emplace_back(stream_id.Utf8());
webrtc_sender_->SetStreams(std::move(ids));
} }
const scoped_refptr<webrtc::PeerConnectionInterface> native_peer_connection_; const scoped_refptr<webrtc::PeerConnectionInterface> native_peer_connection_;
...@@ -373,11 +402,11 @@ struct RTCRtpSenderImpl::RTCRtpSenderInternalTraits { ...@@ -373,11 +402,11 @@ struct RTCRtpSenderImpl::RTCRtpSenderInternalTraits {
// RTCRtpSenderInternal owns AdapterRefs which have to be destroyed on the // RTCRtpSenderInternal owns AdapterRefs which have to be destroyed on the
// main thread, this ensures delete always happens there. // main thread, this ensures delete always happens there.
if (!sender->main_task_runner_->BelongsToCurrentThread()) { if (!sender->main_task_runner_->BelongsToCurrentThread()) {
sender->main_task_runner_->PostTask( PostCrossThreadTask(
FROM_HERE, *sender->main_task_runner_.get(), FROM_HERE,
base::BindOnce( CrossThreadBindOnce(
&RTCRtpSenderImpl::RTCRtpSenderInternalTraits::Destruct, &RTCRtpSenderImpl::RTCRtpSenderInternalTraits::Destruct,
base::Unretained(sender))); CrossThreadUnretained(sender)));
return; return;
} }
delete sender; delete sender;
...@@ -480,11 +509,7 @@ void RTCRtpSenderImpl::GetStats( ...@@ -480,11 +509,7 @@ void RTCRtpSenderImpl::GetStats(
} }
void RTCRtpSenderImpl::SetStreams(const Vector<String>& stream_ids) { void RTCRtpSenderImpl::SetStreams(const Vector<String>& stream_ids) {
std::vector<std::string> ids; internal_->SetStreams(stream_ids);
for (auto stream_id : stream_ids)
ids.emplace_back(stream_id.Utf8());
internal_->SetStreams(std::move(ids));
} }
void RTCRtpSenderImpl::ReplaceTrack(blink::WebMediaStreamTrack with_track, void RTCRtpSenderImpl::ReplaceTrack(blink::WebMediaStreamTrack with_track,
......
...@@ -108,9 +108,10 @@ class RTCRtpSenderImplTest : public ::testing::Test { ...@@ -108,9 +108,10 @@ class RTCRtpSenderImplTest : public ::testing::Test {
// On complete, |*result_holder| is set with the result of replaceTrack() // On complete, |*result_holder| is set with the result of replaceTrack()
// and the |run_loop| quit. // and the |run_loop| quit.
sender_->ReplaceTrack( sender_->ReplaceTrack(
web_track, base::BindOnce(&RTCRtpSenderImplTest::CallbackOnComplete, web_track,
base::Unretained(this), result_holder.get(), WTF::Bind(&RTCRtpSenderImplTest::CallbackOnComplete,
run_loop.get())); WTF::Unretained(this), WTF::Unretained(result_holder.get()),
WTF::Unretained(run_loop.get())));
// When the resulting callback is invoked, waits for |run_loop| to complete // When the resulting callback is invoked, waits for |run_loop| to complete
// and returns |*result_holder|. // and returns |*result_holder|.
return base::BindOnce(&RTCRtpSenderImplTest::RunLoopAndReturnResult, return base::BindOnce(&RTCRtpSenderImplTest::RunLoopAndReturnResult,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment