diff --git a/fml/concurrent_message_loop.cc b/fml/concurrent_message_loop.cc index 681b983fca3ed..f553654a1195f 100644 --- a/fml/concurrent_message_loop.cc +++ b/fml/concurrent_message_loop.cc @@ -170,4 +170,14 @@ void ConcurrentTaskRunner::PostTask(const fml::closure& task) { task(); } +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; +} + } // namespace fml diff --git a/fml/concurrent_message_loop.h b/fml/concurrent_message_loop.h index c043705935e57..ab8534b6c946a 100644 --- a/fml/concurrent_message_loop.h +++ b/fml/concurrent_message_loop.h @@ -34,6 +34,8 @@ class ConcurrentMessageLoop void PostTaskToAllWorkers(const fml::closure& task); + bool RunsTasksOnCurrentThread(); + private: friend ConcurrentTaskRunner; diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 30cc3d3ecc0ba..ee2d173259216 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -6294,7 +6294,12 @@ class ImmutableBuffer extends NativeFieldWrapperClass1 { final ImmutableBuffer instance = ImmutableBuffer._(0); return _futurize((_Callback callback) { return instance._initFromAsset(encodedKey, callback); - }).then((int length) => instance.._length = length); + }).then((int length) { + if (length == -1) { + throw Exception('Asset not found'); + } + return instance.._length = length; + }); } /// Create a buffer from the file with [path]. diff --git a/lib/ui/painting/immutable_buffer.cc b/lib/ui/painting/immutable_buffer.cc index 0d86b07c24e4a..0f859f3420140 100644 --- a/lib/ui/painting/immutable_buffer.cc +++ b/lib/ui/painting/immutable_buffer.cc @@ -43,7 +43,7 @@ Dart_Handle ImmutableBuffer::init(Dart_Handle buffer_handle, return Dart_Null(); } -Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle buffer_handle, +Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle raw_buffer_handle, Dart_Handle asset_name_handle, Dart_Handle callback_handle) { UIDartState::ThrowIfUIOperationsProhibited(); @@ -62,21 +62,60 @@ Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle buffer_handle, std::string asset_name = std::string{reinterpret_cast(chars), static_cast(asset_length)}; - std::shared_ptr asset_manager = UIDartState::Current() - ->platform_configuration() - ->client() - ->GetAssetManager(); - std::unique_ptr data = asset_manager->GetAsMapping(asset_name); - if (data == nullptr) { - return tonic::ToDart("Asset not found"); - } + auto* dart_state = UIDartState::Current(); + auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner(); + auto* buffer_callback_ptr = + new tonic::DartPersistentValue(dart_state, callback_handle); + auto* buffer_handle_ptr = + new tonic::DartPersistentValue(dart_state, raw_buffer_handle); + auto asset_manager = UIDartState::Current() + ->platform_configuration() + ->client() + ->GetAssetManager(); - auto size = data->GetSize(); - const void* bytes = static_cast(data->GetMapping()); - auto sk_data = MakeSkDataWithCopy(bytes, size); - auto buffer = fml::MakeRefCounted(sk_data); - buffer->AssociateWithDartWrapper(buffer_handle); - tonic::DartInvoke(callback_handle, {tonic::ToDart(size)}); + auto ui_task = fml::MakeCopyable( + [buffer_callback_ptr, buffer_handle_ptr](const sk_sp& sk_data, + size_t buffer_size) mutable { + std::unique_ptr buffer_handle( + buffer_handle_ptr); + std::unique_ptr buffer_callback( + buffer_callback_ptr); + + auto dart_state = buffer_callback->dart_state().lock(); + if (!dart_state) { + return; + } + tonic::DartState::Scope scope(dart_state); + + if (!sk_data) { + // -1 is used as a sentinel that the file could not be opened. + tonic::DartInvoke(buffer_callback->Get(), {tonic::ToDart(-1)}); + return; + } + auto buffer = fml::MakeRefCounted(sk_data); + buffer->AssociateWithDartWrapper(buffer_handle->Get()); + tonic::DartInvoke(buffer_callback->Get(), {tonic::ToDart(buffer_size)}); + }); + + dart_state->GetConcurrentTaskRunner()->PostTask( + [asset_name = std::move(asset_name), + asset_manager = std::move(asset_manager), + ui_task_runner = std::move(ui_task_runner), ui_task] { + std::unique_ptr mapping = + asset_manager->GetAsMapping(asset_name); + + sk_sp sk_data; + size_t buffer_size = 0; + if (mapping != nullptr) { + buffer_size = mapping->GetSize(); + const void* bytes = static_cast(mapping->GetMapping()); + sk_data = MakeSkDataWithCopy(bytes, buffer_size); + } + ui_task_runner->PostTask( + [sk_data = std::move(sk_data), ui_task = ui_task, buffer_size]() { + ui_task(sk_data, buffer_size); + }); + }); return Dart_Null(); } @@ -101,20 +140,24 @@ Dart_Handle ImmutableBuffer::initFromFile(Dart_Handle raw_buffer_handle, auto* dart_state = UIDartState::Current(); auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner(); - auto buffer_callback = - std::make_unique(dart_state, callback_handle); - auto buffer_handle = std::make_unique( - dart_state, raw_buffer_handle); + auto* buffer_callback_ptr = + new tonic::DartPersistentValue(dart_state, callback_handle); + auto* buffer_handle_ptr = + new tonic::DartPersistentValue(dart_state, raw_buffer_handle); auto ui_task = fml::MakeCopyable( - [buffer_callback = std::move(buffer_callback), - buffer_handle = std::move(buffer_handle)](const sk_sp& sk_data, - size_t buffer_size) mutable { + [buffer_callback_ptr, buffer_handle_ptr](const sk_sp& sk_data, + size_t buffer_size) mutable { + std::unique_ptr buffer_handle( + buffer_handle_ptr); + std::unique_ptr buffer_callback( + buffer_callback_ptr); auto dart_state = buffer_callback->dart_state().lock(); if (!dart_state) { return; } tonic::DartState::Scope scope(dart_state); + if (!sk_data) { // -1 is used as a sentinel that the file could not be opened. tonic::DartInvoke(buffer_callback->Get(), {tonic::ToDart(-1)}); diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index 605e3d951d230..b88289a093f74 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -470,3 +470,11 @@ Future testPluginUtilitiesCallbackHandle() async { } notifyNativeBool(true); } + +@pragma('vm:entry-point') +Future testThatAssetLoadingHappensOnWorkerThread() async { + try { + await ImmutableBuffer.fromAsset('DoesNotExist'); + } catch (err) { /* Do nothing */ } + notifyNative(); +} diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 449822cd91c32..36c86463d8534 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -19,6 +19,7 @@ #include "flutter/flow/layers/layer_raster_cache_item.h" #include "flutter/flow/layers/platform_view_layer.h" #include "flutter/flow/layers/transform_layer.h" +#include "flutter/fml/backtrace.h" #include "flutter/fml/command_line.h" #include "flutter/fml/dart/dart_converter.h" #include "flutter/fml/make_copyable.h" @@ -194,6 +195,42 @@ class TestAssetResolver : public AssetResolver { AssetResolver::AssetResolverType type_; }; +class ThreadCheckingAssetResolver : public AssetResolver { + public: + explicit ThreadCheckingAssetResolver( + 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 { + if (asset_name == "FontManifest.json") { + // This file is loaded directly by the engine. + return nullptr; + } + mapping_requests.push_back(asset_name); + EXPECT_TRUE(concurrent_loop_->RunsTasksOnCurrentThread()) + << fml::BacktraceHere(); + return nullptr; + } + + mutable std::vector mapping_requests; + + private: + std::shared_ptr concurrent_loop_; +}; + static bool ValidateShell(Shell* shell) { if (!shell) { return false; @@ -3805,6 +3842,39 @@ TEST_F(ShellTest, SpawnWorksWithOnError) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); } +TEST_F(ShellTest, ImmutableBufferLoadsAssetOnBackgroundThread) { + Settings settings = CreateSettingsForFixture(); + auto task_runner = CreateNewThread(); + TaskRunners task_runners("test", task_runner, task_runner, task_runner, + task_runner); + std::unique_ptr shell = CreateShell(settings, task_runners); + + fml::CountDownLatch latch(1); + AddNativeCallback("NotifyNative", + CREATE_NATIVE_ENTRY([&](auto args) { latch.CountDown(); })); + + // Create the surface needed by rasterizer + PlatformViewNotifyCreated(shell.get()); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("testThatAssetLoadingHappensOnWorkerThread"); + auto asset_manager = configuration.GetAssetManager(); + auto test_resolver = std::make_unique( + shell->GetDartVM()->GetConcurrentMessageLoop()); + auto leaked_resolver = test_resolver.get(); + asset_manager->PushBack(std::move(test_resolver)); + + RunEngine(shell.get(), std::move(configuration)); + PumpOneFrame(shell.get()); + + latch.Wait(); + + EXPECT_EQ(leaked_resolver->mapping_requests[0], "DoesNotExist"); + + PlatformViewNotifyDestroyed(shell.get()); + DestroyShell(std::move(shell), task_runners); +} + TEST_F(ShellTest, PictureToImageSync) { #if !SHELL_ENABLE_GL // This test uses the GL backend.