Skip to content
This repository has been archived by the owner on Oct 25, 2024. It is now read-only.

Commit

Permalink
Reland "Replace sigslot usages with robocaller library."
Browse files Browse the repository at this point in the history
This is a reland of 40261c3

Note: Instead of changing the type of JsepTransportController->SignalSSLHandshakeError
added a new member with a different name and used it in webrtc code.
After this change do two more follow up CLs to completely remove the old code
from google3.

Original change's description:
> Replace sigslot usages with robocaller library.
>
> - Replace all the top level signals from jsep_transport_controller.
> - There are still sigslot usages in this file so keep the inheritance
>   and that is the reason for not having a binary size gain in this CL.
>
> Bug: webrtc:11943
> Change-Id: I249d3b9710783aef70ba273e082ceeafe3056898
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185540
> Commit-Queue: Lahiru Ginnaliya Gamathige <[email protected]>
> Reviewed-by: Mirko Bonadei <[email protected]>
> Reviewed-by: Karl Wiberg <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#32321}

Bug: webrtc:11943
Change-Id: Ia07394ee395f94836f6b576c3a97d119a7678e1a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186946
Commit-Queue: Lahiru Ginnaliya Gamathige <[email protected]>
Reviewed-by: Karl Wiberg <[email protected]>
Cr-Commit-Position: refs/heads/master@{#32359}
  • Loading branch information
Lahiru Ginnaliya Gamathige authored and Commit Bot committed Oct 9, 2020
1 parent 0591fbb commit c5f7108
Show file tree
Hide file tree
Showing 11 changed files with 105 additions and 55 deletions.
1 change: 1 addition & 0 deletions p2p/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ rtc_library("rtc_p2p") {
"../logging:ice_log",
"../rtc_base",
"../rtc_base:checks",
"../rtc_base:robo_caller",
"../rtc_base:rtc_numerics",
"../rtc_base/experiments:field_trial_parser",
"../rtc_base/synchronization:sequence_checker",
Expand Down
6 changes: 4 additions & 2 deletions p2p/base/dtls_transport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "rtc_base/checks.h"
#include "rtc_base/dscp.h"
#include "rtc_base/logging.h"
#include "rtc_base/robo_caller.h"
#include "rtc_base/rtc_certificate.h"
#include "rtc_base/ssl_stream_adapter.h"
#include "rtc_base/stream.h"
Expand Down Expand Up @@ -358,8 +359,8 @@ bool DtlsTransport::SetupDtls() {
dtls_->SetMaxProtocolVersion(ssl_max_version_);
dtls_->SetServerRole(*dtls_role_);
dtls_->SignalEvent.connect(this, &DtlsTransport::OnDtlsEvent);
dtls_->SignalSSLHandshakeError.connect(this,
&DtlsTransport::OnDtlsHandshakeError);
dtls_->SSLHandshakeErrorSignal.AddReceiver(
[this](rtc::SSLHandshakeError e) { OnDtlsHandshakeError(e); });
if (remote_fingerprint_value_.size() &&
!dtls_->SetPeerCertificateDigest(
remote_fingerprint_algorithm_,
Expand Down Expand Up @@ -821,6 +822,7 @@ void DtlsTransport::set_dtls_state(DtlsTransportState state) {

void DtlsTransport::OnDtlsHandshakeError(rtc::SSLHandshakeError error) {
SignalDtlsHandshakeError(error);
DtlsHandshakeErrorSignal.Send(error);
}

void DtlsTransport::ConfigureHandshakeTimeout() {
Expand Down
3 changes: 3 additions & 0 deletions p2p/base/dtls_transport_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "p2p/base/ice_transport_internal.h"
#include "p2p/base/packet_transport_internal.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/robo_caller.h"
#include "rtc_base/ssl_certificate.h"
#include "rtc_base/ssl_fingerprint.h"
#include "rtc_base/ssl_stream_adapter.h"
Expand Down Expand Up @@ -115,7 +116,9 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal {
sigslot::signal2<DtlsTransportInternal*, DtlsTransportState> SignalDtlsState;

// Emitted whenever the Dtls handshake failed on some transport channel.
// TODO(bugs.webrtc.org/11943): Remove sigslot and use one variable.
sigslot::signal1<rtc::SSLHandshakeError> SignalDtlsHandshakeError;
webrtc::RoboCaller<rtc::SSLHandshakeError> DtlsHandshakeErrorSignal;

protected:
DtlsTransportInternal();
Expand Down
32 changes: 17 additions & 15 deletions pc/jsep_transport_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,8 @@ JsepTransportController::CreateDtlsTransport(
this, &JsepTransportController::OnTransportWritableState_n);
dtls->SignalReceivingState.connect(
this, &JsepTransportController::OnTransportReceivingState_n);
dtls->SignalDtlsHandshakeError.connect(
this, &JsepTransportController::OnDtlsHandshakeError);
dtls->DtlsHandshakeErrorSignal.AddReceiver(
[this](rtc::SSLHandshakeError error) { OnDtlsHandshakeError(error); });
dtls->ice_transport()->SignalGatheringState.connect(
this, &JsepTransportController::OnTransportGatheringState_n);
dtls->ice_transport()->SignalCandidateGathered.connect(
Expand Down Expand Up @@ -1154,7 +1154,8 @@ void JsepTransportController::OnTransportCandidateGathered_n(
std::string transport_name = transport->transport_name();
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_, [this, transport_name, candidate] {
SignalIceCandidatesGathered(transport_name, {candidate});
SignalIceCandidatesGathered.Send(
transport_name, std::vector<cricket::Candidate>{candidate});
});
}

Expand All @@ -1163,20 +1164,21 @@ void JsepTransportController::OnTransportCandidateError_n(
const cricket::IceCandidateErrorEvent& event) {
RTC_DCHECK(network_thread_->IsCurrent());

invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_,
[this, event] { SignalIceCandidateError(event); });
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_, [this, event] {
SignalIceCandidateError.Send(event);
});
}
void JsepTransportController::OnTransportCandidatesRemoved_n(
cricket::IceTransportInternal* transport,
const cricket::Candidates& candidates) {
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_,
[this, candidates] { SignalIceCandidatesRemoved(candidates); });
[this, candidates] { SignalIceCandidatesRemoved.Send(candidates); });
}
void JsepTransportController::OnTransportCandidatePairChanged_n(
const cricket::CandidatePairChangeEvent& event) {
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_, [this, event] {
SignalIceCandidatePairChanged(event);
SignalIceCandidatePairChanged.Send(event);
});
}

Expand Down Expand Up @@ -1317,14 +1319,14 @@ void JsepTransportController::UpdateAggregateStates_n() {
PeerConnectionInterface::kIceConnectionCompleted) {
// Ensure that we never skip over the "connected" state.
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_, [this] {
SignalStandardizedIceConnectionState(
SignalStandardizedIceConnectionState.Send(
PeerConnectionInterface::kIceConnectionConnected);
});
}
standardized_ice_connection_state_ = new_ice_connection_state;
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_, [this, new_ice_connection_state] {
SignalStandardizedIceConnectionState(new_ice_connection_state);
SignalStandardizedIceConnectionState.Send(new_ice_connection_state);
});
}

Expand Down Expand Up @@ -1378,7 +1380,7 @@ void JsepTransportController::UpdateAggregateStates_n() {
combined_connection_state_ = new_combined_state;
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_,
[this, new_combined_state] {
SignalConnectionState(new_combined_state);
SignalConnectionState.Send(new_combined_state);
});
}

Expand All @@ -1392,10 +1394,10 @@ void JsepTransportController::UpdateAggregateStates_n() {
}
if (ice_gathering_state_ != new_gathering_state) {
ice_gathering_state_ = new_gathering_state;
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_,
[this, new_gathering_state] {
SignalIceGatheringState(new_gathering_state);
});
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_, [this, new_gathering_state] {
SignalIceGatheringState.Send(new_gathering_state);
});
}
}

Expand All @@ -1408,7 +1410,7 @@ void JsepTransportController::OnRtcpPacketReceived_n(

void JsepTransportController::OnDtlsHandshakeError(
rtc::SSLHandshakeError error) {
SignalDtlsHandshakeError(error);
SignalDtlsHandshakeError.Send(error);
}

} // namespace webrtc
20 changes: 9 additions & 11 deletions pc/jsep_transport_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,31 +200,29 @@ class JsepTransportController : public sigslot::has_slots<> {
// Else => connecting
RoboCaller<cricket::IceConnectionState> SignalIceConnectionState;

sigslot::signal1<PeerConnectionInterface::PeerConnectionState>
RoboCaller<PeerConnectionInterface::PeerConnectionState>
SignalConnectionState;

sigslot::signal1<PeerConnectionInterface::IceConnectionState>
RoboCaller<PeerConnectionInterface::IceConnectionState>
SignalStandardizedIceConnectionState;

// If all transports done gathering => complete,
// Else if any are gathering => gathering,
// Else => new
sigslot::signal1<cricket::IceGatheringState> SignalIceGatheringState;
RoboCaller<cricket::IceGatheringState> SignalIceGatheringState;

// (mid, candidates)
sigslot::signal2<const std::string&, const std::vector<cricket::Candidate>&>
// [mid, candidates]
RoboCaller<const std::string&, const std::vector<cricket::Candidate>&>
SignalIceCandidatesGathered;

sigslot::signal1<const cricket::IceCandidateErrorEvent&>
SignalIceCandidateError;
RoboCaller<const cricket::IceCandidateErrorEvent&> SignalIceCandidateError;

sigslot::signal1<const std::vector<cricket::Candidate>&>
SignalIceCandidatesRemoved;
RoboCaller<const std::vector<cricket::Candidate>&> SignalIceCandidatesRemoved;

sigslot::signal1<const cricket::CandidatePairChangeEvent&>
RoboCaller<const cricket::CandidatePairChangeEvent&>
SignalIceCandidatePairChanged;

sigslot::signal1<rtc::SSLHandshakeError> SignalDtlsHandshakeError;
RoboCaller<rtc::SSLHandshakeError> SignalDtlsHandshakeError;

private:
RTCError ApplyDescription_n(bool local,
Expand Down
26 changes: 18 additions & 8 deletions pc/jsep_transport_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,24 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
[this](cricket::IceConnectionState s) {
JsepTransportControllerTest::OnConnectionState(s);
});
transport_controller_->SignalStandardizedIceConnectionState.connect(
this, &JsepTransportControllerTest::OnStandardizedIceConnectionState);
transport_controller_->SignalConnectionState.connect(
this, &JsepTransportControllerTest::OnCombinedConnectionState);
transport_controller_->SignalIceGatheringState.connect(
this, &JsepTransportControllerTest::OnGatheringState);
transport_controller_->SignalIceCandidatesGathered.connect(
this, &JsepTransportControllerTest::OnCandidatesGathered);
transport_controller_->SignalConnectionState.AddReceiver(
[this](PeerConnectionInterface::PeerConnectionState s) {
JsepTransportControllerTest::OnCombinedConnectionState(s);
});
transport_controller_->SignalStandardizedIceConnectionState.AddReceiver(
[this](PeerConnectionInterface::IceConnectionState s) {
JsepTransportControllerTest::OnStandardizedIceConnectionState(s);
});
transport_controller_->SignalIceGatheringState.AddReceiver(
[this](cricket::IceGatheringState s) {
JsepTransportControllerTest::OnGatheringState(s);
});
transport_controller_->SignalIceCandidatesGathered.AddReceiver(
[this](const std::string& transport,
const std::vector<cricket::Candidate>& candidates) {
JsepTransportControllerTest::OnCandidatesGathered(transport,
candidates);
});
}

std::unique_ptr<cricket::SessionDescription>
Expand Down
58 changes: 41 additions & 17 deletions pc/peer_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -545,28 +545,52 @@ bool PeerConnection::Initialize(
transport_controller_.reset(new JsepTransportController(
signaling_thread(), network_thread(), port_allocator_.get(),
async_resolver_factory_.get(), config));
transport_controller_->SignalStandardizedIceConnectionState.connect(
this, &PeerConnection::SetStandardizedIceConnectionState);
transport_controller_->SignalConnectionState.connect(
this, &PeerConnection::SetConnectionState);
transport_controller_->SignalIceGatheringState.connect(
this, &PeerConnection::OnTransportControllerGatheringState);
transport_controller_->SignalIceCandidatesGathered.connect(
this, &PeerConnection::OnTransportControllerCandidatesGathered);
transport_controller_->SignalIceCandidateError.connect(
this, &PeerConnection::OnTransportControllerCandidateError);
transport_controller_->SignalIceCandidatesRemoved.connect(
this, &PeerConnection::OnTransportControllerCandidatesRemoved);
transport_controller_->SignalDtlsHandshakeError.connect(
this, &PeerConnection::OnTransportControllerDtlsHandshakeError);
transport_controller_->SignalIceCandidatePairChanged.connect(
this, &PeerConnection::OnTransportControllerCandidateChanged);

transport_controller_->SignalIceConnectionState.AddReceiver(
[this](cricket::IceConnectionState s) {
RTC_DCHECK_RUN_ON(signaling_thread());
OnTransportControllerConnectionState(s);
});
transport_controller_->SignalConnectionState.AddReceiver(
[this](PeerConnectionInterface::PeerConnectionState s) {
RTC_DCHECK_RUN_ON(signaling_thread());
SetConnectionState(s);
});
transport_controller_->SignalStandardizedIceConnectionState.AddReceiver(
[this](PeerConnectionInterface::IceConnectionState s) {
RTC_DCHECK_RUN_ON(signaling_thread());
SetStandardizedIceConnectionState(s);
});
transport_controller_->SignalIceGatheringState.AddReceiver(
[this](cricket::IceGatheringState s) {
RTC_DCHECK_RUN_ON(signaling_thread());
OnTransportControllerGatheringState(s);
});
transport_controller_->SignalIceCandidatesGathered.AddReceiver(
[this](const std::string& transport,
const std::vector<cricket::Candidate>& candidates) {
RTC_DCHECK_RUN_ON(signaling_thread());
OnTransportControllerCandidatesGathered(transport, candidates);
});
transport_controller_->SignalIceCandidateError.AddReceiver(
[this](const cricket::IceCandidateErrorEvent& event) {
RTC_DCHECK_RUN_ON(signaling_thread());
OnTransportControllerCandidateError(event);
});
transport_controller_->SignalIceCandidatesRemoved.AddReceiver(
[this](const std::vector<cricket::Candidate>& c) {
RTC_DCHECK_RUN_ON(signaling_thread());
OnTransportControllerCandidatesRemoved(c);
});
transport_controller_->SignalIceCandidatePairChanged.AddReceiver(
[this](const cricket::CandidatePairChangeEvent& event) {
RTC_DCHECK_RUN_ON(signaling_thread());
OnTransportControllerCandidateChanged(event);
});
transport_controller_->SignalDtlsHandshakeError.AddReceiver(
[this](rtc::SSLHandshakeError event) {
RTC_DCHECK_RUN_ON(signaling_thread());
OnTransportControllerDtlsHandshakeError(event);
});

stats_.reset(new StatsCollector(this));
stats_collector_ = RTCStatsCollector::Create(this);
Expand Down
1 change: 1 addition & 0 deletions rtc_base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ rtc_library("rtc_base") {
deps = [
":checks",
":deprecation",
":robo_caller",
":rtc_task_queue",
":stringutils",
"../api:array_view",
Expand Down
2 changes: 2 additions & 0 deletions rtc_base/openssl_stream_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "rtc_base/openssl_adapter.h"
#include "rtc_base/openssl_digest.h"
#include "rtc_base/openssl_identity.h"
#include "rtc_base/robo_caller.h"
#include "rtc_base/ssl_certificate.h"
#include "rtc_base/stream.h"
#include "rtc_base/task_utils/to_queued_task.h"
Expand Down Expand Up @@ -931,6 +932,7 @@ int OpenSSLStreamAdapter::ContinueSSL() {
RTC_DLOG(LS_VERBOSE) << " -- error " << code << ", " << err_code << ", "
<< ERR_GET_REASON(err_code);
SignalSSLHandshakeError(ssl_handshake_err);
SSLHandshakeErrorSignal.Send(ssl_handshake_err);
return (ssl_error != 0) ? ssl_error : -1;
}

Expand Down
3 changes: 3 additions & 0 deletions rtc_base/ssl_stream_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "absl/memory/memory.h"
#include "rtc_base/deprecation.h"
#include "rtc_base/robo_caller.h"
#include "rtc_base/ssl_certificate.h"
#include "rtc_base/ssl_identity.h"
#include "rtc_base/stream.h"
Expand Down Expand Up @@ -268,7 +269,9 @@ class SSLStreamAdapter : public StreamAdapterInterface {
// authentication.
bool GetClientAuthEnabled() const { return client_auth_enabled_; }

// TODO(bugs.webrtc.org/11943): Remove sigslot and use one variable.
sigslot::signal1<SSLHandshakeError> SignalSSLHandshakeError;
webrtc::RoboCaller<SSLHandshakeError> SSLHandshakeErrorSignal;

private:
// If true (default), the client is required to provide a certificate during
Expand Down
8 changes: 6 additions & 2 deletions test/peer_scenario/scenario_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,12 @@ void ScenarioIceConnectionImpl::SetRemoteSdp(SdpType type,
const std::string& remote_sdp) {
RTC_DCHECK_RUN_ON(signaling_thread_);
remote_description_ = webrtc::CreateSessionDescription(type, remote_sdp);
jsep_controller_->SignalIceCandidatesGathered.connect(
this, &ScenarioIceConnectionImpl::OnCandidates);
jsep_controller_->SignalIceCandidatesGathered.AddReceiver(
[this](const std::string& transport,
const std::vector<cricket::Candidate>& candidate) {
ScenarioIceConnectionImpl::OnCandidates(transport, candidate);
});

auto res = jsep_controller_->SetRemoteDescription(
remote_description_->GetType(), remote_description_->description());
RTC_CHECK(res.ok()) << res.message();
Expand Down

0 comments on commit c5f7108

Please sign in to comment.