Ensure threads are merged when tearing down the Rasterizer (#19919)

fml/message_loop_task_queues.cc

@@ -246,7 +246,7 @@ bool MessageLoopTaskQueues::Unmerge(TaskQueueId owner) {
 bool MessageLoopTaskQueues::Owns(TaskQueueId owner,
                                  TaskQueueId subsumed) const {
   std::lock_guard guard(queue_mutex_);
-  return subsumed == queue_entries_.at(owner)->owner_of || owner == subsumed;
+  return subsumed == queue_entries_.at(owner)->owner_of;
 }
 
 // Subsumed queues will never have pending tasks.

fml/message_loop_task_queues_unittests.cc

@@ -173,6 +173,13 @@ TEST(MessageLoopTaskQueue, NotifyObserversWhileCreatingQueues) {
   before_second_observer.Signal();
   notify_observers.join();
 }
+
+TEST(MessageLoopTaskQueue, QueueDoNotOwnItself) {
+  auto task_queue = fml::MessageLoopTaskQueues::GetInstance();
+  auto queue_id = task_queue->CreateTaskQueue();
+  ASSERT_FALSE(task_queue->Owns(queue_id, queue_id));
+}
+
 // TODO(chunhtai): This unit-test is flaky and sometimes fails asynchronizely
 // after the test has finished.
 // https://github.com/flutter/flutter/issues/43858

fml/raster_thread_merger.cc

@@ -17,23 +17,41 @@ RasterThreadMerger::RasterThreadMerger(fml::TaskQueueId platform_queue_id,
       gpu_queue_id_(gpu_queue_id),
       task_queues_(fml::MessageLoopTaskQueues::GetInstance()),
       lease_term_(kLeaseNotSet) {
-  is_merged_ = task_queues_->Owns(platform_queue_id_, gpu_queue_id_);
+  FML_CHECK(!task_queues_->Owns(platform_queue_id_, gpu_queue_id_));
 }
 
 void RasterThreadMerger::MergeWithLease(size_t lease_term) {
+  if (TaskQueuesAreSame()) {
+    return;
+  }
+
   FML_DCHECK(lease_term > 0) << "lease_term should be positive.";
-  if (!is_merged_) {
-    is_merged_ = task_queues_->Merge(platform_queue_id_, gpu_queue_id_);
+  std::scoped_lock lock(lease_term_mutex_);
+  if (!IsMergedUnSafe()) {
+    bool success = task_queues_->Merge(platform_queue_id_, gpu_queue_id_);
+    FML_CHECK(success) << "Unable to merge the raster and platform threads.";
     lease_term_ = lease_term;
   }
+  merged_condition_.notify_one();
+}
+
+void RasterThreadMerger::UnMergeNow() {
+  if (TaskQueuesAreSame()) {
+    return;
+  }
+
+  std::scoped_lock lock(lease_term_mutex_);
+  lease_term_ = 0;
+  bool success = task_queues_->Unmerge(platform_queue_id_);
+  FML_CHECK(success) << "Unable to un-merge the raster and platform threads.";
 }
 
 bool RasterThreadMerger::IsOnPlatformThread() const {
   return MessageLoop::GetCurrentTaskQueueId() == platform_queue_id_;
 }
 
-bool RasterThreadMerger::IsOnRasterizingThread() const {
-  if (is_merged_) {
+bool RasterThreadMerger::IsOnRasterizingThread() {
+  if (IsMergedUnSafe()) {
     return IsOnPlatformThread();
   } else {
     return !IsOnPlatformThread();
@@ -41,24 +59,45 @@ bool RasterThreadMerger::IsOnRasterizingThread() const {
 }
 
 void RasterThreadMerger::ExtendLeaseTo(size_t lease_term) {
-  FML_DCHECK(lease_term > 0) << "lease_term should be positive.";
+  if (TaskQueuesAreSame()) {
+    return;
+  }
+  std::scoped_lock lock(lease_term_mutex_);
+  FML_DCHECK(IsMergedUnSafe()) << "lease_term should be positive.";
   if (lease_term_ != kLeaseNotSet &&
       static_cast<int>(lease_term) > lease_term_) {
     lease_term_ = lease_term;
   }
 }
 
-bool RasterThreadMerger::IsMerged() const {
-  return is_merged_;
+bool RasterThreadMerger::IsMerged() {
+  std::scoped_lock lock(lease_term_mutex_);
+  return IsMergedUnSafe();
 }
 
-RasterThreadStatus RasterThreadMerger::DecrementLease() {
-  if (!is_merged_) {
-    return RasterThreadStatus::kRemainsUnmerged;
+bool RasterThreadMerger::IsMergedUnSafe() {
+  return lease_term_ > 0 || TaskQueuesAreSame();
+}
+
+bool RasterThreadMerger::TaskQueuesAreSame() {
+  return platform_queue_id_ == gpu_queue_id_;
+}
+
+void RasterThreadMerger::WaitUntilMerged() {
+  if (TaskQueuesAreSame()) {
+    return;
   }
+  FML_CHECK(IsOnPlatformThread());
+  std::unique_lock<std::mutex> lock(lease_term_mutex_);
+  merged_condition_.wait(lock, [&] { return IsMergedUnSafe(); });
+}
 
-  // we haven't been set to merge.
-  if (lease_term_ == kLeaseNotSet) {
+RasterThreadStatus RasterThreadMerger::DecrementLease() {
+  if (TaskQueuesAreSame()) {
+    return RasterThreadStatus::kRemainsMerged;
+  }
+  std::unique_lock<std::mutex> lock(lease_term_mutex_);
+  if (!IsMergedUnSafe()) {
     return RasterThreadStatus::kRemainsUnmerged;
   }
 
@@ -66,9 +105,9 @@ RasterThreadStatus RasterThreadMerger::DecrementLease() {
       << "lease_term should always be positive when merged.";
   lease_term_--;
   if (lease_term_ == 0) {
-    bool success = task_queues_->Unmerge(platform_queue_id_);
-    FML_CHECK(success) << "Unable to un-merge the raster and platform threads.";
-    is_merged_ = false;
+    // |UnMergeNow| is going to acquire the lock again.
+    lock.unlock();
+    UnMergeNow();
     return RasterThreadStatus::kUnmergedNow;
   }
 

fml/raster_thread_merger.h

@@ -5,6 +5,8 @@
 #ifndef FML_SHELL_COMMON_TASK_RUNNER_MERGER_H_
 #define FML_SHELL_COMMON_TASK_RUNNER_MERGER_H_
 
+#include <condition_variable>
+#include <mutex>
 #include "flutter/fml/macros.h"
 #include "flutter/fml/memory/ref_counted.h"
 #include "flutter/fml/message_loop_task_queues.h"
@@ -28,15 +30,37 @@ class RasterThreadMerger
   // When the caller merges with a lease term of say 2. The threads
   // are going to remain merged until 2 invocations of |DecreaseLease|,
   // unless an |ExtendLeaseTo| gets called.
+  //
+  // If the task queues are the same, we consider them statically merged.
+  // When task queues are statically merged this method becomes no-op.
   void MergeWithLease(size_t lease_term);
 
+  // Un-merges the threads now, and resets the lease term to 0.
+  //
+  // Must be executed on the raster task runner.
+  //
+  // If the task queues are the same, we consider them statically merged.
+  // When task queues are statically merged, we never unmerge them and
+  // this method becomes no-op.
+  void UnMergeNow();
+
+  // If the task queues are the same, we consider them statically merged.
+  // When task queues are statically merged this method becomes no-op.
   void ExtendLeaseTo(size_t lease_term);
 
   // Returns |RasterThreadStatus::kUnmergedNow| if this call resulted in
   // splitting the raster and platform threads. Reduces the lease term by 1.
+  //
+  // If the task queues are the same, we consider them statically merged.
+  // When task queues are statically merged this method becomes no-op.
   RasterThreadStatus DecrementLease();
 
-  bool IsMerged() const;
+  bool IsMerged();
+
+  // Waits until the threads are merged.
+  //
+  // Must run on the platform task runner.
+  void WaitUntilMerged();
 
   RasterThreadMerger(fml::TaskQueueId platform_queue_id,
                      fml::TaskQueueId gpu_queue_id);
@@ -44,7 +68,7 @@ class RasterThreadMerger
   // Returns true if the current thread owns rasterizing.
   // When the threads are merged, platform thread owns rasterizing.
   // When un-merged, raster thread owns rasterizing.
-  bool IsOnRasterizingThread() const;
+  bool IsOnRasterizingThread();
 
   // Returns true if the current thread is the platform thread.
   bool IsOnPlatformThread() const;
@@ -55,7 +79,13 @@ class RasterThreadMerger
   fml::TaskQueueId gpu_queue_id_;
   fml::RefPtr<fml::MessageLoopTaskQueues> task_queues_;
   std::atomic_int lease_term_;
-  bool is_merged_;
+  std::condition_variable merged_condition_;
+  std::mutex lease_term_mutex_;
+
+  bool IsMergedUnSafe();
+  // The platform_queue_id and gpu_queue_id are exactly the same.
+  // We consider the threads are always merged and cannot be unmerged.
+  bool TaskQueuesAreSame();
 
   FML_FRIEND_REF_COUNTED_THREAD_SAFE(RasterThreadMerger);
   FML_FRIEND_MAKE_REF_COUNTED(RasterThreadMerger);

fml/raster_thread_merger_unittests.cc

@@ -208,3 +208,96 @@ TEST(RasterThreadMerger, LeaseExtension) {
   thread1.join();
   thread2.join();
 }
+
+TEST(RasterThreadMerger, WaitUntilMerged) {
+  fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger;
+
+  fml::AutoResetWaitableEvent create_thread_merger_latch;
+  fml::MessageLoop* loop_platform = nullptr;
+  fml::AutoResetWaitableEvent latch_platform;
+  fml::AutoResetWaitableEvent term_platform;
+  fml::AutoResetWaitableEvent latch_merged;
+  std::thread thread_platform([&]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    loop_platform = &fml::MessageLoop::GetCurrent();
+    latch_platform.Signal();
+    create_thread_merger_latch.Wait();
+    raster_thread_merger->WaitUntilMerged();
+    latch_merged.Signal();
+    term_platform.Wait();
+  });
+
+  const int kNumFramesMerged = 5;
+  fml::MessageLoop* loop_raster = nullptr;
+  fml::AutoResetWaitableEvent term_raster;
+  std::thread thread_raster([&]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    loop_raster = &fml::MessageLoop::GetCurrent();
+    latch_platform.Wait();
+    fml::TaskQueueId qid_platform =
+        loop_platform->GetTaskRunner()->GetTaskQueueId();
+    fml::TaskQueueId qid_raster =
+        loop_raster->GetTaskRunner()->GetTaskQueueId();
+    raster_thread_merger =
+        fml::MakeRefCounted<fml::RasterThreadMerger>(qid_platform, qid_raster);
+    ASSERT_FALSE(raster_thread_merger->IsMerged());
+    create_thread_merger_latch.Signal();
+    raster_thread_merger->MergeWithLease(kNumFramesMerged);
+    term_raster.Wait();
+  });
+
+  latch_merged.Wait();
+  ASSERT_TRUE(raster_thread_merger->IsMerged());
+
+  for (int i = 0; i < kNumFramesMerged; i++) {
+    ASSERT_TRUE(raster_thread_merger->IsMerged());
+    raster_thread_merger->DecrementLease();
+  }
+
+  ASSERT_FALSE(raster_thread_merger->IsMerged());
+
+  term_platform.Signal();
+  term_raster.Signal();
+  thread_platform.join();
+  thread_raster.join();
+}
+
+TEST(RasterThreadMerger, HandleTaskQueuesAreTheSame) {
+  fml::MessageLoop* loop1 = nullptr;
+  fml::AutoResetWaitableEvent latch1;
+  fml::AutoResetWaitableEvent term1;
+  std::thread thread1([&loop1, &latch1, &term1]() {
+    fml::MessageLoop::EnsureInitializedForCurrentThread();
+    loop1 = &fml::MessageLoop::GetCurrent();
+    latch1.Signal();
+    term1.Wait();
+  });
+
+  latch1.Wait();
+
+  fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId();
+  fml::TaskQueueId qid2 = qid1;
+  const auto raster_thread_merger_ =
+      fml::MakeRefCounted<fml::RasterThreadMerger>(qid1, qid2);
+  // Statically merged.
+  ASSERT_TRUE(raster_thread_merger_->IsMerged());
+
+  // Test decrement lease and unmerge are both no-ops.
+  // The task queues should be always merged.
+  const int kNumFramesMerged = 5;
+  raster_thread_merger_->MergeWithLease(kNumFramesMerged);
+
+  for (int i = 0; i < kNumFramesMerged; i++) {
+    ASSERT_TRUE(raster_thread_merger_->IsMerged());
+    raster_thread_merger_->DecrementLease();
+  }
+
+  ASSERT_TRUE(raster_thread_merger_->IsMerged());
+
+  // Wait until merged should also return immediately.
+  raster_thread_merger_->WaitUntilMerged();
+  ASSERT_TRUE(raster_thread_merger_->IsMerged());
+
+  term1.Signal();
+  thread1.join();
+}

shell/common/rasterizer.cc

@@ -77,6 +77,10 @@ void Rasterizer::Teardown() {
   compositor_context_->OnGrContextDestroyed();
   surface_.reset();
   last_layer_tree_.reset();
+  if (raster_thread_merger_.get() != nullptr &&
+      raster_thread_merger_.get()->IsMerged()) {
+    raster_thread_merger_->UnMergeNow();
+  }
 }
 
 void Rasterizer::NotifyLowMemoryWarning() const {
@@ -659,6 +663,24 @@ std::optional<size_t> Rasterizer::GetResourceCacheMaxBytes() const {
   return std::nullopt;
 }
 
+bool Rasterizer::EnsureThreadsAreMerged() {
+  if (surface_ == nullptr || raster_thread_merger_.get() == nullptr) {
+    return false;
+  }
+  fml::TaskRunner::RunNowOrPostTask(
+      delegate_.GetTaskRunners().GetRasterTaskRunner(),
+      [weak_this = weak_factory_.GetWeakPtr(),
+       thread_merger = raster_thread_merger_]() {
+        if (weak_this->surface_ == nullptr) {
+          return;
+        }
+        thread_merger->MergeWithLease(10);
+      });
+  raster_thread_merger_->WaitUntilMerged();
+  FML_DCHECK(raster_thread_merger_->IsMerged());
+  return true;
+}
+
 Rasterizer::Screenshot::Screenshot() {}
 
 Rasterizer::Screenshot::Screenshot(sk_sp<SkData> p_data, SkISize p_size)

shell/common/rasterizer.h

@@ -389,6 +389,22 @@ class Rasterizer final : public SnapshotDelegate {
   ///
   std::optional<size_t> GetResourceCacheMaxBytes() const;
 
+  //----------------------------------------------------------------------------
+  /// @brief      Makes sure the raster task runner and the platform task runner
+  ///             are merged.
+  ///
+  /// @attention  If raster and platform task runners are not the same or not
+  ///             merged, this method will try to merge the task runners,
+  ///             blocking the current thread until the 2 task runners are
+  ///             merged.
+  ///
+  /// @return     `true` if raster and platform task runners are the same.
+  ///             `true` if/when raster and platform task runners are merged.
+  ///             `false` if the surface or the |RasterThreadMerger| has not
+  ///             been initialized.
+  ///
+  bool EnsureThreadsAreMerged();
+
  private:
   Delegate& delegate_;
   std::unique_ptr<Surface> surface_;

shell/common/shell.cc

@@ -736,47 +736,21 @@ void Shell::OnPlatformViewDestroyed() {
     fml::TaskRunner::RunNowOrPostTask(io_task_runner, io_task);
   };
 
-  // The normal flow executed by this method is that the platform thread is
-  // starting the sequence and waiting on the latch. Later the UI thread posts
-  // raster_task to the raster thread triggers signaling the latch(on the IO
-  // thread). If the raster and the platform threads are the same this results
-  // in a deadlock as the raster_task will never be posted to the plaform/raster
-  // thread that is blocked on a latch.  To avoid the described deadlock, if the
-  // raster and the platform threads are the same, should_post_raster_task will
-  // be false, and then instead of posting a task to the raster thread, the ui
-  // thread just signals the latch and the platform/raster thread follows with
-  // executing raster_task.
-  bool should_post_raster_task = task_runners_.GetRasterTaskRunner() !=
-                                 task_runners_.GetPlatformTaskRunner();
-
-  auto ui_task = [engine = engine_->GetWeakPtr(),
-                  raster_task_runner = task_runners_.GetRasterTaskRunner(),
-                  raster_task, should_post_raster_task, &latch]() {
+  auto ui_task = [engine = engine_->GetWeakPtr(), &latch]() {
     if (engine) {
       engine->OnOutputSurfaceDestroyed();
     }
-    // Step 1: Next, tell the raster thread that its rasterizer should suspend
-    // access to the underlying surface.
-    if (should_post_raster_task) {
-      fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task);
-    } else {
-      // See comment on should_post_raster_task, in this case we just unblock
-      // the platform thread.
-      latch.Signal();
-    }
+    latch.Signal();
   };
 
   // Step 0: Post a task onto the UI thread to tell the engine that its output
   // surface is about to go away.
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);
   latch.Wait();
-  if (!should_post_raster_task) {
-    // See comment on should_post_raster_task, in this case the raster_task
-    // wasn't executed, and we just run it here as the platform thread
-    // is the raster thread.
-    raster_task();
-    latch.Wait();
-  }
+  rasterizer_->EnsureThreadsAreMerged();
+  fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(),
+                                    raster_task);
+  latch.Wait();
 }
 
 // |PlatformView::Delegate|

shell/common/shell_test.cc

@@ -49,6 +49,16 @@ void ShellTest::PlatformViewNotifyCreated(Shell* shell) {
   latch.Wait();
 }
 
+void ShellTest::PlatformViewNotifyDestroyed(Shell* shell) {
+  fml::AutoResetWaitableEvent latch;
+  fml::TaskRunner::RunNowOrPostTask(
+      shell->GetTaskRunners().GetPlatformTaskRunner(), [shell, &latch]() {
+        shell->GetPlatformView()->NotifyDestroyed();
+        latch.Signal();
+      });
+  latch.Wait();
+}
+
 void ShellTest::RunEngine(Shell* shell, RunConfiguration configuration) {
   fml::AutoResetWaitableEvent latch;
   fml::TaskRunner::RunNowOrPostTask(

shell/common/shell_test.h

@@ -50,6 +50,8 @@ class ShellTest : public FixtureTest {
 
   static void PlatformViewNotifyCreated(
       Shell* shell);  // This creates the surface
+  static void PlatformViewNotifyDestroyed(
+      Shell* shell);  // This destroys the surface
   static void RunEngine(Shell* shell, RunConfiguration configuration);
   static void RestartEngine(Shell* shell, RunConfiguration configuration);
 

shell/common/shell_test_external_view_embedder.cc

@@ -2,6 +2,25 @@
 
 namespace flutter {
 
+ShellTestExternalViewEmbedder::ShellTestExternalViewEmbedder(
+    const EndFrameCallBack& end_frame_call_back,
+    PostPrerollResult post_preroll_result,
+    bool support_thread_merging)
+    : end_frame_call_back_(end_frame_call_back),
+      post_preroll_result_(post_preroll_result),
+      support_thread_merging_(support_thread_merging) {
+  resubmit_once_ = false;
+}
+
+void ShellTestExternalViewEmbedder::UpdatePostPrerollResult(
+    PostPrerollResult post_preroll_result) {
+  post_preroll_result_ = post_preroll_result;
+}
+
+void ShellTestExternalViewEmbedder::SetResubmitOnce() {
+  resubmit_once_ = true;
+}
+
 // |ExternalViewEmbedder|
 void ShellTestExternalViewEmbedder::CancelFrame() {}
 
@@ -21,6 +40,10 @@ void ShellTestExternalViewEmbedder::PrerollCompositeEmbeddedView(
 PostPrerollResult ShellTestExternalViewEmbedder::PostPrerollAction(
     fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
   FML_DCHECK(raster_thread_merger);
+  if (resubmit_once_) {
+    resubmit_once_ = false;
+    return PostPrerollResult::kResubmitFrame;
+  }
   return post_preroll_result_;
 }
 

shell/common/shell_test_external_view_embedder.h

@@ -20,13 +20,18 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
 
   ShellTestExternalViewEmbedder(const EndFrameCallBack& end_frame_call_back,
                                 PostPrerollResult post_preroll_result,
-                                bool support_thread_merging)
-      : end_frame_call_back_(end_frame_call_back),
-        post_preroll_result_(post_preroll_result),
-        support_thread_merging_(support_thread_merging) {}
+                                bool support_thread_merging);
 
   ~ShellTestExternalViewEmbedder() = default;
 
+  // Updates the post preroll result so the |PostPrerollAction| after always
+  // returns the new `post_preroll_result`.
+  void UpdatePostPrerollResult(PostPrerollResult post_preroll_result);
+
+  // Updates the post preroll result to `PostPrerollResult::kResubmitFrame` for
+  // only the next frame.
+  void SetResubmitOnce();
+
  private:
   // |ExternalViewEmbedder|
   void CancelFrame() override;
@@ -69,7 +74,8 @@ class ShellTestExternalViewEmbedder final : public ExternalViewEmbedder {
   bool SupportsDynamicThreadMerging() override;
 
   const EndFrameCallBack end_frame_call_back_;
-  const PostPrerollResult post_preroll_result_;
+  PostPrerollResult post_preroll_result_;
+  bool resubmit_once_;
 
   bool support_thread_merging_;
 

shell/common/shell_unittests.cc

@@ -67,6 +67,32 @@ static bool ValidateShell(Shell* shell) {
   return true;
 }
 
+static bool RasterizerHasLayerTree(Shell* shell) {
+  fml::AutoResetWaitableEvent latch;
+  bool has_layer_tree = false;
+  fml::TaskRunner::RunNowOrPostTask(
+      shell->GetTaskRunners().GetRasterTaskRunner(),
+      [shell, &latch, &has_layer_tree]() {
+        has_layer_tree = shell->GetRasterizer()->GetLastLayerTree() != nullptr;
+        latch.Signal();
+      });
+  latch.Wait();
+  return has_layer_tree;
+}
+
+static void ValidateDestroyPlatformView(Shell* shell) {
+  ASSERT_TRUE(shell != nullptr);
+  ASSERT_TRUE(shell->IsSetup());
+
+  // To validate destroy platform view, we must ensure the rasterizer has a
+  // layer tree before the platform view is destroyed.
+  ASSERT_TRUE(RasterizerHasLayerTree(shell));
+
+  ShellTest::PlatformViewNotifyDestroyed(shell);
+  // Validate the layer tree is destroyed
+  ASSERT_FALSE(RasterizerHasLayerTree(shell));
+}
+
 TEST_F(ShellTest, InitializeWithInvalidThreads) {
   ASSERT_FALSE(DartVMRef::IsInstanceRunning());
   Settings settings = CreateSettingsForFixture();
@@ -454,7 +480,6 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) {
                     CREATE_NATIVE_ENTRY(nativeOnBeginFrame));
 
   RunEngine(shell.get(), std::move(configuration));
-
   PumpOneFrame(shell.get());
 
   // Check that timing is properly set. This implies that
@@ -477,7 +502,7 @@ TEST_F(ShellTest, FrameRasterizedCallbackIsCalled) {
 // support for raster thread merger for Fuchsia.
 TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) {
   auto settings = CreateSettingsForFixture();
-  fml::AutoResetWaitableEvent endFrameLatch;
+  fml::AutoResetWaitableEvent end_frame_latch;
   bool end_frame_called = false;
   auto end_frame_callback =
       [&](bool should_resubmit_frame,
@@ -485,7 +510,7 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) {
         ASSERT_TRUE(raster_thread_merger.get() == nullptr);
         ASSERT_FALSE(should_resubmit_frame);
         end_frame_called = true;
-        endFrameLatch.Signal();
+        end_frame_latch.Signal();
       };
   auto external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
       end_frame_callback, PostPrerollResult::kResubmitFrame, false);
@@ -516,7 +541,7 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) {
   };
 
   PumpOneFrame(shell.get(), 100, 100, builder);
-  endFrameLatch.Wait();
+  end_frame_latch.Wait();
 
   ASSERT_TRUE(end_frame_called);
 
@@ -526,7 +551,7 @@ TEST_F(ShellTest, ExternalEmbedderNoThreadMerger) {
 TEST_F(ShellTest,
        ExternalEmbedderEndFrameIsCalledWhenPostPrerollResultIsResubmit) {
   auto settings = CreateSettingsForFixture();
-  fml::AutoResetWaitableEvent endFrameLatch;
+  fml::AutoResetWaitableEvent end_frame_latch;
   bool end_frame_called = false;
   auto end_frame_callback =
       [&](bool should_resubmit_frame,
@@ -534,7 +559,7 @@ TEST_F(ShellTest,
         ASSERT_TRUE(raster_thread_merger.get() != nullptr);
         ASSERT_TRUE(should_resubmit_frame);
         end_frame_called = true;
-        endFrameLatch.Signal();
+        end_frame_latch.Signal();
       };
   auto external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
       end_frame_callback, PostPrerollResult::kResubmitFrame, true);
@@ -565,14 +590,312 @@ TEST_F(ShellTest,
   };
 
   PumpOneFrame(shell.get(), 100, 100, builder);
-  endFrameLatch.Wait();
+  end_frame_latch.Wait();
 
   ASSERT_TRUE(end_frame_called);
 
   DestroyShell(std::move(shell));
 }
+
+TEST_F(ShellTest, OnPlatformViewDestroyAfterMergingThreads) {
+  const size_t ThreadMergingLease = 10;
+  auto settings = CreateSettingsForFixture();
+  fml::AutoResetWaitableEvent end_frame_latch;
+  auto end_frame_callback =
+      [&](bool should_resubmit_frame,
+          fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
+        if (should_resubmit_frame && !raster_thread_merger->IsMerged()) {
+          raster_thread_merger->MergeWithLease(ThreadMergingLease);
+        }
+        end_frame_latch.Signal();
+      };
+  auto external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
+      end_frame_callback, PostPrerollResult::kSuccess, true);
+  // Set resubmit once to trigger thread merging.
+  external_view_embedder->SetResubmitOnce();
+  auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
+                           false, external_view_embedder);
+
+  // Create the surface needed by rasterizer
+  PlatformViewNotifyCreated(shell.get());
+
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("emptyMain");
+
+  RunEngine(shell.get(), std::move(configuration));
+
+  LayerTreeBuilder builder = [&](std::shared_ptr<ContainerLayer> root) {
+    SkPictureRecorder recorder;
+    SkCanvas* recording_canvas =
+        recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80));
+    recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80),
+                               SkPaint(SkColor4f::FromColor(SK_ColorRED)));
+    auto sk_picture = recorder.finishRecordingAsPicture();
+    fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
+        this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
+    auto picture_layer = std::make_shared<PictureLayer>(
+        SkPoint::Make(10, 10),
+        flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
+    root->Add(picture_layer);
+  };
+
+  PumpOneFrame(shell.get(), 100, 100, builder);
+  // Pump one frame to trigger thread merging.
+  end_frame_latch.Wait();
+  // Pump another frame to ensure threads are merged and a regular layer tree is
+  // submitted.
+  PumpOneFrame(shell.get(), 100, 100, builder);
+  // Threads are merged here. PlatformViewNotifyDestroy should be executed
+  // successfully.
+  ASSERT_TRUE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+  ValidateDestroyPlatformView(shell.get());
+
+  // Ensure threads are unmerged after platform view destroy
+  ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+
+  // Validate the platform view can be recreated and destroyed again
+  ValidateShell(shell.get());
+
+  DestroyShell(std::move(shell));
+}
+
+TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) {
+  const size_t ThreadMergingLease = 10;
+  auto settings = CreateSettingsForFixture();
+  fml::AutoResetWaitableEvent end_frame_latch;
+  auto end_frame_callback =
+      [&](bool should_resubmit_frame,
+          fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
+        if (should_resubmit_frame && !raster_thread_merger->IsMerged()) {
+          raster_thread_merger->MergeWithLease(ThreadMergingLease);
+        }
+        end_frame_latch.Signal();
+      };
+  // Start with a regular layer tree with `PostPrerollResult::kSuccess` so we
+  // can later check if the rasterizer is tore down using
+  // |ValidateDestroyPlatformView|
+  auto external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
+      end_frame_callback, PostPrerollResult::kSuccess, true);
+
+  auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
+                           false, external_view_embedder);
+
+  // Create the surface needed by rasterizer
+  PlatformViewNotifyCreated(shell.get());
+
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("emptyMain");
+
+  RunEngine(shell.get(), std::move(configuration));
+
+  LayerTreeBuilder builder = [&](std::shared_ptr<ContainerLayer> root) {
+    SkPictureRecorder recorder;
+    SkCanvas* recording_canvas =
+        recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80));
+    recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80),
+                               SkPaint(SkColor4f::FromColor(SK_ColorRED)));
+    auto sk_picture = recorder.finishRecordingAsPicture();
+    fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
+        this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
+    auto picture_layer = std::make_shared<PictureLayer>(
+        SkPoint::Make(10, 10),
+        flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
+    root->Add(picture_layer);
+  };
+
+  PumpOneFrame(shell.get(), 100, 100, builder);
+  // Pump one frame and threads aren't merged
+  end_frame_latch.Wait();
+  ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+
+  // Pump a frame with `PostPrerollResult::kResubmitFrame` to start merging
+  // threads
+  external_view_embedder->SetResubmitOnce();
+  PumpOneFrame(shell.get(), 100, 100, builder);
+
+  // Now destroy the platform view immediately.
+  // Two things can happen here:
+  // 1. Threads haven't merged. 2. Threads has already merged.
+  // |Shell:OnPlatformViewDestroy| should be able to handle both cases.
+  ValidateDestroyPlatformView(shell.get());
+
+  // Ensure threads are unmerged after platform view destroy
+  ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+
+  // Validate the platform view can be recreated and destroyed again
+  ValidateShell(shell.get());
+
+  DestroyShell(std::move(shell));
+}
+
+TEST_F(ShellTest,
+       OnPlatformViewDestroyWithThreadMergerWhileThreadsAreUnmerged) {
+  auto settings = CreateSettingsForFixture();
+  fml::AutoResetWaitableEvent end_frame_latch;
+  auto end_frame_callback =
+      [&](bool should_resubmit_frame,
+          fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
+        end_frame_latch.Signal();
+      };
+  auto external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
+      end_frame_callback, PostPrerollResult::kSuccess, true);
+  auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
+                           false, external_view_embedder);
+
+  // Create the surface needed by rasterizer
+  PlatformViewNotifyCreated(shell.get());
+
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("emptyMain");
+
+  RunEngine(shell.get(), std::move(configuration));
+
+  LayerTreeBuilder builder = [&](std::shared_ptr<ContainerLayer> root) {
+    SkPictureRecorder recorder;
+    SkCanvas* recording_canvas =
+        recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80));
+    recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80),
+                               SkPaint(SkColor4f::FromColor(SK_ColorRED)));
+    auto sk_picture = recorder.finishRecordingAsPicture();
+    fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
+        this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
+    auto picture_layer = std::make_shared<PictureLayer>(
+        SkPoint::Make(10, 10),
+        flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
+    root->Add(picture_layer);
+  };
+  PumpOneFrame(shell.get(), 100, 100, builder);
+  end_frame_latch.Wait();
+
+  // Threads should not be merged.
+  ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+  ValidateDestroyPlatformView(shell.get());
+
+  // Ensure threads are unmerged after platform view destroy
+  ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+
+  // Validate the platform view can be recreated and destroyed again
+  ValidateShell(shell.get());
+
+  DestroyShell(std::move(shell));
+}
+
+TEST_F(ShellTest, OnPlatformViewDestroyWithoutRasterThreadMerger) {
+  auto settings = CreateSettingsForFixture();
+
+  auto shell = CreateShell(std::move(settings), GetTaskRunnersForFixture(),
+                           false, nullptr);
+
+  // Create the surface needed by rasterizer
+  PlatformViewNotifyCreated(shell.get());
+
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("emptyMain");
+
+  RunEngine(shell.get(), std::move(configuration));
+
+  LayerTreeBuilder builder = [&](std::shared_ptr<ContainerLayer> root) {
+    SkPictureRecorder recorder;
+    SkCanvas* recording_canvas =
+        recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80));
+    recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80),
+                               SkPaint(SkColor4f::FromColor(SK_ColorRED)));
+    auto sk_picture = recorder.finishRecordingAsPicture();
+    fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
+        this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
+    auto picture_layer = std::make_shared<PictureLayer>(
+        SkPoint::Make(10, 10),
+        flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
+    root->Add(picture_layer);
+  };
+  PumpOneFrame(shell.get(), 100, 100, builder);
+
+  // Threads should not be merged.
+  ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+  ValidateDestroyPlatformView(shell.get());
+
+  // Ensure threads are unmerged after platform view destroy
+  ASSERT_FALSE(fml::TaskRunnerChecker::RunsOnTheSameThread(
+      shell->GetTaskRunners().GetRasterTaskRunner()->GetTaskQueueId(),
+      shell->GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId()));
+
+  // Validate the platform view can be recreated and destroyed again
+  ValidateShell(shell.get());
+
+  DestroyShell(std::move(shell));
+}
 #endif
 
+TEST_F(ShellTest, OnPlatformViewDestroyWithStaticThreadMerging) {
+  auto settings = CreateSettingsForFixture();
+  fml::AutoResetWaitableEvent end_frame_latch;
+  auto end_frame_callback =
+      [&](bool should_resubmit_frame,
+          fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger) {
+        end_frame_latch.Signal();
+      };
+  auto external_view_embedder = std::make_shared<ShellTestExternalViewEmbedder>(
+      end_frame_callback, PostPrerollResult::kSuccess, true);
+  ThreadHost thread_host(
+      "io.flutter.test." + GetCurrentTestName() + ".",
+      ThreadHost::Type::Platform | ThreadHost::Type::IO | ThreadHost::Type::UI);
+  TaskRunners task_runners(
+      "test",
+      thread_host.platform_thread->GetTaskRunner(),  // platform
+      thread_host.platform_thread->GetTaskRunner(),  // raster
+      thread_host.ui_thread->GetTaskRunner(),        // ui
+      thread_host.io_thread->GetTaskRunner()         // io
+  );
+  auto shell = CreateShell(std::move(settings), std::move(task_runners), false,
+                           external_view_embedder);
+
+  // Create the surface needed by rasterizer
+  PlatformViewNotifyCreated(shell.get());
+
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("emptyMain");
+
+  RunEngine(shell.get(), std::move(configuration));
+
+  LayerTreeBuilder builder = [&](std::shared_ptr<ContainerLayer> root) {
+    SkPictureRecorder recorder;
+    SkCanvas* recording_canvas =
+        recorder.beginRecording(SkRect::MakeXYWH(0, 0, 80, 80));
+    recording_canvas->drawRect(SkRect::MakeXYWH(0, 0, 80, 80),
+                               SkPaint(SkColor4f::FromColor(SK_ColorRED)));
+    auto sk_picture = recorder.finishRecordingAsPicture();
+    fml::RefPtr<SkiaUnrefQueue> queue = fml::MakeRefCounted<SkiaUnrefQueue>(
+        this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0));
+    auto picture_layer = std::make_shared<PictureLayer>(
+        SkPoint::Make(10, 10),
+        flutter::SkiaGPUObject<SkPicture>({sk_picture, queue}), false, false);
+    root->Add(picture_layer);
+  };
+  PumpOneFrame(shell.get(), 100, 100, builder);
+  end_frame_latch.Wait();
+
+  ValidateDestroyPlatformView(shell.get());
+
+  // Validate the platform view can be recreated and destroyed again
+  ValidateShell(shell.get());
+
+  DestroyShell(std::move(shell), std::move(task_runners));
+}
+
 TEST(SettingsTest, FrameTimingSetsAndGetsProperly) {
   // Ensure that all phases are in kPhases.
   ASSERT_EQ(sizeof(FrameTiming::kPhases),

shell/platform/darwin/ios/framework/Source/FlutterEngine.mm

@@ -508,43 +508,19 @@ static constexpr int kNumProfilerSamplesPerSec = 5;
   flutter::Shell::CreateCallback<flutter::Rasterizer> on_create_rasterizer =
       [](flutter::Shell& shell) { return std::make_unique<flutter::Rasterizer>(shell); };
 
-  if (flutter::IsIosEmbeddedViewsPreviewEnabled()) {
-    // Embedded views requires the gpu and the platform views to be the same.
-    // The plan is to eventually dynamically merge the threads when there's a
-    // platform view in the layer tree.
-    // For now we use a fixed thread configuration with the same thread used as the
-    // gpu and platform task runner.
-    // TODO(amirh/chinmaygarde): remove this, and dynamically change the thread configuration.
-    // https://github.com/flutter/flutter/issues/23975
-
-    flutter::TaskRunners task_runners(threadLabel.UTF8String,                          // label
-                                      fml::MessageLoop::GetCurrent().GetTaskRunner(),  // platform
-                                      fml::MessageLoop::GetCurrent().GetTaskRunner(),  // raster
-                                      _threadHost.ui_thread->GetTaskRunner(),          // ui
-                                      _threadHost.io_thread->GetTaskRunner()           // io
-    );
-    // Create the shell. This is a blocking operation.
-    _shell = flutter::Shell::Create(std::move(task_runners),  // task runners
-                                    std::move(platformData),  // platform data
-                                    std::move(settings),      // settings
-                                    on_create_platform_view,  // platform view creation
-                                    on_create_rasterizer      // rasterizer creation
-    );
-  } else {
-    flutter::TaskRunners task_runners(threadLabel.UTF8String,                          // label
-                                      fml::MessageLoop::GetCurrent().GetTaskRunner(),  // platform
-                                      _threadHost.raster_thread->GetTaskRunner(),      // raster
-                                      _threadHost.ui_thread->GetTaskRunner(),          // ui
-                                      _threadHost.io_thread->GetTaskRunner()           // io
-    );
-    // Create the shell. This is a blocking operation.
-    _shell = flutter::Shell::Create(std::move(task_runners),  // task runners
-                                    std::move(platformData),  // platform data
-                                    std::move(settings),      // settings
-                                    on_create_platform_view,  // platform view creation
-                                    on_create_rasterizer      // rasterizer creation
-    );
-  }
+  flutter::TaskRunners task_runners(threadLabel.UTF8String,                          // label
+                                    fml::MessageLoop::GetCurrent().GetTaskRunner(),  // platform
+                                    _threadHost.raster_thread->GetTaskRunner(),      // raster
+                                    _threadHost.ui_thread->GetTaskRunner(),          // ui
+                                    _threadHost.io_thread->GetTaskRunner()           // io
+  );
+  // Create the shell. This is a blocking operation.
+  _shell = flutter::Shell::Create(std::move(task_runners),  // task runners
+                                  std::move(platformData),  // window data
+                                  std::move(settings),      // settings
+                                  on_create_platform_view,  // platform view creation
+                                  on_create_rasterizer      // rasterzier creation
+  );
 
   if (_shell == nullptr) {
     FML_LOG(ERROR) << "Could not start a shell FlutterEngine with entrypoint: "