Avoid deadlocks in CheckForFrameChanges

Some delegates called from this method may try accessing |current_frame_| which
can cause deadlocks. Therefore perform these checks right after |current_frame_|
is set and lock is released.

Bug: 901744
Change-Id: I56605cca8bad81a498a177ff8230598abfdd03fd
Reviewed-on: https://chromium-review.googlesource.com/c/1321092Reviewed-by: CJ DiMeglio <lethalantidote@chromium.org>
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605947}

content/renderer/media/stream/webmediaplayer_ms_compositor.cc

@@ -526,7 +526,13 @@ void WebMediaPlayerMSCompositor::SetCurrentFrame(
 
   scoped_refptr<media::VideoFrame> old_frame = std::move(current_frame_);
   current_frame_ = frame;
-  CheckForFrameChanges(old_frame, frame);
+
+  // Complete the checks after |current_frame_| is accessible to avoid
+  // deadlocks, see https://crbug.com/901744.
+  video_frame_compositor_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&WebMediaPlayerMSCompositor::CheckForFrameChanges, this,
+                     old_frame, frame));
 }
 
 void WebMediaPlayerMSCompositor::CheckForFrameChanges(

content/renderer/media/stream/webmediaplayer_ms_unittest.cc

@@ -429,12 +429,27 @@ class MockWebVideoFrameSubmitter : public blink::WebVideoFrameSubmitter {
                     blink::WebFrameSinkDestroyedCallback));
   MOCK_METHOD0(StartRendering, void());
   MOCK_METHOD0(StopRendering, void());
-  MOCK_METHOD1(Initialize, void(cc::VideoFrameProvider*));
+  MOCK_METHOD1(MockInitialize, void(cc::VideoFrameProvider*));
   MOCK_METHOD1(SetRotation, void(media::VideoRotation));
-  MOCK_METHOD1(SetIsOpaque, void(bool));
+  MOCK_METHOD1(MockSetIsOpaque, void(bool));
   MOCK_METHOD1(UpdateSubmissionState, void(bool));
   MOCK_METHOD1(SetForceSubmit, void(bool));
   MOCK_CONST_METHOD0(IsDrivingFrameUpdates, bool());
+
+  void Initialize(cc::VideoFrameProvider* provider) override {
+    provider_ = provider;
+    MockInitialize(provider);
+  }
+
+  // This method may try accessing frames, see deadlock case in
+  // https://crbug.com/901744.
+  void SetIsOpaque(bool opaque) override {
+    auto frame = provider_->GetCurrentFrame();
+    MockSetIsOpaque(opaque);
+  }
+
+ private:
+  cc::VideoFrameProvider* provider_;
 };
 
 // The class is used to generate a MockVideoProvider in
@@ -1168,7 +1183,7 @@ TEST_P(WebMediaPlayerMSTest, OpacityChange) {
   provider->QueueFrames(timestamps, false);
   if (enable_surface_layer_for_video_) {
     EXPECT_CALL(*surface_layer_bridge_ptr_, SetContentsOpaque(false));
-    EXPECT_CALL(*submitter_ptr_, SetIsOpaque(false));
+    EXPECT_CALL(*submitter_ptr_, MockSetIsOpaque(false));
   }
   message_loop_controller_.RunAndWaitForStatus(
       media::PipelineStatus::PIPELINE_OK);
@@ -1182,7 +1197,7 @@ TEST_P(WebMediaPlayerMSTest, OpacityChange) {
   provider->QueueFrames(timestamps, true);
   if (enable_surface_layer_for_video_) {
     EXPECT_CALL(*surface_layer_bridge_ptr_, SetContentsOpaque(true));
-    EXPECT_CALL(*submitter_ptr_, SetIsOpaque(true));
+    EXPECT_CALL(*submitter_ptr_, MockSetIsOpaque(true));
   }
   message_loop_controller_.RunAndWaitForStatus(
       media::PipelineStatus::PIPELINE_OK);