From f3ff14c00b43b04b501f9c8333352b8202920b4e Mon Sep 17 00:00:00 2001 From: Yves Gerey Date: Thu, 25 Oct 2018 10:28:12 +0200 Subject: [PATCH] Properly setup MockPeerConnectionObserver in tests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL prevents dereferencing potentially null pointer by: * Setting the pointer in client code. * Checking the pointer before use. Bug: webrtc:9855 Change-Id: I90c3d00eedfa4bf97954f4795a83e28894cc40f7 Reviewed-on: https://webrtc-review.googlesource.com/c/107706 Reviewed-by: Henrik Boström Reviewed-by: Patrik Höglund Commit-Queue: Yves Gerey Cr-Commit-Position: refs/heads/master@{#25405} --- pc/peerconnectioninterface_unittest.cc | 12 ++++++++++++ pc/test/mockpeerconnectionobservers.h | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index 86a04bfb09..c4cf081886 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -3956,6 +3956,7 @@ class PeerConnectionMediaConfigTest : public testing::Test { rtc::scoped_refptr pc( pcf_->CreatePeerConnection(config, nullptr, nullptr, &observer_)); EXPECT_TRUE(pc.get()); + observer_.SetPeerConnectionInterface(pc.get()); return pc->GetConfiguration().media_config; } @@ -3963,6 +3964,17 @@ class PeerConnectionMediaConfigTest : public testing::Test { MockPeerConnectionObserver observer_; }; +// This sanity check validates the test infrastructure itself. +TEST_F(PeerConnectionMediaConfigTest, TestCreateAndClose) { + PeerConnectionInterface::RTCConfiguration config; + rtc::scoped_refptr pc( + pcf_->CreatePeerConnection(config, nullptr, nullptr, &observer_)); + EXPECT_TRUE(pc.get()); + observer_.SetPeerConnectionInterface(pc.get()); // Required. + pc->Close(); // No abort -> ok. + SUCCEED(); +} + // This test verifies the default behaviour with no constraints and a // default RTCConfiguration. TEST_F(PeerConnectionMediaConfigTest, TestDefaults) { diff --git a/pc/test/mockpeerconnectionobservers.h b/pc/test/mockpeerconnectionobservers.h index 6f5e9bc665..62c0c9415f 100644 --- a/pc/test/mockpeerconnectionobservers.h +++ b/pc/test/mockpeerconnectionobservers.h @@ -67,6 +67,7 @@ class MockPeerConnectionObserver : public PeerConnectionObserver { } void OnSignalingChange( PeerConnectionInterface::SignalingState new_state) override { + RTC_DCHECK(pc_); RTC_DCHECK(pc_->signaling_state() == new_state); state_ = new_state; } @@ -92,6 +93,7 @@ class MockPeerConnectionObserver : public PeerConnectionObserver { void OnIceConnectionChange( PeerConnectionInterface::IceConnectionState new_state) override { + RTC_DCHECK(pc_); RTC_DCHECK(pc_->ice_connection_state() == new_state); // When ICE is finished, the caller will get to a kIceConnectionCompleted // state, because it has the ICE controlling role, while the callee @@ -104,12 +106,14 @@ class MockPeerConnectionObserver : public PeerConnectionObserver { } void OnIceGatheringChange( PeerConnectionInterface::IceGatheringState new_state) override { + RTC_DCHECK(pc_); RTC_DCHECK(pc_->ice_gathering_state() == new_state); ice_gathering_complete_ = new_state == PeerConnectionInterface::kIceGatheringComplete; callback_triggered_ = true; } void OnIceCandidate(const IceCandidateInterface* candidate) override { + RTC_DCHECK(pc_); RTC_DCHECK(PeerConnectionInterface::kIceGatheringNew != pc_->ice_gathering_state()); candidates_.push_back(absl::make_unique(