Deprecate RemoveTrack (old signature)

This also removes all internal usage of RemoveTrack, and changes
the replacement function to RemoveTrackOrError rather than RemoveTrackNew.

Bug: webrtc:9534
Change-Id: Idf7bb17495686de77c70428dcbfb12278328ce59
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244094
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35624}

api/peer_connection_interface.cc

@@ -41,12 +41,6 @@ PeerConnectionInterface::RTCConfiguration::RTCConfiguration(
 
 PeerConnectionInterface::RTCConfiguration::~RTCConfiguration() = default;
 
-RTCError PeerConnectionInterface::RemoveTrackNew(
-    rtc::scoped_refptr<RtpSenderInterface> sender) {
-  return RTCError(RemoveTrack(sender) ? RTCErrorType::NONE
-                                      : RTCErrorType::INTERNAL_ERROR);
-}
-
 RTCError PeerConnectionInterface::SetConfiguration(
     const PeerConnectionInterface::RTCConfiguration& config) {
   return RTCError();

api/peer_connection_interface.h

@@ -805,23 +805,41 @@ class RTC_EXPORT PeerConnectionInterface : public rtc::RefCountInterface {
       rtc::scoped_refptr<MediaStreamTrackInterface> track,
       const std::vector<std::string>& stream_ids) = 0;
 
-  // Remove an RtpSender from this PeerConnection.
+  // Removes the connection between a MediaStreamTrack and the PeerConnection.
-  // Returns true on success.
+  // Stops sending on the RtpSender and marks the
-  // TODO(steveanton): Replace with signature that returns RTCError.
-  virtual bool RemoveTrack(RtpSenderInterface* sender) = 0;
-
-  // Plan B semantics: Removes the RtpSender from this PeerConnection.
-  // Unified Plan semantics: Stop sending on the RtpSender and mark the
   // corresponding RtpTransceiver direction as no longer sending.
+  // https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-removetrack
   //
   // Errors:
   // - INVALID_PARAMETER: `sender` is null or (Plan B only) the sender is not
   //       associated with this PeerConnection.
   // - INVALID_STATE: PeerConnection is closed.
+  //
+  // Plan B semantics: Removes the RtpSender from this PeerConnection.
+  //
   // TODO(bugs.webrtc.org/9534): Rename to RemoveTrack once the other signature
-  // is removed.
+  // is removed; remove default implementation once upstream is updated.
+  virtual RTCError RemoveTrackOrError(
+      rtc::scoped_refptr<RtpSenderInterface> sender) {
+    RTC_CHECK_NOTREACHED();
+    return RTCError();
+  }
+
+  // Legacy API for removing a track from the PeerConnection.
+  // Returns true on success.
+  // TODO(bugs.webrtc.org/9534): Replace with signature that returns RTCError.
+  ABSL_DEPRECATED("Use RemoveTrackOrError")
+  virtual bool RemoveTrack(RtpSenderInterface* sender) {
+    return RemoveTrackOrError(rtc::scoped_refptr<RtpSenderInterface>(sender))
+        .ok();
+  }
+
+  // Old name for the new API. Will be removed when clients are updated.
+  ABSL_DEPRECATED("Use RemoveTrackOrError")
   virtual RTCError RemoveTrackNew(
-      rtc::scoped_refptr<RtpSenderInterface> sender);
+      rtc::scoped_refptr<RtpSenderInterface> sender) {
+    return RemoveTrackOrError(sender);
+  }
 
   // AddTransceiver creates a new RtpTransceiver and adds it to the set of
   // transceivers. Adding a transceiver will cause future calls to CreateOffer

api/test/dummy_peer_connection.h

@@ -47,7 +47,7 @@ class DummyPeerConnection : public PeerConnectionInterface {
 
   bool RemoveTrack(RtpSenderInterface* sender) override { return false; }
 
-  RTCError RemoveTrackNew(
+  RTCError RemoveTrackOrError(
       rtc::scoped_refptr<RtpSenderInterface> sender) override {
     return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, "Not implemented");
   }

api/test/mock_peerconnectioninterface.h

@@ -50,7 +50,7 @@ class MockPeerConnectionInterface
               (override));
   MOCK_METHOD(bool, RemoveTrack, (RtpSenderInterface*), (override));
   MOCK_METHOD(RTCError,
-              RemoveTrackNew,
+              RemoveTrackOrError,
               (rtc::scoped_refptr<RtpSenderInterface>),
               (override));
   MOCK_METHOD(RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>>,

pc/peer_connection.cc

@@ -847,12 +847,7 @@ RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> PeerConnection::AddTrack(
   return sender_or_error;
 }
 
-bool PeerConnection::RemoveTrack(RtpSenderInterface* sender) {
+RTCError PeerConnection::RemoveTrackOrError(
-  TRACE_EVENT0("webrtc", "PeerConnection::RemoveTrack");
-  return RemoveTrackNew(rtc::scoped_refptr<RtpSenderInterface>(sender)).ok();
-}
-
-RTCError PeerConnection::RemoveTrackNew(
     rtc::scoped_refptr<RtpSenderInterface> sender) {
   RTC_DCHECK_RUN_ON(signaling_thread());
   if (!sender) {

pc/peer_connection.h

@@ -141,8 +141,7 @@ class PeerConnection : public PeerConnectionInternal,
   RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> AddTrack(
       rtc::scoped_refptr<MediaStreamTrackInterface> track,
       const std::vector<std::string>& stream_ids) override;
-  bool RemoveTrack(RtpSenderInterface* sender) override;
+  RTCError RemoveTrackOrError(
-  RTCError RemoveTrackNew(
       rtc::scoped_refptr<RtpSenderInterface> sender) override;
 
   RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>> AddTransceiver(

pc/peer_connection_proxy.h

@@ -35,8 +35,9 @@ PROXY_METHOD2(RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>>,
               AddTrack,
               rtc::scoped_refptr<MediaStreamTrackInterface>,
               const std::vector<std::string>&)
-PROXY_METHOD1(bool, RemoveTrack, RtpSenderInterface*)
+PROXY_METHOD1(RTCError,
-PROXY_METHOD1(RTCError, RemoveTrackNew, rtc::scoped_refptr<RtpSenderInterface>)
+              RemoveTrackOrError,
+              rtc::scoped_refptr<RtpSenderInterface>)
 PROXY_METHOD1(RTCErrorOr<rtc::scoped_refptr<RtpTransceiverInterface>>,
               AddTransceiver,
               rtc::scoped_refptr<MediaStreamTrackInterface>)

pc/scenario_tests/goog_cc_test.cc

@@ -93,7 +93,7 @@ TEST(GoogCcPeerScenarioTest, MAYBE_NoBweChangeFromVideoUnmute) {
 
   // Resume video but stop audio. Bandwidth should not drop.
   video.capturer->Start();
-  RTCError status = caller->pc()->RemoveTrackNew(audio.sender);
+  RTCError status = caller->pc()->RemoveTrackOrError(audio.sender);
   ASSERT_TRUE(status.ok());
   audio.track->set_enabled(false);
   for (int i = 0; i < 10; i++) {

pc/test/fake_peer_connection_base.h

@@ -52,7 +52,7 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
 
   bool RemoveTrack(RtpSenderInterface* sender) override { return false; }
 
-  RTCError RemoveTrackNew(
+  RTCError RemoveTrackOrError(
       rtc::scoped_refptr<RtpSenderInterface> sender) override {
     return RTCError(RTCErrorType::UNSUPPORTED_OPERATION);
   }

sdk/android/src/jni/pc/peer_connection.cc

@@ -776,8 +776,10 @@ static jboolean JNI_PeerConnection_RemoveTrack(
     JNIEnv* jni,
     const JavaParamRef<jobject>& j_pc,
     jlong native_sender) {
-  return ExtractNativePC(jni, j_pc)->RemoveTrack(
+  return ExtractNativePC(jni, j_pc)
-      reinterpret_cast<RtpSenderInterface*>(native_sender));
+      ->RemoveTrackOrError(rtc::scoped_refptr<RtpSenderInterface>(
+          reinterpret_cast<RtpSenderInterface*>(native_sender)))
+      .ok();
 }
 
 static ScopedJavaLocalRef<jobject> JNI_PeerConnection_AddTransceiverWithTrack(

sdk/objc/api/peerconnection/RTCPeerConnection.mm

@@ -515,7 +515,7 @@ void PeerConnectionDelegateAdapter::OnRemoveTrack(
 }
 
 - (BOOL)removeTrack:(RTC_OBJC_TYPE(RTCRtpSender) *)sender {
-  bool result = _peerConnection->RemoveTrack(sender.nativeRtpSender);
+  bool result = _peerConnection->RemoveTrackOrError(sender.nativeRtpSender).ok();
   if (!result) {
     RTCLogError(@"Failed to remote track %@", sender);
   }