A bunch of interfaces: Return scoped_ptr<SSLCertificate>
Instead of using a raw pointer output parameter. This affects
SSLStreamAdapter::GetPeerCertificate
Transport::GetRemoteSSLCertificate
TransportChannel::GetRemoteSSLCertificate
TransportController::GetRemoteSSLCertificate
WebRtcSession::GetRemoteSSLCertificate
This is a good idea in general, but will also be very convenient when
scoped_ptr is gone, since unique_ptr doesn't have an .accept() method.
BUG=webrtc:5520
Review URL: https://codereview.webrtc.org/1802013002
Cr-Commit-Position: refs/heads/master@{#12262}
diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc
index 0182a37..0901fc6 100644
--- a/webrtc/api/statscollector.cc
+++ b/webrtc/api/statscollector.cc
@@ -702,9 +702,10 @@
local_cert_report_id = r->id();
}
- rtc::scoped_ptr<rtc::SSLCertificate> cert;
- if (pc_->session()->GetRemoteSSLCertificate(
- transport_iter.second.transport_name, cert.accept())) {
+ rtc::scoped_ptr<rtc::SSLCertificate> cert =
+ pc_->session()->GetRemoteSSLCertificate(
+ transport_iter.second.transport_name);
+ if (cert) {
StatsReport* r = AddCertificateReports(cert.get());
if (r)
remote_cert_report_id = r->id();
diff --git a/webrtc/api/statscollector_unittest.cc b/webrtc/api/statscollector_unittest.cc
index 3b04383..5873e73 100644
--- a/webrtc/api/statscollector_unittest.cc
+++ b/webrtc/api/statscollector_unittest.cc
@@ -82,9 +82,15 @@
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));
+
+ // Workaround for gmock's inability to cope with move-only return values.
+ rtc::scoped_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate(
+ const std::string& transport_name) override {
+ return rtc::scoped_ptr<rtc::SSLCertificate>(
+ GetRemoteSSLCertificate_ReturnsRawPointer(transport_name));
+ }
+ MOCK_METHOD1(GetRemoteSSLCertificate_ReturnsRawPointer,
+ rtc::SSLCertificate*(const std::string& transport_name));
};
// The factory isn't really used; it just satisfies the base PeerConnection.
@@ -662,10 +668,11 @@
VerifyVoiceReceiverInfoReport(track_report, *voice_receiver_info);
}
- void TestCertificateReports(const rtc::FakeSSLCertificate& local_cert,
- const std::vector<std::string>& local_ders,
- const rtc::FakeSSLCertificate& remote_cert,
- const std::vector<std::string>& remote_ders) {
+ void TestCertificateReports(
+ const rtc::FakeSSLCertificate& local_cert,
+ const std::vector<std::string>& local_ders,
+ rtc::scoped_ptr<rtc::FakeSSLCertificate> remote_cert,
+ const std::vector<std::string>& remote_ders) {
StatsCollectorForTest stats(&pc_);
StatsReports reports; // returned values.
@@ -694,10 +701,9 @@
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_, GetRemoteSSLCertificate_ReturnsRawPointer(
+ transport_stats.transport_name))
+ .WillOnce(Return(remote_cert.release()));
EXPECT_CALL(session_, GetTransportStats(_))
.WillOnce(DoAll(SetArgPointee<0>(session_stats),
Return(true)));
@@ -807,8 +813,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
const char kVideoChannelName[] = "video";
@@ -853,8 +859,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
const char kVideoChannelName[] = "video";
@@ -965,8 +971,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
@@ -1037,8 +1043,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
// The transport_name known by the video channel.
@@ -1121,8 +1127,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
// The transport_name known by the video channel.
@@ -1172,8 +1178,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
@@ -1333,9 +1339,11 @@
remote_ders[1] = "non-";
remote_ders[2] = "intersecting";
remote_ders[3] = "set";
- rtc::FakeSSLCertificate remote_cert(DersToPems(remote_ders));
+ rtc::scoped_ptr<rtc::FakeSSLCertificate> remote_cert(
+ new rtc::FakeSSLCertificate(DersToPems(remote_ders)));
- TestCertificateReports(local_cert, local_ders, remote_cert, remote_ders);
+ TestCertificateReports(local_cert, local_ders, std::move(remote_cert),
+ remote_ders);
}
// This test verifies that all certificates without chains are correctly
@@ -1347,10 +1355,12 @@
// Build remote certificate.
std::string remote_der = "This is somebody else's der.";
- rtc::FakeSSLCertificate remote_cert(DerToPem(remote_der));
+ rtc::scoped_ptr<rtc::FakeSSLCertificate> remote_cert(
+ new rtc::FakeSSLCertificate(DerToPem(remote_der)));
TestCertificateReports(local_cert, std::vector<std::string>(1, local_der),
- remote_cert, std::vector<std::string>(1, remote_der));
+ std::move(remote_cert),
+ std::vector<std::string>(1, remote_der));
}
// This test verifies that the stats are generated correctly when no
@@ -1360,8 +1370,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
StatsReports reports; // returned values.
@@ -1417,8 +1427,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
StatsReports reports; // returned values.
@@ -1469,11 +1479,12 @@
// Build a remote certificate with an unsupported digest algorithm.
std::string remote_der = "This is somebody else's der.";
- rtc::FakeSSLCertificate remote_cert(DerToPem(remote_der));
- remote_cert.set_digest_algorithm("foobar");
+ rtc::scoped_ptr<rtc::FakeSSLCertificate> remote_cert(
+ new rtc::FakeSSLCertificate(DerToPem(remote_der)));
+ remote_cert->set_digest_algorithm("foobar");
TestCertificateReports(local_cert, std::vector<std::string>(1, local_der),
- remote_cert, std::vector<std::string>());
+ std::move(remote_cert), std::vector<std::string>());
}
// This test verifies that a local stats object can get statistics via
@@ -1483,8 +1494,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The transport_name known by the voice channel.
@@ -1518,8 +1529,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The transport_name known by the voice channel.
@@ -1547,8 +1558,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The transport_name known by the voice channel.
@@ -1608,8 +1619,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The transport_name known by the voice channel.
@@ -1695,8 +1706,8 @@
EXPECT_CALL(session_, GetLocalCertificate(_, _))
.WillRepeatedly(Return(false));
- EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _))
- .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
// The transport_name known by the voice channel.
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index 19f5aa4..f546285 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -1039,10 +1039,10 @@
certificate);
}
-bool WebRtcSession::GetRemoteSSLCertificate(const std::string& transport_name,
- rtc::SSLCertificate** cert) {
+rtc::scoped_ptr<rtc::SSLCertificate> WebRtcSession::GetRemoteSSLCertificate(
+ const std::string& transport_name) {
ASSERT(signaling_thread()->IsCurrent());
- return transport_controller_->GetRemoteSSLCertificate(transport_name, cert);
+ return transport_controller_->GetRemoteSSLCertificate(transport_name);
}
bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) {
diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h
index 6164731..01ec526 100644
--- a/webrtc/api/webrtcsession.h
+++ b/webrtc/api/webrtcsession.h
@@ -292,8 +292,8 @@
rtc::scoped_refptr<rtc::RTCCertificate>* certificate);
// Caller owns returned certificate
- virtual bool GetRemoteSSLCertificate(const std::string& transport_name,
- rtc::SSLCertificate** cert);
+ virtual rtc::scoped_ptr<rtc::SSLCertificate> GetRemoteSSLCertificate(
+ const std::string& transport_name);
cricket::DataChannelType data_channel_type() const;