diff --git a/fml/concurrent_message_loop.cc b/fml/concurrent_message_loop.cc index b58431b7df3d2..2b6e6e6f760f5 100644 --- a/fml/concurrent_message_loop.cc +++ b/fml/concurrent_message_loop.cc @@ -6,6 +6,7 @@ #include +#include "concurrent_message_loop.h" #include "flutter/fml/thread.h" #include "flutter/fml/trace_event.h" @@ -148,6 +149,16 @@ std::vector ConcurrentMessageLoop::GetThreadTasksLocked() { return pending_tasks; } +bool ConcurrentMessageLoop::RunsTasksOnCurrentThread() { + std::scoped_lock lock(tasks_mutex_); + for (const auto& worker_thread_id : worker_thread_ids_) { + if (worker_thread_id == std::this_thread::get_id()) { + return true; + } + } + return false; +} + ConcurrentTaskRunner::ConcurrentTaskRunner( std::weak_ptr weak_loop) : weak_loop_(std::move(weak_loop)) {} diff --git a/fml/concurrent_message_loop.h b/fml/concurrent_message_loop.h index ebd8de983508d..457365e754b1a 100644 --- a/fml/concurrent_message_loop.h +++ b/fml/concurrent_message_loop.h @@ -34,6 +34,8 @@ class ConcurrentMessageLoop void PostTaskToAllWorkers(fml::closure task); + bool RunsTasksOnCurrentThread(); + private: friend ConcurrentTaskRunner; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 408567435613f..241e92873f588 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -118,7 +118,7 @@ std::unique_ptr Engine::Spawn( /*delegate=*/delegate, /*dispatcher_maker=*/dispatcher_maker, /*image_decoder_task_runner=*/ - runtime_controller_->GetDartVM()->GetConcurrentWorkerTaskRunner(), + concurrent_task_runner(), /*task_runners=*/task_runners_, /*settings=*/settings, /*animator=*/std::move(animator), @@ -547,16 +547,19 @@ void Engine::HandleAssetPlatformMessage( std::string asset_name(reinterpret_cast(data.GetMapping()), data.GetSize()); - if (asset_manager_) { + if (!asset_manager_) { + response->CompleteEmpty(); + return; + } + concurrent_task_runner()->PostTask([asset_manager = asset_manager_, + asset_name = std::move(asset_name), + response = std::move(response)] { std::unique_ptr asset_mapping = - asset_manager_->GetAsMapping(asset_name); + asset_manager->GetAsMapping(asset_name); if (asset_mapping) { response->Complete(std::move(asset_mapping)); - return; } - } - - response->CompleteEmpty(); + }); } const std::string& Engine::GetLastEntrypoint() const { diff --git a/shell/common/engine.h b/shell/common/engine.h index ca73c591432a6..66ef219838206 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -36,6 +36,10 @@ namespace flutter { +namespace testing { +class EngineTest; +} + //------------------------------------------------------------------------------ /// The engine is a component owned by the shell that resides on the UI task /// runner and is responsible for managing the needs of the root isolate and its @@ -979,6 +983,12 @@ class Engine final : public RuntimeDelegate, PointerDataDispatcher::Delegate { bool GetAssetAsBuffer(const std::string& name, std::vector* data); + std::shared_ptr concurrent_task_runner() const { + FML_DCHECK(runtime_controller_); + return runtime_controller_->GetDartVM()->GetConcurrentWorkerTaskRunner(); + } + + friend class testing::EngineTest; friend class testing::ShellTest; FML_DISALLOW_COPY_AND_ASSIGN(Engine); diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index e841060c636ac..1aa34b8e56af0 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -6,6 +6,7 @@ #include +#include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/fixture_test.h" @@ -15,11 +16,13 @@ #include "rapidjson/stringbuffer.h" #include "rapidjson/writer.h" +#include "flutter/fml/backtrace.h" + ///\note Deprecated MOCK_METHOD macros used until this issue is resolved: // https://github.com/google/googletest/issues/2490 namespace flutter { - +namespace testing { namespace { class MockDelegate : public Engine::Delegate { public: @@ -100,6 +103,41 @@ std::unique_ptr MakePlatformMessage( return message; } +class FakeAssetResolver : public AssetResolver { + public: + FakeAssetResolver(std::shared_ptr concurrent_loop) + : concurrent_loop_(std::move(concurrent_loop)) {} + + // |AssetResolver| + bool IsValid() const override { return true; } + + // |AssetResolver| + bool IsValidAfterAssetManagerChange() const override { return true; } + + // |AssetResolver| + AssetResolverType GetType() const { + return AssetResolverType::kApkAssetProvider; + } + + // |AssetResolver| + std::unique_ptr GetAsMapping( + const std::string& asset_name) const override { + mapping_requests.push_back(asset_name); + // TODO(dnfield): FontManifest is loaded differently than most assets. + // Should we load it on a background thread? + if (asset_name != "FontManifest.json") + EXPECT_TRUE(concurrent_loop_->RunsTasksOnCurrentThread()) + << fml::BacktraceHere(); + return nullptr; + } + + mutable std::vector mapping_requests; + + private: + std::shared_ptr concurrent_loop_; +}; +} // namespace + class EngineTest : public testing::FixtureTest { public: EngineTest() @@ -123,6 +161,11 @@ class EngineTest : public testing::FixtureTest { latch.Wait(); } + void HandlePlatformMessage(Engine* engine, + std::unique_ptr message) { + engine->HandlePlatformMessage(std::move(message)); + } + protected: void SetUp() override { settings_ = CreateSettingsForFixture(); @@ -141,7 +184,6 @@ class EngineTest : public testing::FixtureTest { std::unique_ptr runtime_controller_; std::shared_ptr image_decoder_task_runner_; }; -} // namespace TEST_F(EngineTest, Create) { PostUITaskSync([this] { @@ -328,4 +370,51 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) { }); } +TEST_F(EngineTest, AssetIOIsHandledOnWorkerThread) { + PostUITaskSync([this] { + MockRuntimeDelegate client; + auto mock_runtime_controller = + std::make_unique(client, task_runners_); + auto vm_ref = DartVMRef::Create(settings_); + EXPECT_CALL(*mock_runtime_controller, GetDartVM()) + .WillRepeatedly(::testing::Return(vm_ref.get())); + EXPECT_CALL(*mock_runtime_controller, IsRootIsolateRunning()) + .WillRepeatedly(::testing::Return(true)); + EXPECT_CALL(*mock_runtime_controller, DispatchPlatformMessage(::testing::_)) + .WillRepeatedly(::testing::Return(true)); + auto engine = std::make_unique( + /*delegate=*/delegate_, + /*dispatcher_maker=*/dispatcher_maker_, + /*image_decoder_task_runner=*/image_decoder_task_runner_, + /*task_runners=*/task_runners_, + /*settings=*/settings_, + /*animator=*/std::move(animator_), + /*io_manager=*/io_manager_, + /*font_collection=*/std::make_shared(), + /*runtime_controller=*/std::move(mock_runtime_controller)); + + fml::RefPtr response = + fml::MakeRefCounted(); + std::string asset_name = "foo.png"; + auto message = std::make_unique( + "flutter/assets", + fml::MallocMapping::Copy(asset_name.c_str(), asset_name.length()), + response); + + auto asset_manager = std::make_shared(); + auto concurrent_loop = vm_ref->GetConcurrentMessageLoop(); + auto resolver = std::make_unique(concurrent_loop); + auto raw_resolver = resolver.get(); + asset_manager->PushBack(std::move(resolver)); + engine->UpdateAssetManager(asset_manager); + HandlePlatformMessage(engine.get(), std::move(message)); + + fml::CountDownLatch latch(concurrent_loop->GetWorkerCount()); + concurrent_loop->PostTaskToAllWorkers([&latch] { latch.CountDown(); }); + latch.Wait(); + EXPECT_EQ(raw_resolver->mapping_requests.back(), "foo.png"); + }); +} + +} // namespace testing } // namespace flutter