Only generate one CNAME per PeerConnection.

webrtc/api/peerconnectioninterface_unittest.cc

Index: webrtc/api/peerconnectioninterface_unittest.cc
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index 14a067995b42205c698b5581d32b73f1972e1449..4d0f8d90cc542cb49d3cbf123efaa36a4b117aae 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -935,6 +935,39 @@ class PeerConnectionInterfaceTest : public testing::Test {
     ASSERT_TRUE(stream->AddTrack(video_track));
   }
 
+  void GetStreamCnameInOfferAnswer(std::string& cname, bool is_offer) {

[pthatcher1, 2016/05/05 18:50:51]

I think it would be cleaner to break this up into separate methods:

scoped_ptr<SessionDescriptionInterface> CreateOfferWithOneAudioStream() {
  CreatePeerConnection();
  AddVoiceStream(kStreamLabel1);
  scoped_ptr<SessionDescriptionInterface> offer;
  ASSERT_TRUE(DoCreateOffer(offer.get(), nullptr));
  return offer;  
}

scoped_ptr<SessionDescriptionInterface> CreateAnswerWithOneAudioStream() {
  scoped_ptr<SessionDescriptionInterface> offer =
CreateOfferWithOneAudioStream();  
  EXPECT_TRUE(DoSetRemoteDescription(offer.release()));
  rtc::scoped_ptr<SessionDescriptionInterface> answer;
  EXPECT_TRUE(DoCreateAnswer(answer.get(), nullptr))
  return answer;
}

const std::string& GetFirstAudioStreamCname(const SessionDescriptionInterface*
desc) {
  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;
}


Then the tests can be this:

scoped_ptr<SessionDescriptionInterface> offer1 =
CreateOfferWithOneAudioStream();
scoped_ptr<SessionDescriptionInterface> offer2 =
CreateOfferWithOneAudioStream();
EXPECT_NE(GetFirstAudioStreamCname(offer1), GetFirstAudioStreamCname(offer2)) 


scoped_ptr<SessionDescriptionInterface> answer1 =
CreateAnswerWithOneAudioStream();
scoped_ptr<SessionDescriptionInterface> answer2 =
CreateAnswerWithOneAudioStream();
EXPECT_NE(GetFirstAudioStreamCname(answer1), GetFirstAudioStreamCname(answer2))

[Zhi Huang, 2016/05/06 01:36:53]

This looks nice!

+    CreatePeerConnection();
+    AddVoiceStream(kStreamLabel1);
+    scoped_ptr<SessionDescriptionInterface> offer;
+    ASSERT_TRUE(DoCreateOffer(&offer, nullptr));
+
+    const cricket::ContentInfo* audio_content =
+        cricket::GetFirstAudioContent(offer->description());
+    const cricket::AudioContentDescription* audio_desc =
+        static_cast<const cricket::AudioContentDescription*>(
+            audio_content->description);
+    if (is_offer) {
+      cname = audio_desc->streams()[0].cname;
+      return;
+    }
+
+    EXPECT_TRUE(DoSetRemoteDescription(offer.release()));
+    rtc::scoped_ptr<SessionDescriptionInterface> answer;
+    EXPECT_TRUE(DoCreateAnswer(&answer, nullptr));
+    audio_content = cricket::GetFirstAudioContent(answer->description());
+    audio_desc = static_cast<const cricket::AudioContentDescription*>(
+        audio_content->description);
+    cname = audio_desc->streams()[0].cname;
+  }
+
+  void GetStreamCnameInOffer(std::string& cname) {
+    GetStreamCnameInOfferAnswer(cname, true);
+  }
+
+  void GetStreamCnameInAnswer(std::string& cname) {
+    GetStreamCnameInOfferAnswer(cname, false);

[pthatcher1, 2016/05/05 18:50:51]

Instead of passing in a non-const ref, please pass in a pointer.

Better yet, just return a const std::string&, as I outlined in the previous
comment.

+  }
+
   cricket::FakePortAllocator* port_allocator_ = nullptr;
   scoped_refptr<webrtc::PeerConnectionFactoryInterface> pc_factory_;
   scoped_refptr<PeerConnectionInterface> pc_;
@@ -942,6 +975,23 @@ class PeerConnectionInterfaceTest : public testing::Test {
   rtc::scoped_refptr<StreamCollection> reference_collection_;
 };
 
+// Generate different CNAMEs when PeerConnections are created.
+// The CNAME will be generated randomly. It is possible that
+// the test fails, though the possibility is low.
+TEST_F(PeerConnectionInterfaceTest, CnameGenerationInOffer) {
+  std::string cname1, cname2;
+  GetStreamCnameInOffer(cname1);
+  GetStreamCnameInOffer(cname2);
+  EXPECT_NE(cname1, cname2);
+}
+
+TEST_F(PeerConnectionInterfaceTest, CnameGenerationInAnswer) {
+  std::string cname1, cname2;
+  GetStreamCnameInAnswer(cname1);
+  GetStreamCnameInAnswer(cname2);
+  EXPECT_NE(cname1, cname2);
+}
+
 TEST_F(PeerConnectionInterfaceTest,
        CreatePeerConnectionWithDifferentConfigurations) {
   CreatePeerConnectionWithDifferentConfigurations();