Add ability to disable the raster thread merger

fml/raster_thread_merger.cc

@@ -16,16 +16,19 @@ RasterThreadMerger::RasterThreadMerger(fml::TaskQueueId platform_queue_id,
     : platform_queue_id_(platform_queue_id),
       gpu_queue_id_(gpu_queue_id),
       task_queues_(fml::MessageLoopTaskQueues::GetInstance()),
-      lease_term_(kLeaseNotSet) {
+      lease_term_(kLeaseNotSet),
+      enabled_(true) {
   FML_CHECK(!task_queues_->Owns(platform_queue_id_, gpu_queue_id_));
 }
 
 void RasterThreadMerger::MergeWithLease(size_t lease_term) {
+  FML_DCHECK(lease_term > 0) << "lease_term should be positive.";
   if (TaskQueuesAreSame()) {
     return;
   }
-
-  FML_DCHECK(lease_term > 0) << "lease_term should be positive.";
+  if (!IsEnabledUnSafe()) {
+    return;
+  }
   std::scoped_lock lock(lease_term_mutex_);
   if (!IsMergedUnSafe()) {
     bool success = task_queues_->Merge(platform_queue_id_, gpu_queue_id_);
@@ -36,11 +39,13 @@ void RasterThreadMerger::MergeWithLease(size_t lease_term) {
 }
 
 void RasterThreadMerger::UnMergeNow() {
+  std::scoped_lock lock(lease_term_mutex_);
   if (TaskQueuesAreSame()) {
     return;
   }
-
-  std::scoped_lock lock(lease_term_mutex_);
+  if (!IsEnabledUnSafe()) {
+    return;
+  }
   lease_term_ = 0;
   bool success = task_queues_->Unmerge(platform_queue_id_);
   FML_CHECK(success) << "Unable to un-merge the raster and platform threads.";
@@ -50,7 +55,7 @@ bool RasterThreadMerger::IsOnPlatformThread() const {
   return MessageLoop::GetCurrentTaskQueueId() == platform_queue_id_;
 }
 
-bool RasterThreadMerger::IsOnRasterizingThread() {
+bool RasterThreadMerger::IsOnRasterizingThread() const {
   if (IsMergedUnSafe()) {
     return IsOnPlatformThread();
   } else {
@@ -75,11 +80,30 @@ bool RasterThreadMerger::IsMerged() {
   return IsMergedUnSafe();
 }
 
-bool RasterThreadMerger::IsMergedUnSafe() {
+void RasterThreadMerger::Enable() {
+  std::scoped_lock lock(lease_term_mutex_);
+  enabled_ = true;
+}
+
+void RasterThreadMerger::Disable() {
+  std::scoped_lock lock(lease_term_mutex_);
+  enabled_ = false;
+}
+
+bool RasterThreadMerger::IsEnabled() {
+  std::scoped_lock lock(lease_term_mutex_);
+  return IsEnabledUnSafe();
+}
+
+bool RasterThreadMerger::IsEnabledUnSafe() const {
+  return enabled_;
+}
+
+bool RasterThreadMerger::IsMergedUnSafe() const {
   return lease_term_ > 0 || TaskQueuesAreSame();
 }
 
-bool RasterThreadMerger::TaskQueuesAreSame() {
+bool RasterThreadMerger::TaskQueuesAreSame() const {
   return platform_queue_id_ == gpu_queue_id_;
 }
 

fml/raster_thread_merger.h

@@ -68,11 +68,23 @@ 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();
+  bool IsOnRasterizingThread() const;
 
   // Returns true if the current thread is the platform thread.
   bool IsOnPlatformThread() const;
 
+  // Enables the thread merger.
+  void Enable();
+
+  // Disables the thread merger. Once disabled, any call to
+  // |MergeWithLease| or |UnMergeNow| results in a noop.
+  void Disable();
+
+  // Whether the thread merger is enabled. By default, the thread merger is
+  // enabled. If false, calls to |MergeWithLease| or |UnMergeNow| results in a
+  // noop.
+  bool IsEnabled();
+
  private:
   static const int kLeaseNotSet;
   fml::TaskQueueId platform_queue_id_;
@@ -81,11 +93,15 @@ class RasterThreadMerger
   std::atomic_int lease_term_;
   std::condition_variable merged_condition_;
   std::mutex lease_term_mutex_;
+  bool enabled_;
+
+  bool IsMergedUnSafe() const;
+
+  bool IsEnabledUnSafe() const;
 
-  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();
+  bool TaskQueuesAreSame() const;
 
   FML_FRIEND_REF_COUNTED_THREAD_SAFE(RasterThreadMerger);
   FML_FRIEND_MAKE_REF_COUNTED(RasterThreadMerger);

shell/common/rasterizer.cc

@@ -77,9 +77,17 @@ 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::EnableThreadMergerIfNeeded() {
+  if (raster_thread_merger_) {
+    raster_thread_merger_->Enable();
+  }
+}
+
+void Rasterizer::DisableThreadMergerIfNeeded() {
+  if (raster_thread_merger_) {
+    raster_thread_merger_->Disable();
   }
 }
 
@@ -663,24 +671,6 @@ 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

@@ -390,20 +390,28 @@ 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.
+  /// @brief      Enables the thread merger if the external view embedder
+  ///             supports dynamic thread merging.
   ///
-  /// @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.
+  /// @attention  This method is thread-safe. When the thread merger is enabled,
+  ///             the raster task queue can run in the platform thread at any
+  ///             time.
   ///
-  /// @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.
+  /// @see        `ExternalViewEmbedder`
   ///
-  bool EnsureThreadsAreMerged();
+  void EnableThreadMergerIfNeeded();
+
+  //----------------------------------------------------------------------------
+  /// @brief      Disables the thread merger if the external view embedder
+  ///             supports dynamic thread merging.
+  ///
+  /// @attention  This method is thread-safe. When the thread merger is
+  ///             disabled, the raster task queue will continue to run in the
+  ///             same thread until |EnableThreadMergerIfNeeded| is called.
+  ///
+  /// @see        `ExternalViewEmbedder`
+  ///
+  void DisableThreadMergerIfNeeded();
 
  private:
   Delegate& delegate_;

shell/common/shell.cc

@@ -619,6 +619,9 @@ void Shell::OnPlatformViewCreated(std::unique_ptr<Surface> surface) {
   FML_DCHECK(is_setup_);
   FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread());
 
+  // Enables the thread merger which may be used by the external view embedder.
+  rasterizer_->EnableThreadMergerIfNeeded();
+
   // Note:
   // This is a synchronous operation because certain platforms depend on
   // setup/suspension of all activities that may be interacting with the GPU in
@@ -708,6 +711,14 @@ void Shell::OnPlatformViewDestroyed() {
   FML_DCHECK(is_setup_);
   FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread());
 
+  // Prevent any request to merge the raster and platform threads while the
+  // platform view is destroyed.
+  //
+  // This prevents a dead lock where the platform thread is blocked waiting for
+  // the latch, but the latch is never released because the raster queue is all
+  // the sudden running on the platform thread.
+  rasterizer_->DisableThreadMergerIfNeeded();
+
   // Note:
   // This is a synchronous operation because certain platforms depend on
   // setup/suspension of all activities that may be interacting with the GPU in
@@ -747,7 +758,6 @@ void Shell::OnPlatformViewDestroyed() {
   // surface is about to go away.
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetUITaskRunner(), ui_task);
   latch.Wait();
-  rasterizer_->EnsureThreadsAreMerged();
   fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(),
                                     raster_task);
   latch.Wait();