Disallows submitting frames when lifetime of FrameSinkHolder is

extended.

Display compositor can asynchronously request new frames when the
FrameSinkHolder is in its extended lifetime state. It is problematic
since in this scenario the callback `get_compositor_frame_callback_`
is null as the hosting FrameSinkHost is deleted which leads to crashes.

Bug: 1392895
Test: Covered by unittests
Change-Id: I0a0e959937bb0c76bc7642126dc1da42a79e71a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4270413
Reviewed-by: Sean Kau <skau@chromium.org>
Commit-Queue: Zoraiz Naeem <zoraiznaeem@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108046}

ash/frame_sink/frame_sink_holder.cc

@@ -108,8 +108,11 @@ void FrameSinkHolder::SubmitCompositorFrame(bool synchronous_draw) {
   // We cannot request to submit a frame via `SubmitCompositorFrame()` if we are
   // in auto_update mode.
   DCHECK(!auto_update_);
+  // Once the lifetime of FrameSinkHolder is extended, we should not submit new
+  // frames since the `get_compositor_frame_callback_` can become null.
+  DCHECK(!WaitingToScheduleDelete());
 
-  if (delete_pending_ || auto_update_) {
+  if (delete_pending_ || auto_update_ || WaitingToScheduleDelete()) {
     return;
   }
 
@@ -155,6 +158,13 @@ void FrameSinkHolder::SubmitCompositorFrameInternal(
 void FrameSinkHolder::OnBeginFrameSourcePausedChanged(bool paused) {}
 
 bool FrameSinkHolder::OnBeginFrameDerivedImpl(const viz::BeginFrameArgs& args) {
+  // Once the lifetime of FrameSinkHolder is extended, we should not submit new
+  // frames asynchronously since the `get_compositor_frame_callback_` can become
+  // null.
+  if (WaitingToScheduleDelete()) {
+    return false;
+  }
+
   viz::BeginFrameAck current_begin_frame_ack(args, false);
 
   first_frame_requested_ = true;

ash/frame_sink/frame_sink_holder_unittest.cc

@@ -350,6 +350,41 @@ TEST_F(FrameSinkHolderTest, HandlingAsynchronousFrameRequests_NoAutoUpdate) {
   EXPECT_TRUE(test_api.IsPendingFrameAck());
 }
 
+TEST_F(FrameSinkHolderTest, DontSubmitNewFramesWhenWaitingToDeleteSinkHolder) {
+  FrameSinkHolderTestApi test_api(frame_sink_holder_.get());
+  base::RunLoop loop;
+
+  viz::ResourceId id_1 =
+      GetResourceManager().OfferResource(std::make_unique<UiResource>());
+
+  frame_factory_->SetFrameResources({id_1});
+  frame_factory_->SetFrameMetaData(gfx::Size(100, 100), 1.0);
+
+  // Call OnBeginFrame so that FrameSinkHolder can know that it can submit
+  // frames synchronously.
+  frame_sink_holder_->OnBeginFrame(CreateFakeBeginFrameArgs());
+  frame_sink_holder_->SubmitCompositorFrame(/*synchronous_draw=*/true);
+  EXPECT_EQ(layer_tree_frame_sink_->num_frames_received(), 1);
+
+  // The lifetime of frame_sink_holder has been extended since there are still
+  // some exported resources.
+  EXPECT_FALSE(FrameSinkHolder::DeleteWhenLastResourceHasBeenReclaimed(
+      std::move(frame_sink_holder_), host_window_));
+
+  ASSERT_TRUE(holder_weak_ptr_);
+
+  // During deletion FrameSinkHolder submits a empty frame.
+  EXPECT_EQ(layer_tree_frame_sink_->num_frames_received(), 2);
+
+  layer_tree_frame_sink_->ResetLatestFrameState();
+
+  holder_weak_ptr_->OnBeginFrame(CreateFakeBeginFrameArgs());
+
+  // Confirms that FrameSinkHolder did not submit a new frame on asynchronous
+  // request.
+  EXPECT_EQ(layer_tree_frame_sink_->num_frames_received(), 2);
+}
+
 TEST_F(FrameSinkHolderTest,
        DeleteSinkHolderImmediatelyWhenNoFramesIsSubmitted) {
   FrameSinkHolderTestApi test_api(frame_sink_holder_.get());