diff --git a/fml/raster_thread_merger.cc b/fml/raster_thread_merger.cc index 62b696db70aa4..206d2157e874b 100644 --- a/fml/raster_thread_merger.cc +++ b/fml/raster_thread_merger.cc @@ -16,17 +16,20 @@ 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) { + std::scoped_lock lock(lease_term_mutex_); if (TaskQueuesAreSame()) { return; } - + 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_); FML_CHECK(success) << "Unable to merge the raster and platform threads."; @@ -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_; } @@ -100,7 +124,9 @@ RasterThreadStatus RasterThreadMerger::DecrementLease() { if (!IsMergedUnSafe()) { return RasterThreadStatus::kRemainsUnmerged; } - + if (!IsEnabledUnSafe()) { + return RasterThreadStatus::kRemainsMerged; + } FML_DCHECK(lease_term_ > 0) << "lease_term should always be positive when merged."; lease_term_--; diff --git a/fml/raster_thread_merger.h b/fml/raster_thread_merger.h index b01cf76a11436..785dd000b1857 100644 --- a/fml/raster_thread_merger.h +++ b/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); diff --git a/fml/raster_thread_merger_unittests.cc b/fml/raster_thread_merger_unittests.cc index 15e95831a101e..67d1da4d20576 100644 --- a/fml/raster_thread_merger_unittests.cc +++ b/fml/raster_thread_merger_unittests.cc @@ -305,6 +305,167 @@ TEST(RasterThreadMerger, HandleTaskQueuesAreTheSame) { thread1.join(); } +TEST(RasterThreadMerger, Enable) { + 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(); + }); + + fml::MessageLoop* loop2 = nullptr; + fml::AutoResetWaitableEvent latch2; + fml::AutoResetWaitableEvent term2; + std::thread thread2([&loop2, &latch2, &term2]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + loop2 = &fml::MessageLoop::GetCurrent(); + latch2.Signal(); + term2.Wait(); + }); + + latch1.Wait(); + latch2.Wait(); + + fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId(); + fml::TaskQueueId qid2 = loop2->GetTaskRunner()->GetTaskQueueId(); + const auto raster_thread_merger_ = + fml::MakeRefCounted(qid1, qid2); + + raster_thread_merger_->Disable(); + raster_thread_merger_->MergeWithLease(1); + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->Enable(); + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->MergeWithLease(1); + ASSERT_TRUE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->DecrementLease(); + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + term1.Signal(); + term2.Signal(); + thread1.join(); + thread2.join(); +} + +TEST(RasterThreadMerger, Disable) { + 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(); + }); + + fml::MessageLoop* loop2 = nullptr; + fml::AutoResetWaitableEvent latch2; + fml::AutoResetWaitableEvent term2; + std::thread thread2([&loop2, &latch2, &term2]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + loop2 = &fml::MessageLoop::GetCurrent(); + latch2.Signal(); + term2.Wait(); + }); + + latch1.Wait(); + latch2.Wait(); + + fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId(); + fml::TaskQueueId qid2 = loop2->GetTaskRunner()->GetTaskQueueId(); + const auto raster_thread_merger_ = + fml::MakeRefCounted(qid1, qid2); + + raster_thread_merger_->Disable(); + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->MergeWithLease(1); + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->Enable(); + raster_thread_merger_->MergeWithLease(1); + ASSERT_TRUE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->Disable(); + raster_thread_merger_->UnMergeNow(); + ASSERT_TRUE(raster_thread_merger_->IsMerged()); + + { + auto decrement_result = raster_thread_merger_->DecrementLease(); + ASSERT_EQ(fml::RasterThreadStatus::kRemainsMerged, decrement_result); + } + + ASSERT_TRUE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->Enable(); + raster_thread_merger_->UnMergeNow(); + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + raster_thread_merger_->MergeWithLease(1); + + ASSERT_TRUE(raster_thread_merger_->IsMerged()); + + { + auto decrement_result = raster_thread_merger_->DecrementLease(); + ASSERT_EQ(fml::RasterThreadStatus::kUnmergedNow, decrement_result); + } + + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + term1.Signal(); + term2.Signal(); + thread1.join(); + thread2.join(); +} + +TEST(RasterThreadMerger, IsEnabled) { + 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(); + }); + + fml::MessageLoop* loop2 = nullptr; + fml::AutoResetWaitableEvent latch2; + fml::AutoResetWaitableEvent term2; + std::thread thread2([&loop2, &latch2, &term2]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + loop2 = &fml::MessageLoop::GetCurrent(); + latch2.Signal(); + term2.Wait(); + }); + + latch1.Wait(); + latch2.Wait(); + + fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId(); + fml::TaskQueueId qid2 = loop2->GetTaskRunner()->GetTaskQueueId(); + const auto raster_thread_merger_ = + fml::MakeRefCounted(qid1, qid2); + ASSERT_TRUE(raster_thread_merger_->IsEnabled()); + + raster_thread_merger_->Disable(); + ASSERT_FALSE(raster_thread_merger_->IsEnabled()); + + raster_thread_merger_->Enable(); + ASSERT_TRUE(raster_thread_merger_->IsEnabled()); + + term1.Signal(); + term2.Signal(); + thread1.join(); + thread2.join(); +} + TEST(RasterThreadMerger, RunExpiredTasksWhileFirstTaskMergesThreads) { fml::MessageLoop* loop_platform = nullptr; fml::AutoResetWaitableEvent latch1; diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 0cc894d7d4e76..5bae45f0078a8 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -81,7 +81,8 @@ void Rasterizer::Setup(std::unique_ptr surface) { // TODO(sanjayc77): https://github.com/flutter/flutter/issues/53179. Add // support for raster thread merger for Fuchsia. if (surface_->GetExternalViewEmbedder() && - surface_->GetExternalViewEmbedder()->SupportsDynamicThreadMerging()) { + surface_->GetExternalViewEmbedder()->SupportsDynamicThreadMerging() && + !raster_thread_merger_) { const auto platform_id = delegate_.GetTaskRunners().GetPlatformTaskRunner()->GetTaskQueueId(); const auto gpu_id = @@ -99,10 +100,23 @@ void Rasterizer::Teardown() { if (raster_thread_merger_.get() != nullptr && raster_thread_merger_.get()->IsMerged()) { + FML_DCHECK(raster_thread_merger_->IsEnabled()); 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(); + } +} + void Rasterizer::NotifyLowMemoryWarning() const { if (!surface_) { FML_DLOG(INFO) @@ -668,21 +682,6 @@ std::optional Rasterizer::GetResourceCacheMaxBytes() const { return std::nullopt; } -void Rasterizer::EnsureThreadsAreMerged() { - if (surface_ == nullptr || raster_thread_merger_.get() == nullptr) { - return; - } - const size_t ThreadMergeLeaseTermDefault = 10; - fml::TaskRunner::RunNowOrPostTask( - delegate_.GetTaskRunners().GetRasterTaskRunner(), - [weak_this = weak_factory_.GetWeakPtr(), - thread_merger = raster_thread_merger_]() { - thread_merger->MergeWithLease(ThreadMergeLeaseTermDefault); - }); - raster_thread_merger_->WaitUntilMerged(); - FML_DCHECK(raster_thread_merger_->IsMerged()); -} - Rasterizer::Screenshot::Screenshot() {} Rasterizer::Screenshot::Screenshot(sk_sp p_data, SkISize p_size) diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index e2b0caaa84780..723f0ee8cfd8a 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -392,15 +392,28 @@ class Rasterizer final : public SnapshotDelegate { std::optional 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. /// - void EnsureThreadsAreMerged(); + /// @see `ExternalViewEmbedder` + /// + 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_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 97c4cde047d27..4438d9bd5cde9 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -619,6 +619,32 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + // Prevent any request to change the thread configuration for raster and + // platform queues while the platform view is being created. + // + // This prevents false positives such as this method starts assuming that the + // raster and platform queues have a given thread configuration, but then the + // configuration is changed by a task, and the asumption is not longer true. + // + // This incorrect assumption can lead to dead lock. + // See `should_post_raster_task` for more. + rasterizer_->DisableThreadMergerIfNeeded(); + + // 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 which signals the latch. 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. + const bool should_post_raster_task = + !fml::TaskRunnerChecker::RunsOnTheSameThread( + task_runners_.GetRasterTaskRunner()->GetTaskQueueId(), + task_runners_.GetPlatformTaskRunner()->GetTaskQueueId()); + // Note: // This is a synchronous operation because certain platforms depend on // setup/suspension of all activities that may be interacting with the GPU in @@ -630,6 +656,9 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { surface = std::move(surface), // &latch]() mutable { if (rasterizer) { + // Enables the thread merger which may be used by the external view + // embedder. + rasterizer->EnableThreadMergerIfNeeded(); rasterizer->Setup(std::move(surface)); } @@ -640,19 +669,6 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { latch.Signal(); }); - // 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 which signals the latch. 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, @@ -708,6 +724,32 @@ void Shell::OnPlatformViewDestroyed() { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + // Prevent any request to change the thread configuration for raster and + // platform queues while the platform view is being destroyed. + // + // This prevents false positives such as this method starts assuming that the + // raster and platform queues have a given thread configuration, but then the + // configuration is changed by a task, and the asumption is not longer true. + // + // This incorrect assumption can lead to dead lock. + // See `should_post_raster_task` for more. + rasterizer_->DisableThreadMergerIfNeeded(); + + // 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. + const bool should_post_raster_task = + !fml::TaskRunnerChecker::RunsOnTheSameThread( + task_runners_.GetRasterTaskRunner()->GetTaskQueueId(), + task_runners_.GetPlatformTaskRunner()->GetTaskQueueId()); + // Note: // This is a synchronous operation because certain platforms depend on // setup/suspension of all activities that may be interacting with the GPU in @@ -730,27 +772,43 @@ void Shell::OnPlatformViewDestroyed() { io_task_runner = task_runners_.GetIOTaskRunner(), io_task]() { if (rasterizer) { + // Enables the thread merger which is required prior tearing down the + // rasterizer. If the raster and platform threads are merged, tearing down + // the rasterizer unmerges the threads. + rasterizer->EnableThreadMergerIfNeeded(); rasterizer->Teardown(); } // Step 2: Next, tell the IO thread to complete its remaining work. fml::TaskRunner::RunNowOrPostTask(io_task_runner, io_task); }; - auto ui_task = [engine = engine_->GetWeakPtr(), &latch]() { + auto ui_task = [engine = engine_->GetWeakPtr(), + raster_task_runner = task_runners_.GetRasterTaskRunner(), + raster_task, should_post_raster_task, &latch]() { if (engine) { engine->OnOutputSurfaceDestroyed(); } - latch.Signal(); + 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(); + } }; // 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(); - rasterizer_->EnsureThreadsAreMerged(); - fml::TaskRunner::RunNowOrPostTask(task_runners_.GetRasterTaskRunner(), - raster_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(); + } } // |PlatformView::Delegate| diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index f368475d7a1d3..050fae899320d 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -597,6 +597,46 @@ TEST_F(ShellTest, DestroyShell(std::move(shell)); } +TEST_F(ShellTest, OnPlatformViewDestroyDisablesThreadMerger) { + auto settings = CreateSettingsForFixture(); + fml::AutoResetWaitableEvent end_frame_latch; + fml::RefPtr raster_thread_merger; + auto end_frame_callback = + [&](bool should_resubmit_frame, + fml::RefPtr thread_merger) { + raster_thread_merger = thread_merger; + end_frame_latch.Signal(); + }; + auto external_view_embedder = std::make_shared( + 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)); + + PumpOneFrame(shell.get()); + + end_frame_latch.Wait(); + ASSERT_TRUE(raster_thread_merger->IsEnabled()); + + ValidateDestroyPlatformView(shell.get()); + ASSERT_TRUE(raster_thread_merger->IsEnabled()); + + // Validate the platform view can be recreated and destroyed again + ValidateShell(shell.get()); + ASSERT_TRUE(raster_thread_merger->IsEnabled()); + + DestroyShell(std::move(shell)); +} + TEST_F(ShellTest, OnPlatformViewDestroyAfterMergingThreads) { const size_t ThreadMergingLease = 10; auto settings = CreateSettingsForFixture(); @@ -664,14 +704,14 @@ TEST_F(ShellTest, OnPlatformViewDestroyAfterMergingThreads) { } TEST_F(ShellTest, OnPlatformViewDestroyWhenThreadsAreMerging) { - const size_t ThreadMergingLease = 10; + const size_t kThreadMergingLease = 10; auto settings = CreateSettingsForFixture(); fml::AutoResetWaitableEvent end_frame_latch; auto end_frame_callback = [&](bool should_resubmit_frame, fml::RefPtr raster_thread_merger) { if (should_resubmit_frame && !raster_thread_merger->IsMerged()) { - raster_thread_merger->MergeWithLease(ThreadMergingLease); + raster_thread_merger->MergeWithLease(kThreadMergingLease); } end_frame_latch.Signal(); }; diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.cc b/shell/platform/android/external_view_embedder/external_view_embedder.cc index b5479284aa0ef..a78ad935044f1 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder.cc @@ -81,6 +81,10 @@ void AndroidExternalViewEmbedder::SubmitFrame( // Don't submit the current frame if the frame will be resubmitted. return; } + if (!FrameHasPlatformLayers()) { + frame->Submit(); + return; + } std::unordered_map> overlay_layers; std::unordered_map> pictures; @@ -148,8 +152,7 @@ void AndroidExternalViewEmbedder::SubmitFrame( // // Skip a frame if the embedding is switching surfaces, and indicate in // `PostPrerollAction` that this frame must be resubmitted. - auto should_submit_current_frame = - previous_frame_view_count_ > 0 || current_frame_view_count == 0; + auto should_submit_current_frame = previous_frame_view_count_ > 0; if (should_submit_current_frame) { frame->Submit(); } @@ -223,24 +226,28 @@ PostPrerollResult AndroidExternalViewEmbedder::PostPrerollAction( // // To keep the rasterizer running on the platform thread one more frame, // `kDefaultMergedLeaseDuration` must be at least `1`. - bool has_platform_views = composition_order_.size() > 0; - if (has_platform_views) { - if (raster_thread_merger->IsMerged()) { - raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); - } else { - // Merge the raster and platform threads in `EndFrame`. - should_run_rasterizer_on_platform_thread_ = true; - CancelFrame(); - return PostPrerollResult::kResubmitFrame; - } - // Surface switch requires to resubmit the frame. - if (previous_frame_view_count_ == 0) { - return PostPrerollResult::kResubmitFrame; - } + if (!FrameHasPlatformLayers()) { + return PostPrerollResult::kSuccess; + } + if (raster_thread_merger->IsMerged()) { + raster_thread_merger->ExtendLeaseTo(kDefaultMergedLeaseDuration); + } else { + // Merge the raster and platform threads in `EndFrame`. + should_run_rasterizer_on_platform_thread_ = true; + CancelFrame(); + return PostPrerollResult::kResubmitFrame; + } + // Surface switch requires to resubmit the frame. + if (previous_frame_view_count_ == 0) { + return PostPrerollResult::kResubmitFrame; } return PostPrerollResult::kSuccess; } +bool AndroidExternalViewEmbedder::FrameHasPlatformLayers() { + return composition_order_.size() > 0; +} + // |ExternalViewEmbedder| SkCanvas* AndroidExternalViewEmbedder::GetRootCanvas() { // On Android, the root surface is created from the on-screen render target. @@ -286,7 +293,14 @@ void AndroidExternalViewEmbedder::EndFrame( fml::RefPtr raster_thread_merger) { if (should_resubmit_frame && should_run_rasterizer_on_platform_thread_) { raster_thread_merger->MergeWithLease(kDefaultMergedLeaseDuration); - should_run_rasterizer_on_platform_thread_ = false; + if (raster_thread_merger->IsMerged()) { + // The raster thread merger may be disabled if the rasterizer is being + // teared down. + // + // Therefore, set `should_run_rasterizer_on_platform_thread_` to `false` + // only if the thread merger was able to set the lease. + should_run_rasterizer_on_platform_thread_ = false; + } } surface_pool_->RecycleLayers(); // JNI method must be called on the platform thread. diff --git a/shell/platform/android/external_view_embedder/external_view_embedder.h b/shell/platform/android/external_view_embedder/external_view_embedder.h index ce755f252a8ac..349142d097bc9 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder.h +++ b/shell/platform/android/external_view_embedder/external_view_embedder.h @@ -132,6 +132,9 @@ class AndroidExternalViewEmbedder final : public ExternalViewEmbedder { // Resets the state. void Reset(); + // Whether the layer tree in the current frame has platform layers. + bool FrameHasPlatformLayers(); + // Creates a Surface when needed or recycles an existing one. // Finally, draws the picture on the frame's canvas. std::unique_ptr CreateSurfaceIfNeeded(GrDirectContext* context, diff --git a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc index bdb8ca1c6c286..e84801dca0a5a 100644 --- a/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc +++ b/shell/platform/android/external_view_embedder/external_view_embedder_unittests.cc @@ -564,5 +564,33 @@ TEST(AndroidExternalViewEmbedder, SupportsDynamicThreadMerging) { ASSERT_TRUE(embedder->SupportsDynamicThreadMerging()); } +TEST(AndroidExternalViewEmbedder, DisableThreadMerger) { + auto jni_mock = std::make_shared(); + auto embedder = + std::make_unique(nullptr, jni_mock, nullptr); + + auto raster_thread_merger = GetThreadMergerFromRasterThread(); + ASSERT_FALSE(raster_thread_merger->IsMerged()); + + // The shell may disable the thread merger during `OnPlatformViewDestroyed`. + raster_thread_merger->Disable(); + + EXPECT_CALL(*jni_mock, FlutterViewBeginFrame()).Times(0); + + embedder->BeginFrame(SkISize::Make(10, 20), nullptr, 1.0, + raster_thread_merger); + // Push a platform view. + embedder->PrerollCompositeEmbeddedView( + 0, std::make_unique()); + + auto postpreroll_result = embedder->PostPrerollAction(raster_thread_merger); + ASSERT_EQ(PostPrerollResult::kResubmitFrame, postpreroll_result); + + EXPECT_CALL(*jni_mock, FlutterViewEndFrame()).Times(0); + embedder->EndFrame(/*should_resubmit_frame=*/true, raster_thread_merger); + + ASSERT_FALSE(raster_thread_merger->IsMerged()); +} + } // namespace testing } // namespace flutter