RTCPeerConnection.addTrack and removeTrack added (behind flag)

third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp

Index: third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
diff --git a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp b/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
index e4d46030e55e6c5fdeb58076043f34c5ef75beda..92d2a9d87adb054a9d68b830a260ffb363b46960 100644
--- a/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
+++ b/third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp
@@ -1110,6 +1110,8 @@ void RTCPeerConnection::addStream(ScriptState* script_state,
   if (!valid)
     exception_state.ThrowDOMException(kSyntaxError,
                                       "Unable to add the provided stream.");
+  // Ensure |rtp_senders_| is up-to-date.
+  getSenders();
 }
 
 void RTCPeerConnection::removeStream(MediaStream* stream,
@@ -1236,6 +1238,75 @@ HeapVector<Member<RTCRtpReceiver>> RTCPeerConnection::getReceivers() {
   return rtp_receivers;
 }
 
+RTCRtpSender* RTCPeerConnection::addTrack(MediaStreamTrack* track,
+                                          MediaStreamVector streams,
+                                          ExceptionState& exception_state) {
+  DCHECK(track);
+  DCHECK(track->Component());
+  if (ThrowExceptionIfSignalingStateClosed(signaling_state_, exception_state))
+    return nullptr;
+  if (streams.size() >= 2) {
+    // TODO(hbos): Don't throw an exception when this is supported by the lower
+    // layers. https://crbug.com/webrtc/7932
+    exception_state.ThrowDOMException(
+        kNotSupportedError,
+        "Adding a track to multiple streams is not supported.");
+    return nullptr;
+  }
+  for (const auto sender_entry : rtp_senders_) {
+    RTCRtpSender* sender = sender_entry.value;
+    if (sender->track() == track) {
+      exception_state.ThrowDOMException(
+          kInvalidAccessError, "A sender already exists for the track.");
+      return nullptr;
+    }
+  }
+
+  WebVector<WebMediaStream> web_streams(streams.size());
+  for (size_t i = 0; i < streams.size(); ++i) {
+    web_streams[i] = streams[i]->Descriptor();
+  }
+  std::unique_ptr<WebRTCRtpSender> web_rtp_sender =
+      peer_handler_->AddTrack(track->Component(), web_streams);
+  if (!web_rtp_sender) {
+    exception_state.ThrowDOMException(
+        kNotSupportedError, "A sender could not be created for this track.");
+    return nullptr;
+  }
+
+  uintptr_t id = web_rtp_sender->Id();
+  DCHECK(rtp_senders_.find(id) == rtp_senders_.end());
+  RTCRtpSender* rtp_sender = new RTCRtpSender(std::move(web_rtp_sender), track);

[Guido Urdaneta, 2017/07/05 15:37:06]

nit: I think the preferred style in Blink is to have the constructor private and
add a static create method to ensure that the object is always created in the
Oilpan heap.
Check code from other modules to be sure if that's actually the norm.

[hbos_chromium, 2017/07/06 12:31:37]

Yes, done. I didn't make it Create because unlike the other Create this is not a
WebIDL constructor, I didn't think about forcing it on the Oilpan heap and other
code does in fact use Create for all constructors, even ones not defined in
WebIDL.

+  tracks_.insert(track->Component(), track);
+  rtp_senders_.insert(id, rtp_sender);
+  return rtp_sender;
+}
+
+void RTCPeerConnection::removeTrack(RTCRtpSender* sender,
+                                    ExceptionState& exception_state) {
+  DCHECK(sender);

[Guido Urdaneta, 2017/07/05 15:37:06]

Is this DCHECK right? What prevents the application from sending null here?

[hbos_chromium, 2017/07/06 12:31:38]

Yes, it's non-optional in the IDL so null should already have thrown an
exception for us.

+  if (ThrowExceptionIfSignalingStateClosed(signaling_state_, exception_state))
+    return;
+  if (rtp_senders_.find(sender->web_rtp_sender()->Id()) == rtp_senders_.end()) {
+    exception_state.ThrowDOMException(
+        kInvalidAccessError,
+        "The sender was not created by this peer connection.");
+  }
+
+  if (!peer_handler_->RemoveTrack(sender->web_rtp_sender())) {
+    // Operation aborted. This indicates that the sender is no longer used by
+    // the peer connection, i.e. that it was removed due to setting a remote
+    // description of type "rollback".
+    // Note: until re-using senders is supported these are also removed by

[Guido Urdaneta, 2017/07/05 15:37:06]

Is reusing senders not supported by the WebRTC library? If so, indicate it here.
From the comment it's not clear what layer has the limitation.

[hbos_chromium, 2017/07/06 12:31:37]

Yes I was referring to the WebRTC library limitations, comment updated.

[Guido Urdaneta, 2017/07/06 12:43:42]

Add crbug.com reference.

+    // |RemoveTrack|.
+    return;
+  }
+  // Successfully removing the track results in the sender's track property
+  // being nulled.
+  DCHECK(!sender->web_rtp_sender()->Track());
+  sender->SetTrack(nullptr);
+}
+
 RTCDataChannel* RTCPeerConnection::createDataChannel(
     ScriptState* script_state,
     String label,