Add PeerConnection IsClosed check.
Add IsClosed check when excuting some functions so that they can return early if the PeerConnection is closed.
The observer will not be called after the PeerConnection is closed.
BUG=webrtc:5861
Review-Url: https://codereview.webrtc.org/1975453002
Cr-Commit-Position: refs/heads/master@{#13544}
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index 843a312..419f69c 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -40,6 +40,7 @@
#include "webrtc/media/base/fakevideocapturer.h"
#include "webrtc/media/sctp/sctpdataengine.h"
#include "webrtc/p2p/base/fakeportallocator.h"
+#include "webrtc/p2p/base/faketransportcontroller.h"
#include "webrtc/pc/mediasession.h"
static const char kStreamLabel1[] = "local_stream_1";
@@ -491,11 +492,13 @@
void OnIceConnectionChange(
PeerConnectionInterface::IceConnectionState new_state) override {
EXPECT_EQ(pc_->ice_connection_state(), new_state);
+ callback_triggered = true;
}
void OnIceGatheringChange(
PeerConnectionInterface::IceGatheringState new_state) override {
EXPECT_EQ(pc_->ice_gathering_state(), new_state);
ice_complete_ = new_state == PeerConnectionInterface::kIceGatheringComplete;
+ callback_triggered = true;
}
void OnIceCandidate(const webrtc::IceCandidateInterface* candidate) override {
EXPECT_NE(PeerConnectionInterface::kIceGatheringNew,
@@ -507,6 +510,16 @@
last_candidate_.reset(webrtc::CreateIceCandidate(candidate->sdp_mid(),
candidate->sdp_mline_index(), sdp, NULL));
EXPECT_TRUE(last_candidate_.get() != NULL);
+ callback_triggered = true;
+ }
+
+ void OnIceCandidatesRemoved(
+ const std::vector<cricket::Candidate>& candidates) override {
+ callback_triggered = true;
+ }
+
+ void OnIceConnectionReceivingChange(bool receiving) override {
+ callback_triggered = true;
}
// Returns the label of the last added stream.
@@ -529,6 +542,7 @@
rtc::scoped_refptr<StreamCollection> remote_streams_;
bool renegotiation_needed_ = false;
bool ice_complete_ = false;
+ bool callback_triggered = false;
private:
scoped_refptr<MediaStreamInterface> last_added_stream_;
@@ -537,6 +551,36 @@
} // namespace
+// The PeerConnectionMediaConfig tests below verify that configuration
+// and constraints are propagated into the MediaConfig passed to
+// CreateMediaController. These settings are intended for MediaChannel
+// constructors, but that is not exercised by these unittest.
+class PeerConnectionFactoryForTest : public webrtc::PeerConnectionFactory {
+ public:
+ webrtc::MediaControllerInterface* CreateMediaController(
+ const cricket::MediaConfig& config) const override {
+ create_media_controller_called_ = true;
+ create_media_controller_config_ = config;
+
+ webrtc::MediaControllerInterface* mc =
+ PeerConnectionFactory::CreateMediaController(config);
+ EXPECT_TRUE(mc != nullptr);
+ return mc;
+ }
+
+ cricket::TransportController* CreateTransportController(
+ cricket::PortAllocator* port_allocator) override {
+ transport_controller = new cricket::TransportController(
+ rtc::Thread::Current(), rtc::Thread::Current(), port_allocator);
+ return transport_controller;
+ }
+
+ cricket::TransportController* transport_controller;
+ // Mutable, so they can be modified in the above const-declared method.
+ mutable bool create_media_controller_called_ = false;
+ mutable cricket::MediaConfig create_media_controller_config_;
+};
+
class PeerConnectionInterfaceTest : public testing::Test {
protected:
PeerConnectionInterfaceTest() {
@@ -550,6 +594,9 @@
rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(),
nullptr, nullptr, nullptr);
ASSERT_TRUE(pc_factory_);
+ pc_factory_for_test_ =
+ new rtc::RefCountedObject<PeerConnectionFactoryForTest>();
+ pc_factory_for_test_->Initialize();
}
void CreatePeerConnection() {
@@ -732,7 +779,9 @@
} else {
pc_->SetRemoteDescription(observer, desc);
}
- EXPECT_EQ_WAIT(true, observer->called(), kTimeout);
+ if (pc_->signaling_state() != PeerConnectionInterface::kClosed) {
+ EXPECT_EQ_WAIT(true, observer->called(), kTimeout);
+ }
return observer->result();
}
@@ -994,11 +1043,37 @@
cricket::FakePortAllocator* port_allocator_ = nullptr;
scoped_refptr<webrtc::PeerConnectionFactoryInterface> pc_factory_;
+ scoped_refptr<PeerConnectionFactoryForTest> pc_factory_for_test_;
scoped_refptr<PeerConnectionInterface> pc_;
MockPeerConnectionObserver observer_;
rtc::scoped_refptr<StreamCollection> reference_collection_;
};
+// Test that no callbacks on the PeerConnectionObserver are called after the
+// PeerConnection is closed.
+TEST_F(PeerConnectionInterfaceTest, CloseAndTestCallbackFunctions) {
+ scoped_refptr<PeerConnectionInterface> pc(
+ pc_factory_for_test_->CreatePeerConnection(
+ PeerConnectionInterface::RTCConfiguration(), nullptr, nullptr,
+ nullptr, &observer_));
+ observer_.SetPeerConnectionInterface(pc.get());
+ pc->Close();
+
+ // No callbacks is expected to be called.
+ observer_.callback_triggered = false;
+ std::vector<cricket::Candidate> candidates;
+ pc_factory_for_test_->transport_controller->SignalGatheringState(
+ cricket::IceGatheringState{});
+ pc_factory_for_test_->transport_controller->SignalCandidatesGathered(
+ "", candidates);
+ pc_factory_for_test_->transport_controller->SignalConnectionState(
+ cricket::IceConnectionState{});
+ pc_factory_for_test_->transport_controller->SignalCandidatesRemoved(
+ candidates);
+ pc_factory_for_test_->transport_controller->SignalReceiving(false);
+ EXPECT_FALSE(observer_.callback_triggered);
+}
+
// Generate different CNAMEs when PeerConnections are created.
// The CNAMEs are expected to be generated randomly. It is possible
// that the test fails, though the possibility is very low.
@@ -2555,28 +2630,6 @@
EXPECT_TRUE(ContainsSender(new_senders, kVideoTracks[0], kStreams[1]));
}
-// The PeerConnectionMediaConfig tests below verify that configuration
-// and constraints are propagated into the MediaConfig passed to
-// CreateMediaController. These settings are intended for MediaChannel
-// constructors, but that is not exercised by these unittest.
-class PeerConnectionFactoryForTest : public webrtc::PeerConnectionFactory {
- public:
- webrtc::MediaControllerInterface* CreateMediaController(
- const cricket::MediaConfig& config) const override {
- create_media_controller_called_ = true;
- create_media_controller_config_ = config;
-
- webrtc::MediaControllerInterface* mc =
- PeerConnectionFactory::CreateMediaController(config);
- EXPECT_TRUE(mc != nullptr);
- return mc;
- }
-
- // Mutable, so they can be modified in the above const-declared method.
- mutable bool create_media_controller_called_ = false;
- mutable cricket::MediaConfig create_media_controller_config_;
-};
-
class PeerConnectionMediaConfigTest : public testing::Test {
protected:
void SetUp() override {