Add PeerConnection IsClosed check.

webrtc/api/peerconnectioninterface_unittest.cc

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 22 matching lines @@
 #include "webrtc/api/videocapturertracksource.h"
 #include "webrtc/api/videotrack.h"
 #include "webrtc/base/gunit.h"
 #include "webrtc/base/ssladapter.h"
 #include "webrtc/base/sslstreamadapter.h"
 #include "webrtc/base/stringutils.h"
 #include "webrtc/base/thread.h"
 #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";
 static const char kStreamLabel2[] = "local_stream_2";
 static const char kStreamLabel3[] = "local_stream_3";
 static const int kDefaultStunPort = 3478;
 static const char kStunAddressOnly[] = "stun:address";
 static const char kStunInvalidPort[] = "stun:address:-1";
 static const char kStunAddressPortAndMore1[] = "stun:address:port:more";
 static const char kStunAddressPortAndMore2[] = "stun:address:port more";
@@ skipping 431 matching lines @@
   }
   void OnRenegotiationNeeded() override { renegotiation_needed_ = true; }
   void OnDataChannel(
       rtc::scoped_refptr<DataChannelInterface> data_channel) override {
     last_datachannel_ = data_channel;
   }
 
   void OnIceConnectionChange(
       PeerConnectionInterface::IceConnectionState new_state) override {
     EXPECT_EQ(pc_->ice_connection_state(), new_state);
+    is_triggered = true;
   }
   void OnIceGatheringChange(
       PeerConnectionInterface::IceGatheringState new_state) override {
     EXPECT_EQ(pc_->ice_gathering_state(), new_state);
     ice_complete_ = new_state == PeerConnectionInterface::kIceGatheringComplete;
+    is_triggered = true;
   }
   void OnIceCandidate(const webrtc::IceCandidateInterface* candidate) override {
     EXPECT_NE(PeerConnectionInterface::kIceGatheringNew,
               pc_->ice_gathering_state());
 
     std::string sdp;
     EXPECT_TRUE(candidate->ToString(&sdp));
     EXPECT_LT(0u, sdp.size());
     last_candidate_.reset(webrtc::CreateIceCandidate(candidate->sdp_mid(),
         candidate->sdp_mline_index(), sdp, NULL));
     EXPECT_TRUE(last_candidate_.get() != NULL);
+    is_triggered = true;
+  }
+
+  void OnIceCandidatesRemoved(
+      const std::vector<cricket::Candidate>& candidates) override {
+    is_triggered = true;
+  }
+
+  void OnIceConnectionReceivingChange(bool receiving) override {
+    is_triggered = true;
   }
 
   // Returns the label of the last added stream.
   // Empty string if no stream have been added.
   std::string GetLastAddedStreamLabel() {
     if (last_added_stream_.get())
       return last_added_stream_->label();
     return "";
   }
   std::string GetLastRemovedStreamLabel() {
     if (last_removed_stream_.get())
       return last_removed_stream_->label();
     return "";
   }
 
   scoped_refptr<PeerConnectionInterface> pc_;
   PeerConnectionInterface::SignalingState state_;
   std::unique_ptr<IceCandidateInterface> last_candidate_;
   scoped_refptr<DataChannelInterface> last_datachannel_;
   rtc::scoped_refptr<StreamCollection> remote_streams_;
   bool renegotiation_needed_ = false;
   bool ice_complete_ = false;
+  bool is_triggered = false;

[Taylor Brandstetter, 2016/06/21 23:50:02]

Maybe rename to "callback_triggered"?

[Zhi Huang, 2016/06/22 22:01:08]

OK. This is more readable.

 
  private:
   scoped_refptr<MediaStreamInterface> last_added_stream_;
   scoped_refptr<MediaStreamInterface> last_removed_stream_;
 };
 
 }  // 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() {
 #ifdef WEBRTC_ANDROID
     webrtc::InitializeAndroidObjects();
 #endif
   }
 
   virtual void SetUp() {
     pc_factory_ = webrtc::CreatePeerConnectionFactory(
         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() {
     CreatePeerConnection(PeerConnectionInterface::RTCConfiguration(), nullptr);
   }
 
   void CreatePeerConnection(webrtc::MediaConstraintsInterface* constraints) {
     CreatePeerConnection(PeerConnectionInterface::RTCConfiguration(),
                          constraints);
   }
@@ skipping 162 matching lines @@
 
   bool DoSetSessionDescription(SessionDescriptionInterface* desc, bool local) {
     rtc::scoped_refptr<MockSetSessionDescriptionObserver>
         observer(new rtc::RefCountedObject<
             MockSetSessionDescriptionObserver>());
     if (local) {
       pc_->SetLocalDescription(observer, desc);
     } 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();
   }
 
   bool DoSetLocalDescription(SessionDescriptionInterface* desc) {
     return DoSetSessionDescription(desc, true);
   }
 
   bool DoSetRemoteDescription(SessionDescriptionInterface* desc) {
     return DoSetSessionDescription(desc, false);
   }
@@ skipping 241 matching lines @@
     const cricket::ContentInfo* audio_content =
         cricket::GetFirstAudioContent(desc->description());
     const cricket::AudioContentDescription* audio_desc =
         static_cast<const cricket::AudioContentDescription*>(
             audio_content->description);
     return audio_desc->streams()[0].cname;
   }
 
   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_F(PeerConnectionInterfaceTest, CloseAndTestCallbackFunctions) {

[Taylor Brandstetter, 2016/06/21 23:50:02]

You could add a comment explaining that this is testing that no callbacks on the
PeerConnectionObserver are called after the PeerConnection is closed.

+  scoped_refptr<PeerConnectionInterface> pc(
+      pc_factory_for_test_->CreatePeerConnection(
+          PeerConnectionInterface::RTCConfiguration(), nullptr, nullptr,
+          nullptr, &observer_));
+  observer_.SetPeerConnectionInterface(pc.get());
+  pc->Close();
+
+  observer_.is_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_.is_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.
 TEST_F(PeerConnectionInterfaceTest, CnameGenerationInOffer) {
   std::unique_ptr<SessionDescriptionInterface> offer1 =
       CreateOfferWithOneAudioStream();
   std::unique_ptr<SessionDescriptionInterface> offer2 =
       CreateOfferWithOneAudioStream();
   EXPECT_NE(GetFirstAudioStreamCname(offer1.get()),
             GetFirstAudioStreamCname(offer2.get()));
@@ skipping 1512 matching lines @@
 
   auto new_senders = pc_->GetSenders();
   // Should be the same senders as before, but with updated stream id.
   // Note that this behavior is subject to change in the future.
   // We may decide the PC should ignore existing tracks in AddStream.
   EXPECT_EQ(senders, new_senders);
   EXPECT_TRUE(ContainsSender(new_senders, kAudioTracks[0], kStreams[1]));
   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 {
     pcf_ = new rtc::RefCountedObject<PeerConnectionFactoryForTest>();
     pcf_->Initialize();
   }
   const cricket::MediaConfig& TestCreatePeerConnection(
       const PeerConnectionInterface::RTCConfiguration& config,
       const MediaConstraintsInterface *constraints) {
     pcf_->create_media_controller_called_ = false;
@@ skipping 244 matching lines @@
   FakeConstraints updated_answer_c;
   answer_c.SetMandatoryReceiveAudio(false);
   answer_c.SetMandatoryReceiveVideo(false);
 
   cricket::MediaSessionOptions updated_answer_options;
   EXPECT_TRUE(
       ParseConstraintsForAnswer(&updated_answer_c, &updated_answer_options));
   EXPECT_TRUE(updated_answer_options.has_audio());
   EXPECT_TRUE(updated_answer_options.has_video());
 }