[M90] Add CloseOnClone flag to VideoFrameHandle

This CL temporary adds a flag that allows VideoFrameHandles to be closed
automatically after clone() is called.

This is used by MediaStreamVideoTrackUnderlyingSource to mark frames for
closure before posting them to a endpoint has has been transferred.
Without this, posted VideoFrames cannot be closed, and this results in
stalls (see attached bug).

To detect whether or not a stream endpoint has been transferred, we use
a ReadableStreamTransferringOptimizer which doesn't attempt to perform
any optimizations, but notifies the original MSVTUS that there was an
attempt to optimize stream transfers.

(cherry picked from commit 27a978b)

Bug: 1182497
Change-Id: Ibbd5017b973cf2736b9bda7cdacd9b789a1173f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2729456
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#859630}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2739870
Auto-Submit: Thomas Guilbert <tguilbert@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4430@{#163}
Cr-Branched-From: e5ce7dc-refs/heads/master@{#857950}

third_party/blink/renderer/modules/mediastream/media_stream_track_processor.cc

@@ -142,7 +142,8 @@ void MediaStreamTrackProcessor::CreateVideoSourceStream(
       MakeGarbageCollected<MediaStreamVideoTrackUnderlyingSource>(
           script_state, input_track_->Component(), buffer_size_);
   source_stream_ = ReadableStream::CreateWithCountQueueingStrategy(
-      script_state, video_underlying_source_, /*high_water_mark=*/0);
+      script_state, video_underlying_source_, /*high_water_mark=*/0,
+      video_underlying_source_->GetStreamTransferOptimizer());
 
   source_closer_ = MakeGarbageCollected<UnderlyingSourceCloser>(
       input_track_, video_underlying_source_);

third_party/blink/renderer/modules/mediastream/media_stream_video_track_underlying_source.cc

@@ -19,6 +19,38 @@
 
 namespace blink {
 
+// Temporary workaround for crbug.com/1182497.
+// Doesn't perform any stream optimization, but instead
+// lets MediaStreamVideoTrackUnderlyingSource know that
+// its stream endpoint has been transferred, and that it should mark its video
+// frames for closure when they are cloned().
+class StreamTransferNotifier final
+    : public ReadableStreamTransferringOptimizer {
+  USING_FAST_MALLOC(StreamTransferNotifier);
+  using OptimizerCallback = CrossThreadOnceClosure;
+
+ public:
+  StreamTransferNotifier(
+      scoped_refptr<base::SingleThreadTaskRunner> original_runner,
+      OptimizerCallback callback)
+      : original_runner_(std::move(original_runner)),
+        callback_(std::move(callback)) {}
+
+  UnderlyingSourceBase* PerformInProcessOptimization(
+      ScriptState* script_state) override {
+    // Send a message back to MediaStreamVideoTrackUnderlyingSource.
+    PostCrossThreadTask(*original_runner_, FROM_HERE, std::move(callback_));
+
+    // Return nullptr will mean that no optimization was performed, and streams
+    // will post internally.
+    return nullptr;
+  }
+
+ private:
+  scoped_refptr<base::SingleThreadTaskRunner> original_runner_;
+  OptimizerCallback callback_;
+};
+
 MediaStreamVideoTrackUnderlyingSource::MediaStreamVideoTrackUnderlyingSource(
     ScriptState* script_state,
     MediaStreamComponent* track,
@@ -94,6 +126,19 @@ void MediaStreamVideoTrackUnderlyingSource::Close() {
   queue_.clear();
 }
 
+std::unique_ptr<ReadableStreamTransferringOptimizer>
+MediaStreamVideoTrackUnderlyingSource::GetStreamTransferOptimizer() {
+  auto stream_transferred_cb = [](MediaStreamVideoTrackUnderlyingSource* self) {
+    if (self)
+      self->stream_was_transferred_ = true;
+  };
+
+  return std::make_unique<StreamTransferNotifier>(
+      main_task_runner_,
+      CrossThreadBindOnce(stream_transferred_cb,
+                          WrapCrossThreadWeakPersistent(this)));
+}
+
 void MediaStreamVideoTrackUnderlyingSource::OnFrameFromTrack(
     scoped_refptr<media::VideoFrame> media_frame,
     std::vector<scoped_refptr<media::VideoFrame>> /*scaled_media_frames*/,
@@ -143,6 +188,10 @@ void MediaStreamVideoTrackUnderlyingSource::SendFrameToStream(
 
   VideoFrame* video_frame = MakeGarbageCollected<VideoFrame>(
       std::move(media_frame), GetExecutionContext());
+
+  if (stream_was_transferred_)
+    video_frame->handle()->SetCloseOnClone();
+
   Controller()->Enqueue(video_frame);
   is_pending_pull_ = false;
 }

third_party/blink/renderer/modules/mediastream/media_stream_video_track_underlying_source.h

@@ -7,6 +7,7 @@
 
 #include "base/threading/thread_checker.h"
 #include "third_party/blink/public/web/modules/mediastream/media_stream_video_sink.h"
+#include "third_party/blink/renderer/core/streams/readable_stream_transferring_optimizer.h"
 #include "third_party/blink/renderer/core/streams/underlying_source_base.h"
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/platform/wtf/deque.h"
@@ -47,6 +48,9 @@ class MODULES_EXPORT MediaStreamVideoTrackUnderlyingSource
   void Close();
   void Trace(Visitor*) const override;
 
+  std::unique_ptr<ReadableStreamTransferringOptimizer>
+  GetStreamTransferOptimizer();
+
  private:
   void OnFrameFromTrack(
       scoped_refptr<media::VideoFrame> media_frame,
@@ -58,6 +62,10 @@ class MODULES_EXPORT MediaStreamVideoTrackUnderlyingSource
   void SendFrameToStream(scoped_refptr<media::VideoFrame> media_frame);
   void ProcessPullRequest();
 
+  // Used when a stream endpoint was transferred to another realm, to
+  // automatically close frames as they are posted to the other stream.
+  bool stream_was_transferred_ = false;
+
   const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
   const Member<MediaStreamComponent> track_;
 

third_party/blink/renderer/modules/webcodecs/video_frame_handle.cc

@@ -65,16 +65,24 @@ sk_sp<SkImage> VideoFrameHandle::sk_image() {
 
 void VideoFrameHandle::Invalidate() {
   WTF::MutexLocker locker(mutex_);
-  frame_.reset();
-  sk_image_.reset();
-  close_auditor_.reset();
+  InvalidateLocked();
+}
+
+void VideoFrameHandle::SetCloseOnClone() {
+  WTF::MutexLocker locker(mutex_);
+  close_on_clone_ = true;
 }
 
 scoped_refptr<VideoFrameHandle> VideoFrameHandle::Clone() {
   WTF::MutexLocker locker(mutex_);
-  return frame_ ? base::MakeRefCounted<VideoFrameHandle>(frame_, sk_image_,
-                                                         close_auditor_)
-                : nullptr;
+  auto cloned_handle = frame_ ? base::MakeRefCounted<VideoFrameHandle>(
+                                    frame_, sk_image_, close_auditor_)
+                              : nullptr;
+
+  if (close_on_clone_)
+    InvalidateLocked();
+
+  return cloned_handle;
 }
 
 scoped_refptr<VideoFrameHandle> VideoFrameHandle::CloneForInternalUse() {
@@ -83,4 +91,12 @@ scoped_refptr<VideoFrameHandle> VideoFrameHandle::CloneForInternalUse() {
                 : nullptr;
 }
 
+void VideoFrameHandle::InvalidateLocked() {
+  mutex_.AssertAcquired();
+
+  frame_.reset();
+  sk_image_.reset();
+  close_auditor_.reset();
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/webcodecs/video_frame_handle.h

@@ -58,6 +58,12 @@ class MODULES_EXPORT VideoFrameHandle
   // blink::VideoFrames and blink::Planes that hold a reference to |this|.
   void Invalidate();
 
+  // Temporary workaround for crbug.com/1182497.
+  // Overrides the next call to Clone() to return |this| without adding a new
+  // reference. This prevents cloning handles when internally posting frames to
+  // a transferred stream, which cannot be cleared.
+  void SetCloseOnClone();
+
   // Clones this VideoFrameHandle into a new VideoFrameHandle object.
   scoped_refptr<VideoFrameHandle> Clone();
 
@@ -69,6 +75,13 @@ class MODULES_EXPORT VideoFrameHandle
   friend class WTF::ThreadSafeRefCounted<VideoFrameHandle>;
   ~VideoFrameHandle();
 
+  // |mutex_| must be held before calling into this.
+  void InvalidateLocked();
+
+  // Flag that prevents the creation of a new handle during the next Clone()
+  // call. Used as a temporary workaround for crbug.com/1182497.
+  bool close_on_clone_ = false;
+
   WTF::Mutex mutex_;
   sk_sp<SkImage> sk_image_;
   scoped_refptr<media::VideoFrame> frame_;

third_party/blink/web_tests/wpt_internal/mediastream/frame-reader-worker.js

@@ -0,0 +1,39 @@
+var frameCount = 0;
+
+self.addEventListener('message', function(e) {
+
+  const frameStream = e.data.stream;
+  const frameReader = frameStream.getReader();
+
+  const framesToRead = 20;
+
+  var closeStream = function() {
+    frameReader.releaseLock();
+    frameStream.cancel();
+  }
+
+  frameReader.read().then(function processFrame({done, value}) {
+    if(done) {
+      self.postMessage({ success: false, message: "Stream is ended before we could read enough frames" });
+      closeStream();
+      return;
+    }
+
+    if (value.codedWitdh == 0) {
+      self.postMessage({ success: false, message: "Video frame is invalid" });
+      closeStream();
+      value.close();
+      return;
+    }
+
+    value.close();
+
+    if(++frameCount == framesToRead) {
+      self.postMessage({ success: true, message: "Ran as expected" });
+      closeStream();
+      return;
+    }
+
+    frameReader.read().then(processFrame);
+  })
+}, false);

third_party/blink/web_tests/wpt_internal/mediastream/mediastreamtrackprocessor-transfer-to-worker.html

@@ -0,0 +1,35 @@
+<!DOCTYPE html>
+<title>Transferring Streams from MediaStreamTrackProcessor.</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="frame-reader-worker.js"></script>
+<script>
+
+promise_test(async t => {
+  var video = document.createElement('video');
+  video.src = "/media/400x300-red-resize-200x150-green.mp4";
+  await test_driver.bless("Play video element", () => {
+    return video.play();
+  });
+
+  var track = video.captureStream().getVideoTracks()[0];
+  t.add_cleanup(() => track.stop());
+
+  var processor = new MediaStreamTrackProcessor(track);
+  var worker = new Worker("frame-reader-worker.js");
+
+  var testSuccess = new Promise(resolve => {
+    worker.addEventListener('message', resolve);
+  }).then(message => {
+      assert_true(message.data.success, message.data.message);
+  });
+
+  var readable = processor.readable;
+  worker.postMessage({ stream: readable }, [readable]);
+
+  return testSuccess;
+}, "Test we can transfer a Video MediaStreamTrackProcessor stream");
+
+</script>