diff --git a/fml/message_loop_task_queues.cc b/fml/message_loop_task_queues.cc index 8df08fa1b244d..1d5a9091083f3 100644 --- a/fml/message_loop_task_queues.cc +++ b/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. diff --git a/fml/message_loop_task_queues_unittests.cc b/fml/message_loop_task_queues_unittests.cc index a1c2df0a47f6c..1086bb28b8013 100644 --- a/fml/message_loop_task_queues_unittests.cc +++ b/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 diff --git a/fml/raster_thread_merger.cc b/fml/raster_thread_merger.cc index b29c303371b8a..62b696db70aa4 100644 --- a/fml/raster_thread_merger.cc +++ b/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(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 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 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; } diff --git a/fml/raster_thread_merger.h b/fml/raster_thread_merger.h index 7c0318ff77d26..b01cf76a11436 100644 --- a/fml/raster_thread_merger.h +++ b/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 +#include #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 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); diff --git a/fml/raster_thread_merger_unittests.cc b/fml/raster_thread_merger_unittests.cc index a182723a79191..f3df1c1bb2259 100644 --- a/fml/raster_thread_merger_unittests.cc +++ b/fml/raster_thread_merger_unittests.cc @@ -208,3 +208,96 @@ TEST(RasterThreadMerger, LeaseExtension) { thread1.join(); thread2.join(); } + +TEST(RasterThreadMerger, WaitUntilMerged) { + fml::RefPtr 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(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(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(); +} diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index ca5a099253b6a..292de2e234f18 100644 --- a/shell/common/rasterizer.cc +++ b/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 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 p_data, SkISize p_size) diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 2a45e765a3f7a..8fbfd973150ea 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -389,6 +389,22 @@ class Rasterizer final : public SnapshotDelegate { /// std::optional 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_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 13bd40638991a..97c4cde047d27 100644 --- a/shell/common/shell.cc +++ b/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| diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index b3a64f3fd6993..8144d3accaacd 100644 --- a/shell/common/shell_test.cc +++ b/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( diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index e534fcec3cef0..c61aba37c7647 100644 --- a/shell/common/shell_test.h +++ b/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); diff --git a/shell/common/shell_test_external_view_embedder.cc b/shell/common/shell_test_external_view_embedder.cc index 105f109dbab31..07557768a46b7 100644 --- a/shell/common/shell_test_external_view_embedder.cc +++ b/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 raster_thread_merger) { FML_DCHECK(raster_thread_merger); + if (resubmit_once_) { + resubmit_once_ = false; + return PostPrerollResult::kResubmitFrame; + } return post_preroll_result_; } diff --git a/shell/common/shell_test_external_view_embedder.h b/shell/common/shell_test_external_view_embedder.h index 8b855a0a7f4f6..6d90879c74e17 100644 --- a/shell/common/shell_test_external_view_embedder.h +++ b/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_; diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 8917f093df5f7..f368475d7a1d3 100644 --- a/shell/common/shell_unittests.cc +++ b/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( 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( 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 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( + 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 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 queue = fml::MakeRefCounted( + this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); + auto picture_layer = std::make_shared( + SkPoint::Make(10, 10), + flutter::SkiaGPUObject({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 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( + 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 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 queue = fml::MakeRefCounted( + this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); + auto picture_layer = std::make_shared( + SkPoint::Make(10, 10), + flutter::SkiaGPUObject({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 raster_thread_merger) { + end_frame_latch.Signal(); + }; + auto external_view_embedder = std::make_shared( + 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 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 queue = fml::MakeRefCounted( + this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); + auto picture_layer = std::make_shared( + SkPoint::Make(10, 10), + flutter::SkiaGPUObject({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 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 queue = fml::MakeRefCounted( + this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); + auto picture_layer = std::make_shared( + SkPoint::Make(10, 10), + flutter::SkiaGPUObject({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 raster_thread_merger) { + end_frame_latch.Signal(); + }; + auto external_view_embedder = std::make_shared( + 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 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 queue = fml::MakeRefCounted( + this->GetCurrentTaskRunner(), fml::TimeDelta::FromSeconds(0)); + auto picture_layer = std::make_shared( + SkPoint::Make(10, 10), + flutter::SkiaGPUObject({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), diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index bdd995b6b2c1e..13394a976c92c 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -508,43 +508,19 @@ - (BOOL)createShell:(NSString*)entrypoint flutter::Shell::CreateCallback on_create_rasterizer = [](flutter::Shell& shell) { return std::make_unique(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: "