Reland of TransportController refactoring. (patchset #1 id:1 of https://codereview.webrtc.org/1358413003/ )
Reason for revert:
This CL just landed: https://codereview.chromium.org/1323243006/
Which fixes the FYI bots for the original CL, and breaks them for this revert.
Original issue's description:
> Revert of TransportController refactoring. (patchset #6 id:100001 of https://codereview.webrtc.org/1350523003/ )
>
> Reason for revert:
> This CL causes problems with the WebRTC-in-Chromium FYI bots. Presumably it needs to be done in several steps, where removed files are emptied instead of removed in the first step.
>
> Original issue's description:
> > TransportController refactoring.
> >
> > Getting rid of TransportProxy, and in its place adding a
> > TransportController class which will facilitate access to and manage
> > the lifetimes of Transports. These Transports will now be accessed
> > solely from the worker thread, simplifying their implementation.
> >
> > This refactoring also pulls Transport-related code out of BaseSession.
> > Which means that BaseChannels will now rely on the TransportController
> > interface to create channels, rather than BaseSession.
> >
> > Committed: https://crrev.com/47ee2f3b9f33e8938948c482c921d4e13a3acd83
> > Cr-Commit-Position: refs/heads/master@{#10022}
>
> TBR=pthatcher@webrtc.org,deadbeef@webrtc.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
>
> Committed: https://crrev.com/a81a42f584baa0d93a4b93da9632415e8922450c
> Cr-Commit-Position: refs/heads/master@{#10024}
TBR=pthatcher@webrtc.org,torbjorng@webrtc.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Review URL: https://codereview.webrtc.org/1361773005
Cr-Commit-Position: refs/heads/master@{#10036}
diff --git a/talk/app/webrtc/objctests/RTCPeerConnectionSyncObserver.m b/talk/app/webrtc/objctests/RTCPeerConnectionSyncObserver.m
index 5070b78..daebc95 100644
--- a/talk/app/webrtc/objctests/RTCPeerConnectionSyncObserver.m
+++ b/talk/app/webrtc/objctests/RTCPeerConnectionSyncObserver.m
@@ -196,8 +196,11 @@
if (newState == RTCICEGatheringGathering) {
return;
}
+ NSAssert([_expectedICEGatheringChanges count] > 0,
+ @"Unexpected ICE gathering state change");
int expectedState = [self popFirstElementAsInt:_expectedICEGatheringChanges];
- NSAssert(expectedState == (int)newState, @"Empty expectation array");
+ NSAssert(expectedState == (int)newState,
+ @"ICE gathering state should match expectation");
}
- (void)peerConnection:(RTCPeerConnection*)peerConnection
@@ -205,8 +208,11 @@
// See TODO(fischman) in RTCPeerConnectionTest.mm about Completed.
if (newState == RTCICEConnectionCompleted)
return;
+ NSAssert([_expectedICEConnectionChanges count] > 0,
+ @"Unexpected ICE connection state change");
int expectedState = [self popFirstElementAsInt:_expectedICEConnectionChanges];
- NSAssert(expectedState == (int)newState, @"Empty expectation array");
+ NSAssert(expectedState == (int)newState,
+ @"ICE connection state should match expectation");
}
- (void)peerConnection:(RTCPeerConnection*)peerConnection
diff --git a/talk/app/webrtc/objctests/RTCPeerConnectionTest.mm b/talk/app/webrtc/objctests/RTCPeerConnectionTest.mm
index 050c9f4..f0703f5 100644
--- a/talk/app/webrtc/objctests/RTCPeerConnectionTest.mm
+++ b/talk/app/webrtc/objctests/RTCPeerConnectionTest.mm
@@ -182,7 +182,10 @@
EXPECT_GT([answerSDP.description length], 0);
[offeringExpectations expectICECandidates:2];
- [answeringExpectations expectICECandidates:2];
+ // It's possible to only have 1 ICE candidate for the answerer, since we use
+ // BUNDLE and rtcp-mux by default, and don't provide any ICE servers in this
+ // test.
+ [answeringExpectations expectICECandidates:1];
sdpObserver = [[RTCSessionDescriptionSyncObserver alloc] init];
[answeringExpectations expectSignalingChange:RTCSignalingStable];
diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index 41df847..1215655 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -616,6 +616,10 @@
}
SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer);
signaling_thread()->Post(this, MSG_SET_SESSIONDESCRIPTION_SUCCESS, msg);
+ // MaybeStartGathering needs to be called after posting
+ // MSG_SET_SESSIONDESCRIPTION_SUCCESS, so that we don't signal any candidates
+ // before signaling that SetLocalDescription completed.
+ session_->MaybeStartGathering();
}
void PeerConnection::SetRemoteDescription(
@@ -868,6 +872,11 @@
void PeerConnection::OnIceConnectionChange(
PeerConnectionInterface::IceConnectionState new_state) {
ASSERT(signaling_thread()->IsCurrent());
+ // After transitioning to "closed", ignore any additional states from
+ // WebRtcSession (such as "disconnected").
+ if (ice_connection_state_ == kIceConnectionClosed) {
+ return;
+ }
ice_connection_state_ = new_state;
observer_->OnIceConnectionChange(ice_connection_state_);
}
diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc
index 76ac76d..0212347 100644
--- a/talk/app/webrtc/statscollector.cc
+++ b/talk/app/webrtc/statscollector.cc
@@ -697,24 +697,18 @@
// expose them in stats reports. All channels in a transport share the
// same local and remote certificates.
//
- // Note that Transport::GetCertificate and Transport::GetRemoteCertificate
- // invoke method calls on the worker thread and block this thread, but
- // messages are still processed on this thread, which may blow way the
- // existing transports. So we cannot reuse |transport| after these calls.
StatsReport::Id local_cert_report_id, remote_cert_report_id;
-
- cricket::Transport* transport =
- session_->GetTransport(transport_iter.second.content_name);
rtc::scoped_refptr<rtc::RTCCertificate> certificate;
- if (transport && transport->GetCertificate(&certificate)) {
+ if (session_->GetLocalCertificate(transport_iter.second.transport_name,
+ &certificate)) {
StatsReport* r = AddCertificateReports(&(certificate->ssl_certificate()));
if (r)
local_cert_report_id = r->id();
}
- transport = session_->GetTransport(transport_iter.second.content_name);
rtc::scoped_ptr<rtc::SSLCertificate> cert;
- if (transport && transport->GetRemoteSSLCertificate(cert.accept())) {
+ if (session_->GetRemoteSSLCertificate(transport_iter.second.transport_name,
+ cert.accept())) {
StatsReport* r = AddCertificateReports(cert.get());
if (r)
remote_cert_report_id = r->id();
@@ -722,7 +716,7 @@
for (const auto& channel_iter : transport_iter.second.channel_stats) {
StatsReport::Id id(StatsReport::NewComponentId(
- transport_iter.second.content_name, channel_iter.component));
+ transport_iter.second.transport_name, channel_iter.component));
StatsReport* channel_report = reports_.ReplaceOrAddNew(id);
channel_report->set_timestamp(stats_gathering_started_);
channel_report->AddInt(StatsReport::kStatsValueNameComponent,
@@ -939,7 +933,6 @@
StatsReport* report = entry.second;
report->set_timestamp(stats_gathering_started_);
}
-
}
void StatsCollector::ClearUpdateStatsCacheForTest() {
diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc
index 9b037c4..0a9d947 100644
--- a/talk/app/webrtc/statscollector_unittest.cc
+++ b/talk/app/webrtc/statscollector_unittest.cc
@@ -27,6 +27,8 @@
#include <stdio.h>
+#include <algorithm>
+
#include "talk/app/webrtc/statscollector.h"
#include "talk/app/webrtc/mediastream.h"
@@ -45,7 +47,7 @@
#include "webrtc/base/fakesslidentity.h"
#include "webrtc/base/gunit.h"
#include "webrtc/base/network.h"
-#include "webrtc/p2p/base/fakesession.h"
+#include "webrtc/p2p/base/faketransportcontroller.h"
using rtc::scoped_ptr;
using testing::_;
@@ -89,7 +91,12 @@
MOCK_METHOD2(GetLocalTrackIdBySsrc, bool(uint32, std::string*));
MOCK_METHOD2(GetRemoteTrackIdBySsrc, bool(uint32, std::string*));
MOCK_METHOD1(GetTransportStats, bool(cricket::SessionStats*));
- MOCK_METHOD1(GetTransport, cricket::Transport*(const std::string&));
+ MOCK_METHOD2(GetLocalCertificate,
+ bool(const std::string& transport_name,
+ rtc::scoped_refptr<rtc::RTCCertificate>* certificate));
+ MOCK_METHOD2(GetRemoteSSLCertificate,
+ bool(const std::string& transport_name,
+ rtc::SSLCertificate** cert));
};
class MockVideoMediaChannel : public cricket::FakeVideoMediaChannel {
@@ -500,7 +507,7 @@
cricket::TransportStats transport_stats;
cricket::TransportChannelStats channel_stats;
channel_stats.component = 1;
- transport_stats.content_name = kTransportName;
+ transport_stats.transport_name = kTransportName;
transport_stats.channel_stats.push_back(channel_stats);
session_stats_.transport_stats[kTransportName] = transport_stats;
@@ -647,36 +654,27 @@
channel_stats.ssl_cipher = "the-ssl-cipher";
cricket::TransportStats transport_stats;
- transport_stats.content_name = "audio";
+ transport_stats.transport_name = "audio";
transport_stats.channel_stats.push_back(channel_stats);
cricket::SessionStats session_stats;
- session_stats.transport_stats[transport_stats.content_name] =
+ session_stats.transport_stats[transport_stats.transport_name] =
transport_stats;
- // Fake certificates to report.
+ // Fake certificate to report
rtc::scoped_refptr<rtc::RTCCertificate> local_certificate(
rtc::RTCCertificate::Create(rtc::scoped_ptr<rtc::FakeSSLIdentity>(
- new rtc::FakeSSLIdentity(local_cert)).Pass()));
- rtc::scoped_ptr<rtc::FakeSSLCertificate> remote_cert_copy(
- remote_cert.GetReference());
-
- // Fake transport object.
- rtc::scoped_ptr<cricket::FakeTransport> transport(
- new cricket::FakeTransport(
- session_.signaling_thread(),
- session_.worker_thread(),
- transport_stats.content_name));
- transport->SetCertificate(local_certificate);
- cricket::FakeTransportChannel* channel =
- static_cast<cricket::FakeTransportChannel*>(
- transport->CreateChannel(channel_stats.component));
- EXPECT_FALSE(channel == NULL);
- channel->SetRemoteSSLCertificate(remote_cert_copy.get());
+ new rtc::FakeSSLIdentity(local_cert))
+ .Pass()));
// Configure MockWebRtcSession
- EXPECT_CALL(session_, GetTransport(transport_stats.content_name))
- .WillRepeatedly(Return(transport.get()));
+ EXPECT_CALL(session_,
+ GetLocalCertificate(transport_stats.transport_name, _))
+ .WillOnce(DoAll(SetArgPointee<1>(local_certificate), Return(true)));
+ EXPECT_CALL(session_,
+ GetRemoteSSLCertificate(transport_stats.transport_name, _))
+ .WillOnce(
+ DoAll(SetArgPointee<1>(remote_cert.GetReference()), Return(true)));
EXPECT_CALL(session_, GetTransportStats(_))
.WillOnce(DoAll(SetArgPointee<0>(session_stats),
Return(true)));
@@ -790,14 +788,17 @@
TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) {
StatsCollectorForTest stats(&session_);
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
EXPECT_CALL(session_, GetTransportStats(_))
.WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
Return(true)));
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -833,14 +834,17 @@
TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) {
StatsCollectorForTest stats(&session_);
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
EXPECT_CALL(session_, GetTransportStats(_))
.WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
Return(true)));
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -946,13 +950,16 @@
TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) {
StatsCollectorForTest stats(&session_);
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
EXPECT_CALL(session_, GetTransportStats(_))
.WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
Return(true)));
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -1011,11 +1018,13 @@
TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) {
StatsCollectorForTest stats(&session_);
- // Ignore unused callback (logspam).
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
- // The content_name known by the video channel.
+ // The transport_name known by the video channel.
const std::string kVcName("vcname");
cricket::VideoChannel video_channel(rtc::Thread::Current(),
media_channel, NULL, kVcName, false);
@@ -1073,7 +1082,7 @@
StatsCollectorForTest stats(&session_);
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
- // The content_name known by the video channel.
+ // The transport_name known by the video channel.
const std::string kVcName("vcname");
cricket::VideoChannel video_channel(rtc::Thread::Current(),
media_channel, NULL, kVcName, false);
@@ -1096,11 +1105,13 @@
TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) {
StatsCollectorForTest stats(&session_);
- // Ignore unused callback (logspam).
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
- // The content_name known by the video channel.
+ // The transport_name known by the video channel.
const std::string kVcName("vcname");
cricket::VideoChannel video_channel(rtc::Thread::Current(),
media_channel, NULL, kVcName, false);
@@ -1145,13 +1156,16 @@
TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) {
StatsCollectorForTest stats(&session_);
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
EXPECT_CALL(session_, GetTransportStats(_))
.WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
Return(true)));
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -1330,6 +1344,11 @@
TEST_F(StatsCollectorTest, NoTransport) {
StatsCollectorForTest stats(&session_);
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
StatsReports reports; // returned values.
// Fake stats to process.
@@ -1337,16 +1356,14 @@
channel_stats.component = 1;
cricket::TransportStats transport_stats;
- transport_stats.content_name = "audio";
+ transport_stats.transport_name = "audio";
transport_stats.channel_stats.push_back(channel_stats);
cricket::SessionStats session_stats;
- session_stats.transport_stats[transport_stats.content_name] =
+ session_stats.transport_stats[transport_stats.transport_name] =
transport_stats;
// Configure MockWebRtcSession
- EXPECT_CALL(session_, GetTransport(transport_stats.content_name))
- .WillRepeatedly(ReturnNull());
EXPECT_CALL(session_, GetTransportStats(_))
.WillOnce(DoAll(SetArgPointee<0>(session_stats),
Return(true)));
@@ -1389,6 +1406,11 @@
TEST_F(StatsCollectorTest, NoCertificates) {
StatsCollectorForTest stats(&session_);
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
StatsReports reports; // returned values.
// Fake stats to process.
@@ -1396,23 +1418,18 @@
channel_stats.component = 1;
cricket::TransportStats transport_stats;
- transport_stats.content_name = "audio";
+ transport_stats.transport_name = "audio";
transport_stats.channel_stats.push_back(channel_stats);
cricket::SessionStats session_stats;
- session_stats.transport_stats[transport_stats.content_name] =
+ session_stats.transport_stats[transport_stats.transport_name] =
transport_stats;
// Fake transport object.
rtc::scoped_ptr<cricket::FakeTransport> transport(
- new cricket::FakeTransport(
- session_.signaling_thread(),
- session_.worker_thread(),
- transport_stats.content_name));
+ new cricket::FakeTransport(transport_stats.transport_name));
// Configure MockWebRtcSession
- EXPECT_CALL(session_, GetTransport(transport_stats.content_name))
- .WillRepeatedly(Return(transport.get()));
EXPECT_CALL(session_, GetTransportStats(_))
.WillOnce(DoAll(SetArgPointee<0>(session_stats),
Return(true)));
@@ -1458,12 +1475,13 @@
TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) {
StatsCollectorForTest stats(&session_);
- // Ignore unused callback (logspam).
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
- // The content_name known by the voice channel.
+ // The transport_name known by the voice channel.
const std::string kVcName("vcname");
cricket::VoiceChannel voice_channel(rtc::Thread::Current(),
media_engine_, media_channel, NULL, kVcName, false);
@@ -1492,11 +1510,13 @@
TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) {
StatsCollectorForTest stats(&session_);
- // Ignore unused callback (logspam).
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
- // The content_name known by the voice channel.
+ // The transport_name known by the voice channel.
const std::string kVcName("vcname");
cricket::VoiceChannel voice_channel(rtc::Thread::Current(),
media_engine_, media_channel, NULL, kVcName, false);
@@ -1519,11 +1539,13 @@
TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) {
StatsCollectorForTest stats(&session_);
- // Ignore unused callback (logspam).
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
- // The content_name known by the voice channel.
+ // The transport_name known by the voice channel.
const std::string kVcName("vcname");
cricket::VoiceChannel voice_channel(rtc::Thread::Current(),
media_engine_, media_channel, NULL, kVcName, false);
@@ -1578,11 +1600,13 @@
TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) {
StatsCollectorForTest stats(&session_);
- // Ignore unused callback (logspam).
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
- // The content_name known by the voice channel.
+ // The transport_name known by the voice channel.
const std::string kVcName("vcname");
cricket::VoiceChannel voice_channel(rtc::Thread::Current(),
media_engine_, media_channel, NULL, kVcName, false);
@@ -1663,11 +1687,13 @@
TEST_F(StatsCollectorTest, TwoLocalTracksWithSameSsrc) {
StatsCollectorForTest stats(&session_);
- // Ignore unused callback (logspam).
- EXPECT_CALL(session_, GetTransport(_))
- .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
+ .WillRepeatedly(Return(false));
+
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
- // The content_name known by the voice channel.
+ // The transport_name known by the voice channel.
const std::string kVcName("vcname");
cricket::VoiceChannel voice_channel(rtc::Thread::Current(),
media_engine_, media_channel, NULL, kVcName, false);
diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc
index 0c0e44d..bd1cd1a 100644
--- a/talk/app/webrtc/webrtcsession.cc
+++ b/talk/app/webrtc/webrtcsession.cc
@@ -31,6 +31,7 @@
#include <algorithm>
#include <vector>
+#include <set>
#include "talk/app/webrtc/jsepicecandidate.h"
#include "talk/app/webrtc/jsepsessiondescription.h"
@@ -86,6 +87,7 @@
"Couldn't set up DTLS-SRTP on RTP channel.";
const char kDtlsSetupFailureRtcp[] =
"Couldn't set up DTLS-SRTP on RTCP channel.";
+const char kEnableBundleFailed[] = "Failed to enable BUNDLE.";
const int kMaxUnsignalledRecvStreams = 20;
IceCandidatePairType GetIceCandidatePairCounter(
@@ -543,7 +545,6 @@
worker_thread,
port_allocator,
rtc::ToString(rtc::CreateRandomId64() & LLONG_MAX),
- cricket::NS_JINGLE_RTP,
false),
// RFC 3264: The numeric value of the session id and version in the
// o line MUST be representable with a "64 bit signed integer".
@@ -558,6 +559,14 @@
data_channel_type_(cricket::DCT_NONE),
ice_restart_latch_(new IceRestartAnswerLatch),
metrics_observer_(NULL) {
+ transport_controller()->SignalConnectionState.connect(
+ this, &WebRtcSession::OnTransportControllerConnectionState);
+ transport_controller()->SignalReceiving.connect(
+ this, &WebRtcSession::OnTransportControllerReceiving);
+ transport_controller()->SignalGatheringState.connect(
+ this, &WebRtcSession::OnTransportControllerGatheringState);
+ transport_controller()->SignalCandidatesGathered.connect(
+ this, &WebRtcSession::OnTransportControllerCandidatesGathered);
}
WebRtcSession::~WebRtcSession() {
@@ -583,12 +592,12 @@
bool WebRtcSession::Initialize(
const PeerConnectionFactoryInterface::Options& options,
- const MediaConstraintsInterface* constraints,
+ const MediaConstraintsInterface* constraints,
rtc::scoped_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
const PeerConnectionInterface::RTCConfiguration& rtc_configuration) {
bundle_policy_ = rtc_configuration.bundle_policy;
rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy;
- SetSslMaxProtocolVersion(options.ssl_max_version);
+ transport_controller()->SetSslMaxProtocolVersion(options.ssl_max_version);
// Obtain a certificate from RTCConfiguration if any were provided (optional).
rtc::scoped_refptr<rtc::RTCCertificate> certificate;
@@ -613,10 +622,8 @@
// Enable DTLS by default if we have an identity store or a certificate.
dtls_enabled_ = (dtls_identity_store || certificate);
// |constraints| can override the default |dtls_enabled_| value.
- if (FindConstraint(
- constraints,
- MediaConstraintsInterface::kEnableDtlsSrtp,
- &value, nullptr)) {
+ if (FindConstraint(constraints, MediaConstraintsInterface::kEnableDtlsSrtp,
+ &value, nullptr)) {
dtls_enabled_ = value;
}
}
@@ -736,35 +743,21 @@
if (!dtls_enabled_) {
// Construct with DTLS disabled.
webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory(
- signaling_thread(),
- channel_manager_,
- mediastream_signaling_,
- this,
- id(),
- data_channel_type_));
+ signaling_thread(), channel_manager_, mediastream_signaling_, this,
+ id(), data_channel_type_));
} else {
// Construct with DTLS enabled.
if (!certificate) {
// Use the |dtls_identity_store| to generate a certificate.
RTC_DCHECK(dtls_identity_store);
webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory(
- signaling_thread(),
- channel_manager_,
- mediastream_signaling_,
- dtls_identity_store.Pass(),
- this,
- id(),
- data_channel_type_));
+ signaling_thread(), channel_manager_, mediastream_signaling_,
+ dtls_identity_store.Pass(), this, id(), data_channel_type_));
} else {
// Use the already generated certificate.
webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory(
- signaling_thread(),
- channel_manager_,
- mediastream_signaling_,
- certificate,
- this,
- id(),
- data_channel_type_));
+ signaling_thread(), channel_manager_, mediastream_signaling_,
+ certificate, this, id(), data_channel_type_));
}
}
@@ -791,26 +784,12 @@
void WebRtcSession::Terminate() {
SetState(STATE_RECEIVEDTERMINATE);
- RemoveUnusedChannelsAndTransports(NULL);
+ RemoveUnusedChannels(NULL);
ASSERT(!voice_channel_);
ASSERT(!video_channel_);
ASSERT(!data_channel_);
}
-bool WebRtcSession::StartCandidatesAllocation() {
- // SpeculativelyConnectTransportChannels, will call ConnectChannels method
- // from TransportProxy to start gathering ice candidates.
- SpeculativelyConnectAllTransportChannels();
- if (!saved_candidates_.empty()) {
- // If there are saved candidates which arrived before local description is
- // set, copy those to remote description.
- CopySavedCandidates(remote_desc_.get());
- }
- // Push remote candidates present in remote description to transport channels.
- UseCandidatesInSessionDescription(remote_desc_.get());
- return true;
-}
-
void WebRtcSession::SetSdesPolicy(cricket::SecurePolicy secure_policy) {
webrtc_session_desc_factory_->SetSdesPolicy(secure_policy);
}
@@ -826,17 +805,7 @@
return false;
}
- // TODO(mallinath) - Return role of each transport, as role may differ from
- // one another.
- // In current implementaion we just return the role of first transport in the
- // transport map.
- for (cricket::TransportMap::const_iterator iter = transport_proxies().begin();
- iter != transport_proxies().end(); ++iter) {
- if (iter->second->impl()) {
- return iter->second->impl()->GetSslRole(role);
- }
- }
- return false;
+ return transport_controller()->GetSslRole(role);
}
void WebRtcSession::CreateOffer(
@@ -852,6 +821,8 @@
bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc,
std::string* err_desc) {
+ ASSERT(signaling_thread()->IsCurrent());
+
// Takes the ownership of |desc| regardless of the result.
rtc::scoped_ptr<SessionDescriptionInterface> desc_temp(desc);
@@ -884,16 +855,24 @@
return BadLocalSdp(desc->type(), kCreateChannelFailed, err_desc);
}
- // Remove channel and transport proxies, if MediaContentDescription is
- // rejected.
- RemoveUnusedChannelsAndTransports(local_desc_->description());
+ // Remove unused channels if MediaContentDescription is rejected.
+ RemoveUnusedChannels(local_desc_->description());
if (!UpdateSessionState(action, cricket::CS_LOCAL, err_desc)) {
return false;
}
- // Kick starting the ice candidates allocation.
- StartCandidatesAllocation();
+ if (remote_description()) {
+ // Now that we have a local description, we can push down remote candidates
+ // that we stored, and those from the remote description.
+ if (!saved_candidates_.empty()) {
+ // If there are saved candidates which arrived before the local
+ // description was set, copy those to the remote description.
+ CopySavedCandidates(remote_desc_.get());
+ }
+ // Push remote candidates in remote description to transport channels.
+ UseCandidatesInSessionDescription(remote_desc_.get());
+ }
// Update state and SSRC of local MediaStreams and DataChannels based on the
// local session description.
@@ -911,6 +890,8 @@
bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc,
std::string* err_desc) {
+ ASSERT(signaling_thread()->IsCurrent());
+
// Takes the ownership of |desc| regardless of the result.
rtc::scoped_ptr<SessionDescriptionInterface> desc_temp(desc);
@@ -927,9 +908,8 @@
return BadRemoteSdp(desc->type(), kCreateChannelFailed, err_desc);
}
- // Remove channel and transport proxies, if MediaContentDescription is
- // rejected.
- RemoveUnusedChannelsAndTransports(desc->description());
+ // Remove unused channels if MediaContentDescription is rejected.
+ RemoveUnusedChannels(desc->description());
// NOTE: Candidates allocation will be initiated only when SetLocalDescription
// is called.
@@ -988,6 +968,8 @@
bool WebRtcSession::UpdateSessionState(
Action action, cricket::ContentSource source,
std::string* err_desc) {
+ ASSERT(signaling_thread()->IsCurrent());
+
// If there's already a pending error then no state transition should happen.
// But all call-sites should be verifying this before calling us!
ASSERT(error() == cricket::BaseSession::ERROR_NONE);
@@ -1021,7 +1003,21 @@
if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) {
return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
}
- MaybeEnableMuxingSupport();
+ const cricket::ContentGroup* local_bundle =
+ BaseSession::local_description()->GetGroupByName(
+ cricket::GROUP_TYPE_BUNDLE);
+ const cricket::ContentGroup* remote_bundle =
+ BaseSession::remote_description()->GetGroupByName(
+ cricket::GROUP_TYPE_BUNDLE);
+ if (local_bundle && remote_bundle) {
+ // The answerer decides the transport to bundle on
+ const cricket::ContentGroup* answer_bundle =
+ (source == cricket::CS_LOCAL ? local_bundle : remote_bundle);
+ if (!EnableBundle(*answer_bundle)) {
+ LOG(LS_WARNING) << "Failed to enable BUNDLE.";
+ return BadAnswerSdp(source, kEnableBundleFailed, err_desc);
+ }
+ }
EnableChannels();
SetState(source == cricket::CS_LOCAL ?
STATE_SENTACCEPT : STATE_RECEIVEDACCEPT);
@@ -1070,32 +1066,101 @@
bool WebRtcSession::GetTransportStats(cricket::SessionStats* stats) {
ASSERT(signaling_thread()->IsCurrent());
+ return (GetChannelTransportStats(voice_channel(), stats) &&
+ GetChannelTransportStats(video_channel(), stats) &&
+ GetChannelTransportStats(data_channel(), stats));
+}
- const auto get_transport_stats = [stats](const std::string& content_name,
- cricket::Transport* transport) {
- const std::string& transport_id = transport->content_name();
- stats->proxy_to_transport[content_name] = transport_id;
- if (stats->transport_stats.find(transport_id)
- != stats->transport_stats.end()) {
- // Transport stats already done for this transport.
+bool WebRtcSession::GetChannelTransportStats(cricket::BaseChannel* ch,
+ cricket::SessionStats* stats) {
+ ASSERT(signaling_thread()->IsCurrent());
+ if (!ch) {
+ // Not using this channel.
+ return true;
+ }
+
+ const std::string& content_name = ch->content_name();
+ const std::string& transport_name = ch->transport_name();
+ stats->proxy_to_transport[content_name] = transport_name;
+ if (stats->transport_stats.find(transport_name) !=
+ stats->transport_stats.end()) {
+ // Transport stats already done for this transport.
+ return true;
+ }
+
+ cricket::TransportStats tstats;
+ if (!transport_controller()->GetStats(transport_name, &tstats)) {
+ return false;
+ }
+
+ stats->transport_stats[transport_name] = tstats;
+ return true;
+}
+
+bool WebRtcSession::GetLocalCertificate(
+ const std::string& transport_name,
+ rtc::scoped_refptr<rtc::RTCCertificate>* certificate) {
+ ASSERT(signaling_thread()->IsCurrent());
+ return transport_controller()->GetLocalCertificate(transport_name,
+ certificate);
+}
+
+bool WebRtcSession::GetRemoteSSLCertificate(const std::string& transport_name,
+ rtc::SSLCertificate** cert) {
+ ASSERT(signaling_thread()->IsCurrent());
+ return transport_controller()->GetRemoteSSLCertificate(transport_name, cert);
+}
+
+cricket::BaseChannel* WebRtcSession::GetChannel(
+ const std::string& content_name) {
+ if (voice_channel() && voice_channel()->content_name() == content_name) {
+ return voice_channel();
+ }
+ if (video_channel() && video_channel()->content_name() == content_name) {
+ return video_channel();
+ }
+ if (data_channel() && data_channel()->content_name() == content_name) {
+ return data_channel();
+ }
+ return nullptr;
+}
+
+bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) {
+ const std::string* first_content_name = bundle.FirstContentName();
+ if (!first_content_name) {
+ LOG(LS_WARNING) << "Tried to BUNDLE with no contents.";
+ return false;
+ }
+ const std::string& transport_name = *first_content_name;
+ cricket::BaseChannel* first_channel = GetChannel(transport_name);
+
+ auto maybe_set_transport = [this, bundle, transport_name,
+ first_channel](cricket::BaseChannel* ch) {
+ if (!ch || !bundle.HasContentName(ch->content_name())) {
return true;
}
- cricket::TransportStats tstats;
- if (!transport->GetStats(&tstats)) {
- return false;
+ if (ch->transport_name() == transport_name) {
+ LOG(LS_INFO) << "BUNDLE already enabled for " << ch->content_name()
+ << " on " << transport_name << ".";
+ return true;
}
- stats->transport_stats[transport_id] = tstats;
+ if (!ch->SetTransport(transport_name)) {
+ LOG(LS_WARNING) << "Failed to enable BUNDLE for " << ch->content_name();
+ return false;
+ }
+ LOG(LS_INFO) << "Enabled BUNDLE for " << ch->content_name() << " on "
+ << transport_name << ".";
return true;
};
- for (const auto& kv : transport_proxies()) {
- cricket::Transport* transport = kv.second->impl();
- if (transport && !get_transport_stats(kv.first, transport)) {
- return false;
- }
+ if (!maybe_set_transport(voice_channel()) ||
+ !maybe_set_transport(video_channel()) ||
+ !maybe_set_transport(data_channel())) {
+ return false;
}
+
return true;
}
@@ -1401,13 +1466,18 @@
void WebRtcSession::OnCertificateReady(
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
- SetCertificate(certificate);
+ transport_controller()->SetLocalCertificate(certificate);
}
bool WebRtcSession::waiting_for_certificate_for_testing() const {
return webrtc_session_desc_factory_->waiting_for_certificate_for_testing();
}
+const rtc::scoped_refptr<rtc::RTCCertificate>&
+WebRtcSession::certificate_for_testing() {
+ return transport_controller()->certificate_for_testing();
+}
+
void WebRtcSession::SetIceConnectionState(
PeerConnectionInterface::IceConnectionState state) {
if (ice_connection_state_ == state) {
@@ -1418,6 +1488,8 @@
// WebRtcSession does not implement "kIceConnectionClosed" (that is handled
// within PeerConnection). This switch statement should compile away when
// ASSERTs are disabled.
+ LOG(LS_INFO) << "Changing IceConnectionState " << ice_connection_state_
+ << " => " << state;
switch (ice_connection_state_) {
case PeerConnectionInterface::kIceConnectionNew:
ASSERT(state == PeerConnectionInterface::kIceConnectionChecking);
@@ -1458,70 +1530,52 @@
}
}
-void WebRtcSession::OnTransportRequestSignaling(
- cricket::Transport* transport) {
- ASSERT(signaling_thread()->IsCurrent());
- transport->OnSignalingReady();
- if (ice_observer_) {
- ice_observer_->OnIceGatheringChange(
- PeerConnectionInterface::kIceGatheringGathering);
- }
-}
-
-void WebRtcSession::OnTransportConnecting(cricket::Transport* transport) {
- ASSERT(signaling_thread()->IsCurrent());
- // start monitoring for the write state of the transport.
- OnTransportWritable(transport);
-}
-
-void WebRtcSession::OnTransportWritable(cricket::Transport* transport) {
- ASSERT(signaling_thread()->IsCurrent());
- if (transport->all_channels_writable()) {
- SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
- } else if (transport->HasChannels()) {
- // If the current state is Connected or Completed, then there were writable
- // channels but now there are not, so the next state must be Disconnected.
- if (ice_connection_state_ ==
- PeerConnectionInterface::kIceConnectionConnected ||
- ice_connection_state_ ==
- PeerConnectionInterface::kIceConnectionCompleted) {
- SetIceConnectionState(
- PeerConnectionInterface::kIceConnectionDisconnected);
- }
- }
-}
-
-void WebRtcSession::OnTransportCompleted(cricket::Transport* transport) {
- ASSERT(signaling_thread()->IsCurrent());
- PeerConnectionInterface::IceConnectionState old_state = ice_connection_state_;
- SetIceConnectionState(PeerConnectionInterface::kIceConnectionCompleted);
- // Only report once when Ice connection is completed.
- if (old_state != PeerConnectionInterface::kIceConnectionCompleted) {
- cricket::TransportStats stats;
- if (metrics_observer_ && transport->GetStats(&stats)) {
- ReportBestConnectionState(stats);
- ReportNegotiatedCiphers(stats);
- }
- }
-}
-
-void WebRtcSession::OnTransportFailed(cricket::Transport* transport) {
- ASSERT(signaling_thread()->IsCurrent());
- SetIceConnectionState(PeerConnectionInterface::kIceConnectionFailed);
-}
-
-void WebRtcSession::OnTransportReceiving(cricket::Transport* transport) {
- ASSERT(signaling_thread()->IsCurrent());
- // The ice connection is considered receiving if at least one transport is
- // receiving on any channels.
- bool receiving = false;
- for (const auto& kv : transport_proxies()) {
- cricket::Transport* transport = kv.second->impl();
- if (transport && transport->any_channel_receiving()) {
- receiving = true;
+void WebRtcSession::OnTransportControllerConnectionState(
+ cricket::IceConnectionState state) {
+ switch (state) {
+ case cricket::kIceConnectionConnecting:
+ // If the current state is Connected or Completed, then there were
+ // writable channels but now there are not, so the next state must
+ // be Disconnected.
+ // kIceConnectionConnecting is currently used as the default,
+ // un-connected state by the TransportController, so its only use is
+ // detecting disconnections.
+ if (ice_connection_state_ ==
+ PeerConnectionInterface::kIceConnectionConnected ||
+ ice_connection_state_ ==
+ PeerConnectionInterface::kIceConnectionCompleted) {
+ SetIceConnectionState(
+ PeerConnectionInterface::kIceConnectionDisconnected);
+ }
break;
- }
+ case cricket::kIceConnectionFailed:
+ SetIceConnectionState(PeerConnectionInterface::kIceConnectionFailed);
+ break;
+ case cricket::kIceConnectionConnected:
+ LOG(LS_INFO) << "Changing to ICE connected state because "
+ << "all transports are writable.";
+ SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
+ break;
+ case cricket::kIceConnectionCompleted:
+ LOG(LS_INFO) << "Changing to ICE completed state because "
+ << "all transports are complete.";
+ if (ice_connection_state_ !=
+ PeerConnectionInterface::kIceConnectionConnected) {
+ // If jumping directly from "checking" to "connected",
+ // signal "connected" first.
+ SetIceConnectionState(PeerConnectionInterface::kIceConnectionConnected);
+ }
+ SetIceConnectionState(PeerConnectionInterface::kIceConnectionCompleted);
+ if (metrics_observer_) {
+ ReportTransportStats();
+ }
+ break;
+ default:
+ ASSERT(false);
}
+}
+
+void WebRtcSession::OnTransportControllerReceiving(bool receiving) {
SetIceConnectionReceiving(receiving);
}
@@ -1535,18 +1589,27 @@
}
}
-void WebRtcSession::OnTransportProxyCandidatesReady(
- cricket::TransportProxy* proxy, const cricket::Candidates& candidates) {
+void WebRtcSession::OnTransportControllerCandidatesGathered(
+ const std::string& transport_name,
+ const cricket::Candidates& candidates) {
ASSERT(signaling_thread()->IsCurrent());
- ProcessNewLocalCandidate(proxy->content_name(), candidates);
-}
+ int sdp_mline_index;
+ if (!GetLocalCandidateMediaIndex(transport_name, &sdp_mline_index)) {
+ LOG(LS_ERROR) << "OnTransportControllerCandidatesGathered: content name "
+ << transport_name << " not found";
+ return;
+ }
-void WebRtcSession::OnCandidatesAllocationDone() {
- ASSERT(signaling_thread()->IsCurrent());
- if (ice_observer_) {
- ice_observer_->OnIceGatheringChange(
- PeerConnectionInterface::kIceGatheringComplete);
- ice_observer_->OnIceComplete();
+ for (cricket::Candidates::const_iterator citer = candidates.begin();
+ citer != candidates.end(); ++citer) {
+ // Use transport_name as the candidate media id.
+ JsepIceCandidate candidate(transport_name, sdp_mline_index, *citer);
+ if (ice_observer_) {
+ ice_observer_->OnIceCandidate(&candidate);
+ }
+ if (local_desc_) {
+ local_desc_->AddCandidate(&candidate);
+ }
}
}
@@ -1562,29 +1625,6 @@
data_channel_->Enable(true);
}
-void WebRtcSession::ProcessNewLocalCandidate(
- const std::string& content_name,
- const cricket::Candidates& candidates) {
- int sdp_mline_index;
- if (!GetLocalCandidateMediaIndex(content_name, &sdp_mline_index)) {
- LOG(LS_ERROR) << "ProcessNewLocalCandidate: content name "
- << content_name << " not found";
- return;
- }
-
- for (cricket::Candidates::const_iterator citer = candidates.begin();
- citer != candidates.end(); ++citer) {
- // Use content_name as the candidate media id.
- JsepIceCandidate candidate(content_name, sdp_mline_index, *citer);
- if (ice_observer_) {
- ice_observer_->OnIceCandidate(&candidate);
- }
- if (local_desc_) {
- local_desc_->AddCandidate(&candidate);
- }
- }
-}
-
// Returns the media index for a local ice candidate given the content name.
bool WebRtcSession::GetLocalCandidateMediaIndex(const std::string& content_name,
int* sdp_mline_index) {
@@ -1649,7 +1689,8 @@
candidates.push_back(candidate->candidate());
// Invoking BaseSession method to handle remote candidates.
std::string error;
- if (OnRemoteCandidates(content.name, candidates, &error)) {
+ if (transport_controller()->AddRemoteCandidates(content.name, candidates,
+ &error)) {
// Candidates successfully submitted for checking.
if (ice_connection_state_ == PeerConnectionInterface::kIceConnectionNew ||
ice_connection_state_ ==
@@ -1673,8 +1714,7 @@
return true;
}
-void WebRtcSession::RemoveUnusedChannelsAndTransports(
- const SessionDescription* desc) {
+void WebRtcSession::RemoveUnusedChannels(const SessionDescription* desc) {
// Destroy video_channel_ first since it may have a pointer to the
// voice_channel_.
const cricket::ContentInfo* video_info =
@@ -1684,7 +1724,6 @@
SignalVideoChannelDestroyed();
const std::string content_name = video_channel_->content_name();
channel_manager_->DestroyVideoChannel(video_channel_.release());
- DestroyTransportProxy(content_name);
}
const cricket::ContentInfo* voice_info =
@@ -1694,7 +1733,6 @@
SignalVoiceChannelDestroyed();
const std::string content_name = voice_channel_->content_name();
channel_manager_->DestroyVoiceChannel(voice_channel_.release());
- DestroyTransportProxy(content_name);
}
const cricket::ContentInfo* data_info =
@@ -1704,7 +1742,6 @@
SignalDataChannelDestroyed();
const std::string content_name = data_channel_->content_name();
channel_manager_->DestroyDataChannel(data_channel_.release());
- DestroyTransportProxy(content_name);
}
}
@@ -1749,7 +1786,7 @@
}
}
- // Enable bundle before when kMaxBundle policy is in effect.
+ // Enable BUNDLE immediately when kBundlePolicyMaxBundle is in effect.
if (bundle_policy_ == PeerConnectionInterface::kBundlePolicyMaxBundle) {
const cricket::ContentGroup* bundle_group = desc->GetGroupByName(
cricket::GROUP_TYPE_BUNDLE);
@@ -1757,7 +1794,7 @@
LOG(LS_WARNING) << "max-bundle specified without BUNDLE specified";
return false;
}
- if (!BaseSession::BundleContentGroup(bundle_group)) {
+ if (!EnableBundle(*bundle_group)) {
LOG(LS_WARNING) << "max-bundle failed to enable bundling.";
return false;
}
@@ -1768,7 +1805,8 @@
bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content) {
voice_channel_.reset(channel_manager_->CreateVoiceChannel(
- media_controller_.get(), this, content->name, true, audio_options_));
+ media_controller_.get(), transport_controller(), content->name, true,
+ audio_options_));
if (!voice_channel_) {
return false;
}
@@ -1780,7 +1818,8 @@
bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content) {
video_channel_.reset(channel_manager_->CreateVideoChannel(
- media_controller_.get(), this, content->name, true, video_options_));
+ media_controller_.get(), transport_controller(), content->name, true,
+ video_options_));
if (!video_channel_) {
return false;
}
@@ -1793,7 +1832,7 @@
bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content) {
bool sctp = (data_channel_type_ == cricket::DCT_SCTP);
data_channel_.reset(channel_manager_->CreateDataChannel(
- this, content->name, !sctp, data_channel_type_));
+ transport_controller(), content->name, !sctp, data_channel_type_));
if (!data_channel_) {
return false;
}
@@ -1974,7 +2013,6 @@
const SessionDescriptionInterface* remote_desc,
bool* valid) {
*valid = true;;
- cricket::TransportProxy* transport_proxy = NULL;
const SessionDescriptionInterface* current_remote_desc =
remote_desc ? remote_desc : remote_description();
@@ -1996,12 +2034,53 @@
cricket::ContentInfo content =
current_remote_desc->description()->contents()[mediacontent_index];
- transport_proxy = GetTransportProxy(content.name);
+ cricket::BaseChannel* channel = GetChannel(content.name);
+ if (!channel) {
+ return false;
+ }
- return transport_proxy && transport_proxy->local_description_set() &&
- transport_proxy->remote_description_set();
+ return transport_controller()->ReadyForRemoteCandidates(
+ channel->transport_name());
}
+void WebRtcSession::OnTransportControllerGatheringState(
+ cricket::IceGatheringState state) {
+ ASSERT(signaling_thread()->IsCurrent());
+ if (state == cricket::kIceGatheringGathering) {
+ if (ice_observer_) {
+ ice_observer_->OnIceGatheringChange(
+ PeerConnectionInterface::kIceGatheringGathering);
+ }
+ } else if (state == cricket::kIceGatheringComplete) {
+ if (ice_observer_) {
+ ice_observer_->OnIceGatheringChange(
+ PeerConnectionInterface::kIceGatheringComplete);
+ ice_observer_->OnIceComplete();
+ }
+ }
+}
+
+void WebRtcSession::ReportTransportStats() {
+ // Use a set so we don't report the same stats twice if two channels share
+ // a transport.
+ std::set<std::string> transport_names;
+ if (voice_channel()) {
+ transport_names.insert(voice_channel()->transport_name());
+ }
+ if (video_channel()) {
+ transport_names.insert(video_channel()->transport_name());
+ }
+ if (data_channel()) {
+ transport_names.insert(data_channel()->transport_name());
+ }
+ for (const auto& name : transport_names) {
+ cricket::TransportStats stats;
+ if (transport_controller()->GetStats(name, &stats)) {
+ ReportBestConnectionState(stats);
+ ReportNegotiatedCiphers(stats);
+ }
+ }
+}
// Walk through the ConnectionInfos to gather best connection usage
// for IPv4 and IPv6.
void WebRtcSession::ReportBestConnectionState(
@@ -2069,13 +2148,13 @@
PeerConnectionMetricsName srtp_name;
PeerConnectionMetricsName ssl_name;
- if (stats.content_name == cricket::CN_AUDIO) {
+ if (stats.transport_name == cricket::CN_AUDIO) {
srtp_name = kAudioSrtpCipher;
ssl_name = kAudioSslCipher;
- } else if (stats.content_name == cricket::CN_VIDEO) {
+ } else if (stats.transport_name == cricket::CN_VIDEO) {
srtp_name = kVideoSrtpCipher;
ssl_name = kVideoSslCipher;
- } else if (stats.content_name == cricket::CN_DATA) {
+ } else if (stats.transport_name == cricket::CN_DATA) {
srtp_name = kDataSrtpCipher;
ssl_name = kDataSslCipher;
} else {
diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h
index 582d6d0..1ad9a69 100644
--- a/talk/app/webrtc/webrtcsession.h
+++ b/talk/app/webrtc/webrtcsession.h
@@ -29,6 +29,7 @@
#define TALK_APP_WEBRTC_WEBRTCSESSION_H_
#include <string>
+#include <vector>
#include "talk/app/webrtc/datachannel.h"
#include "talk/app/webrtc/dtmfsender.h"
@@ -49,7 +50,6 @@
class ChannelManager;
class DataChannel;
class StatsReport;
-class Transport;
class VideoCapturer;
class VideoChannel;
class VoiceChannel;
@@ -77,6 +77,8 @@
extern const char kSessionErrorDesc[];
extern const char kDtlsSetupFailureRtp[];
extern const char kDtlsSetupFailureRtcp[];
+extern const char kEnableBundleFailed[];
+
// Maximum number of received video streams that will be processed by webrtc
// even if they are not signalled beforehand.
extern const int kMaxUnsignalledRecvStreams;
@@ -235,6 +237,19 @@
// This avoids exposing the internal structures used to track them.
virtual bool GetTransportStats(cricket::SessionStats* stats);
+ // Get stats for a specific channel
+ bool GetChannelTransportStats(cricket::BaseChannel* ch,
+ cricket::SessionStats* stats);
+
+ // virtual so it can be mocked in unit tests
+ virtual bool GetLocalCertificate(
+ const std::string& transport_name,
+ rtc::scoped_refptr<rtc::RTCCertificate>* certificate);
+
+ // Caller owns returned certificate
+ virtual bool GetRemoteSSLCertificate(const std::string& transport_name,
+ rtc::SSLCertificate** cert);
+
// Implements DataChannelFactory.
rtc::scoped_refptr<DataChannel> CreateDataChannel(
const std::string& label,
@@ -254,6 +269,7 @@
// For unit test.
bool waiting_for_certificate_for_testing() const;
+ const rtc::scoped_refptr<rtc::RTCCertificate>& certificate_for_testing();
void set_metrics_observer(
webrtc::MetricsObserverInterface* metrics_observer) {
@@ -269,9 +285,6 @@
kAnswer,
};
- // Invokes ConnectChannels() on transport proxies, which initiates ice
- // candidates allocation.
- bool StartCandidatesAllocation();
bool UpdateSessionState(Action action, cricket::ContentSource source,
std::string* err_desc);
static Action GetAction(const std::string& type);
@@ -281,25 +294,13 @@
cricket::ContentSource source,
std::string* error_desc);
-
- // Transport related callbacks, override from cricket::BaseSession.
- virtual void OnTransportRequestSignaling(cricket::Transport* transport);
- virtual void OnTransportConnecting(cricket::Transport* transport);
- virtual void OnTransportWritable(cricket::Transport* transport);
- virtual void OnTransportCompleted(cricket::Transport* transport);
- virtual void OnTransportFailed(cricket::Transport* transport);
- virtual void OnTransportProxyCandidatesReady(
- cricket::TransportProxy* proxy,
- const cricket::Candidates& candidates);
- virtual void OnCandidatesAllocationDone();
- void OnTransportReceiving(cricket::Transport* transport) override;
+ cricket::BaseChannel* GetChannel(const std::string& content_name);
+ // Cause all the BaseChannels in the bundle group to have the same
+ // transport channel.
+ bool EnableBundle(const cricket::ContentGroup& bundle);
// Enables media channels to allow sending of media.
void EnableChannels();
- // Creates a JsepIceCandidate and adds it to the local session description
- // and notify observers. Called when a new local candidate have been found.
- void ProcessNewLocalCandidate(const std::string& content_name,
- const cricket::Candidates& candidates);
// Returns the media index for a local ice candidate given the content name.
// Returns false if the local session description does not have a media
// content called |content_name|.
@@ -312,8 +313,7 @@
bool UseCandidate(const IceCandidateInterface* candidate);
// Deletes the corresponding channel of contents that don't exist in |desc|.
// |desc| can be null. This means that all channels are deleted.
- void RemoveUnusedChannelsAndTransports(
- const cricket::SessionDescription* desc);
+ void RemoveUnusedChannels(const cricket::SessionDescription* desc);
// Allocates media channels based on the |desc|. If |desc| doesn't have
// the BUNDLE option, this method will disable BUNDLE in PortAllocator.
@@ -362,10 +362,20 @@
const SessionDescriptionInterface* remote_desc,
bool* valid);
+ void OnTransportControllerConnectionState(cricket::IceConnectionState state);
+ void OnTransportControllerReceiving(bool receiving);
+ void OnTransportControllerGatheringState(cricket::IceGatheringState state);
+ void OnTransportControllerCandidatesGathered(
+ const std::string& transport_name,
+ const cricket::Candidates& candidates);
+
std::string GetSessionErrorMsg();
- // Invoked when OnTransportCompleted is signaled to gather the usage
- // of IPv4/IPv6 as best connection.
+ // Invoked when TransportController connection completion is signaled.
+ // Reports stats for all transports in use.
+ void ReportTransportStats();
+
+ // Gather the usage of IPv4/IPv6 as best connection.
void ReportBestConnectionState(const cricket::TransportStats& stats);
void ReportNegotiatedCiphers(const cricket::TransportStats& stats);
diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc
index 8bd97e5..6dc0fc7 100644
--- a/talk/app/webrtc/webrtcsession_unittest.cc
+++ b/talk/app/webrtc/webrtcsession_unittest.cc
@@ -25,6 +25,8 @@
* ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+#include <vector>
+
#include "talk/app/webrtc/audiotrack.h"
#include "talk/app/webrtc/fakemetricsobserver.h"
#include "talk/app/webrtc/jsepicecandidate.h"
@@ -163,8 +165,8 @@
const std::string& newlines,
std::string* message) {
const std::string tmp = line + newlines;
- rtc::replace_substrs(line.c_str(), line.length(),
- tmp.c_str(), tmp.length(), message);
+ rtc::replace_substrs(line.c_str(), line.length(), tmp.c_str(), tmp.length(),
+ message);
}
class MockIceObserver : public webrtc::IceObserver {
@@ -244,12 +246,52 @@
}
virtual ~WebRtcSessionForTest() {}
- using cricket::BaseSession::GetTransportProxy;
+ // Note that these methods are only safe to use if the signaling thread
+ // is the same as the worker thread
+ cricket::TransportChannel* voice_rtp_transport_channel() {
+ return rtp_transport_channel(voice_channel());
+ }
+
+ cricket::TransportChannel* voice_rtcp_transport_channel() {
+ return rtcp_transport_channel(voice_channel());
+ }
+
+ cricket::TransportChannel* video_rtp_transport_channel() {
+ return rtp_transport_channel(video_channel());
+ }
+
+ cricket::TransportChannel* video_rtcp_transport_channel() {
+ return rtcp_transport_channel(video_channel());
+ }
+
+ cricket::TransportChannel* data_rtp_transport_channel() {
+ return rtp_transport_channel(data_channel());
+ }
+
+ cricket::TransportChannel* data_rtcp_transport_channel() {
+ return rtcp_transport_channel(data_channel());
+ }
+
using webrtc::WebRtcSession::SetAudioPlayout;
using webrtc::WebRtcSession::SetAudioSend;
using webrtc::WebRtcSession::SetCaptureDevice;
using webrtc::WebRtcSession::SetVideoPlayout;
using webrtc::WebRtcSession::SetVideoSend;
+
+ private:
+ cricket::TransportChannel* rtp_transport_channel(cricket::BaseChannel* ch) {
+ if (!ch) {
+ return nullptr;
+ }
+ return ch->transport_channel();
+ }
+
+ cricket::TransportChannel* rtcp_transport_channel(cricket::BaseChannel* ch) {
+ if (!ch) {
+ return nullptr;
+ }
+ return ch->rtcp_transport_channel();
+ }
};
class WebRtcSessionCreateSDPObserverForTest
@@ -375,9 +417,9 @@
EXPECT_EQ(PeerConnectionInterface::kIceGatheringNew,
observer_.ice_gathering_state_);
- EXPECT_TRUE(session_->Initialize(
- options_, constraints_.get(), dtls_identity_store.Pass(),
- rtc_configuration));
+ EXPECT_TRUE(session_->Initialize(options_, constraints_.get(),
+ dtls_identity_store.Pass(),
+ rtc_configuration));
session_->set_metrics_observer(metrics_observer_);
}
@@ -490,13 +532,6 @@
session_->video_channel() != NULL);
}
- void CheckTransportChannels() const {
- EXPECT_TRUE(session_->GetChannel(cricket::CN_AUDIO, 1) != NULL);
- EXPECT_TRUE(session_->GetChannel(cricket::CN_AUDIO, 2) != NULL);
- EXPECT_TRUE(session_->GetChannel(cricket::CN_VIDEO, 1) != NULL);
- EXPECT_TRUE(session_->GetChannel(cricket::CN_VIDEO, 2) != NULL);
- }
-
void VerifyCryptoParams(const cricket::SessionDescription* sdp) {
ASSERT_TRUE(session_.get() != NULL);
const cricket::ContentInfo* content = cricket::GetFirstAudioContent(sdp);
@@ -722,6 +757,7 @@
}
void SetLocalDescriptionWithoutError(SessionDescriptionInterface* desc) {
EXPECT_TRUE(session_->SetLocalDescription(desc, NULL));
+ session_->MaybeStartGathering();
}
void SetLocalDescriptionExpectState(SessionDescriptionInterface* desc,
BaseSession::State expected_state) {
@@ -968,15 +1004,10 @@
SetRemoteDescriptionWithoutError(new_answer);
EXPECT_TRUE_WAIT(observer_.oncandidatesready_, kIceCandidatesTimeout);
EXPECT_EQ(expected_candidate_num, observer_.mline_0_candidates_.size());
- EXPECT_EQ(expected_candidate_num, observer_.mline_1_candidates_.size());
- for (size_t i = 0; i < observer_.mline_0_candidates_.size(); ++i) {
- cricket::Candidate c0 = observer_.mline_0_candidates_[i];
- cricket::Candidate c1 = observer_.mline_1_candidates_[i];
- if (bundle) {
- EXPECT_TRUE(c0.IsEquivalent(c1));
- } else {
- EXPECT_FALSE(c0.IsEquivalent(c1));
- }
+ if (bundle) {
+ EXPECT_EQ(0, observer_.mline_1_candidates_.size());
+ } else {
+ EXPECT_EQ(expected_candidate_num, observer_.mline_1_candidates_.size());
}
}
// Tests that we can only send DTMF when the dtmf codec is supported.
@@ -1001,7 +1032,7 @@
// initial ICE convergences.
class LoopbackNetworkConfiguration {
- public:
+ public:
LoopbackNetworkConfiguration()
: test_ipv6_network_(false),
test_extra_ipv4_network_(false),
@@ -1150,11 +1181,8 @@
// Clearing the rules, session should move back to completed state.
loopback_network_manager.ClearRules(fss_.get());
- // Session is automatically calling OnSignalingReady after creation of
- // new portallocator session which will allocate new set of candidates.
LOG(LS_INFO) << "Firewall Rules cleared";
-
EXPECT_EQ_WAIT(PeerConnectionInterface::kIceConnectionCompleted,
observer_.ice_connection_state_,
kIceCandidatesTimeout);
@@ -1707,15 +1735,14 @@
// a DTLS fingerprint when DTLS is required.
TEST_P(WebRtcSessionTest, TestSetRemoteNonDtlsAnswerWhenDtlsOn) {
MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
- // Enable both SDES and DTLS, so that offer won't be outright rejected as a
- // result of using the "UDP/TLS/RTP/SAVPF" profile.
InitWithDtls(GetParam());
- session_->SetSdesPolicy(cricket::SEC_ENABLED);
SessionDescriptionInterface* offer = CreateOffer();
cricket::MediaSessionOptions options;
options.recv_video = true;
+ rtc::scoped_ptr<SessionDescriptionInterface> temp_offer(
+ CreateRemoteOffer(options, cricket::SEC_ENABLED));
JsepSessionDescription* answer =
- CreateRemoteAnswer(offer, options, cricket::SEC_ENABLED);
+ CreateRemoteAnswer(temp_offer.get(), options, cricket::SEC_ENABLED);
// SetRemoteDescription and SetLocalDescription will take the ownership of
// the offer and answer.
@@ -2017,7 +2044,7 @@
EXPECT_LT(0u, candidates->count());
candidates = local_desc->candidates(1);
ASSERT_TRUE(candidates != NULL);
- EXPECT_LT(0u, candidates->count());
+ EXPECT_EQ(0u, candidates->count());
// Update the session descriptions.
mediastream_signaling_.SendAudioVideoStream1();
@@ -2029,7 +2056,7 @@
EXPECT_LT(0u, candidates->count());
candidates = local_desc->candidates(1);
ASSERT_TRUE(candidates != NULL);
- EXPECT_LT(0u, candidates->count());
+ EXPECT_EQ(0u, candidates->count());
}
// Test that we can set a remote session description with remote candidates.
@@ -2073,23 +2100,17 @@
// Wait until at least one local candidate has been collected.
EXPECT_TRUE_WAIT(0u < observer_.mline_0_candidates_.size(),
kIceCandidatesTimeout);
- EXPECT_TRUE_WAIT(0u < observer_.mline_1_candidates_.size(),
- kIceCandidatesTimeout);
rtc::scoped_ptr<SessionDescriptionInterface> local_offer(CreateOffer());
ASSERT_TRUE(local_offer->candidates(kMediaContentIndex0) != NULL);
EXPECT_LT(0u, local_offer->candidates(kMediaContentIndex0)->count());
- ASSERT_TRUE(local_offer->candidates(kMediaContentIndex1) != NULL);
- EXPECT_LT(0u, local_offer->candidates(kMediaContentIndex1)->count());
SessionDescriptionInterface* remote_offer(CreateRemoteOffer());
SetRemoteDescriptionWithoutError(remote_offer);
SessionDescriptionInterface* answer = CreateAnswer(NULL);
ASSERT_TRUE(answer->candidates(kMediaContentIndex0) != NULL);
EXPECT_LT(0u, answer->candidates(kMediaContentIndex0)->count());
- ASSERT_TRUE(answer->candidates(kMediaContentIndex1) != NULL);
- EXPECT_LT(0u, answer->candidates(kMediaContentIndex1)->count());
SetLocalDescriptionWithoutError(answer);
}
@@ -2131,8 +2152,14 @@
CreateAnswer(NULL);
SetLocalDescriptionWithoutError(answer);
- EXPECT_TRUE(session_->GetTransportProxy("audio_content_name") != NULL);
- EXPECT_TRUE(session_->GetTransportProxy("video_content_name") != NULL);
+ cricket::TransportChannel* voice_transport_channel =
+ session_->voice_rtp_transport_channel();
+ EXPECT_TRUE(voice_transport_channel != NULL);
+ EXPECT_EQ(voice_transport_channel->transport_name(), "audio_content_name");
+ cricket::TransportChannel* video_transport_channel =
+ session_->video_rtp_transport_channel();
+ EXPECT_TRUE(video_transport_channel != NULL);
+ EXPECT_EQ(video_transport_channel->transport_name(), "video_content_name");
EXPECT_TRUE((video_channel_ = media_engine_->GetVideoChannel(0)) != NULL);
EXPECT_TRUE((voice_channel_ = media_engine_->GetVoiceChannel(0)) != NULL);
}
@@ -2692,20 +2719,23 @@
SessionDescriptionInterface* answer = CreateAnswer(NULL);
SetLocalDescriptionWithoutError(answer);
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
- cricket::Transport* t = session_->GetTransport("audio");
+ cricket::BaseChannel* voice_channel = session_->voice_channel();
+ ASSERT(voice_channel != NULL);
// Checks if one of the transport channels contains a connection using a given
// port.
- auto connection_with_remote_port = [t](int port) {
- cricket::TransportStats stats;
- t->GetStats(&stats);
- for (auto& chan_stat : stats.channel_stats) {
- for (auto& conn_info : chan_stat.connection_infos) {
- if (conn_info.remote_candidate.address().port() == port) {
- return true;
+ auto connection_with_remote_port = [this, voice_channel](int port) {
+ cricket::SessionStats stats;
+ session_->GetChannelTransportStats(voice_channel, &stats);
+ for (auto& kv : stats.transport_stats) {
+ for (auto& chan_stat : kv.second.channel_stats) {
+ for (auto& conn_info : chan_stat.connection_infos) {
+ if (conn_info.remote_candidate.address().port() == port) {
+ return true;
+ }
}
}
}
@@ -2758,7 +2788,7 @@
EXPECT_FALSE(connection_with_remote_port(6000));
}
-// kBundlePolicyBalanced bundle policy and answer contains BUNDLE.
+// kBundlePolicyBalanced BUNDLE policy and answer contains BUNDLE.
TEST_F(WebRtcSessionTest, TestBalancedBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced);
mediastream_signaling_.SendAudioVideoStream1();
@@ -2769,19 +2799,19 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_NE(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
}
-// kBundlePolicyBalanced bundle policy but no BUNDLE in the answer.
+// kBundlePolicyBalanced BUNDLE policy but no BUNDLE in the answer.
TEST_F(WebRtcSessionTest, TestBalancedNoBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyBalanced);
mediastream_signaling_.SendAudioVideoStream1();
@@ -2792,8 +2822,8 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_NE(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
mediastream_signaling_.SendAudioVideoStream2();
@@ -2807,8 +2837,8 @@
modified_answer->Initialize(answer_copy, "1", "1");
SetRemoteDescriptionWithoutError(modified_answer); //
- EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_NE(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
}
// kBundlePolicyMaxBundle policy with BUNDLE in the answer.
@@ -2822,16 +2852,49 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
+}
+
+// kBundlePolicyMaxBundle policy with BUNDLE in the answer, but no
+// audio content in the answer.
+TEST_F(WebRtcSessionTest, TestMaxBundleRejectAudio) {
+ InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxBundle);
+ mediastream_signaling_.SendAudioVideoStream1();
+
+ PeerConnectionInterface::RTCOfferAnswerOptions options;
+ options.use_rtp_mux = true;
+
+ SessionDescriptionInterface* offer = CreateOffer(options);
+ SetLocalDescriptionWithoutError(offer);
+
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
+
+ mediastream_signaling_.SendAudioVideoStream2();
+ cricket::MediaSessionOptions recv_options;
+ recv_options.recv_audio = false;
+ recv_options.recv_video = true;
+ SessionDescriptionInterface* answer =
+ CreateRemoteAnswer(session_->local_description(), recv_options);
+ SetRemoteDescriptionWithoutError(answer);
+
+ EXPECT_TRUE(NULL == session_->voice_channel());
+ EXPECT_TRUE(NULL != session_->video_rtp_transport_channel());
+
+ session_->Terminate();
+ EXPECT_TRUE(NULL == session_->voice_rtp_transport_channel());
+ EXPECT_TRUE(NULL == session_->voice_rtcp_transport_channel());
+ EXPECT_TRUE(NULL == session_->video_rtp_transport_channel());
+ EXPECT_TRUE(NULL == session_->video_rtcp_transport_channel());
}
// kBundlePolicyMaxBundle policy but no BUNDLE in the answer.
@@ -2845,8 +2908,8 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
mediastream_signaling_.SendAudioVideoStream2();
@@ -2860,8 +2923,45 @@
modified_answer->Initialize(answer_copy, "1", "1");
SetRemoteDescriptionWithoutError(modified_answer);
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
+}
+
+// kBundlePolicyMaxBundle policy with BUNDLE in the remote offer.
+TEST_F(WebRtcSessionTest, TestMaxBundleBundleInRemoteOffer) {
+ InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxBundle);
+ mediastream_signaling_.SendAudioVideoStream1();
+
+ SessionDescriptionInterface* offer = CreateRemoteOffer();
+ SetRemoteDescriptionWithoutError(offer);
+
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
+
+ mediastream_signaling_.SendAudioVideoStream2();
+ SessionDescriptionInterface* answer = CreateAnswer(nullptr);
+ SetLocalDescriptionWithoutError(answer);
+
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
+}
+
+// kBundlePolicyMaxBundle policy but no BUNDLE in the remote offer.
+TEST_F(WebRtcSessionTest, TestMaxBundleNoBundleInRemoteOffer) {
+ InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxBundle);
+ mediastream_signaling_.SendAudioVideoStream1();
+
+ // Remove BUNDLE from the offer.
+ rtc::scoped_ptr<SessionDescriptionInterface> offer(CreateRemoteOffer());
+ cricket::SessionDescription* offer_copy = offer->description()->Copy();
+ offer_copy->RemoveGroupByName(cricket::GROUP_TYPE_BUNDLE);
+ JsepSessionDescription* modified_offer =
+ new JsepSessionDescription(JsepSessionDescription::kOffer);
+ modified_offer->Initialize(offer_copy, "1", "1");
+
+ // Expect an error when applying the remote description
+ SetRemoteDescriptionExpectError(JsepSessionDescription::kOffer,
+ kCreateChannelFailed, modified_offer);
}
// kBundlePolicyMaxCompat bundle policy and answer contains BUNDLE.
@@ -2875,8 +2975,8 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_NE(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
@@ -2885,11 +2985,11 @@
// This should lead to an audio-only call but isn't implemented
// correctly yet.
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
}
-// kBundlePolicyMaxCompat bundle policy but no BUNDLE in the answer.
+// kBundlePolicyMaxCompat BUNDLE policy but no BUNDLE in the answer.
TEST_F(WebRtcSessionTest, TestMaxCompatNoBundleInAnswer) {
InitWithBundlePolicy(PeerConnectionInterface::kBundlePolicyMaxCompat);
mediastream_signaling_.SendAudioVideoStream1();
@@ -2899,8 +2999,8 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_NE(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
mediastream_signaling_.SendAudioVideoStream2();
@@ -2914,8 +3014,8 @@
modified_answer->Initialize(answer_copy, "1", "1");
SetRemoteDescriptionWithoutError(modified_answer); //
- EXPECT_NE(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_NE(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
}
// kBundlePolicyMaxbundle and then we call SetRemoteDescription first.
@@ -2929,8 +3029,8 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetRemoteDescriptionWithoutError(offer);
- EXPECT_EQ(session_->GetTransportProxy("audio")->impl(),
- session_->GetTransportProxy("video")->impl());
+ EXPECT_EQ(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
}
TEST_F(WebRtcSessionTest, TestRequireRtcpMux) {
@@ -2941,16 +3041,16 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_FALSE(session_->GetTransportProxy("audio")->impl()->HasChannel(2));
- EXPECT_FALSE(session_->GetTransportProxy("video")->impl()->HasChannel(2));
+ EXPECT_TRUE(session_->voice_rtcp_transport_channel() == NULL);
+ EXPECT_TRUE(session_->video_rtcp_transport_channel() == NULL);
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
- EXPECT_FALSE(session_->GetTransportProxy("audio")->impl()->HasChannel(2));
- EXPECT_FALSE(session_->GetTransportProxy("video")->impl()->HasChannel(2));
+ EXPECT_TRUE(session_->voice_rtcp_transport_channel() == NULL);
+ EXPECT_TRUE(session_->video_rtcp_transport_channel() == NULL);
}
TEST_F(WebRtcSessionTest, TestNegotiateRtcpMux) {
@@ -2961,16 +3061,16 @@
SessionDescriptionInterface* offer = CreateOffer(options);
SetLocalDescriptionWithoutError(offer);
- EXPECT_TRUE(session_->GetTransportProxy("audio")->impl()->HasChannel(2));
- EXPECT_TRUE(session_->GetTransportProxy("video")->impl()->HasChannel(2));
+ EXPECT_TRUE(session_->voice_rtcp_transport_channel() != NULL);
+ EXPECT_TRUE(session_->video_rtcp_transport_channel() != NULL);
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
- EXPECT_FALSE(session_->GetTransportProxy("audio")->impl()->HasChannel(2));
- EXPECT_FALSE(session_->GetTransportProxy("video")->impl()->HasChannel(2));
+ EXPECT_TRUE(session_->voice_rtcp_transport_channel() == NULL);
+ EXPECT_TRUE(session_->video_rtcp_transport_channel() == NULL);
}
// This test verifies that SetLocalDescription and SetRemoteDescription fails
@@ -2991,11 +3091,11 @@
rtc::replace_substrs(rtcp_mux.c_str(), rtcp_mux.length(),
xrtcp_mux.c_str(), xrtcp_mux.length(),
&offer_str);
- JsepSessionDescription *local_offer =
+ JsepSessionDescription* local_offer =
new JsepSessionDescription(JsepSessionDescription::kOffer);
EXPECT_TRUE((local_offer)->Initialize(offer_str, NULL));
SetLocalDescriptionOfferExpectError(kBundleWithoutRtcpMux, local_offer);
- JsepSessionDescription *remote_offer =
+ JsepSessionDescription* remote_offer =
new JsepSessionDescription(JsepSessionDescription::kOffer);
EXPECT_TRUE((remote_offer)->Initialize(offer_str, NULL));
SetRemoteDescriptionOfferExpectError(kBundleWithoutRtcpMux, remote_offer);
@@ -3258,8 +3358,8 @@
candidate1);
EXPECT_TRUE(offer->AddCandidate(&ice_candidate1));
SetRemoteDescriptionWithoutError(offer);
- ASSERT_TRUE(session_->GetTransportProxy("audio") != NULL);
- ASSERT_TRUE(session_->GetTransportProxy("video") != NULL);
+ ASSERT_TRUE(session_->voice_rtp_transport_channel() != NULL);
+ ASSERT_TRUE(session_->video_rtp_transport_channel() != NULL);
// Pump for 1 second and verify that no candidates are generated.
rtc::Thread::Current()->ProcessMessages(1000);
@@ -3268,8 +3368,6 @@
SessionDescriptionInterface* answer = CreateAnswer(NULL);
SetLocalDescriptionWithoutError(answer);
- EXPECT_TRUE(session_->GetTransportProxy("audio")->negotiated());
- EXPECT_TRUE(session_->GetTransportProxy("video")->negotiated());
EXPECT_TRUE_WAIT(observer_.oncandidatesready_, kIceCandidatesTimeout);
}
@@ -3304,7 +3402,7 @@
// will be set as per MediaSessionDescriptionFactory.
std::string offer_str;
offer->ToString(&offer_str);
- SessionDescriptionInterface *jsep_offer_str =
+ SessionDescriptionInterface* jsep_offer_str =
CreateSessionDescription(JsepSessionDescription::kOffer, offer_str, NULL);
SetLocalDescriptionWithoutError(jsep_offer_str);
EXPECT_FALSE(session_->voice_channel()->secure_required());
@@ -3657,8 +3755,8 @@
TEST_P(WebRtcSessionTest,
TestMultipleCreateOfferBeforeIdentityRequestReturnSuccess) {
MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
- VerifyMultipleAsyncCreateDescription(
- GetParam(), CreateSessionDescriptionRequest::kOffer);
+ VerifyMultipleAsyncCreateDescription(GetParam(),
+ CreateSessionDescriptionRequest::kOffer);
}
// Verifies that CreateOffer fails when Multiple CreateOffer calls are made
@@ -3881,31 +3979,31 @@
rtc::Socket::Option::OPT_RCVBUF, 8000);
int option_val;
- EXPECT_TRUE(session_->video_channel()->transport_channel()->GetOption(
+ EXPECT_TRUE(session_->video_rtp_transport_channel()->GetOption(
rtc::Socket::Option::OPT_SNDBUF, &option_val));
EXPECT_EQ(4000, option_val);
- EXPECT_FALSE(session_->voice_channel()->transport_channel()->GetOption(
+ EXPECT_FALSE(session_->voice_rtp_transport_channel()->GetOption(
rtc::Socket::Option::OPT_SNDBUF, &option_val));
- EXPECT_TRUE(session_->voice_channel()->transport_channel()->GetOption(
+ EXPECT_TRUE(session_->voice_rtp_transport_channel()->GetOption(
rtc::Socket::Option::OPT_RCVBUF, &option_val));
EXPECT_EQ(8000, option_val);
- EXPECT_FALSE(session_->video_channel()->transport_channel()->GetOption(
+ EXPECT_FALSE(session_->video_rtp_transport_channel()->GetOption(
rtc::Socket::Option::OPT_RCVBUF, &option_val));
- EXPECT_NE(session_->voice_channel()->transport_channel(),
- session_->video_channel()->transport_channel());
+ EXPECT_NE(session_->voice_rtp_transport_channel(),
+ session_->video_rtp_transport_channel());
mediastream_signaling_.SendAudioVideoStream2();
SessionDescriptionInterface* answer =
CreateRemoteAnswer(session_->local_description());
SetRemoteDescriptionWithoutError(answer);
- EXPECT_TRUE(session_->voice_channel()->transport_channel()->GetOption(
+ EXPECT_TRUE(session_->voice_rtp_transport_channel()->GetOption(
rtc::Socket::Option::OPT_SNDBUF, &option_val));
EXPECT_EQ(4000, option_val);
- EXPECT_TRUE(session_->voice_channel()->transport_channel()->GetOption(
+ EXPECT_TRUE(session_->voice_rtp_transport_channel()->GetOption(
rtc::Socket::Option::OPT_RCVBUF, &option_val));
EXPECT_EQ(8000, option_val);
}
@@ -3941,6 +4039,7 @@
// currently fails because upon disconnection and reconnection OnIceComplete is
// called more than once without returning to IceGatheringGathering.
-INSTANTIATE_TEST_CASE_P(
- WebRtcSessionTests, WebRtcSessionTest,
- testing::Values(ALREADY_GENERATED, DTLS_IDENTITY_STORE));
+INSTANTIATE_TEST_CASE_P(WebRtcSessionTests,
+ WebRtcSessionTest,
+ testing::Values(ALREADY_GENERATED,
+ DTLS_IDENTITY_STORE));
diff --git a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc
index caa53df..f6414d3 100644
--- a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc
+++ b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc
@@ -165,9 +165,15 @@
WebRtcSession* session,
const std::string& session_id,
cricket::DataChannelType dct)
- : WebRtcSessionDescriptionFactory(
- signaling_thread, channel_manager, mediastream_signaling, nullptr,
- nullptr, session, session_id, dct, false) {
+ : WebRtcSessionDescriptionFactory(signaling_thread,
+ channel_manager,
+ mediastream_signaling,
+ nullptr,
+ nullptr,
+ session,
+ session_id,
+ dct,
+ false) {
LOG(LS_VERBOSE) << "DTLS-SRTP disabled.";
}
@@ -226,9 +232,9 @@
// We already have a certificate but we wait to do SetIdentity; if we do
// it in the constructor then the caller has not had a chance to connect to
// SignalIdentityReady.
- signaling_thread_->Post(this, MSG_USE_CONSTRUCTOR_CERTIFICATE,
- new rtc::ScopedRefMessageData<rtc::RTCCertificate>(
- certificate));
+ signaling_thread_->Post(
+ this, MSG_USE_CONSTRUCTOR_CERTIFICATE,
+ new rtc::ScopedRefMessageData<rtc::RTCCertificate>(certificate));
}
WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() {
@@ -254,8 +260,6 @@
delete msg.pdata;
}
}
-
- transport_desc_factory_.set_certificate(nullptr);
}
void WebRtcSessionDescriptionFactory::CreateOffer(
diff --git a/talk/app/webrtc/webrtcsessiondescriptionfactory.h b/talk/app/webrtc/webrtcsessiondescriptionfactory.h
index b42a551..52b8da5 100644
--- a/talk/app/webrtc/webrtcsessiondescriptionfactory.h
+++ b/talk/app/webrtc/webrtcsessiondescriptionfactory.h
@@ -90,13 +90,12 @@
public sigslot::has_slots<> {
public:
// Construct with DTLS disabled.
- WebRtcSessionDescriptionFactory(
- rtc::Thread* signaling_thread,
- cricket::ChannelManager* channel_manager,
- MediaStreamSignaling* mediastream_signaling,
- WebRtcSession* session,
- const std::string& session_id,
- cricket::DataChannelType dct);
+ WebRtcSessionDescriptionFactory(rtc::Thread* signaling_thread,
+ cricket::ChannelManager* channel_manager,
+ MediaStreamSignaling* mediastream_signaling,
+ WebRtcSession* session,
+ const std::string& session_id,
+ cricket::DataChannelType dct);
// Construct with DTLS enabled using the specified |dtls_identity_store| to
// generate a certificate.