Clone MediaStream video tracks correctly with spec-compliant constraints.

content/renderer/media/media_stream_center.cc

Index: content/renderer/media/media_stream_center.cc
diff --git a/content/renderer/media/media_stream_center.cc b/content/renderer/media/media_stream_center.cc
index 283df659d9c55fdb7bfca9b9895a660de88d0c89..48b5480fb1e9ebd6a51dc2a9f87f446159470f11 100644
--- a/content/renderer/media/media_stream_center.cc
+++ b/content/renderer/media/media_stream_center.cc
@@ -83,15 +83,46 @@ void CreateNativeVideoMediaStreamTrack(
         native_source, constraints,
         MediaStreamVideoSource::ConstraintsCallback(), track.isEnabled()));
   } else {
-    // TODO(guidou): All existing uses are without constraints except for the
-    // case of cloning a track. To fix this, create a new function in
-    // MediaStreamCenter specifically for cloning.
     writable_track.setTrackData(new MediaStreamVideoTrack(
         native_source, MediaStreamVideoSource::ConstraintsCallback(),
         track.isEnabled()));
   }
 }
 
+void CloneNativeVideoMediaStreamTrack(
+    const blink::WebMediaStreamTrack& original,
+    const blink::WebMediaStreamTrack& clone) {
+  DCHECK(!clone.getTrackData());
+  blink::WebMediaStreamSource source = clone.source();
+  DCHECK_EQ(source.getType(), blink::WebMediaStreamSource::TypeVideo);
+  MediaStreamVideoSource* native_source =
+      MediaStreamVideoSource::GetVideoSource(source);
+  DCHECK(native_source);
+  blink::WebMediaStreamTrack writable_clone(clone);

[hbos_chromium, 2017/04/04 11:21:43]

Can you add a comment explaining that doing setTrackData on the "writable" sets
the track data of the original |clone|'s MediaStreamComponent?
Same comment should be added to CreateNativeVideoMediaStreamTrack.

Alternatively, does CloneNativeVideoMediaStreamTrack have to take a const
argument (blink::MediaStreamCenter has non-const ptrs)? This seems like a way of
avoiding essentially a const_cast since a const-WebMediaStreamTrack leaks a
non-const TrackData pointer, an odd thing to do, but existing behavior.

[Guido Urdaneta, 2017/04/04 15:42:53]

Went instead for passing by value. WebMSTrack just wraps a pointer to
MSComponent, so it's OK to pass by value.

+  if (IsOldVideoConstraints()) {
+    // TODO(perkj): The constraints to use here should be passed from blink when
+    // a new track is created. For cloning, it should be the constraints of the
+    // cloned track and not the originating source.
+    // Also - source.constraints() returns an uninitialized constraint if the
+    // source is coming from a remote video track. See http://crbug/287805.
+    blink::WebMediaConstraints constraints = source.constraints();
+    if (constraints.isNull())
+      constraints.initialize();
+    writable_clone.setTrackData(new MediaStreamVideoTrack(
+        native_source, constraints,
+        MediaStreamVideoSource::ConstraintsCallback(), clone.isEnabled()));
+  } else {
+    MediaStreamVideoTrack* original_track =
+        MediaStreamVideoTrack::GetVideoTrack(original);
+    DCHECK(original_track);
+    writable_clone.setTrackData(new MediaStreamVideoTrack(
+        native_source, original_track->adapter_settings(),
+        original_track->noise_reduction(), original_track->is_screencast(),
+        original_track->min_frame_rate(),
+        MediaStreamVideoSource::ConstraintsCallback(), clone.isEnabled()));
+  }
+}
+
 }  // namespace
 
 MediaStreamCenter::MediaStreamCenter(
@@ -116,6 +147,23 @@ void MediaStreamCenter::didCreateMediaStreamTrack(
   }
 }
 
+void MediaStreamCenter::didCloneMediaStreamTrack(
+    const blink::WebMediaStreamTrack& original,
+    const blink::WebMediaStreamTrack& clone) {
+  DCHECK(!clone.isNull());
+  DCHECK(!clone.getTrackData());
+  DCHECK(!clone.source().isNull());
+
+  switch (clone.source().getType()) {
+    case blink::WebMediaStreamSource::TypeAudio:
+      CreateNativeAudioMediaStreamTrack(clone);
+      break;
+    case blink::WebMediaStreamSource::TypeVideo:
+      CloneNativeVideoMediaStreamTrack(original, clone);
+      break;
+  }
+}
+
 void MediaStreamCenter::didSetContentHint(
     const blink::WebMediaStreamTrack& track) {
   MediaStreamTrack* native_track = MediaStreamTrack::GetTrack(track);