diff --git a/shell/common/rasterizer_unittests.cc b/shell/common/rasterizer_unittests.cc index 96321df4eb1a3..47433ff6edc6b 100644 --- a/shell/common/rasterizer_unittests.cc +++ b/shell/common/rasterizer_unittests.cc @@ -219,7 +219,7 @@ TEST( fml::MessageLoop::EnsureInitializedForCurrentThread(); TaskRunners task_runners("test", fml::MessageLoop::GetCurrent().GetTaskRunner(), - fml::MessageLoop::GetCurrent().GetTaskRunner(), + thread_host.raster_thread->GetTaskRunner(), thread_host.ui_thread->GetTaskRunner(), thread_host.io_thread->GetTaskRunner()); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index e919c1b86a2f2..8f5c568616dc1 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -716,28 +716,8 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { // 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 = - !task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread(); - // Note: // This is a synchronous operation because certain platforms depend on // setup/suspension of all activities that may be interacting with the GPU in @@ -764,21 +744,13 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { auto ui_task = [engine = engine_->GetWeakPtr(), // raster_task_runner = task_runners_.GetRasterTaskRunner(), // - raster_task, should_post_raster_task, - &latch // - ] { + raster_task]() { if (engine) { engine->OnOutputSurfaceCreated(); } // Step 2: Next, tell the raster thread that it should create a surface for // its rasterizer. - 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(); - } + fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task); }; // Threading: Capture platform view by raw pointer and not the weak pointer. @@ -809,12 +781,6 @@ void Shell::OnPlatformViewCreated(std::unique_ptr surface) { fml::TaskRunner::RunNowOrPostTask(task_runners_.GetIOTaskRunner(), io_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(); - } } // |PlatformView::Delegate| @@ -825,28 +791,8 @@ void Shell::OnPlatformViewDestroyed() { // 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 = - !task_runners_.GetRasterTaskRunner()->RunsTasksOnCurrentThread(); - // Note: // This is a synchronous operation because certain platforms depend on // setup/suspension of all activities that may be interacting with the GPU in @@ -881,17 +827,11 @@ void Shell::OnPlatformViewDestroyed() { auto ui_task = [engine = engine_->GetWeakPtr(), raster_task_runner = task_runners_.GetRasterTaskRunner(), - raster_task, should_post_raster_task, &latch]() { + raster_task]() { if (engine) { engine->OnOutputSurfaceDestroyed(); } - 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(); - } + fml::TaskRunner::RunNowOrPostTask(raster_task_runner, raster_task); }; // Step 0: Post a task onto the UI thread to tell the engine that its output @@ -899,13 +839,6 @@ void Shell::OnPlatformViewDestroyed() { 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(); - } } // |PlatformView::Delegate| diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 0daddce767641..6bb221e779c0b 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -345,26 +345,6 @@ TEST_F(ShellTest, InitializeWithDisabledGpu) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); } -TEST_F(ShellTest, InitializeWithGPUAndPlatformThreadsTheSame) { - ASSERT_FALSE(DartVMRef::IsInstanceRunning()); - Settings settings = CreateSettingsForFixture(); - 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)); - ASSERT_TRUE(DartVMRef::IsInstanceRunning()); - ASSERT_TRUE(ValidateShell(shell.get())); - DestroyShell(std::move(shell), std::move(task_runners)); - ASSERT_FALSE(DartVMRef::IsInstanceRunning()); -} - TEST_F(ShellTest, FixturesAreFunctional) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); auto settings = CreateSettingsForFixture(); @@ -1097,7 +1077,7 @@ TEST_F(ShellTest, TaskRunners task_runners( "test", thread_host.platform_thread->GetTaskRunner(), // platform - thread_host.platform_thread->GetTaskRunner(), // raster + thread_host.raster_thread->GetTaskRunner(), // raster thread_host.ui_thread->GetTaskRunner(), // ui thread_host.io_thread->GetTaskRunner() // io ); diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index b6744999753eb..edd93c9046996 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -1719,85 +1719,6 @@ TEST_F(EmbedderTest, CanCreateEmbedderWithCustomRenderTaskRunner) { } } -//------------------------------------------------------------------------------ -/// Asserts that the render task runner can be the same as the platform task -/// runner. -/// -TEST_F(EmbedderTest, - CanCreateEmbedderWithCustomRenderTaskRunnerTheSameAsPlatformTaskRunner) { - // A new thread needs to be created for the platform thread because the test - // can't wait for assertions to be completed on the same thread that services - // platform task runner tasks. - auto platform_task_runner = CreateNewThread("platform_thread"); - - static std::mutex engine_mutex; - static UniqueEngine engine; - fml::AutoResetWaitableEvent task_latch; - bool task_executed = false; - EmbedderTestTaskRunner common_task_runner( - platform_task_runner, [&](FlutterTask task) { - std::scoped_lock engine_lock(engine_mutex); - if (engine.is_valid()) { - ASSERT_EQ(FlutterEngineRunTask(engine.get(), &task), kSuccess); - task_executed = true; - task_latch.Signal(); - } - }); - - platform_task_runner->PostTask([&]() { - EmbedderConfigBuilder builder( - GetEmbedderContext(EmbedderTestContextType::kOpenGLContext)); - builder.SetDartEntrypoint("can_render_scene_without_custom_compositor"); - builder.SetOpenGLRendererConfig(SkISize::Make(800, 600)); - builder.SetRenderTaskRunner( - &common_task_runner.GetFlutterTaskRunnerDescription()); - builder.SetPlatformTaskRunner( - &common_task_runner.GetFlutterTaskRunnerDescription()); - - { - std::scoped_lock lock(engine_mutex); - engine = builder.InitializeEngine(); - } - - ASSERT_EQ(FlutterEngineRunInitialized(engine.get()), kSuccess); - - ASSERT_TRUE(engine.is_valid()); - - FlutterWindowMetricsEvent event = {}; - event.struct_size = sizeof(event); - event.width = 800; - event.height = 600; - event.pixel_ratio = 1.0; - ASSERT_EQ(FlutterEngineSendWindowMetricsEvent(engine.get(), &event), - kSuccess); - }); - - task_latch.Wait(); - - // Don't use the task latch because that may be called multiple time - // (including during the shutdown process). - fml::AutoResetWaitableEvent shutdown_latch; - - platform_task_runner->PostTask([&]() { - ASSERT_TRUE(task_executed); - ASSERT_EQ(FlutterEngineDeinitialize(engine.get()), kSuccess); - - { - std::scoped_lock engine_lock(engine_mutex); - engine.reset(); - } - shutdown_latch.Signal(); - }); - - shutdown_latch.Wait(); - - { - std::scoped_lock engine_lock(engine_mutex); - // Engine should have been killed by this point. - ASSERT_FALSE(engine.is_valid()); - } -} - TEST_F(EmbedderTest, CompositorMustBeAbleToRenderKnownScenePixelRatioOnSurface) { auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext);