Enable/Disable from shell delegate

Author: Emmanuel Garcia

fml/raster_thread_merger.cc

@@ -22,14 +22,13 @@ RasterThreadMerger::RasterThreadMerger(fml::TaskQueueId platform_queue_id,
 }
 
 void RasterThreadMerger::MergeWithLease(size_t lease_term) {
+  FML_DCHECK(lease_term > 0) << "lease_term should be positive.";
   if (TaskQueuesAreSame()) {
     return;
   }
-  if (!IsEnabled()) {
+  if (!IsEnabledUnSafe()) {
     return;
   }
-
-  FML_DCHECK(lease_term > 0) << "lease_term should be positive.";
   std::scoped_lock lock(lease_term_mutex_);
   if (!IsMergedUnSafe()) {
     bool success = task_queues_->Merge(platform_queue_id_, gpu_queue_id_);
@@ -40,14 +39,13 @@ void RasterThreadMerger::MergeWithLease(size_t lease_term) {
 }
 
 void RasterThreadMerger::UnMergeNow() {
+  std::scoped_lock lock(lease_term_mutex_);
   if (TaskQueuesAreSame()) {
     return;
   }
-  if (!IsEnabled()) {
+  if (!IsEnabledUnSafe()) {
     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.";
@@ -57,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 {
@@ -77,7 +75,7 @@ void RasterThreadMerger::ExtendLeaseTo(size_t lease_term) {
   }
 }
 
-bool RasterThreadMerger::IsMerged() {
+bool RasterThreadMerger::IsMerged() const {
   std::scoped_lock lock(lease_term_mutex_);
   return IsMergedUnSafe();
 }
@@ -94,14 +92,18 @@ void RasterThreadMerger::Disable() {
 
 bool RasterThreadMerger::IsEnabled() const {
   std::scoped_lock lock(lease_term_mutex_);
+  return IsEnabledUnSafe();
+}
+
+bool RasterThreadMerger::IsEnabledUnSafe() const {
   return enabled_;
 }
 
-bool RasterThreadMerger::IsMergedUnSafe() {
+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

@@ -55,7 +55,7 @@ class RasterThreadMerger
   // When task queues are statically merged this method becomes no-op.
   RasterThreadStatus DecrementLease();
 
-  bool IsMerged();
+  bool IsMerged() const;
 
   // Waits until the threads are merged.
   //
@@ -68,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();
+  bool IsOnRasterizingThread() const;
 
   // Returns true if the current thread is the platform thread.
   bool IsOnPlatformThread() const;
@@ -95,10 +95,13 @@ class RasterThreadMerger
   std::mutex lease_term_mutex_;
   bool enabled_;
 
-  bool IsMergedUnSafe();
+  bool IsMergedUnSafe() const;
+
+  bool IsEnabledUnSafe() const;
+
   // 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();
   }
 }
 
@@ -146,11 +154,12 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
 
   // Merging the thread as we know the next `Draw` should be run on the platform
   // thread.
-  if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr) {
-    auto should_resubmit_frame = raster_status == RasterStatus::kResubmit;
-    surface_->GetExternalViewEmbedder()->EndFrame(should_resubmit_frame,
-                                                  raster_thread_merger_);
-  }
+  // if (surface_ != nullptr && surface_->GetExternalViewEmbedder() != nullptr)
+  // {
+  //   auto should_resubmit_frame = raster_status == RasterStatus::kResubmit;
+  //   surface_->GetExternalViewEmbedder()->EndFrame(should_resubmit_frame,
+  //                                                 raster_thread_merger_);
+  // }
 
   // Consume as many pipeline items as possible. But yield the event loop
   // between successive tries.
@@ -358,12 +367,12 @@ RasterStatus Rasterizer::DoDraw(
   //   - |Draw| for B yields as its on the wrong thread.
   // This enqueue ensures that we attempt to consume from the right
   // thread one more time after un-merge.
-  if (raster_thread_merger_) {
-    if (raster_thread_merger_->DecrementLease() ==
-        fml::RasterThreadStatus::kUnmergedNow) {
-      return RasterStatus::kEnqueuePipeline;
-    }
-  }
+  // if (raster_thread_merger_) {
+  //   if (raster_thread_merger_->DecrementLease() ==
+  //       fml::RasterThreadStatus::kUnmergedNow) {
+  //     return RasterStatus::kEnqueuePipeline;
+  //   }
+  // }
 
   return raster_status;
 }
@@ -377,7 +386,7 @@ RasterStatus Rasterizer::DrawToSurface(flutter::LayerTree& layer_tree) {
   // for instrumentation.
   compositor_context_->ui_time().SetLapTime(layer_tree.build_time());
 
-  auto* external_view_embedder = surface_->GetExternalViewEmbedder();
+  flutter::ExternalViewEmbedder* external_view_embedder = nullptr;
 
   SkCanvas* embedder_root_canvas = nullptr;
   if (external_view_embedder != nullptr) {
@@ -663,24 +672,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

@@ -389,21 +389,9 @@ 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();
+  void EnableThreadMergerIfNeeded();
+
+  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_->EnabledThreadMergerIfNeeded();
+
   // 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();