Capture the layer tree pipeline as a weak pointer in the OnAnimatorDraw task (flutter#26210)

Author: jason-simmons

shell/common/animator.cc

@@ -29,12 +29,12 @@ Animator::Animator(Delegate& delegate,
       waiter_(std::move(waiter)),
       dart_frame_deadline_(0),
 #if SHELL_ENABLE_METAL
-      layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(2)),
+      layer_tree_pipeline_(std::make_shared<LayerTreePipeline>(2)),
 #else   // SHELL_ENABLE_METAL
       // TODO(dnfield): We should remove this logic and set the pipeline depth
       // back to 2 in this case. See
       // https://github.com/flutter/engine/pull/9132 for discussion.
-      layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(
+      layer_tree_pipeline_(std::make_shared<LayerTreePipeline>(
           task_runners.GetPlatformTaskRunner() ==
                   task_runners.GetRasterTaskRunner()
               ? 1

shell/common/animator.h

@@ -36,7 +36,7 @@ class Animator final {
     virtual void OnAnimatorNotifyIdle(int64_t deadline) = 0;
 
     virtual void OnAnimatorDraw(
-        fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
+        std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
         std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder) = 0;
 
     virtual void OnAnimatorDrawLastLayerTree(
@@ -106,7 +106,7 @@ class Animator final {
   std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder_;
   uint64_t frame_request_number_ = 1;
   int64_t dart_frame_deadline_;
-  fml::RefPtr<LayerTreePipeline> layer_tree_pipeline_;
+  std::shared_ptr<LayerTreePipeline> layer_tree_pipeline_;
   fml::Semaphore pending_frame_semaphore_;
   LayerTreePipeline::ProducerContinuation producer_continuation_;
   bool paused_;

shell/common/pipeline.h

@@ -27,7 +27,7 @@ size_t GetNextPipelineTraceID();
 /// A thread-safe queue of resources for a single consumer and a single
 /// producer.
 template <class R>
-class Pipeline : public fml::RefCountedThreadSafe<Pipeline<R>> {
+class Pipeline {
  public:
   using Resource = R;
   using ResourcePtr = std::unique_ptr<Resource>;

shell/common/pipeline_unittests.cc

@@ -19,7 +19,7 @@ using IntPipeline = Pipeline<int>;
 using Continuation = IntPipeline::ProducerContinuation;
 
 TEST(PipelineTest, ConsumeOneVal) {
-  fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(2);
+  std::shared_ptr<IntPipeline> pipeline = std::make_shared<IntPipeline>(2);
 
   Continuation continuation = pipeline->Produce();
 
@@ -34,7 +34,7 @@ TEST(PipelineTest, ConsumeOneVal) {
 }
 
 TEST(PipelineTest, ContinuationCanOnlyBeUsedOnce) {
-  fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(2);
+  std::shared_ptr<IntPipeline> pipeline = std::make_shared<IntPipeline>(2);
 
   Continuation continuation = pipeline->Produce();
 
@@ -59,7 +59,7 @@ TEST(PipelineTest, ContinuationCanOnlyBeUsedOnce) {
 
 TEST(PipelineTest, PushingMoreThanDepthCompletesFirstSubmission) {
   const int depth = 1;
-  fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(depth);
+  std::shared_ptr<IntPipeline> pipeline = std::make_shared<IntPipeline>(depth);
 
   Continuation continuation_1 = pipeline->Produce();
   Continuation continuation_2 = pipeline->Produce();
@@ -78,7 +78,7 @@ TEST(PipelineTest, PushingMoreThanDepthCompletesFirstSubmission) {
 
 TEST(PipelineTest, PushingMultiProcessesInOrder) {
   const int depth = 2;
-  fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(depth);
+  std::shared_ptr<IntPipeline> pipeline = std::make_shared<IntPipeline>(depth);
 
   Continuation continuation_1 = pipeline->Produce();
   Continuation continuation_2 = pipeline->Produce();
@@ -100,7 +100,7 @@ TEST(PipelineTest, PushingMultiProcessesInOrder) {
 
 TEST(PipelineTest, ProduceIfEmptyDoesNotConsumeWhenQueueIsNotEmpty) {
   const int depth = 2;
-  fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(depth);
+  std::shared_ptr<IntPipeline> pipeline = std::make_shared<IntPipeline>(depth);
 
   Continuation continuation_1 = pipeline->Produce();
   Continuation continuation_2 = pipeline->ProduceIfEmpty();
@@ -118,7 +118,7 @@ TEST(PipelineTest, ProduceIfEmptyDoesNotConsumeWhenQueueIsNotEmpty) {
 
 TEST(PipelineTest, ProduceIfEmptySuccessfulIfQueueIsEmpty) {
   const int depth = 1;
-  fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(depth);
+  std::shared_ptr<IntPipeline> pipeline = std::make_shared<IntPipeline>(depth);
 
   Continuation continuation_1 = pipeline->ProduceIfEmpty();
 

shell/common/rasterizer.cc

@@ -158,7 +158,7 @@ void Rasterizer::DrawLastLayerTree(
 
 void Rasterizer::Draw(
     std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder,
-    fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
+    std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
     LayerTreeDiscardCallback discardCallback) {
   TRACE_EVENT_WITH_FRAME_NUMBER(frame_timings_recorder, "flutter",
                                 "GPURasterizer::Draw");

shell/common/rasterizer.h

@@ -244,7 +244,7 @@ class Rasterizer final : public SnapshotDelegate {
   ///                             is discarded instead of being rendered
   ///
   void Draw(std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder,
-            fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
+            std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
             LayerTreeDiscardCallback discardCallback = NoDiscard);
 
   //----------------------------------------------------------------------------

shell/common/rasterizer_unittests.cc

@@ -105,7 +105,7 @@ TEST(RasterizerTest, drawEmptyPipeline) {
   rasterizer->Setup(std::move(surface));
   fml::AutoResetWaitableEvent latch;
   thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
-    auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+    auto pipeline = std::make_shared<Pipeline<LayerTree>>(/*depth=*/10);
     rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, nullptr);
     latch.Signal();
   });
@@ -157,7 +157,7 @@ TEST(RasterizerTest,
   rasterizer->Setup(std::move(surface));
   fml::AutoResetWaitableEvent latch;
   thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
-    auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+    auto pipeline = std::make_shared<Pipeline<LayerTree>>(/*depth=*/10);
     auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
                                                   /*device_pixel_ratio=*/2.0f);
     bool result = pipeline->Produce().Complete(std::move(layer_tree));
@@ -211,7 +211,7 @@ TEST(
   rasterizer->Setup(std::move(surface));
   fml::AutoResetWaitableEvent latch;
   thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
-    auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+    auto pipeline = std::make_shared<Pipeline<LayerTree>>(/*depth=*/10);
     auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
                                                   /*device_pixel_ratio=*/2.0f);
     bool result = pipeline->Produce().Complete(std::move(layer_tree));
@@ -270,7 +270,7 @@ TEST(
 
   rasterizer->Setup(std::move(surface));
 
-  auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+  auto pipeline = std::make_shared<Pipeline<LayerTree>>(/*depth=*/10);
   auto layer_tree = std::make_unique<LayerTree>(/*frame_size=*/SkISize(),
                                                 /*device_pixel_ratio=*/2.0f);
   bool result = pipeline->Produce().Complete(std::move(layer_tree));
@@ -307,7 +307,7 @@ TEST(RasterizerTest, externalViewEmbedderDoesntEndFrameWhenNoSurfaceIsSet) {
 
   fml::AutoResetWaitableEvent latch;
   thread_host.raster_thread->GetTaskRunner()->PostTask([&] {
-    auto pipeline = fml::AdoptRef(new Pipeline<LayerTree>(/*depth=*/10));
+    auto pipeline = std::make_shared<Pipeline<LayerTree>>(/*depth=*/10);
     auto no_discard = [](LayerTree&) { return false; };
     rasterizer->Draw(CreateFinishedBuildRecorder(), pipeline, no_discard);
     latch.Signal();

shell/common/shell.cc

@@ -1128,7 +1128,7 @@ void Shell::OnAnimatorNotifyIdle(int64_t deadline) {
 
 // |Animator::Delegate|
 void Shell::OnAnimatorDraw(
-    fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
+    std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
     std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder) {
   FML_DCHECK(is_setup_);
 
@@ -1153,12 +1153,16 @@ void Shell::OnAnimatorDraw(
   task_runners_.GetRasterTaskRunner()->PostTask(fml::MakeCopyable(
       [&waiting_for_first_frame = waiting_for_first_frame_,
        &waiting_for_first_frame_condition = waiting_for_first_frame_condition_,
-       rasterizer = rasterizer_->GetWeakPtr(), pipeline = std::move(pipeline),
+       rasterizer = rasterizer_->GetWeakPtr(),
+       weak_pipeline = std::weak_ptr<Pipeline<LayerTree>>(pipeline),
        discard_callback = std::move(discard_callback),
        frame_timings_recorder = std::move(frame_timings_recorder)]() mutable {
         if (rasterizer) {
-          rasterizer->Draw(std::move(frame_timings_recorder), pipeline,
-                           std::move(discard_callback));
+          std::shared_ptr<Pipeline<LayerTree>> pipeline = weak_pipeline.lock();
+          if (pipeline) {
+            rasterizer->Draw(std::move(frame_timings_recorder),
+                             std::move(pipeline), std::move(discard_callback));
+          }
 
           if (waiting_for_first_frame.load()) {
             waiting_for_first_frame.store(false);

shell/common/shell.h

@@ -539,7 +539,7 @@ class Shell final : public PlatformView::Delegate,
 
   // |Animator::Delegate|
   void OnAnimatorDraw(
-      fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline,
+      std::shared_ptr<Pipeline<flutter::LayerTree>> pipeline,
       std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder) override;
 
   // |Animator::Delegate|