Fix current_direction() when stopping_ but not stopped_

Also add an unit test for RtpTransceiver under Unified Plan, and
refactor so that we no longer use StopInternal() internally.
This will make removing it easier.

Bug: chromium:980879
Change-Id: I46219112e3aba8e7513c08336b10e95b1ea5d68b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/182681
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31999}

pc/peer_connection.cc

@@ -3337,7 +3337,7 @@ RTCError PeerConnection::ApplyRemoteDescription(
       if (content->rejected && !transceiver->stopped()) {
         RTC_LOG(LS_INFO) << "Stopping transceiver for MID=" << content->name
                          << " since the media section was rejected.";
-        transceiver->StopInternal();
+        transceiver->internal()->StopTransceiverProcedure();
       }
       if (!content->rejected &&
           RtpTransceiverDirectionHasRecv(local_direction)) {

pc/rtp_transceiver.cc

@@ -321,7 +321,7 @@ RTCError RtpTransceiver::SetDirectionWithError(
 
 absl::optional<RtpTransceiverDirection> RtpTransceiver::current_direction()
     const {
-  if (unified_plan_ && stopping())
+  if (unified_plan_ && stopped())
     return webrtc::RtpTransceiverDirection::kStopped;
 
   return current_direction_;
@@ -354,7 +354,11 @@ void RtpTransceiver::StopSendingAndReceiving() {
 
 RTCError RtpTransceiver::StopStandard() {
   RTC_DCHECK_RUN_ON(thread_);
-  RTC_DCHECK(unified_plan_);
+  // If we're on Plan B, do what Stop() used to do there.
+  if (!unified_plan_) {
+    StopInternal();
+    return RTCError::OK();
+  }
   // 1. Let transceiver be the RTCRtpTransceiver object on which the method is
   // invoked.
   //
@@ -380,7 +384,12 @@ RTCError RtpTransceiver::StopStandard() {
 }
 
 void RtpTransceiver::StopInternal() {
+  StopTransceiverProcedure();
+}
+
+void RtpTransceiver::StopTransceiverProcedure() {
   RTC_DCHECK_RUN_ON(thread_);
+  // As specified in the "Stop the RTCRtpTransceiver" procedure
   // 1. If transceiver.[[Stopping]] is false, stop sending and receiving given
   // transceiver.
   if (!stopping_)

pc/rtp_transceiver.h

@@ -177,6 +177,10 @@ class RtpTransceiver final
   // PeerConnection is closed.
   void SetPeerConnectionClosed();
 
+  // Executes the "stop the RTCRtpTransceiver" procedure from
+  // the webrtc-pc specification, described under the stop() method.
+  void StopTransceiverProcedure();
+
   // Fired when the RtpTransceiver state changes such that negotiation is now
   // needed (e.g., in response to a direction change).
   sigslot::signal0<> SignalNegotiationNeeded;

pc/rtp_transceiver_unittest.cc

@@ -79,6 +79,40 @@ TEST(RtpTransceiverTest, CanUnsetChannelOnStoppedTransceiver) {
   EXPECT_EQ(nullptr, transceiver.channel());
 }
 
+class RtpTransceiverUnifiedPlanTest : public ::testing::Test {
+ public:
+  RtpTransceiverUnifiedPlanTest()
+      : channel_manager_(std::make_unique<cricket::FakeMediaEngine>(),
+                         std::make_unique<cricket::FakeDataEngine>(),
+                         rtc::Thread::Current(),
+                         rtc::Thread::Current()),
+        transceiver_(RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
+                         rtc::Thread::Current(),
+                         new rtc::RefCountedObject<MockRtpSenderInternal>()),
+                     RtpReceiverProxyWithInternal<RtpReceiverInternal>::Create(
+                         rtc::Thread::Current(),
+                         new rtc::RefCountedObject<MockRtpReceiverInternal>()),
+                     &channel_manager_,
+                     channel_manager_.GetSupportedAudioRtpHeaderExtensions()) {}
+
+  cricket::ChannelManager channel_manager_;
+  RtpTransceiver transceiver_;
+};
+
+// Basic tests for Stop()
+TEST_F(RtpTransceiverUnifiedPlanTest, StopSetsDirection) {
+  EXPECT_EQ(RtpTransceiverDirection::kInactive, transceiver_.direction());
+  EXPECT_FALSE(transceiver_.current_direction());
+  transceiver_.StopStandard();
+  EXPECT_EQ(RtpTransceiverDirection::kStopped, transceiver_.direction());
+  EXPECT_FALSE(transceiver_.current_direction());
+  transceiver_.StopTransceiverProcedure();
+  EXPECT_TRUE(transceiver_.current_direction());
+  EXPECT_EQ(RtpTransceiverDirection::kStopped, transceiver_.direction());
+  EXPECT_EQ(RtpTransceiverDirection::kStopped,
+            *transceiver_.current_direction());
+}
+
 class RtpTransceiverTestForHeaderExtensions : public ::testing::Test {
  public:
   RtpTransceiverTestForHeaderExtensions()