Reland "Implement RtpParameters.transaction_id for PC RtpSenderInterface"

This is a reland of 5faf36ef3c582350fba5ef97a3549e440d81a283
The issue in Chrome has been fixed and this should be safe to reland.

TBR=deadbeef

Original change's description:
> Implement RtpParameters.transaction_id for PC RtpSenderInterface
>
> The transaction_id field should be refreshed for every getParameters()
> call and checked at each setParameters() call.
> This also checks that getParameters() was ever called to return a proper
> error code.
>
> Bug: webrtc:7580
> Change-Id: I6c6fe289542e486fc422cdc61577982b0529d4c1
> Reviewed-on: https://webrtc-review.googlesource.com/70820
> Commit-Queue: Florent Castelli <orphis@webrtc.org>
> Reviewed-by: Sami Kalliomäki <sakal@webrtc.org>
> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> Reviewed-by: Kári Helgason <kthelgason@webrtc.org>
> Reviewed-by: Steve Anton <steveanton@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#23120}

Bug: webrtc:7580
Change-Id: Iabd41fb21afdf452c039d5513824ae334f8d1d3f
Reviewed-on: https://webrtc-review.googlesource.com/76980
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23247}

4 files changed, 165 insertions, 9 deletions

diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc
index 5cfbb13637..3b0bbf886a 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -187,12 +187,15 @@ bool AudioRtpSender::SetTrack(MediaStreamTrackInterface* track) {
   return true;
 }
 
-RtpParameters AudioRtpSender::GetParameters() const {
+RtpParameters AudioRtpSender::GetParameters() {
   if (!media_channel_ || stopped_) {
     return RtpParameters();
   }
   return worker_thread_->Invoke<RtpParameters>(RTC_FROM_HERE, [&] {
-    return media_channel_->GetRtpSendParameters(ssrc_);
+    RtpParameters result = media_channel_->GetRtpSendParameters(ssrc_);
+    last_transaction_id_ = rtc::CreateRandomUuid();
+    result.transaction_id = last_transaction_id_.value();
+    return result;
   });
 }
 
@@ -201,8 +204,23 @@ RTCError AudioRtpSender::SetParameters(const RtpParameters& parameters) {
   if (!media_channel_ || stopped_) {
     return RTCError(RTCErrorType::INVALID_STATE);
   }
+  if (!last_transaction_id_) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::INVALID_STATE,
+        "Failed to set parameters since getParameters() has never been called"
+        " on this sender");
+  }
+  if (last_transaction_id_ != parameters.transaction_id) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::INVALID_MODIFICATION,
+        "Failed to set parameters since the transaction_id doesn't match"
+        " the last value returned from getParameters()");
+  }
+
   return worker_thread_->Invoke<RTCError>(RTC_FROM_HERE, [&] {
-    return media_channel_->SetRtpSendParameters(ssrc_, parameters);
+    RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters);
+    last_transaction_id_.reset();
+    return result;
   });
 }
 
@@ -373,12 +391,15 @@ bool VideoRtpSender::SetTrack(MediaStreamTrackInterface* track) {
   return true;
 }
 
-RtpParameters VideoRtpSender::GetParameters() const {
+RtpParameters VideoRtpSender::GetParameters() {
   if (!media_channel_ || stopped_) {
     return RtpParameters();
   }
   return worker_thread_->Invoke<RtpParameters>(RTC_FROM_HERE, [&] {
-    return media_channel_->GetRtpSendParameters(ssrc_);
+    RtpParameters result = media_channel_->GetRtpSendParameters(ssrc_);
+    last_transaction_id_ = rtc::CreateRandomUuid();
+    result.transaction_id = last_transaction_id_.value();
+    return result;
   });
 }
 
@@ -387,8 +408,23 @@ RTCError VideoRtpSender::SetParameters(const RtpParameters& parameters) {
   if (!media_channel_ || stopped_) {
     return RTCError(RTCErrorType::INVALID_STATE);
   }
+  if (!last_transaction_id_) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::INVALID_STATE,
+        "Failed to set parameters since getParameters() has never been called"
+        " on this sender");
+  }
+  if (last_transaction_id_ != parameters.transaction_id) {
+    LOG_AND_RETURN_ERROR(
+        RTCErrorType::INVALID_MODIFICATION,
+        "Failed to set parameters since the transaction_id doesn't match"
+        " the last value returned from getParameters()");
+  }
+
   return worker_thread_->Invoke<RTCError>(RTC_FROM_HERE, [&] {
-    return media_channel_->SetRtpSendParameters(ssrc_, parameters);
+    RTCError result = media_channel_->SetRtpSendParameters(ssrc_, parameters);
+    last_transaction_id_.reset();
+    return result;
   });
 }
 
diff --git a/pc/rtpsender.h b/pc/rtpsender.h
index ef41485898..4077b9ca1f 100644
--- a/pc/rtpsender.h
+++ b/pc/rtpsender.h
@@ -128,7 +128,7 @@ class AudioRtpSender : public DtmfProviderInterface,
 
   std::vector<std::string> stream_ids() const override { return stream_ids_; }
 
-  RtpParameters GetParameters() const override;
+  RtpParameters GetParameters() override;
   RTCError SetParameters(const RtpParameters& parameters) override;
 
   rtc::scoped_refptr<DtmfSenderInterface> GetDtmfSender() const override;
@@ -172,6 +172,7 @@ class AudioRtpSender : public DtmfProviderInterface,
   StatsCollector* stats_;
   rtc::scoped_refptr<AudioTrackInterface> track_;
   rtc::scoped_refptr<DtmfSenderInterface> dtmf_sender_proxy_;
+  rtc::Optional<std::string> last_transaction_id_;
   uint32_t ssrc_ = 0;
   bool cached_track_enabled_ = false;
   bool stopped_ = false;
@@ -216,7 +217,7 @@ class VideoRtpSender : public ObserverInterface,
 
   std::vector<std::string> stream_ids() const override { return stream_ids_; }
 
-  RtpParameters GetParameters() const override;
+  RtpParameters GetParameters() override;
   RTCError SetParameters(const RtpParameters& parameters) override;
 
   rtc::scoped_refptr<DtmfSenderInterface> GetDtmfSender() const override;
@@ -253,6 +254,7 @@ class VideoRtpSender : public ObserverInterface,
   std::vector<std::string> stream_ids_;
   cricket::VideoMediaChannel* media_channel_ = nullptr;
   rtc::scoped_refptr<VideoTrackInterface> track_;
+  rtc::Optional<std::string> last_transaction_id_;
   uint32_t ssrc_ = 0;
   VideoTrackInterface::ContentHint cached_track_content_hint_ =
       VideoTrackInterface::ContentHint::kNone;
diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc
index d9b543b8c2..b084b01e1a 100644
--- a/pc/rtpsenderreceiver_unittest.cc
+++ b/pc/rtpsenderreceiver_unittest.cc
@@ -606,6 +606,65 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCanSetParameters) {
   DestroyAudioRtpSender();
 }
 
+TEST_F(RtpSenderReceiverTest,
+       AudioSenderMustCallGetParametersBeforeSetParameters) {
+  CreateAudioRtpSender();
+
+  RtpParameters params;
+  RTCError result = audio_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type());
+
+  DestroyAudioRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest,
+       AudioSenderSetParametersInvalidatesTransactionId) {
+  CreateAudioRtpSender();
+
+  RtpParameters params = audio_rtp_sender_->GetParameters();
+  EXPECT_EQ(1u, params.encodings.size());
+  EXPECT_TRUE(audio_rtp_sender_->SetParameters(params).ok());
+  RTCError result = audio_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type());
+
+  DestroyAudioRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest, AudioSenderDetectTransactionIdModification) {
+  CreateAudioRtpSender();
+
+  RtpParameters params = audio_rtp_sender_->GetParameters();
+  params.transaction_id = "";
+  RTCError result = audio_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type());
+
+  DestroyAudioRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest, AudioSenderCheckTransactionIdRefresh) {
+  CreateAudioRtpSender();
+
+  RtpParameters params = audio_rtp_sender_->GetParameters();
+  EXPECT_NE(params.transaction_id.size(), 0);
+  auto saved_transaction_id = params.transaction_id;
+  params = audio_rtp_sender_->GetParameters();
+  EXPECT_NE(saved_transaction_id, params.transaction_id);
+
+  DestroyAudioRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest, AudioSenderSetParametersOldValueFail) {
+  CreateAudioRtpSender();
+
+  RtpParameters params = audio_rtp_sender_->GetParameters();
+  RtpParameters second_params = audio_rtp_sender_->GetParameters();
+
+  RTCError result = audio_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type());
+
+  DestroyAudioRtpSender();
+}
+
 TEST_F(RtpSenderReceiverTest, SetAudioMaxSendBitrate) {
   CreateAudioRtpSender();
 
@@ -664,6 +723,65 @@ TEST_F(RtpSenderReceiverTest, VideoSenderCanSetParameters) {
   DestroyVideoRtpSender();
 }
 
+TEST_F(RtpSenderReceiverTest,
+       VideoSenderMustCallGetParametersBeforeSetParameters) {
+  CreateVideoRtpSender();
+
+  RtpParameters params;
+  RTCError result = video_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type());
+
+  DestroyVideoRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest,
+       VideoSenderSetParametersInvalidatesTransactionId) {
+  CreateVideoRtpSender();
+
+  RtpParameters params = video_rtp_sender_->GetParameters();
+  EXPECT_EQ(1u, params.encodings.size());
+  EXPECT_TRUE(video_rtp_sender_->SetParameters(params).ok());
+  RTCError result = video_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_STATE, result.type());
+
+  DestroyVideoRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest, VideoSenderDetectTransactionIdModification) {
+  CreateVideoRtpSender();
+
+  RtpParameters params = video_rtp_sender_->GetParameters();
+  params.transaction_id = "";
+  RTCError result = video_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type());
+
+  DestroyVideoRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest, VideoSenderCheckTransactionIdRefresh) {
+  CreateVideoRtpSender();
+
+  RtpParameters params = video_rtp_sender_->GetParameters();
+  EXPECT_NE(params.transaction_id.size(), 0);
+  auto saved_transaction_id = params.transaction_id;
+  params = video_rtp_sender_->GetParameters();
+  EXPECT_NE(saved_transaction_id, params.transaction_id);
+
+  DestroyVideoRtpSender();
+}
+
+TEST_F(RtpSenderReceiverTest, VideoSenderSetParametersOldValueFail) {
+  CreateVideoRtpSender();
+
+  RtpParameters params = video_rtp_sender_->GetParameters();
+  RtpParameters second_params = video_rtp_sender_->GetParameters();
+
+  RTCError result = video_rtp_sender_->SetParameters(params);
+  EXPECT_EQ(RTCErrorType::INVALID_MODIFICATION, result.type());
+
+  DestroyVideoRtpSender();
+}
+
 TEST_F(RtpSenderReceiverTest, SetVideoMaxSendBitrate) {
   CreateVideoRtpSender();
 
diff --git a/pc/test/mock_rtpsenderinternal.h b/pc/test/mock_rtpsenderinternal.h
index 5988c7b3c1..3ddffec7a1 100644
--- a/pc/test/mock_rtpsenderinternal.h
+++ b/pc/test/mock_rtpsenderinternal.h
@@ -29,7 +29,7 @@ class MockRtpSenderInternal : public RtpSenderInternal {
   MOCK_CONST_METHOD0(media_type, cricket::MediaType());
   MOCK_CONST_METHOD0(id, std::string());
   MOCK_CONST_METHOD0(stream_ids, std::vector<std::string>());
-  MOCK_CONST_METHOD0(GetParameters, RtpParameters());
+  MOCK_METHOD0(GetParameters, RtpParameters());
   MOCK_METHOD1(SetParameters, RTCError(const RtpParameters&));
   MOCK_CONST_METHOD0(GetDtmfSender, rtc::scoped_refptr<DtmfSenderInterface>());