Revert "[engine] move asset mapping copy to background thread (#39918)" (#40147)
Revert "[engine] move asset mapping copy to background thread"
diff --git a/fml/concurrent_message_loop.cc b/fml/concurrent_message_loop.cc
index f553654..681b983 100644
--- a/fml/concurrent_message_loop.cc
+++ b/fml/concurrent_message_loop.cc
@@ -170,14 +170,4 @@
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 ab8534b..c043705 100644
--- a/fml/concurrent_message_loop.h
+++ b/fml/concurrent_message_loop.h
@@ -34,8 +34,6 @@
void PostTaskToAllWorkers(const fml::closure& task);
- bool RunsTasksOnCurrentThread();
-
private:
friend ConcurrentTaskRunner;
diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart
index ee2d173..30cc3d3 100644
--- a/lib/ui/painting.dart
+++ b/lib/ui/painting.dart
@@ -6294,12 +6294,7 @@
final ImmutableBuffer instance = ImmutableBuffer._(0);
return _futurize((_Callback<int> callback) {
return instance._initFromAsset(encodedKey, callback);
- }).then((int length) {
- if (length == -1) {
- throw Exception('Asset not found');
- }
- return instance.._length = length;
- });
+ }).then((int length) => 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 0595eb8..0d86b07 100644
--- a/lib/ui/painting/immutable_buffer.cc
+++ b/lib/ui/painting/immutable_buffer.cc
@@ -43,7 +43,7 @@
return Dart_Null();
}
-Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle raw_buffer_handle,
+Dart_Handle ImmutableBuffer::initFromAsset(Dart_Handle buffer_handle,
Dart_Handle asset_name_handle,
Dart_Handle callback_handle) {
UIDartState::ThrowIfUIOperationsProhibited();
@@ -62,55 +62,21 @@
std::string asset_name = std::string{reinterpret_cast<const char*>(chars),
static_cast<size_t>(asset_length)};
- auto* dart_state = UIDartState::Current();
- auto ui_task_runner = dart_state->GetTaskRunners().GetUITaskRunner();
- auto buffer_callback =
- std::make_unique<tonic::DartPersistentValue>(dart_state, callback_handle);
- auto buffer_handle = std::make_unique<tonic::DartPersistentValue>(
- dart_state, raw_buffer_handle);
- auto asset_manager = UIDartState::Current()
- ->platform_configuration()
- ->client()
- ->GetAssetManager();
+ std::shared_ptr<AssetManager> asset_manager = UIDartState::Current()
+ ->platform_configuration()
+ ->client()
+ ->GetAssetManager();
+ std::unique_ptr<fml::Mapping> data = asset_manager->GetAsMapping(asset_name);
+ if (data == nullptr) {
+ return tonic::ToDart("Asset not found");
+ }
- auto ui_task = fml::MakeCopyable(
- [buffer_callback = std::move(buffer_callback),
- buffer_handle = std::move(buffer_handle)](const sk_sp<SkData>& sk_data,
- size_t buffer_size) mutable {
- 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<ImmutableBuffer>(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<fml::Mapping> mapping =
- asset_manager->GetAsMapping(asset_name);
-
- sk_sp<SkData> sk_data;
- size_t buffer_size = 0;
- if (mapping != nullptr) {
- buffer_size = mapping->GetSize();
- const void* bytes = static_cast<const void*>(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);
- });
- });
+ auto size = data->GetSize();
+ const void* bytes = static_cast<const void*>(data->GetMapping());
+ auto sk_data = MakeSkDataWithCopy(bytes, size);
+ auto buffer = fml::MakeRefCounted<ImmutableBuffer>(sk_data);
+ buffer->AssociateWithDartWrapper(buffer_handle);
+ tonic::DartInvoke(callback_handle, {tonic::ToDart(size)});
return Dart_Null();
}
diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart
index b88289a..605e3d9 100644
--- a/shell/common/fixtures/shell_test.dart
+++ b/shell/common/fixtures/shell_test.dart
@@ -470,11 +470,3 @@
}
notifyNativeBool(true);
}
-
-@pragma('vm:entry-point')
-Future<void> 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 36c8646..449822c 100644
--- a/shell/common/shell_unittests.cc
+++ b/shell/common/shell_unittests.cc
@@ -19,7 +19,6 @@
#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"
@@ -195,42 +194,6 @@
AssetResolver::AssetResolverType type_;
};
-class ThreadCheckingAssetResolver : public AssetResolver {
- public:
- explicit ThreadCheckingAssetResolver(
- std::shared_ptr<fml::ConcurrentMessageLoop> 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<fml::Mapping> 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<std::string> mapping_requests;
-
- private:
- std::shared_ptr<fml::ConcurrentMessageLoop> concurrent_loop_;
-};
-
static bool ValidateShell(Shell* shell) {
if (!shell) {
return false;
@@ -3842,39 +3805,6 @@
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> 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<ThreadCheckingAssetResolver>(
- 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.
