From a4c018a2742e9ed76b8300a680c187c0d5e84304 Mon Sep 17 00:00:00 2001 From: zsunkun Date: Fri, 20 Aug 2021 20:48:04 +0800 Subject: [PATCH 1/2] Share the io_manager between parent and spawn engine --- runtime/runtime_controller.cc | 12 +++- runtime/runtime_controller.h | 4 +- shell/common/engine.cc | 13 +++- shell/common/engine.h | 6 +- shell/common/engine_unittests.cc | 6 +- shell/common/shell.cc | 34 +++++++---- shell/common/shell.h | 6 +- shell/common/shell_unittests.cc | 101 +++++++++++++++++++++++++++++++ 8 files changed, 159 insertions(+), 23 deletions(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index f7dfa4fbf8a94..f27b832965a73 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -48,7 +48,15 @@ std::unique_ptr RuntimeController::Spawn( const std::function& p_idle_notification_callback, const fml::closure& p_isolate_create_callback, const fml::closure& p_isolate_shutdown_callback, - std::shared_ptr p_persistent_isolate_data) const { + std::shared_ptr p_persistent_isolate_data, + fml::WeakPtr io_manager, + fml::WeakPtr image_decoder) const { + UIDartState::Context spawned_context{ + context_.task_runners, context_.snapshot_delegate, + std::move(io_manager), context_.unref_queue, + std::move(image_decoder), context_.image_generator_registry, + advisory_script_uri, advisory_script_entrypoint, + context_.volatile_path_tracker}; auto result = std::make_unique(p_client, // vm_, // @@ -58,7 +66,7 @@ std::unique_ptr RuntimeController::Spawn( p_isolate_create_callback, // p_isolate_shutdown_callback, // p_persistent_isolate_data, // - context_); // + spawned_context); // result->spawning_isolate_ = root_isolate_; return result; } diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 7b46139a52815..9fd346a3d1350 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -102,7 +102,9 @@ class RuntimeController : public PlatformConfigurationClient { const std::function& idle_notification_callback, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, - std::shared_ptr persistent_isolate_data) const; + std::shared_ptr persistent_isolate_data, + fml::WeakPtr io_manager, + fml::WeakPtr image_decoder) const; // |PlatformConfigurationClient| ~RuntimeController() override; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 13056e7755c4c..e0b39a6c69c28 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -113,7 +113,8 @@ std::unique_ptr Engine::Spawn( const PointerDataDispatcherMaker& dispatcher_maker, Settings settings, std::unique_ptr animator, - const std::string& initial_route) const { + const std::string& initial_route, + fml::WeakPtr io_manager) const { auto result = std::make_unique( /*delegate=*/delegate, /*dispatcher_maker=*/dispatcher_maker, @@ -122,7 +123,7 @@ std::unique_ptr Engine::Spawn( /*task_runners=*/task_runners_, /*settings=*/settings, /*animator=*/std::move(animator), - /*io_manager=*/runtime_controller_->GetIOManager(), + /*io_manager=*/io_manager, /*font_collection=*/font_collection_, /*runtime_controller=*/nullptr); result->runtime_controller_ = runtime_controller_->Spawn( @@ -132,7 +133,9 @@ std::unique_ptr Engine::Spawn( settings_.idle_notification_callback, // idle notification callback settings_.isolate_create_callback, // isolate create callback settings_.isolate_shutdown_callback, // isolate shutdown callback - settings_.persistent_isolate_data // persistent isolate data + settings_.persistent_isolate_data, // persistent isolate data + io_manager, // io_manager + result->GetImageDecoderWeakPtr() // imageDecoder ); result->initial_route_ = initial_route; return result; @@ -153,6 +156,10 @@ std::shared_ptr Engine::GetAssetManager() { return asset_manager_; } +fml::WeakPtr Engine::GetImageDecoderWeakPtr() { + return image_decoder_.GetWeakPtr(); +} + fml::WeakPtr Engine::GetImageGeneratorRegistry() { return image_generator_registry_.GetWeakPtr(); } diff --git a/shell/common/engine.h b/shell/common/engine.h index b578bf07585c1..af4b8d37d7631 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -376,7 +376,8 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { const PointerDataDispatcherMaker& dispatcher_maker, Settings settings, std::unique_ptr animator, - const std::string& initial_route) const; + const std::string& initial_route, + fml::WeakPtr io_manager) const; //---------------------------------------------------------------------------- /// @brief Destroys the engine engine. Called by the shell on the UI task @@ -791,6 +792,9 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { // Return the asset manager associated with the current engine, or nullptr. std::shared_ptr GetAssetManager(); + // Return the weak_ptr of ImageDecoder. + fml::WeakPtr GetImageDecoderWeakPtr(); + //---------------------------------------------------------------------------- /// @brief Get the `ImageGeneratorRegistry` associated with the current /// engine. diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index e841060c636ac..3fcd122be2bdd 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -269,7 +269,7 @@ TEST_F(EngineTest, SpawnSharesFontLibrary) { /*runtime_controller=*/std::move(mock_runtime_controller)); auto spawn = engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, - std::string()); + std::string(), io_manager_); EXPECT_TRUE(spawn != nullptr); EXPECT_EQ(&engine->GetFontCollection(), &spawn->GetFontCollection()); }); @@ -294,8 +294,8 @@ TEST_F(EngineTest, SpawnWithCustomInitialRoute) { /*font_collection=*/std::make_shared(), /*runtime_controller=*/std::move(mock_runtime_controller)); - auto spawn = - engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, "/foo"); + auto spawn = engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, + "/foo", io_manager_); EXPECT_TRUE(spawn != nullptr); ASSERT_EQ("/foo", spawn->InitialRoute()); }); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index f7c91472facab..85119cbc0c49a 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -151,6 +151,7 @@ std::unique_ptr Shell::Create( return CreateWithSnapshot(std::move(platform_data), // std::move(task_runners), // /*parent_merger=*/nullptr, // + /*parent_io_manager=*/nullptr, // std::move(settings), // std::move(vm), // std::move(isolate_snapshot), // @@ -162,6 +163,7 @@ std::unique_ptr Shell::Create( std::unique_ptr Shell::CreateShellOnPlatformThread( DartVMRef vm, fml::RefPtr parent_merger, + std::shared_ptr parent_io_manager, TaskRunners task_runners, const PlatformData& platform_data, Settings settings, @@ -216,7 +218,7 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( // first because it has state that the other subsystems depend on. It must // first be booted and the necessary references obtained to initialize the // other subsystems. - std::promise> io_manager_promise; + std::promise> io_manager_promise; auto io_manager_future = io_manager_promise.get_future(); std::promise> weak_io_manager_promise; auto weak_io_manager_future = weak_io_manager_promise.get_future(); @@ -234,18 +236,24 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( io_task_runner, [&io_manager_promise, // &weak_io_manager_promise, // + &parent_io_manager, // &unref_queue_promise, // platform_view = platform_view->GetWeakPtr(), // io_task_runner, // is_backgrounded_sync_switch = shell->GetIsGpuDisabledSyncSwitch() // ]() { TRACE_EVENT0("flutter", "ShellSetupIOSubsystem"); - auto io_manager = std::make_unique( - platform_view.getUnsafe()->CreateResourceContext(), - is_backgrounded_sync_switch, io_task_runner); + std::shared_ptr io_manager; + if (parent_io_manager) { + io_manager = parent_io_manager; + } else { + io_manager = std::make_shared( + platform_view.getUnsafe()->CreateResourceContext(), + is_backgrounded_sync_switch, io_task_runner); + } weak_io_manager_promise.set_value(io_manager->GetWeakPtr()); unref_queue_promise.set_value(io_manager->GetSkiaUnrefQueue()); - io_manager_promise.set_value(std::move(io_manager)); + io_manager_promise.set_value(io_manager); }); // Send dispatcher_maker to the engine constructor because shell won't have @@ -305,6 +313,7 @@ std::unique_ptr Shell::CreateWithSnapshot( const PlatformData& platform_data, TaskRunners task_runners, fml::RefPtr parent_thread_merger, + std::shared_ptr parent_io_manager, Settings settings, DartVMRef vm, fml::RefPtr isolate_snapshot, @@ -331,6 +340,7 @@ std::unique_ptr Shell::CreateWithSnapshot( [&latch, // &shell, // parent_thread_merger, // + parent_io_manager, // task_runners = std::move(task_runners), // platform_data = std::move(platform_data), // settings = std::move(settings), // @@ -343,6 +353,7 @@ std::unique_ptr Shell::CreateWithSnapshot( shell = CreateShellOnPlatformThread( std::move(vm), // parent_thread_merger, // + parent_io_manager, // std::move(task_runners), // std::move(platform_data), // std::move(settings), // @@ -490,9 +501,9 @@ std::unique_ptr Shell::Spawn( fml::SyncSwitch::Handlers() .SetIfFalse([&is_gpu_disabled] { is_gpu_disabled = false; }) .SetIfTrue([&is_gpu_disabled] { is_gpu_disabled = true; })); - std::unique_ptr result = (CreateWithSnapshot( + std::unique_ptr result = CreateWithSnapshot( PlatformData{}, task_runners_, rasterizer_->GetRasterThreadMerger(), - GetSettings(), vm_, vm_->GetVMData()->GetIsolateSnapshot(), + io_manager_, GetSettings(), vm_, vm_->GetVMData()->GetIsolateSnapshot(), on_create_platform_view, on_create_rasterizer, [engine = this->engine_.get(), initial_route]( Engine::Delegate& delegate, @@ -508,9 +519,10 @@ std::unique_ptr Shell::Spawn( /*dispatcher_maker=*/dispatcher_maker, /*settings=*/settings, /*animator=*/std::move(animator), - /*initial_route=*/initial_route); + /*initial_route=*/initial_route, + /*io_manager=*/std::move(io_manager)); }, - is_gpu_disabled)); + is_gpu_disabled); result->shared_resource_context_ = io_manager_->GetSharedResourceContext(); result->RunEngine(std::move(run_configuration)); return result; @@ -613,7 +625,7 @@ bool Shell::IsSetup() const { bool Shell::Setup(std::unique_ptr platform_view, std::unique_ptr engine, std::unique_ptr rasterizer, - std::unique_ptr io_manager) { + std::shared_ptr io_manager) { if (is_setup_) { return false; } @@ -626,7 +638,7 @@ bool Shell::Setup(std::unique_ptr platform_view, platform_message_handler_ = platform_view_->GetPlatformMessageHandler(); engine_ = std::move(engine); rasterizer_ = std::move(rasterizer); - io_manager_ = std::move(io_manager); + io_manager_ = io_manager; // Set the external view embedder for the rasterizer. auto view_embedder = platform_view_->CreateExternalViewEmbedder(); diff --git a/shell/common/shell.h b/shell/common/shell.h index 87ba86cb83ac5..0448da1c424d7 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -415,7 +415,7 @@ class Shell final : public PlatformView::Delegate, std::unique_ptr platform_view_; // on platform task runner std::unique_ptr engine_; // on UI task runner std::unique_ptr rasterizer_; // on raster task runner - std::unique_ptr io_manager_; // on IO task runner + std::shared_ptr io_manager_; // on IO task runner std::shared_ptr is_gpu_disabled_sync_switch_; std::shared_ptr volatile_path_tracker_; std::shared_ptr platform_message_handler_; @@ -480,6 +480,7 @@ class Shell final : public PlatformView::Delegate, static std::unique_ptr CreateShellOnPlatformThread( DartVMRef vm, fml::RefPtr parent_merger, + std::shared_ptr parent_io_manager, TaskRunners task_runners, const PlatformData& platform_data, Settings settings, @@ -492,6 +493,7 @@ class Shell final : public PlatformView::Delegate, const PlatformData& platform_data, TaskRunners task_runners, fml::RefPtr parent_thread_merger, + std::shared_ptr parent_io_manager, Settings settings, DartVMRef vm, fml::RefPtr isolate_snapshot, @@ -503,7 +505,7 @@ class Shell final : public PlatformView::Delegate, bool Setup(std::unique_ptr platform_view, std::unique_ptr engine, std::unique_ptr rasterizer, - std::unique_ptr io_manager); + std::shared_ptr io_manager); void ReportTimings(); diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 2aacb4b2f815e..c6f3244e74483 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -2963,6 +2963,107 @@ TEST_F(ShellTest, SpawnWithDartEntrypointArgs) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); } +TEST_F(ShellTest, IOManagerIsSharedBetweenParentAndSpawnedShell) { + auto settings = CreateSettingsForFixture(); + auto shell = CreateShell(settings); + ASSERT_TRUE(ValidateShell(shell.get())); + + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [this, + &spawner = shell, + &settings] { + auto second_configuration = RunConfiguration::InferFromSettings(settings); + ASSERT_TRUE(second_configuration.IsValid()); + second_configuration.SetEntrypoint("emptyMain"); + const std::string initial_route("/foo"); + MockPlatformViewDelegate platform_view_delegate; + auto spawn = spawner->Spawn( + std::move(second_configuration), initial_route, + [&platform_view_delegate](Shell& shell) { + auto result = std::make_unique( + platform_view_delegate, shell.GetTaskRunners()); + ON_CALL(*result, CreateRenderingSurface()) + .WillByDefault(::testing::Invoke( + [] { return std::make_unique(); })); + return result; + }, + [](Shell& shell) { return std::make_unique(shell); }); + ASSERT_TRUE(ValidateShell(spawn.get())); + + PostSync(spawner->GetTaskRunners().GetIOTaskRunner(), [&spawner, &spawn] { + ASSERT_NE(spawner->GetIOManager().get(), nullptr); + ASSERT_EQ(spawner->GetIOManager().get(), spawn->GetIOManager().get()); + }); + + // Destroy the child shell. + DestroyShell(std::move(spawn)); + }); + // Destroy the parent shell. + DestroyShell(std::move(shell)); +} + +TEST_F(ShellTest, IOManagerInSpawnedShellIsNotNullAfterParentShellDestroyed) { + auto settings = CreateSettingsForFixture(); + auto shell = CreateShell(settings); + ASSERT_TRUE(ValidateShell(shell.get())); + + PostSync(shell->GetTaskRunners().GetUITaskRunner(), [&shell] { + // We must get engine on UI thread. + auto runtime_controller = shell->GetEngine()->GetRuntimeController(); + PostSync(shell->GetTaskRunners().GetIOTaskRunner(), + [&shell, &runtime_controller] { + // We must get io_manager on IO thread. + auto io_manager = runtime_controller->GetIOManager(); + // Check io_manager existence. + ASSERT_NE(io_manager.get(), nullptr); + ASSERT_NE(io_manager->GetSkiaUnrefQueue().get(), nullptr); + // Get io_manager directly from shell and check its existence. + ASSERT_NE(shell->GetIOManager().get(), nullptr); + }); + }); + + std::unique_ptr spawn; + + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell, &settings, + &spawn] { + auto second_configuration = RunConfiguration::InferFromSettings(settings); + ASSERT_TRUE(second_configuration.IsValid()); + second_configuration.SetEntrypoint("emptyMain"); + const std::string initial_route("/foo"); + MockPlatformViewDelegate platform_view_delegate; + auto child = shell->Spawn( + std::move(second_configuration), initial_route, + [&platform_view_delegate](Shell& shell) { + auto result = std::make_unique( + platform_view_delegate, shell.GetTaskRunners()); + ON_CALL(*result, CreateRenderingSurface()) + .WillByDefault(::testing::Invoke( + [] { return std::make_unique(); })); + return result; + }, + [](Shell& shell) { return std::make_unique(shell); }); + spawn = std::move(child); + }); + // Destroy the parent shell. + DestroyShell(std::move(shell)); + + PostSync(spawn->GetTaskRunners().GetUITaskRunner(), [&spawn] { + // We must get engine on UI thread. + auto runtime_controller = spawn->GetEngine()->GetRuntimeController(); + PostSync(spawn->GetTaskRunners().GetIOTaskRunner(), + [&spawn, &runtime_controller] { + // We must get io_manager on IO thread. + auto io_manager = runtime_controller->GetIOManager(); + // Check io_manager existence here. + ASSERT_NE(io_manager.get(), nullptr); + ASSERT_NE(io_manager->GetSkiaUnrefQueue().get(), nullptr); + // Get io_manager directly from shell and check its existence. + ASSERT_NE(spawn->GetIOManager().get(), nullptr); + }); + }); + // Destroy the child shell. + DestroyShell(std::move(spawn)); +} + TEST_F(ShellTest, UpdateAssetResolverByTypeReplaces) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); Settings settings = CreateSettingsForFixture(); From d9f404fa85aae65db8d30c2cbb2c65c6a7262ddf Mon Sep 17 00:00:00 2001 From: eggfly Date: Wed, 1 Dec 2021 19:00:05 +0800 Subject: [PATCH 2/2] Fix unittest missing io_manager parameter after merge --- shell/common/engine_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index dcf3d7a74622d..53df307aeec10 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -331,7 +331,7 @@ TEST_F(EngineTest, SpawnResetsViewportMetrics) { EXPECT_EQ(old_platform_data.viewport_metrics.physical_height, kViewHeight); auto spawn = engine->Spawn(delegate_, dispatcher_maker_, settings_, nullptr, - std::string()); + std::string(), io_manager_); EXPECT_TRUE(spawn != nullptr); auto& new_viewport_metrics = spawn->GetRuntimeController()->GetPlatformData().viewport_metrics;