 Chromium Code Reviews
 Chromium Code Reviews Issue 3015473002:
  Reject the descriptions that attempt to change the order of m= section in current local description.
    
  
    Issue 3015473002:
  Reject the descriptions that attempt to change the order of m= section in current local description. 
  | Index: webrtc/pc/peerconnectioninterface_unittest.cc | 
| diff --git a/webrtc/pc/peerconnectioninterface_unittest.cc b/webrtc/pc/peerconnectioninterface_unittest.cc | 
| index c23a79d597b4fe8bdaec83edbd867df52ef3f3f5..6cc6e81abcc54a81511b83f37e22d216b5728dbf 100644 | 
| --- a/webrtc/pc/peerconnectioninterface_unittest.cc | 
| +++ b/webrtc/pc/peerconnectioninterface_unittest.cc | 
| @@ -1241,6 +1241,35 @@ class PeerConnectionInterfaceTest : public testing::Test { | 
| return false; | 
| } | 
| + void ReorderMediaSections(SessionDescriptionInterface* session_description) { | 
| 
Taylor Brandstetter
2017/09/14 00:41:47
nit: This could be simplified by using std::revers
 | 
| + if (!session_description) { | 
| + return; | 
| + } | 
| + auto desc = session_description->description(); | 
| + RTC_DCHECK(desc); | 
| + RTC_DCHECK(desc->contents().size() == desc->transport_infos().size()); | 
| + // Reorder the ContentInfo. | 
| + if (desc->contents().empty()) { | 
| + return; | 
| + } | 
| + size_t num_msections = desc->contents().size(); | 
| + auto first_content_info = desc->contents()[0]; | 
| + for (size_t i = 1; i < num_msections; ++i) { | 
| + desc->contents()[i - 1] = desc->contents()[i]; | 
| + } | 
| + desc->contents()[num_msections - 1] = first_content_info; | 
| + | 
| + // Reorder the TransportInfo. | 
| + if (desc->transport_infos().empty()) { | 
| + return; | 
| + } | 
| + auto first_transport_info = desc->transport_infos()[0]; | 
| + for (size_t i = 1; i < num_msections; ++i) { | 
| + desc->transport_infos()[i - 1] = desc->transport_infos()[i]; | 
| + } | 
| + desc->transport_infos()[num_msections - 1] = first_transport_info; | 
| + } | 
| + | 
| std::unique_ptr<rtc::VirtualSocketServer> vss_; | 
| rtc::AutoSocketServerThread main_; | 
| rtc::scoped_refptr<FakeAudioCaptureModule> fake_audio_capture_module_; | 
| @@ -3818,6 +3847,33 @@ TEST_F(PeerConnectionInterfaceTest, CreateAnswerWithoutRemoteDescription) { | 
| EXPECT_TRUE(DoCreateAnswer(&answer, nullptr)); | 
| } | 
| +// Test that an error is returned if a description is applied that doesn't | 
| +// respect the order of existing media sections. | 
| +TEST_F(PeerConnectionInterfaceTest, | 
| + MediaSectionOrderEnforcedForSubsequentOffers) { | 
| + CreatePeerConnection(); | 
| + FakeConstraints constraints; | 
| + constraints.SetMandatoryReceiveAudio(true); | 
| + constraints.SetMandatoryReceiveVideo(true); | 
| + std::unique_ptr<SessionDescriptionInterface> offer; | 
| + ASSERT_TRUE(DoCreateOffer(&offer, &constraints)); | 
| + EXPECT_TRUE(DoSetRemoteDescription(offer.release())); | 
| + | 
| + std::unique_ptr<SessionDescriptionInterface> answer; | 
| + ASSERT_TRUE(DoCreateAnswer(&answer, nullptr)); | 
| + EXPECT_TRUE(DoSetLocalDescription(answer.release())); | 
| + | 
| + // An remote offer with different m=line order would be rejected. | 
| 
Taylor Brandstetter
2017/09/14 00:41:47
nit: "A" instead of "An", and "should" instead of
 | 
| + ASSERT_TRUE(DoCreateOffer(&offer, &constraints)); | 
| + ReorderMediaSections(offer.get()); | 
| + EXPECT_FALSE(DoSetRemoteDescription(offer.release())); | 
| + | 
| + // A subsequent local offer with different m=line order would be rejected. | 
| + ASSERT_TRUE(DoCreateOffer(&offer, &constraints)); | 
| + ReorderMediaSections(offer.get()); | 
| + EXPECT_FALSE(DoSetLocalDescription(offer.release())); | 
| +} | 
| + | 
| class PeerConnectionMediaConfigTest : public testing::Test { | 
| protected: | 
| void SetUp() override { |