From dd4cbc365f771f5c6fa8e813ea16badf590f016d Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Feb 2020 15:13:33 -0800 Subject: [PATCH 1/8] test --- lib/ui/BUILD.gn | 6 + lib/ui/painting/image_decoder_unittests.cc | 173 ++++++++++++++++++++- 2 files changed, 178 insertions(+), 1 deletion(-) diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index 4cc81dff2154d..012587e174723 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -173,8 +173,14 @@ if (current_toolchain == host_toolchain) { ":ui", ":ui_unittests_fixtures", "//flutter/common", + "//flutter/fml", + "//flutter/lib/snapshot", + "//flutter/runtime", + "//flutter/shell/common", "//flutter/testing:dart", "//flutter/testing:opengl", + "//flutter/third_party/tonic", + "//third_party/dart/runtime/bin:elf_loader", ] } } diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 288eb2abda1b0..e5b06c46c1181 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -6,6 +6,11 @@ #include "flutter/fml/mapping.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/painting/image_decoder.h" +#include "flutter/lib/ui/painting/multi_frame_codec.h" +#include "flutter/runtime/dart_vm.h" +#include "flutter/runtime/dart_vm_lifecycle.h" +#include "flutter/testing/elf_loader.h" +#include "flutter/testing/test_dart_native_resolver.h" #include "flutter/testing/test_gl_surface.h" #include "flutter/testing/testing.h" #include "flutter/testing/thread_test.h" @@ -14,7 +19,56 @@ namespace flutter { namespace testing { -using ImageDecoderFixtureTest = ThreadTest; +class ImageDecoderFixtureTest : public ThreadTest { + public: + ImageDecoderFixtureTest() + : native_resolver_(std::make_shared()), + assets_dir_(fml::OpenDirectory(GetFixturesPath(), + false, + fml::FilePermission::kRead)), + aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary()) {} + + Settings CreateSettingsForFixture() { + Settings settings; + settings.leak_vm = false; + settings.task_observer_add = [](intptr_t, fml::closure) {}; + settings.task_observer_remove = [](intptr_t) {}; + settings.isolate_create_callback = [this]() { + native_resolver_->SetNativeResolverForIsolate(); + }; + SetSnapshotsAndAssets(settings); + return settings; + } + + private: + std::shared_ptr native_resolver_; + fml::UniqueFD assets_dir_; + ELFAOTSymbols aot_symbols_; + + void SetSnapshotsAndAssets(Settings& settings) { + if (!assets_dir_.is_valid()) { + return; + } + + settings.assets_dir = assets_dir_.get(); + + // In JIT execution, all snapshots are present within the binary itself and + // don't need to be explicitly supplied by the embedder. In AOT, these + // snapshots will be present in the application AOT dylib. + if (DartVM::IsRunningPrecompiledCode()) { + PrepareSettingsForAOTWithSymbols(settings, aot_symbols_); + } else { + settings.application_kernels = [this]() { + std::vector> kernel_mappings; + kernel_mappings.emplace_back( + fml::FileMapping::CreateReadOnly(assets_dir_, "kernel_blob.bin")); + return kernel_mappings; + }; + } + } + + FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoderFixtureTest); +}; class TestIOManager final : public IOManager { public: @@ -557,5 +611,122 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { assert_image(decode({}, 100)); } +TEST_F(ImageDecoderFixtureTest, + MultiFrameCodecCanBeCollectedBeforeIOTasksFinish) { + auto settings = CreateSettingsForFixture(); + settings.leak_vm = false; + settings.enable_observatory = false; + auto vm_ref = DartVMRef::Create(settings); + auto vm_data = vm_ref.GetVMData(); + + auto gif_mapping = OpenFixtureAsSkData("hello_loop_2.gif"); + + ASSERT_TRUE(gif_mapping); + + auto gif_codec = SkCodec::MakeFromData(gif_mapping); + ASSERT_TRUE(gif_codec); + + auto loop = fml::ConcurrentMessageLoop::Create(); + TaskRunners runners(GetCurrentTestName(), // label + CreateNewThread("platform"), // platform + CreateNewThread("gpu"), // gpu + CreateNewThread("ui"), // ui + CreateNewThread("io") // io + ); + + fml::AutoResetWaitableEvent latch; + fml::AutoResetWaitableEvent io_latch; + std::unique_ptr io_manager; + std::unique_ptr image_decoder; + + // Setup the IO manager. + runners.GetIOTaskRunner()->PostTask([&]() { + io_manager = std::make_unique(runners.GetIOTaskRunner()); + latch.Signal(); + }); + latch.Wait(); + + auto isolate_weak = DartIsolate::CreateRootIsolate( + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate_snapshot + runners, // task_runners + {}, // window + {}, // snapshot_delegate + io_manager->GetWeakIOManager(), // io_manager + {}, // unref_queue + {}, // image_decoder + "main.dart", // advisory_script_uri + "main", // advisory_script_entrypoint + nullptr, // flags + settings.isolate_create_callback, // isolate create callback + settings.isolate_shutdown_callback // isolate shutdown callback + ); + + auto isolate = isolate_weak.lock(); + + // Setup the image decoder. + runners.GetUITaskRunner()->PostTask([&]() { + image_decoder = std::make_unique( + runners, loop->GetTaskRunner(), io_manager->GetWeakIOManager()); + + latch.Signal(); + }); + latch.Wait(); + + // Latch the IO task runner. + runners.GetIOTaskRunner()->PostTask([&]() { io_latch.Wait(); }); + + runners.GetUITaskRunner()->PostTask([&]() { + fml::RefPtr codec; + + Dart_EnterIsolate(isolate->isolate()); + Dart_EnterScope(); + + Dart_Handle core_lib = Dart_LookupLibrary(Dart_NewStringFromCString("dart:core")); + EXPECT_FALSE(Dart_IsError(core_lib)); + Dart_Handle object_type = Dart_GetType(core_lib, Dart_NewStringFromCString("Object"), 0, nullptr); + EXPECT_FALSE(Dart_IsError(object_type)); + Dart_Handle object = Dart_New(object_type, Dart_EmptyString(), 0, nullptr); + EXPECT_FALSE(Dart_IsError(object)); + Dart_Handle closure = Dart_GetField(object, Dart_NewStringFromCString("toString")); + EXPECT_FALSE(Dart_IsError(closure)); + EXPECT_TRUE(Dart_IsClosure(closure)); + + codec = fml::MakeRefCounted(std::move(gif_codec)); + FML_DLOG(ERROR) << "1"; + codec->getNextFrame(closure); + + FML_DLOG(ERROR) << "2"; + + codec = nullptr; + FML_DLOG(ERROR) << "3"; + + Dart_ExitScope(); + Dart_ExitIsolate(); + + EXPECT_FALSE(codec); + + io_latch.Signal(); + FML_DLOG(ERROR) << "4"; + + latch.Signal(); + }); + latch.Wait(); + + // Destroy the IO manager + runners.GetIOTaskRunner()->PostTask([&]() { + io_manager.reset(); + latch.Signal(); + }); + latch.Wait(); + + // Destroy the image decoder + runners.GetUITaskRunner()->PostTask([&]() { + image_decoder.reset(); + latch.Signal(); + }); + latch.Wait(); +} + } // namespace testing } // namespace flutter From 46af095265630e9164590155672d9711f3b7818e Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Feb 2020 22:31:42 -0800 Subject: [PATCH 2/8] avoid unsafe capture of this in MultiFrameCodec --- lib/ui/fixtures/ui_test.dart | 7 + lib/ui/painting/image_decoder_unittests.cc | 86 ++----- lib/ui/painting/multi_frame_codec.cc | 40 ++- lib/ui/painting/multi_frame_codec.h | 43 ++-- runtime/dart_isolate_unittests.cc | 269 +++++---------------- testing/BUILD.gn | 1 + testing/dart_isolate_runner.h | 198 +++++++++++++++ 7 files changed, 350 insertions(+), 294 deletions(-) create mode 100644 testing/dart_isolate_runner.h diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 6d604e30543bf..dfd68d7416c54 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -3,4 +3,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:ui'; + void main() {} + +@pragma('vm:entry-point') +void frameCallback(FrameInfo info) { + print('called back'); +} diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index e5b06c46c1181..f9aa07879860d 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -9,6 +9,7 @@ #include "flutter/lib/ui/painting/multi_frame_codec.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" +#include "flutter/testing/dart_isolate_runner.h" #include "flutter/testing/elf_loader.h" #include "flutter/testing/test_dart_native_resolver.h" #include "flutter/testing/test_gl_surface.h" @@ -637,7 +638,6 @@ TEST_F(ImageDecoderFixtureTest, fml::AutoResetWaitableEvent latch; fml::AutoResetWaitableEvent io_latch; std::unique_ptr io_manager; - std::unique_ptr image_decoder; // Setup the IO manager. runners.GetIOTaskRunner()->PostTask([&]() { @@ -646,68 +646,41 @@ TEST_F(ImageDecoderFixtureTest, }); latch.Wait(); - auto isolate_weak = DartIsolate::CreateRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate_snapshot - runners, // task_runners - {}, // window - {}, // snapshot_delegate - io_manager->GetWeakIOManager(), // io_manager - {}, // unref_queue - {}, // image_decoder - "main.dart", // advisory_script_uri - "main", // advisory_script_entrypoint - nullptr, // flags - settings.isolate_create_callback, // isolate create callback - settings.isolate_shutdown_callback // isolate shutdown callback - ); - - auto isolate = isolate_weak.lock(); - - // Setup the image decoder. - runners.GetUITaskRunner()->PostTask([&]() { - image_decoder = std::make_unique( - runners, loop->GetTaskRunner(), io_manager->GetWeakIOManager()); - - latch.Signal(); - }); - latch.Wait(); + auto isolate = + RunDartCodeInIsolate(vm_ref, settings, runners, "main", {}, + GetFixturesPath(), io_manager->GetWeakIOManager()); // Latch the IO task runner. runners.GetIOTaskRunner()->PostTask([&]() { io_latch.Wait(); }); runners.GetUITaskRunner()->PostTask([&]() { + fml::AutoResetWaitableEvent isolate_latch; fml::RefPtr codec; - - Dart_EnterIsolate(isolate->isolate()); - Dart_EnterScope(); - - Dart_Handle core_lib = Dart_LookupLibrary(Dart_NewStringFromCString("dart:core")); - EXPECT_FALSE(Dart_IsError(core_lib)); - Dart_Handle object_type = Dart_GetType(core_lib, Dart_NewStringFromCString("Object"), 0, nullptr); - EXPECT_FALSE(Dart_IsError(object_type)); - Dart_Handle object = Dart_New(object_type, Dart_EmptyString(), 0, nullptr); - EXPECT_FALSE(Dart_IsError(object)); - Dart_Handle closure = Dart_GetField(object, Dart_NewStringFromCString("toString")); - EXPECT_FALSE(Dart_IsError(closure)); - EXPECT_TRUE(Dart_IsClosure(closure)); - - codec = fml::MakeRefCounted(std::move(gif_codec)); - FML_DLOG(ERROR) << "1"; - codec->getNextFrame(closure); - - FML_DLOG(ERROR) << "2"; - - codec = nullptr; - FML_DLOG(ERROR) << "3"; - - Dart_ExitScope(); - Dart_ExitIsolate(); + EXPECT_TRUE(isolate->RunInIsolateScope([&]() -> bool { + Dart_Handle library = Dart_LookupLibrary(Dart_NewStringFromCString( + "package:engine_root/ui/fixtures/ui_test.dart")); + if (Dart_IsError(library)) { + isolate_latch.Signal(); + return false; + } + Dart_Handle closure = + Dart_GetField(library, Dart_NewStringFromCString("frameCallback")); + if (Dart_IsError(closure) || !Dart_IsClosure(closure)) { + isolate_latch.Signal(); + return false; + } + + codec = fml::MakeRefCounted(std::move(gif_codec)); + codec->getNextFrame(closure); + codec = nullptr; + isolate_latch.Signal(); + return true; + })); + isolate_latch.Wait(); EXPECT_FALSE(codec); io_latch.Signal(); - FML_DLOG(ERROR) << "4"; latch.Signal(); }); @@ -719,13 +692,6 @@ TEST_F(ImageDecoderFixtureTest, latch.Signal(); }); latch.Wait(); - - // Destroy the image decoder - runners.GetUITaskRunner()->PostTask([&]() { - image_decoder.reset(); - latch.Signal(); - }); - latch.Wait(); } } // namespace testing diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index f650e25a97e61..4013dc4d156fa 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -12,12 +12,18 @@ namespace flutter { MultiFrameCodec::MultiFrameCodec(std::unique_ptr codec) + : state_(new State(std::move(codec))) {} + +MultiFrameCodec::~MultiFrameCodec() { + state_->live_ = false; +} + +MultiFrameCodec::State::State(std::unique_ptr codec) : codec_(std::move(codec)), frameCount_(codec_->getFrameCount()), repetitionCount_(codec_->getRepetitionCount()), - nextFrameIndex_(0) {} - -MultiFrameCodec::~MultiFrameCodec() = default; + nextFrameIndex_(0), + live_(true) {} static void InvokeNextFrameCallback( fml::RefPtr frameInfo, @@ -70,7 +76,7 @@ static bool CopyToBitmap(SkBitmap* dst, return true; } -sk_sp MultiFrameCodec::GetNextFrameImage( +sk_sp MultiFrameCodec::State::GetNextFrameImage( fml::WeakPtr resourceContext) { SkBitmap bitmap = SkBitmap(); SkImageInfo info = codec_->getInfo().makeColorType(kN32_SkColorType); @@ -128,13 +134,20 @@ sk_sp MultiFrameCodec::GetNextFrameImage( } } -void MultiFrameCodec::GetNextFrameAndInvokeCallback( +void MultiFrameCodec::State::GetNextFrameAndInvokeCallback( std::unique_ptr callback, fml::RefPtr ui_task_runner, fml::WeakPtr resourceContext, fml::RefPtr unref_queue, size_t trace_id) { fml::RefPtr frameInfo = NULL; + if (!live_) { + ui_task_runner->PostTask(fml::MakeCopyable( + [callback = std::move(callback), frameInfo, trace_id]() mutable { + InvokeNextFrameCallback(frameInfo, std::move(callback), trace_id); + })); + return; + } sk_sp skImage = GetNextFrameImage(resourceContext); if (skImage) { fml::RefPtr image = CanvasImage::Create(); @@ -164,12 +177,13 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { const auto& task_runners = dart_state->GetTaskRunners(); - task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable( - [callback = std::make_unique( - tonic::DartState::Current(), callback_handle), - this, trace_id, ui_task_runner = task_runners.GetUITaskRunner(), - io_manager = dart_state->GetIOManager()]() mutable { - GetNextFrameAndInvokeCallback( + task_runners.GetIOTaskRunner()->PostTask( + fml::MakeCopyable([callback = std::make_unique( + tonic::DartState::Current(), callback_handle), + state = state_, trace_id, + ui_task_runner = task_runners.GetUITaskRunner(), + io_manager = dart_state->GetIOManager()]() mutable { + state->GetNextFrameAndInvokeCallback( std::move(callback), std::move(ui_task_runner), io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(), trace_id); @@ -179,11 +193,11 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { } int MultiFrameCodec::frameCount() const { - return frameCount_; + return state_->frameCount_; } int MultiFrameCodec::repetitionCount() const { - return repetitionCount_; + return state_->repetitionCount_; } } // namespace flutter diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index 0be63c2a57582..a91765a1ea89d 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -26,24 +26,31 @@ class MultiFrameCodec : public Codec { Dart_Handle getNextFrame(Dart_Handle args) override; private: - const std::unique_ptr codec_; - const int frameCount_; - const int repetitionCount_; - int nextFrameIndex_; - - // The last decoded frame that's required to decode any subsequent frames. - std::unique_ptr lastRequiredFrame_; - // The index of the last decoded required frame. - int lastRequiredFrameIndex_ = -1; - - sk_sp GetNextFrameImage(fml::WeakPtr resourceContext); - - void GetNextFrameAndInvokeCallback( - std::unique_ptr callback, - fml::RefPtr ui_task_runner, - fml::WeakPtr resourceContext, - fml::RefPtr unref_queue, - size_t trace_id); + struct State { + State(std::unique_ptr codec); + + const std::unique_ptr codec_; + const int frameCount_; + const int repetitionCount_; + int nextFrameIndex_; + std::atomic live_; + + // The last decoded frame that's required to decode any subsequent frames. + std::unique_ptr lastRequiredFrame_; + // The index of the last decoded required frame. + int lastRequiredFrameIndex_ = -1; + + sk_sp GetNextFrameImage(fml::WeakPtr resourceContext); + void GetNextFrameAndInvokeCallback( + std::unique_ptr callback, + fml::RefPtr ui_task_runner, + fml::WeakPtr resourceContext, + fml::RefPtr unref_queue, + size_t trace_id); + }; + + // Shared across the UI and IO task runners. + std::shared_ptr state_; FML_FRIEND_MAKE_REF_COUNTED(MultiFrameCodec); FML_FRIEND_REF_COUNTED_THREAD_SAFE(MultiFrameCodec); diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 4998b8f13948b..283f3801a9e8a 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -2,9 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "flutter/fml/make_copyable.h" #include "flutter/fml/mapping.h" -#include "flutter/fml/paths.h" #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/thread.h" @@ -12,6 +10,7 @@ #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/runtime_test.h" +#include "flutter/testing/dart_isolate_runner.h" #include "flutter/testing/testing.h" #include "flutter/testing/thread_test.h" #include "third_party/tonic/converter/dart_converter.h" @@ -96,199 +95,18 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { ASSERT_EQ(destruction_callback_count, 1u); } -class AutoIsolateShutdown { - public: - AutoIsolateShutdown() = default; - - AutoIsolateShutdown(std::shared_ptr isolate, - fml::RefPtr runner) - : isolate_(std::move(isolate)), runner_(std::move(runner)) {} - - ~AutoIsolateShutdown() { - if (!IsValid()) { - return; - } - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - runner_, [isolate = std::move(isolate_), &latch]() { - if (!isolate->Shutdown()) { - FML_LOG(ERROR) << "Could not shutdown isolate."; - FML_CHECK(false); - } - latch.Signal(); - }); - latch.Wait(); - } - - bool IsValid() const { return isolate_ != nullptr && runner_; } - - FML_WARN_UNUSED_RESULT - bool RunInIsolateScope(std::function closure) { - if (!IsValid()) { - return false; - } - - bool result = false; - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - runner_, [this, &result, &latch, closure]() { - tonic::DartIsolateScope scope(isolate_->isolate()); - tonic::DartApiScope api_scope; - if (closure) { - result = closure(); - } - latch.Signal(); - }); - latch.Wait(); - return true; - } - - DartIsolate* get() { - FML_CHECK(isolate_); - return isolate_.get(); - } - - private: - std::shared_ptr isolate_; - fml::RefPtr runner_; - - FML_DISALLOW_COPY_AND_ASSIGN(AutoIsolateShutdown); -}; - -static void RunDartCodeInIsolate(DartVMRef& vm_ref, - std::unique_ptr& result, - const Settings& settings, - fml::RefPtr task_runner, - std::string entrypoint, - const std::vector& args) { - FML_CHECK(task_runner->RunsTasksOnCurrentThread()); - - if (!vm_ref) { - return; - } - - TaskRunners task_runners(GetCurrentTestName(), // - task_runner, // - task_runner, // - task_runner, // - task_runner // - ); - - auto vm_data = vm_ref.GetVMData(); - - if (!vm_data) { - return; - } - - auto weak_isolate = DartIsolate::CreateRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - std::move(task_runners), // task runners - nullptr, // window - {}, // snapshot delegate - {}, // io manager - {}, // unref queue - {}, // image decoder - "main.dart", // advisory uri - "main", // advisory entrypoint - nullptr, // flags - settings.isolate_create_callback, // isolate create callback - settings.isolate_shutdown_callback // isolate shutdown callback - ); - - auto root_isolate = - std::make_unique(weak_isolate.lock(), task_runner); - - if (!root_isolate->IsValid()) { - FML_LOG(ERROR) << "Could not create isolate."; - return; - } - - if (root_isolate->get()->GetPhase() != DartIsolate::Phase::LibrariesSetup) { - FML_LOG(ERROR) << "Created isolate is in unexpected phase."; - return; - } - - if (!DartVM::IsRunningPrecompiledCode()) { - auto kernel_file_path = - fml::paths::JoinPaths({GetFixturesPath(), "kernel_blob.bin"}); - - if (!fml::IsFile(kernel_file_path)) { - FML_LOG(ERROR) << "Could not locate kernel file."; - return; - } - - auto kernel_file = fml::OpenFile(kernel_file_path.c_str(), false, - fml::FilePermission::kRead); - - if (!kernel_file.is_valid()) { - FML_LOG(ERROR) << "Kernel file descriptor was invalid."; - return; - } - - auto kernel_mapping = std::make_unique(kernel_file); - - if (kernel_mapping->GetMapping() == nullptr) { - FML_LOG(ERROR) << "Could not setup kernel mapping."; - return; - } - - if (!root_isolate->get()->PrepareForRunningFromKernel( - std::move(kernel_mapping))) { - FML_LOG(ERROR) - << "Could not prepare to run the isolate from the kernel file."; - return; - } - } else { - if (!root_isolate->get()->PrepareForRunningFromPrecompiledCode()) { - FML_LOG(ERROR) - << "Could not prepare to run the isolate from precompiled code."; - return; - } - } - - if (root_isolate->get()->GetPhase() != DartIsolate::Phase::Ready) { - FML_LOG(ERROR) << "Isolate is in unexpected phase."; - return; - } - - if (!root_isolate->get()->Run(entrypoint, args, - settings.root_isolate_create_callback)) { - FML_LOG(ERROR) << "Could not run the method \"" << entrypoint - << "\" in the isolate."; - return; - } - - root_isolate->get()->AddIsolateShutdownCallback( - settings.root_isolate_shutdown_callback); - - result = std::move(root_isolate); -} - -static std::unique_ptr RunDartCodeInIsolate( - DartVMRef& vm_ref, - const Settings& settings, - fml::RefPtr task_runner, - std::string entrypoint, - const std::vector& args) { - std::unique_ptr result; - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - task_runner, fml::MakeCopyable([&]() mutable { - RunDartCodeInIsolate(vm_ref, result, settings, task_runner, entrypoint, - args); - latch.Signal(); - })); - latch.Wait(); - return result; -} - TEST_F(DartIsolateTest, IsolateCanLoadAndRunDartCode) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetCurrentTaskRunner(), - "main", {}); + TaskRunners task_runners(GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", + {}, GetFixturesPath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); } @@ -297,8 +115,15 @@ TEST_F(DartIsolateTest, IsolateCannotLoadAndRunUnknownDartEntrypoint) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetCurrentTaskRunner(), - "thisShouldNotExist", {}); + TaskRunners task_runners(GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // + ); + auto isolate = + RunDartCodeInIsolate(vm_ref, settings, task_runners, "thisShouldNotExist", + {}, GetFixturesPath()); ASSERT_FALSE(isolate); } @@ -306,8 +131,14 @@ TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, GetCurrentTaskRunner(), - "main", {}); + TaskRunners task_runners(GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", + {}, GetFixturesPath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -330,8 +161,16 @@ TEST_F(DartIsolateTest, CanRegisterNativeCallback) { }))); const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), - "canRegisterNativeCallback", {}); + auto thread = CreateNewThread(); + TaskRunners task_runners(GetCurrentTestName(), // + thread, // + thread, // + thread, // + thread // + ); + auto isolate = + RunDartCodeInIsolate(vm_ref, settings, task_runners, + "canRegisterNativeCallback", {}, GetFixturesPath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); latch.Wait(); @@ -353,8 +192,16 @@ TEST_F(DartIsolateTest, CanSaveCompilationTrace) { const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), - "testCanSaveCompilationTrace", {}); + auto thread = CreateNewThread(); + TaskRunners task_runners(GetCurrentTestName(), // + thread, // + thread, // + thread, // + thread // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, + "testCanSaveCompilationTrace", {}, + GetFixturesPath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); @@ -384,8 +231,16 @@ TEST_F(DartIsolateTest, CanLaunchSecondaryIsolates) { child_shutdown_latch.Signal(); }; auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), - "testCanLaunchSecondaryIsolate", {}); + auto thread = CreateNewThread(); + TaskRunners task_runners(GetCurrentTestName(), // + thread, // + thread, // + thread, // + thread // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, + "testCanLaunchSecondaryIsolate", {}, + GetFixturesPath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); child_shutdown_latch.Wait(); // wait for child isolate to shutdown first @@ -405,8 +260,16 @@ TEST_F(DartIsolateTest, CanRecieveArguments) { const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, CreateNewThread(), - "testCanRecieveArguments", {"arg1"}); + auto thread = CreateNewThread(); + TaskRunners task_runners(GetCurrentTestName(), // + thread, // + thread, // + thread, // + thread // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, + "testCanRecieveArguments", {"arg1"}, + GetFixturesPath()); ASSERT_TRUE(isolate); ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); diff --git a/testing/BUILD.gn b/testing/BUILD.gn index 04a563494209c..1845921e50418 100644 --- a/testing/BUILD.gn +++ b/testing/BUILD.gn @@ -47,6 +47,7 @@ source_set("dart") { "elf_loader.h", "test_dart_native_resolver.cc", "test_dart_native_resolver.h", + "dart_isolate_runner.h", ] public_deps = [ diff --git a/testing/dart_isolate_runner.h b/testing/dart_isolate_runner.h new file mode 100644 index 0000000000000..aca09435ea226 --- /dev/null +++ b/testing/dart_isolate_runner.h @@ -0,0 +1,198 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/fml/make_copyable.h" +#include "flutter/fml/paths.h" +#include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/fml/thread.h" + +namespace flutter { +namespace testing { + +class AutoIsolateShutdown { + public: + AutoIsolateShutdown() = default; + + AutoIsolateShutdown(std::shared_ptr isolate, + fml::RefPtr runner) + : isolate_(std::move(isolate)), runner_(std::move(runner)) {} + + ~AutoIsolateShutdown() { + if (!IsValid()) { + return; + } + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask( + runner_, [isolate = std::move(isolate_), &latch]() { + if (!isolate->Shutdown()) { + FML_LOG(ERROR) << "Could not shutdown isolate."; + FML_CHECK(false); + } + latch.Signal(); + }); + latch.Wait(); + } + + bool IsValid() const { return isolate_ != nullptr && runner_; } + + FML_WARN_UNUSED_RESULT + bool RunInIsolateScope(std::function closure) { + if (!IsValid()) { + return false; + } + + bool result = false; + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask( + runner_, [this, &result, &latch, closure]() { + tonic::DartIsolateScope scope(isolate_->isolate()); + tonic::DartApiScope api_scope; + if (closure) { + result = closure(); + } + latch.Signal(); + }); + latch.Wait(); + return true; + } + + DartIsolate* get() { + FML_CHECK(isolate_); + return isolate_.get(); + } + + private: + std::shared_ptr isolate_; + fml::RefPtr runner_; + + FML_DISALLOW_COPY_AND_ASSIGN(AutoIsolateShutdown); +}; + +static void RunDartCodeInIsolate(DartVMRef& vm_ref, + std::unique_ptr& result, + const Settings& settings, + const TaskRunners& task_runners, + std::string entrypoint, + const std::vector& args, + const std::string& fixtures_path, + fml::WeakPtr io_manager = {}) { + FML_CHECK(task_runners.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + + if (!vm_ref) { + return; + } + + auto vm_data = vm_ref.GetVMData(); + + if (!vm_data) { + return; + } + + auto weak_isolate = DartIsolate::CreateRootIsolate( + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + io_manager, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint + nullptr, // flags + settings.isolate_create_callback, // isolate create callback + settings.isolate_shutdown_callback // isolate shutdown callback + ); + + auto root_isolate = std::make_unique( + weak_isolate.lock(), task_runners.GetUITaskRunner()); + + if (!root_isolate->IsValid()) { + FML_LOG(ERROR) << "Could not create isolate."; + return; + } + + if (root_isolate->get()->GetPhase() != DartIsolate::Phase::LibrariesSetup) { + FML_LOG(ERROR) << "Created isolate is in unexpected phase."; + return; + } + + if (!DartVM::IsRunningPrecompiledCode()) { + auto kernel_file_path = + fml::paths::JoinPaths({fixtures_path, "kernel_blob.bin"}); + + if (!fml::IsFile(kernel_file_path)) { + FML_LOG(ERROR) << "Could not locate kernel file."; + return; + } + + auto kernel_file = fml::OpenFile(kernel_file_path.c_str(), false, + fml::FilePermission::kRead); + + if (!kernel_file.is_valid()) { + FML_LOG(ERROR) << "Kernel file descriptor was invalid."; + return; + } + + auto kernel_mapping = std::make_unique(kernel_file); + + if (kernel_mapping->GetMapping() == nullptr) { + FML_LOG(ERROR) << "Could not setup kernel mapping."; + return; + } + + if (!root_isolate->get()->PrepareForRunningFromKernel( + std::move(kernel_mapping))) { + FML_LOG(ERROR) + << "Could not prepare to run the isolate from the kernel file."; + return; + } + } else { + if (!root_isolate->get()->PrepareForRunningFromPrecompiledCode()) { + FML_LOG(ERROR) + << "Could not prepare to run the isolate from precompiled code."; + return; + } + } + + if (root_isolate->get()->GetPhase() != DartIsolate::Phase::Ready) { + FML_LOG(ERROR) << "Isolate is in unexpected phase."; + return; + } + + if (!root_isolate->get()->Run(entrypoint, args, + settings.root_isolate_create_callback)) { + FML_LOG(ERROR) << "Could not run the method \"" << entrypoint + << "\" in the isolate."; + return; + } + + root_isolate->get()->AddIsolateShutdownCallback( + settings.root_isolate_shutdown_callback); + + result = std::move(root_isolate); +} + +static std::unique_ptr RunDartCodeInIsolate( + DartVMRef& vm_ref, + const Settings& settings, + const TaskRunners& task_runners, + std::string entrypoint, + const std::vector& args, + const std::string& fixtures_path, + fml::WeakPtr io_manager = {}) { + std::unique_ptr result; + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask( + task_runners.GetPlatformTaskRunner(), fml::MakeCopyable([&]() mutable { + RunDartCodeInIsolate(vm_ref, result, settings, task_runners, entrypoint, + args, fixtures_path, io_manager); + latch.Signal(); + })); + latch.Wait(); + return result; +} + +} // namespace testing +} // namespace flutter From c81cfd2eac50d1f451b2c809b17e7336bbfc200f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Feb 2020 22:39:55 -0800 Subject: [PATCH 3/8] gn format --- testing/BUILD.gn | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/BUILD.gn b/testing/BUILD.gn index 1845921e50418..ba6bdf7237898 100644 --- a/testing/BUILD.gn +++ b/testing/BUILD.gn @@ -43,11 +43,11 @@ source_set("dart") { testonly = true sources = [ + "dart_isolate_runner.h", "elf_loader.cc", "elf_loader.h", "test_dart_native_resolver.cc", "test_dart_native_resolver.h", - "dart_isolate_runner.h", ] public_deps = [ From f3e51b98daed398b85e204140aca5ec02af8ce36 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 4 Mar 2020 09:12:34 -0800 Subject: [PATCH 4/8] missing config --- lib/ui/BUILD.gn | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index ca05d1b146721..f6d676d2b6f78 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -162,6 +162,8 @@ if (current_toolchain == host_toolchain) { executable("ui_unittests") { testonly = true + configs += [ "//flutter:export_dynamic_symbols" ] + sources = [ "painting/image_decoder_unittests.cc", "window/pointer_data_packet_converter_unittests.cc", From 4bff01039b67fe64b02dd829f56cb2019df0c008 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 4 Mar 2020 09:57:50 -0800 Subject: [PATCH 5/8] docs --- lib/ui/painting/image_decoder_unittests.cc | 13 +++++++++++++ lib/ui/painting/multi_frame_codec.h | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index f9aa07879860d..fa45b9d4e8057 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -614,6 +614,19 @@ TEST(ImageDecoderTest, VerifySubpixelDecodingPreservesExifOrientation) { TEST_F(ImageDecoderFixtureTest, MultiFrameCodecCanBeCollectedBeforeIOTasksFinish) { + // This test verifies that the MultiFrameCodec safely shares state between + // tasks on the IO and UI runners, and does not allow unsafe memory access if + // the UI object is collected while the IO thread still has pending decode + // work. This could happen in a real application if the engine is collected + // while a multi-frame image is decoding. To exercise this, the test: + // - Starts a Dart VM + // - Latches the IO task runner + // - Create a MultiFrameCodec for an animated gif pointed to a callback + // in the Dart fixture + // - Calls getNextFrame on the UI task runner + // - Collects the MultiFrameCodec object before unlatching the IO task + // runner. + // - Unlatches the IO task runner auto settings = CreateSettingsForFixture(); settings.leak_vm = false; settings.enable_observatory = false; diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index a91765a1ea89d..006e4b68f2681 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -26,6 +26,15 @@ class MultiFrameCodec : public Codec { Dart_Handle getNextFrame(Dart_Handle args) override; private: + // Captures the state shared between the IO and UI task runners. + // + // The state is initialized on the UI task runner when the Dart object is + // created. Decoding occurs on the IO task runner. Since it is possible for + // the UI object to be collected independently of the IO task runner work, + // it is not safe for this state to live directly on the MultiFrameCodec. + // Instead, the MultiFrameCodec creates this object when it is constructed, + // shares it with the IO task runner's decoding work, and sets the live_ + // member to false when it is destructed. struct State { State(std::unique_ptr codec); @@ -33,10 +42,14 @@ class MultiFrameCodec : public Codec { const int frameCount_; const int repetitionCount_; int nextFrameIndex_; + + // Indicates whether the Dart object has been collected on the UI task + // runner. std::atomic live_; // The last decoded frame that's required to decode any subsequent frames. std::unique_ptr lastRequiredFrame_; + // The index of the last decoded required frame. int lastRequiredFrameIndex_ = -1; From f5bbcddd8a6d821efd52e5d7421fc78cb3e85460 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 5 Mar 2020 17:02:50 -0800 Subject: [PATCH 6/8] Review --- lib/ui/painting/image_decoder_unittests.cc | 9 ++---- lib/ui/painting/multi_frame_codec.cc | 32 ++++++++++------------ lib/ui/painting/multi_frame_codec.h | 10 +++---- 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index fa45b9d4e8057..651b59fea6d83 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -37,6 +37,7 @@ class ImageDecoderFixtureTest : public ThreadTest { settings.isolate_create_callback = [this]() { native_resolver_->SetNativeResolverForIsolate(); }; + settings.enable_observatory = false; SetSnapshotsAndAssets(settings); return settings; } @@ -57,7 +58,7 @@ class ImageDecoderFixtureTest : public ThreadTest { // don't need to be explicitly supplied by the embedder. In AOT, these // snapshots will be present in the application AOT dylib. if (DartVM::IsRunningPrecompiledCode()) { - PrepareSettingsForAOTWithSymbols(settings, aot_symbols_); + FML_CHECK(PrepareSettingsForAOTWithSymbols(settings, aot_symbols_)); } else { settings.application_kernels = [this]() { std::vector> kernel_mappings; @@ -628,8 +629,6 @@ TEST_F(ImageDecoderFixtureTest, // runner. // - Unlatches the IO task runner auto settings = CreateSettingsForFixture(); - settings.leak_vm = false; - settings.enable_observatory = false; auto vm_ref = DartVMRef::Create(settings); auto vm_data = vm_ref.GetVMData(); @@ -640,7 +639,6 @@ TEST_F(ImageDecoderFixtureTest, auto gif_codec = SkCodec::MakeFromData(gif_mapping); ASSERT_TRUE(gif_codec); - auto loop = fml::ConcurrentMessageLoop::Create(); TaskRunners runners(GetCurrentTestName(), // label CreateNewThread("platform"), // platform CreateNewThread("gpu"), // gpu @@ -670,8 +668,7 @@ TEST_F(ImageDecoderFixtureTest, fml::AutoResetWaitableEvent isolate_latch; fml::RefPtr codec; EXPECT_TRUE(isolate->RunInIsolateScope([&]() -> bool { - Dart_Handle library = Dart_LookupLibrary(Dart_NewStringFromCString( - "package:engine_root/ui/fixtures/ui_test.dart")); + Dart_Handle library = Dart_RootLibrary(); if (Dart_IsError(library)) { isolate_latch.Signal(); return false; diff --git a/lib/ui/painting/multi_frame_codec.cc b/lib/ui/painting/multi_frame_codec.cc index 4013dc4d156fa..ae2b4bdb83f02 100644 --- a/lib/ui/painting/multi_frame_codec.cc +++ b/lib/ui/painting/multi_frame_codec.cc @@ -14,16 +14,13 @@ namespace flutter { MultiFrameCodec::MultiFrameCodec(std::unique_ptr codec) : state_(new State(std::move(codec))) {} -MultiFrameCodec::~MultiFrameCodec() { - state_->live_ = false; -} +MultiFrameCodec::~MultiFrameCodec() = default; MultiFrameCodec::State::State(std::unique_ptr codec) : codec_(std::move(codec)), frameCount_(codec_->getFrameCount()), repetitionCount_(codec_->getRepetitionCount()), - nextFrameIndex_(0), - live_(true) {} + nextFrameIndex_(0) {} static void InvokeNextFrameCallback( fml::RefPtr frameInfo, @@ -141,13 +138,6 @@ void MultiFrameCodec::State::GetNextFrameAndInvokeCallback( fml::RefPtr unref_queue, size_t trace_id) { fml::RefPtr frameInfo = NULL; - if (!live_) { - ui_task_runner->PostTask(fml::MakeCopyable( - [callback = std::move(callback), frameInfo, trace_id]() mutable { - InvokeNextFrameCallback(frameInfo, std::move(callback), trace_id); - })); - return; - } sk_sp skImage = GetNextFrameImage(resourceContext); if (skImage) { fml::RefPtr image = CanvasImage::Create(); @@ -177,12 +167,18 @@ Dart_Handle MultiFrameCodec::getNextFrame(Dart_Handle callback_handle) { const auto& task_runners = dart_state->GetTaskRunners(); - task_runners.GetIOTaskRunner()->PostTask( - fml::MakeCopyable([callback = std::make_unique( - tonic::DartState::Current(), callback_handle), - state = state_, trace_id, - ui_task_runner = task_runners.GetUITaskRunner(), - io_manager = dart_state->GetIOManager()]() mutable { + task_runners.GetIOTaskRunner()->PostTask(fml::MakeCopyable( + [callback = std::make_unique( + tonic::DartState::Current(), callback_handle), + weak_state = std::weak_ptr(state_), trace_id, + ui_task_runner = task_runners.GetUITaskRunner(), + io_manager = dart_state->GetIOManager()]() mutable { + auto state = weak_state.lock(); + if (!state) { + ui_task_runner->PostTask(fml::MakeCopyable( + [callback = std::move(callback)]() { callback->Clear(); })); + return; + } state->GetNextFrameAndInvokeCallback( std::move(callback), std::move(ui_task_runner), io_manager->GetResourceContext(), io_manager->GetSkiaUnrefQueue(), diff --git a/lib/ui/painting/multi_frame_codec.h b/lib/ui/painting/multi_frame_codec.h index 006e4b68f2681..3ca317cbacadb 100644 --- a/lib/ui/painting/multi_frame_codec.h +++ b/lib/ui/painting/multi_frame_codec.h @@ -41,12 +41,11 @@ class MultiFrameCodec : public Codec { const std::unique_ptr codec_; const int frameCount_; const int repetitionCount_; - int nextFrameIndex_; - - // Indicates whether the Dart object has been collected on the UI task - // runner. - std::atomic live_; + // The non-const members and functions below here are only read or written + // to on the IO thread. They are not safe to access or write on the UI + // thread. + int nextFrameIndex_; // The last decoded frame that's required to decode any subsequent frames. std::unique_ptr lastRequiredFrame_; @@ -54,6 +53,7 @@ class MultiFrameCodec : public Codec { int lastRequiredFrameIndex_ = -1; sk_sp GetNextFrameImage(fml::WeakPtr resourceContext); + void GetNextFrameAndInvokeCallback( std::unique_ptr callback, fml::RefPtr ui_task_runner, From 7438df158a8e914aee9ebdaa0610f0b594fbc872 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Thu, 5 Mar 2020 17:13:20 -0800 Subject: [PATCH 7/8] move to own TU --- lib/ui/BUILD.gn | 2 + lib/ui/painting/image_decoder_test.cc | 53 ++++++++++++++++++++++ lib/ui/painting/image_decoder_test.h | 39 ++++++++++++++++ lib/ui/painting/image_decoder_unittests.cc | 53 +--------------------- 4 files changed, 95 insertions(+), 52 deletions(-) create mode 100644 lib/ui/painting/image_decoder_test.cc create mode 100644 lib/ui/painting/image_decoder_test.h diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index f6d676d2b6f78..235c1acdefdf9 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -165,6 +165,8 @@ if (current_toolchain == host_toolchain) { configs += [ "//flutter:export_dynamic_symbols" ] sources = [ + "painting/image_decoder_test.cc", + "painting/image_decoder_test.h", "painting/image_decoder_unittests.cc", "window/pointer_data_packet_converter_unittests.cc", ] diff --git a/lib/ui/painting/image_decoder_test.cc b/lib/ui/painting/image_decoder_test.cc new file mode 100644 index 0000000000000..6075856e7f38e --- /dev/null +++ b/lib/ui/painting/image_decoder_test.cc @@ -0,0 +1,53 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/lib/ui/painting/image_decoder_test.h" + +namespace flutter { +namespace testing { + +ImageDecoderFixtureTest::ImageDecoderFixtureTest() + : native_resolver_(std::make_shared()), + assets_dir_(fml::OpenDirectory(GetFixturesPath(), + false, + fml::FilePermission::kRead)), + aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary()) {} + +Settings ImageDecoderFixtureTest::CreateSettingsForFixture() { + Settings settings; + settings.leak_vm = false; + settings.task_observer_add = [](intptr_t, fml::closure) {}; + settings.task_observer_remove = [](intptr_t) {}; + settings.isolate_create_callback = [this]() { + native_resolver_->SetNativeResolverForIsolate(); + }; + settings.enable_observatory = false; + SetSnapshotsAndAssets(settings); + return settings; +} + +void ImageDecoderFixtureTest::SetSnapshotsAndAssets(Settings& settings) { + if (!assets_dir_.is_valid()) { + return; + } + + settings.assets_dir = assets_dir_.get(); + + // In JIT execution, all snapshots are present within the binary itself and + // don't need to be explicitly supplied by the embedder. In AOT, these + // snapshots will be present in the application AOT dylib. + if (DartVM::IsRunningPrecompiledCode()) { + FML_CHECK(PrepareSettingsForAOTWithSymbols(settings, aot_symbols_)); + } else { + settings.application_kernels = [this]() { + std::vector> kernel_mappings; + kernel_mappings.emplace_back( + fml::FileMapping::CreateReadOnly(assets_dir_, "kernel_blob.bin")); + return kernel_mappings; + }; + } +} + +} // namespace testing +} // namespace flutter diff --git a/lib/ui/painting/image_decoder_test.h b/lib/ui/painting/image_decoder_test.h new file mode 100644 index 0000000000000..5b7c58f49953e --- /dev/null +++ b/lib/ui/painting/image_decoder_test.h @@ -0,0 +1,39 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_LIB_UI_PAINTING_IMAGE_DECODER_TEST_H_ +#define FLUTTER_LIB_UI_PAINTING_IMAGE_DECODER_TEST_H_ + +#include + +#include "flutter/common/settings.h" +#include "flutter/runtime/dart_vm.h" +#include "flutter/testing/elf_loader.h" +#include "flutter/testing/test_dart_native_resolver.h" +#include "flutter/testing/testing.h" +#include "flutter/testing/thread_test.h" + +namespace flutter { +namespace testing { + +class ImageDecoderFixtureTest : public ThreadTest { + public: + ImageDecoderFixtureTest(); + + Settings CreateSettingsForFixture(); + + private: + std::shared_ptr native_resolver_; + fml::UniqueFD assets_dir_; + ELFAOTSymbols aot_symbols_; + + void SetSnapshotsAndAssets(Settings& settings); + + FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoderFixtureTest); +}; + +} // namespace testing +} // namespace flutter + +#endif // FLUTTER_LIB_UI_PAINTING_IMAGE_DECODER_TEST_H_ diff --git a/lib/ui/painting/image_decoder_unittests.cc b/lib/ui/painting/image_decoder_unittests.cc index 651b59fea6d83..94c008e2959d2 100644 --- a/lib/ui/painting/image_decoder_unittests.cc +++ b/lib/ui/painting/image_decoder_unittests.cc @@ -6,6 +6,7 @@ #include "flutter/fml/mapping.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/painting/image_decoder.h" +#include "flutter/lib/ui/painting/image_decoder_test.h" #include "flutter/lib/ui/painting/multi_frame_codec.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" @@ -20,58 +21,6 @@ namespace flutter { namespace testing { -class ImageDecoderFixtureTest : public ThreadTest { - public: - ImageDecoderFixtureTest() - : native_resolver_(std::make_shared()), - assets_dir_(fml::OpenDirectory(GetFixturesPath(), - false, - fml::FilePermission::kRead)), - aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary()) {} - - Settings CreateSettingsForFixture() { - Settings settings; - settings.leak_vm = false; - settings.task_observer_add = [](intptr_t, fml::closure) {}; - settings.task_observer_remove = [](intptr_t) {}; - settings.isolate_create_callback = [this]() { - native_resolver_->SetNativeResolverForIsolate(); - }; - settings.enable_observatory = false; - SetSnapshotsAndAssets(settings); - return settings; - } - - private: - std::shared_ptr native_resolver_; - fml::UniqueFD assets_dir_; - ELFAOTSymbols aot_symbols_; - - void SetSnapshotsAndAssets(Settings& settings) { - if (!assets_dir_.is_valid()) { - return; - } - - settings.assets_dir = assets_dir_.get(); - - // In JIT execution, all snapshots are present within the binary itself and - // don't need to be explicitly supplied by the embedder. In AOT, these - // snapshots will be present in the application AOT dylib. - if (DartVM::IsRunningPrecompiledCode()) { - FML_CHECK(PrepareSettingsForAOTWithSymbols(settings, aot_symbols_)); - } else { - settings.application_kernels = [this]() { - std::vector> kernel_mappings; - kernel_mappings.emplace_back( - fml::FileMapping::CreateReadOnly(assets_dir_, "kernel_blob.bin")); - return kernel_mappings; - }; - } - } - - FML_DISALLOW_COPY_AND_ASSIGN(ImageDecoderFixtureTest); -}; - class TestIOManager final : public IOManager { public: TestIOManager(fml::RefPtr task_runner, From 7cac8dc7cb349f89818b7ee57b717769368880ac Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 9 Mar 2020 17:31:44 -0700 Subject: [PATCH 8/8] licenses --- ci/licenses_golden/licenses_flutter | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index d0182f4580ab0..6056147ddc95f 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -306,6 +306,8 @@ FILE: ../../../flutter/lib/ui/painting/image.cc FILE: ../../../flutter/lib/ui/painting/image.h FILE: ../../../flutter/lib/ui/painting/image_decoder.cc FILE: ../../../flutter/lib/ui/painting/image_decoder.h +FILE: ../../../flutter/lib/ui/painting/image_decoder_test.cc +FILE: ../../../flutter/lib/ui/painting/image_decoder_test.h FILE: ../../../flutter/lib/ui/painting/image_decoder_unittests.cc FILE: ../../../flutter/lib/ui/painting/image_encoding.cc FILE: ../../../flutter/lib/ui/painting/image_encoding.h