[Backport] CVE-2023-0931: Use after free in Video (1/2)

Cherry-pick of patch originally reviewed
https://chromium-review.googlesource.com/c/chromium/src/+/4227731:
Simplify WebMediaPlayerMSCompositor destruction.

The code was only sometimes calling StopUsingProvider() and posted
the submitter destruction unnecessarily.

Destruction now works the same as in VideoFrameCompositor, where the
class itself is responsible for calling StopUsingProvider() during
its own destruction.

(cherry picked from commit cbd238e85903b7d94910bd2c6362ff9abf9908cc)

Fixed: 1407701
Change-Id: Ia649cb5532519468eea34e12745ed9c990580d82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195824
Auto-Submit: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Tony Herre <toprice@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1098505}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4227731
Owners-Override: Michael Ershov <miersh@google.com>
Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com>
Reviewed-by: Michael Ershov <miersh@google.com>
Cr-Commit-Position: refs/branch-heads/5005@{#1435}
Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/462767
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

3 files changed, 3 insertions, 32 deletions

diff --git a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc
index d73c02e43fb..4d4d7e98871 100644
--- a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc
+++ b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms.cc
@@ -403,9 +403,6 @@ WebMediaPlayerMS::~WebMediaPlayerMS() {
   if (frame_deliverer_)
     io_task_runner_->DeleteSoon(FROM_HERE, frame_deliverer_.release());
 
-  if (compositor_)
-    compositor_->StopUsingProvider();
-
   if (video_frame_provider_)
     video_frame_provider_->Stop();
 
diff --git a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc
index ccec5c67f5b..7ac02b2b986 100644
--- a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc
+++ b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.cc
@@ -236,13 +236,8 @@ WebMediaPlayerMSCompositor::WebMediaPlayerMSCompositor(
 WebMediaPlayerMSCompositor::~WebMediaPlayerMSCompositor() {
   // Ensured by destructor traits.
   DCHECK(video_frame_compositor_task_runner_->BelongsToCurrentThread());
-
-  if (submitter_) {
-    video_frame_compositor_task_runner_->DeleteSoon(FROM_HERE,
-                                                    std::move(submitter_));
-  } else {
-    DCHECK(!video_frame_provider_client_)
-        << "Must call StopUsingProvider() before dtor!";
+  if (video_frame_provider_client_) {
+    video_frame_provider_client_->StopUsingProvider();
   }
 }
 
@@ -262,7 +257,7 @@ void WebMediaPlayerMSCompositorTraits::Destruct(
 
 void WebMediaPlayerMSCompositor::InitializeSubmitter() {
   DCHECK(video_frame_compositor_task_runner_->BelongsToCurrentThread());
-  submitter_->Initialize(this, /* is_media_stream = */ true);
+  submitter_->Initialize(this, /*is_media_stream=*/true);
 }
 
 void WebMediaPlayerMSCompositor::SetIsSurfaceVisible(
@@ -609,15 +604,6 @@ void WebMediaPlayerMSCompositor::ReplaceCurrentFrameWithACopy() {
           WrapRefCounted(this))));
 }
 
-void WebMediaPlayerMSCompositor::StopUsingProvider() {
-  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  PostCrossThreadTask(
-      *video_frame_compositor_task_runner_, FROM_HERE,
-      CrossThreadBindOnce(
-          &WebMediaPlayerMSCompositor::StopUsingProviderInternal,
-          WrapRefCounted(this)));
-}
-
 bool WebMediaPlayerMSCompositor::MapTimestampsToRenderTimeTicks(
     const std::vector<base::TimeDelta>& timestamps,
     std::vector<base::TimeTicks>* wall_clock_times) {
@@ -853,13 +839,6 @@ void WebMediaPlayerMSCompositor::StopRenderingInternal() {
     video_frame_provider_client_->StopRendering();
 }
 
-void WebMediaPlayerMSCompositor::StopUsingProviderInternal() {
-  DCHECK(video_frame_compositor_task_runner_->BelongsToCurrentThread());
-  if (video_frame_provider_client_)
-    video_frame_provider_client_->StopUsingProvider();
-  video_frame_provider_client_ = nullptr;
-}
-
 void WebMediaPlayerMSCompositor::ReplaceCurrentFrameWithACopyInternal() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   scoped_refptr<media::VideoFrame> current_frame_ref;
diff --git a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h
index 59b1a60b7bc..a2d8761d561 100644
--- a/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h
+++ b/chromium/third_party/blink/renderer/modules/mediastream/webmediaplayer_ms_compositor.h
@@ -116,10 +116,6 @@ class MODULES_EXPORT WebMediaPlayerMSCompositor
   void StopRendering();
   void ReplaceCurrentFrameWithACopy();
 
-  // Tell |video_frame_provider_client_| to stop using this instance in
-  // preparation for dtor.
-  void StopUsingProvider();
-
   // Sets a hook to be notified when a new frame is presented, to fulfill a
   // prending video.requestAnimationFrame() request.
   // Can be called from any thread.
@@ -202,7 +198,6 @@ class MODULES_EXPORT WebMediaPlayerMSCompositor
 
   void StartRenderingInternal();
   void StopRenderingInternal();
-  void StopUsingProviderInternal();
   void ReplaceCurrentFrameWithACopyInternal();
 
   void SetAlgorithmEnabledForTesting(bool algorithm_enabled);