From d6d02a99b88cb44a7cd72f86b3eedff47cd00ba6 Mon Sep 17 00:00:00 2001 From: garyqian Date: Thu, 19 Nov 2020 17:46:47 -0800 Subject: [PATCH 01/15] INitial code --- runtime/BUILD.gn | 2 + runtime/dart_deferred_load_handler.cc | 38 +++++++++ runtime/dart_deferred_load_handler.h | 33 ++++++++ runtime/dart_isolate.cc | 80 ++++++++++++++---- runtime/dart_isolate.h | 16 +++- runtime/dart_isolate_unittests.cc | 76 +++++++++-------- runtime/dart_lifecycle_unittests.cc | 26 +++--- runtime/runtime_controller.cc | 115 ++++++++++++++++++++++---- runtime/runtime_controller.h | 35 ++++++++ runtime/runtime_delegate.h | 2 + 10 files changed, 338 insertions(+), 85 deletions(-) create mode 100644 runtime/dart_deferred_load_handler.cc create mode 100644 runtime/dart_deferred_load_handler.h diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index 9788794f7da83..09207b330c3c4 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -37,6 +37,8 @@ group("libdart") { source_set("runtime") { sources = [ + "dart_deferred_load_handler.cc", + "dart_deferred_load_handler.h", "dart_isolate.cc", "dart_isolate.h", "dart_isolate_group_data.cc", diff --git a/runtime/dart_deferred_load_handler.cc b/runtime/dart_deferred_load_handler.cc new file mode 100644 index 0000000000000..0f203ee4937b9 --- /dev/null +++ b/runtime/dart_deferred_load_handler.cc @@ -0,0 +1,38 @@ +// Copyright 2020 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/runtime/dart_deferred_load_handler.h" + +#include "flutter/runtime/runtime_controller.h" + +namespace flutter { + +void* DartDeferredLoadHandler::runtime_controller_ = nullptr; + +Dart_DeferredLoadHandler + DartDeferredLoadHandler::empty_dart_deferred_load_handler = + &DartDeferredLoadHandler::EmptyDartLoadLibrary; + +Dart_DeferredLoadHandler DartDeferredLoadHandler::dart_deferred_load_handler = + &DartDeferredLoadHandler::OnDartLoadLibrary; + +void DartDeferredLoadHandler::RegisterRuntimeController( + void* runtime_controller) { + runtime_controller_ = runtime_controller; +} + +Dart_Handle DartDeferredLoadHandler::OnDartLoadLibrary( + intptr_t loading_unit_id) { + if (runtime_controller_ != nullptr) + return static_cast(runtime_controller_) + ->OnDartLoadLibrary(loading_unit_id); + return Dart_Null(); +} + +Dart_Handle DartDeferredLoadHandler::EmptyDartLoadLibrary( + intptr_t loading_unit_id) { + return Dart_Null(); +} + +} // namespace flutter diff --git a/runtime/dart_deferred_load_handler.h b/runtime/dart_deferred_load_handler.h new file mode 100644 index 0000000000000..a6947570ea132 --- /dev/null +++ b/runtime/dart_deferred_load_handler.h @@ -0,0 +1,33 @@ +// Copyright 2020 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_RUNTIME_DART_DEFERRED_LOAD_HANDLER_H_ +#define FLUTTER_RUNTIME_DART_DEFERRED_LOAD_HANDLER_H_ + +#include +#include + +#include "third_party/dart/runtime/include/dart_api.h" + +namespace flutter { + +class DartDeferredLoadHandler { + public: + // TODO: Fix deps to not use void* + static void RegisterRuntimeController(void* runtime_controller); + + static Dart_DeferredLoadHandler empty_dart_deferred_load_handler; + static Dart_DeferredLoadHandler dart_deferred_load_handler; + + private: + static Dart_Handle OnDartLoadLibrary(intptr_t loading_unit_id); + // No-op function that returns Dart_Null() for when the isolate is not + // expected to handle deferred libraries. + static Dart_Handle EmptyDartLoadLibrary(intptr_t loading_unit_id); + static void* runtime_controller_; +}; + +} // namespace flutter + +#endif // FLUTTER_RUNTIME_DART_DEFERRED_LOAD_HANDLER_H_ diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 8a6215110e426..c40d897252ddc 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "flutter/runtime/dart_isolate.h" +#include "flutter/runtime/dart_deferred_load_handler.h" #include #include @@ -86,6 +87,7 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags isolate_flags, + Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, std::optional dart_entrypoint, @@ -116,6 +118,7 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( advisory_script_uri, // advisory_script_entrypoint, // isolate_flags, // + deferred_load_handler, // isolate_create_callback, // isolate_shutdown_callback // ) @@ -149,8 +152,8 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( } if (settings.root_isolate_create_callback) { - // Isolate callbacks always occur in isolate scope and before user code has - // had a chance to run. + // Isolate callbacks always occur in isolate scope and before user code + // has had a chance to run. tonic::DartState::Scope scope(isolate.get()); settings.root_isolate_create_callback(*isolate.get()); } @@ -186,6 +189,7 @@ std::weak_ptr DartIsolate::CreateRootIsolate( std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, + Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback) { TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate"); @@ -222,7 +226,7 @@ std::weak_ptr DartIsolate::CreateRootIsolate( auto isolate_flags = flags.Get(); Dart_Isolate vm_isolate = CreateDartIsolateGroup( std::move(isolate_group_data), std::move(isolate_data), &isolate_flags, - error.error()); + deferred_load_handler, error.error()); if (error) { FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error.str(); @@ -288,7 +292,8 @@ std::string DartIsolate::GetServiceId() { return service_id; } -bool DartIsolate::Initialize(Dart_Isolate dart_isolate) { +bool DartIsolate::Initialize(Dart_Isolate dart_isolate, + Dart_DeferredLoadHandler& deferred_load_handler) { TRACE_EVENT0("flutter", "DartIsolate::Initialize"); if (phase_ != Phase::Uninitialized) { return false; @@ -320,6 +325,9 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate) { return false; } + // Dart_SetDeferredLoadHandler(&DartDeferredLoadHandler); + Dart_SetDeferredLoadHandler(deferred_load_handler); + if (!UpdateThreadPoolNames()) { return false; } @@ -332,6 +340,33 @@ fml::RefPtr DartIsolate::GetMessageHandlingTaskRunner() const { return message_handling_task_runner_; } +bool DartIsolate::LoadLoadingUnit(intptr_t loading_unit_id, + const uint8_t* snapshot_data, + const uint8_t* snapshot_instructions) { + tonic::DartState::Scope scope(this); + Dart_Handle result = Dart_DeferredLoadComplete(loading_unit_id, snapshot_data, + snapshot_instructions); + if (Dart_IsApiError(result)) { + FML_LOG(ERROR) << "LOADING FAILED " << loading_unit_id; + result = + Dart_DeferredLoadCompleteError(loading_unit_id, Dart_GetError(result), + /*transient*/ true); + return false; + } + return true; +} + +void DartIsolate::LoadLoadingUnitFailure(intptr_t loading_unit_id, + const std::string error_message, + bool transient) { + tonic::DartState::Scope scope(this); + Dart_Handle result = Dart_DeferredLoadCompleteError( + loading_unit_id, error_message.c_str(), transient); + if (Dart_IsApiError(result)) { + FML_LOG(ERROR) << "Dart error: " << Dart_GetError(result); + } +} + void DartIsolate::SetMessageHandlingTaskRunner( fml::RefPtr runner) { if (!IsRootIsolate() || !runner) { @@ -649,8 +684,8 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, bool DartIsolate::Shutdown() { TRACE_EVENT0("flutter", "DartIsolate::Shutdown"); // This call may be re-entrant since Dart_ShutdownIsolate can invoke the - // cleanup callback which deletes the embedder side object of the dart isolate - // (a.k.a. this). + // cleanup callback which deletes the embedder side object of the dart + // isolate (a.k.a. this). if (phase_ == Phase::Shutdown) { return false; } @@ -716,8 +751,9 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( DART_VM_SERVICE_ISOLATE_NAME, // script uri DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint DartIsolate::Flags{flags}, // flags - nullptr, // isolate create callback - nullptr // isolate shutdown callback + DartDeferredLoadHandler::empty_dart_deferred_load_handler, + nullptr, // isolate create callback + nullptr // isolate shutdown callback ); std::shared_ptr service_isolate = weak_service_isolate.lock(); @@ -780,8 +816,9 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( // The VM attempts to start the VM service for us on |Dart_Initialize|. In // such a case, the callback data will be null and the script URI will be // DART_VM_SERVICE_ISOLATE_NAME. In such cases, we just create the service - // isolate like normal but dont hold a reference to it at all. We also start - // this isolate since we will never again reference it from the engine. + // isolate like normal but dont hold a reference to it at all. We also + // start this isolate since we will never again reference it from the + // engine. return DartCreateAndStartServiceIsolate(package_root, // package_config, // flags, // @@ -826,7 +863,8 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( false))); // is_root_isolate Dart_Isolate vm_isolate = CreateDartIsolateGroup( - std::move(isolate_group_data), std::move(isolate_data), flags, error); + std::move(isolate_group_data), std::move(isolate_data), flags, + DartDeferredLoadHandler::empty_dart_deferred_load_handler, error); if (*error) { FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error; @@ -871,7 +909,9 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data, false))); // is_root_isolate // root isolate should have been created via CreateRootIsolate - if (!InitializeIsolate(*embedder_isolate, isolate, error)) { + if (!InitializeIsolate( + *embedder_isolate, isolate, + DartDeferredLoadHandler::empty_dart_deferred_load_handler, error)) { return false; } @@ -887,6 +927,7 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( std::unique_ptr> isolate_group_data, std::unique_ptr> isolate_data, Dart_IsolateFlags* flags, + Dart_DeferredLoadHandler& deferred_load_handler, char** error) { TRACE_EVENT0("flutter", "DartIsolate::CreateDartIsolateGroup"); @@ -902,12 +943,14 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( return nullptr; } - // Ownership of the isolate data objects has been transferred to the Dart VM. + // Ownership of the isolate data objects has been transferred to the Dart + // VM. std::shared_ptr embedder_isolate(*isolate_data); isolate_group_data.release(); isolate_data.release(); - if (!InitializeIsolate(std::move(embedder_isolate), isolate, error)) { + if (!InitializeIsolate(std::move(embedder_isolate), isolate, + deferred_load_handler, error)) { return nullptr; } @@ -917,9 +960,10 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( bool DartIsolate::InitializeIsolate( std::shared_ptr embedder_isolate, Dart_Isolate isolate, + Dart_DeferredLoadHandler& deferred_load_handler, char** error) { TRACE_EVENT0("flutter", "DartIsolate::InitializeIsolate"); - if (!embedder_isolate->Initialize(isolate)) { + if (!embedder_isolate->Initialize(isolate, deferred_load_handler)) { *error = fml::strdup("Embedder could not initialize the Dart isolate."); FML_DLOG(ERROR) << *error; return false; @@ -932,9 +976,9 @@ bool DartIsolate::InitializeIsolate( return false; } - // Root isolates will be setup by the engine and the service isolate (which is - // also a root isolate) by the utility routines in the VM. However, secondary - // isolates will be run by the VM if they are marked as runnable. + // Root isolates will be setup by the engine and the service isolate (which + // is also a root isolate) by the utility routines in the VM. However, + // secondary isolates will be run by the VM if they are marked as runnable. if (!embedder_isolate->IsRootIsolate()) { auto child_isolate_preparer = embedder_isolate->GetIsolateGroupData().GetChildIsolatePreparer(); diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index 5e6913d52b185..ca9ea2b80ad52 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -225,6 +225,7 @@ class DartIsolate : public UIDartState { std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, + Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, std::optional dart_entrypoint, @@ -384,6 +385,14 @@ class DartIsolate : public UIDartState { /// fml::RefPtr GetMessageHandlingTaskRunner() const; + bool LoadLoadingUnit(intptr_t loading_unit_id, + const uint8_t* snapshot_data, + const uint8_t* snapshot_instructions); + + void LoadLoadingUnitFailure(intptr_t loading_unit_id, + const std::string error_message, + bool transient); + private: friend class IsolateConfiguration; class AutoFireClosure { @@ -418,6 +427,7 @@ class DartIsolate : public UIDartState { std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, + Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback); @@ -432,7 +442,9 @@ class DartIsolate : public UIDartState { std::string advisory_script_entrypoint, bool is_root_isolate); - [[nodiscard]] bool Initialize(Dart_Isolate isolate); + [[nodiscard]] bool Initialize( + Dart_Isolate isolate, + Dart_DeferredLoadHandler& deferred_load_handler); void SetMessageHandlingTaskRunner(fml::RefPtr runner); @@ -472,10 +484,12 @@ class DartIsolate : public UIDartState { std::unique_ptr> isolate_group_data, std::unique_ptr> isolate_data, Dart_IsolateFlags* flags, + Dart_DeferredLoadHandler& deferred_load_handler, char** error); static bool InitializeIsolate(std::shared_ptr embedder_isolate, Dart_Isolate isolate, + Dart_DeferredLoadHandler& deferred_load_handler, char** error); // |Dart_IsolateShutdownCallback| diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index a90599fd8a3fb..be62acbabd811 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -8,6 +8,7 @@ #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/thread.h" +#include "flutter/runtime/dart_deferred_load_handler.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/isolate_configuration.h" @@ -52,18 +53,19 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) { IsolateConfiguration::InferFromSettings(settings); auto weak_isolate = DartIsolate::CreateRunningRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - std::move(task_runners), // task runners - nullptr, // window - {}, // snapshot delegate - {}, // hint freed delegate - {}, // io manager - {}, // unref queue - {}, // image decoder - "main.dart", // advisory uri - "main", // advisory entrypoint, - DartIsolate::Flags{}, // flags + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint, + DartIsolate::Flags{}, // flags + DartDeferredLoadHandler::empty_dart_deferred_load_handler, settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint @@ -92,18 +94,19 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { auto isolate_configuration = IsolateConfiguration::InferFromSettings(settings); auto weak_isolate = DartIsolate::CreateRunningRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - std::move(task_runners), // task runners - nullptr, // window - {}, // snapshot delegate - {}, // hint freed delegate - {}, // io manager - {}, // unref queue - {}, // image decoder - "main.dart", // advisory uri - "main", // advisory entrypoint - DartIsolate::Flags{}, // flags + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint + DartIsolate::Flags{}, // flags + DartDeferredLoadHandler::empty_dart_deferred_load_handler, settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint @@ -350,18 +353,19 @@ TEST_F(DartIsolateTest, CanCreateServiceIsolate) { auto isolate_configuration = IsolateConfiguration::InferFromSettings(settings); auto weak_isolate = DartIsolate::CreateRunningRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - std::move(task_runners), // task runners - nullptr, // window - {}, // snapshot delegate - {}, // hint freed delegate - {}, // io manager - {}, // unref queue - {}, // image decoder - "main.dart", // advisory uri - "main", // advisory entrypoint, - DartIsolate::Flags{}, // flags + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint, + DartIsolate::Flags{}, // flags + DartDeferredLoadHandler::empty_dart_deferred_load_handler, settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint diff --git a/runtime/dart_lifecycle_unittests.cc b/runtime/dart_lifecycle_unittests.cc index 8576b3fb11292..793247654a47d 100644 --- a/runtime/dart_lifecycle_unittests.cc +++ b/runtime/dart_lifecycle_unittests.cc @@ -6,6 +6,7 @@ #include "flutter/fml/paths.h" #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/runtime/dart_deferred_load_handler.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/isolate_configuration.h" @@ -57,18 +58,19 @@ static std::shared_ptr CreateAndRunRootIsolate( auto isolate = DartIsolate::CreateRunningRootIsolate( - vm.GetSettings(), // settings - vm.GetIsolateSnapshot(), // isolate_snapshot - runners, // task_runners - {}, // window - {}, // snapshot_delegate - {}, // hint_freed_delegate - {}, // io_manager - {}, // unref_queue - {}, // image_decoder - "main.dart", // advisory_script_uri - entrypoint.c_str(), // advisory_script_entrypoint - DartIsolate::Flags{}, // flags + vm.GetSettings(), // settings + vm.GetIsolateSnapshot(), // isolate_snapshot + runners, // task_runners + {}, // window + {}, // snapshot_delegate + {}, // hint_freed_delegate + {}, // io_manager + {}, // unref_queue + {}, // image_decoder + "main.dart", // advisory_script_uri + entrypoint.c_str(), // advisory_script_entrypoint + DartIsolate::Flags{}, // flags + DartDeferredLoadHandler::empty_dart_deferred_load_handler, settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback, entrypoint, // dart entrypoint diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 3551e1967134e..185f451189c39 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -4,6 +4,9 @@ #include "flutter/runtime/runtime_controller.h" +#include +#include + #include "flutter/fml/message_loop.h" #include "flutter/fml/trace_event.h" #include "flutter/lib/ui/compositing/scene.h" @@ -11,6 +14,7 @@ #include "flutter/lib/ui/window/platform_configuration.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/lib/ui/window/window.h" +#include "flutter/runtime/dart_deferred_load_handler.h" #include "flutter/runtime/isolate_configuration.h" #include "flutter/runtime/runtime_delegate.h" #include "third_party/tonic/dart_message_handler.h" @@ -53,7 +57,9 @@ RuntimeController::RuntimeController( platform_data_(std::move(p_platform_data)), isolate_create_callback_(p_isolate_create_callback), isolate_shutdown_callback_(p_isolate_shutdown_callback), - persistent_isolate_data_(std::move(p_persistent_isolate_data)) {} + persistent_isolate_data_(std::move(p_persistent_isolate_data)) { + DartDeferredLoadHandler::RegisterRuntimeController(this); +} RuntimeController::~RuntimeController() { FML_DCHECK(Dart_CurrentIsolate() == nullptr); @@ -353,23 +359,24 @@ bool RuntimeController::LaunchRootIsolate( auto strong_root_isolate = DartIsolate::CreateRunningRootIsolate( - settings, // - isolate_snapshot_, // - task_runners_, // - std::make_unique(this), // - snapshot_delegate_, // - hint_freed_delegate_, // - io_manager_, // - unref_queue_, // - image_decoder_, // - advisory_script_uri_, // - advisory_script_entrypoint_, // - DartIsolate::Flags{}, // - isolate_create_callback_, // - isolate_shutdown_callback_, // - dart_entrypoint, // - dart_entrypoint_library, // - std::move(isolate_configuration) // + settings, // + isolate_snapshot_, // + task_runners_, // + std::make_unique(this), // + snapshot_delegate_, // + hint_freed_delegate_, // + io_manager_, // + unref_queue_, // + image_decoder_, // + advisory_script_uri_, // + advisory_script_entrypoint_, // + DartIsolate::Flags{}, // + DartDeferredLoadHandler::dart_deferred_load_handler, // + isolate_create_callback_, // + isolate_shutdown_callback_, // + dart_entrypoint, // + dart_entrypoint_library, // + std::move(isolate_configuration) // ) .lock(); @@ -415,6 +422,78 @@ std::optional RuntimeController::GetRootIsolateReturnCode() { return root_isolate_return_code_; } +void RuntimeController::CompleteDartLoadLibrary( + intptr_t loading_unit_id, + std::string lib_name, + std::vector& apkPaths, + std::string abi) { + std::vector searchPaths; + for (std::string apkPath : apkPaths) { + searchPaths.push_back(apkPath + "!/lib/" + abi + "/" + lib_name); + } + + // TODO: Switch to using the NativeLibrary class, eg: + // + // fml::RefPtr native_lib = + // fml::NativeLibrary::Create(lib_name.c_str()); + void* handle = nullptr; + while (handle == nullptr && !searchPaths.empty()) { + std::string path = searchPaths.back(); + handle = ::dlopen(path.c_str(), RTLD_NOW); + searchPaths.pop_back(); + if (handle == nullptr) { + FML_LOG(ERROR) << "Opening lib \"" << lib_name + << "\" failed: " + std::string(::dlerror()); + } + } + if (handle == nullptr) { + root_isolate_.lock()->LoadLoadingUnitFailure( + loading_unit_id, "No lib .so found for provided search paths.", true); + return; + } + + uint8_t* isolate_data = + static_cast(::dlsym(handle, DartSnapshot::kIsolateDataSymbol)); + if (isolate_data == nullptr) { + // Mac sometimes requires an underscore prefix. + std::stringstream underscore_symbol_name; + underscore_symbol_name << "_" << DartSnapshot::kIsolateDataSymbol; + isolate_data = static_cast( + ::dlsym(handle, underscore_symbol_name.str().c_str())); + if (isolate_data == nullptr) { + // FML_LOG(ERROR) << "Could not resolve symbol in library: " + // << DartSnapshot::kIsolateDataSymbol; + root_isolate_.lock()->LoadLoadingUnitFailure( + loading_unit_id, "Could not resolve data symbol in library", true); + return; + } + } + uint8_t* isolate_instructions = static_cast( + ::dlsym(handle, DartSnapshot::kIsolateInstructionsSymbol)); + if (isolate_instructions == nullptr) { + // Mac sometimes requires an underscore prefix. + std::stringstream underscore_symbol_name; + underscore_symbol_name << "_" << DartSnapshot::kIsolateInstructionsSymbol; + isolate_instructions = static_cast( + ::dlsym(handle, underscore_symbol_name.str().c_str())); + if (isolate_data == nullptr) { + // FML_LOG(ERROR) << "Could not resolve symbol in library: " + // << DartSnapshot::kIsolateInstructionsSymbol; + root_isolate_.lock()->LoadLoadingUnitFailure( + loading_unit_id, "Could not resolve instructions symbol in library", + true); + return; + } + } + + root_isolate_.lock()->LoadLoadingUnit(loading_unit_id, isolate_data, + isolate_instructions); +} + +Dart_Handle RuntimeController::OnDartLoadLibrary(intptr_t loading_unit_id) { + return client_.OnDartLoadLibrary(loading_unit_id); +} + RuntimeController::Locale::Locale(std::string language_code_, std::string country_code_, std::string script_code_, diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 22941c7388a1b..02fc925403ccc 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -473,6 +473,41 @@ class RuntimeController : public PlatformConfigurationClient { /// std::optional GetRootIsolateReturnCode(); + //-------------------------------------------------------------------------- + /// @brief Loads the dart shared library from disk and into the dart VM + /// based off of the search parameters. When the dart library is + /// loaded successfully, the dart future returned by the + /// originating loadLibrary() call completes. + /// + /// @param[in] loading_unit_id The unique id of the deferred library's + /// loading unit. + /// + /// @param[in] lib_name The file name of the .so shared library + /// file. + /// + /// @param[in] apkPaths The paths of the APKs that may or may not + /// contain the lib_name file. + /// + /// @param[in] abi The abi of the library, eg, arm64-v8a + /// + void CompleteDartLoadLibrary(intptr_t loading_unit_id, + std::string lib_name, + std::vector& apkPaths, + std::string abi); + + //-------------------------------------------------------------------------- + /// @brief Invoked when the dart VM requests that a deferred library + /// be loaded. Notifies the engine that the requested loading + /// unit should be downloaded and loaded. + /// + /// @param[in] loading_unit_id The unique id of the deferred library's + /// loading unit. + /// + /// @return A Dart_Handle that is Dart_Null on success, and a dart error + /// on failure. + /// + Dart_Handle OnDartLoadLibrary(intptr_t loading_unit_id); + protected: /// Constructor for Mocks. RuntimeController(RuntimeDelegate& client, TaskRunners p_task_runners); diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index 55978b4dbc39f..e5a2113c4380d 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -43,6 +43,8 @@ class RuntimeDelegate { ComputePlatformResolvedLocale( const std::vector& supported_locale_data) = 0; + virtual Dart_Handle OnDartLoadLibrary(intptr_t loading_unit_id) = 0; + protected: virtual ~RuntimeDelegate(); }; From 59ff685c2f67a0885928b5466c6e2ce717d1164c Mon Sep 17 00:00:00 2001 From: garyqian Date: Thu, 19 Nov 2020 18:22:37 -0800 Subject: [PATCH 02/15] Update runtime controller API to new engine API --- runtime/runtime_controller.cc | 74 ++++------------------------------- runtime/runtime_controller.h | 42 ++++++++++++-------- runtime/runtime_delegate.h | 2 +- 3 files changed, 34 insertions(+), 84 deletions(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 185f451189c39..97e0b040d4e17 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -422,76 +422,16 @@ std::optional RuntimeController::GetRootIsolateReturnCode() { return root_isolate_return_code_; } -void RuntimeController::CompleteDartLoadLibrary( +void RuntimeController::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::string lib_name, - std::vector& apkPaths, - std::string abi) { - std::vector searchPaths; - for (std::string apkPath : apkPaths) { - searchPaths.push_back(apkPath + "!/lib/" + abi + "/" + lib_name); - } - - // TODO: Switch to using the NativeLibrary class, eg: - // - // fml::RefPtr native_lib = - // fml::NativeLibrary::Create(lib_name.c_str()); - void* handle = nullptr; - while (handle == nullptr && !searchPaths.empty()) { - std::string path = searchPaths.back(); - handle = ::dlopen(path.c_str(), RTLD_NOW); - searchPaths.pop_back(); - if (handle == nullptr) { - FML_LOG(ERROR) << "Opening lib \"" << lib_name - << "\" failed: " + std::string(::dlerror()); - } - } - if (handle == nullptr) { - root_isolate_.lock()->LoadLoadingUnitFailure( - loading_unit_id, "No lib .so found for provided search paths.", true); - return; - } - - uint8_t* isolate_data = - static_cast(::dlsym(handle, DartSnapshot::kIsolateDataSymbol)); - if (isolate_data == nullptr) { - // Mac sometimes requires an underscore prefix. - std::stringstream underscore_symbol_name; - underscore_symbol_name << "_" << DartSnapshot::kIsolateDataSymbol; - isolate_data = static_cast( - ::dlsym(handle, underscore_symbol_name.str().c_str())); - if (isolate_data == nullptr) { - // FML_LOG(ERROR) << "Could not resolve symbol in library: " - // << DartSnapshot::kIsolateDataSymbol; - root_isolate_.lock()->LoadLoadingUnitFailure( - loading_unit_id, "Could not resolve data symbol in library", true); - return; - } - } - uint8_t* isolate_instructions = static_cast( - ::dlsym(handle, DartSnapshot::kIsolateInstructionsSymbol)); - if (isolate_instructions == nullptr) { - // Mac sometimes requires an underscore prefix. - std::stringstream underscore_symbol_name; - underscore_symbol_name << "_" << DartSnapshot::kIsolateInstructionsSymbol; - isolate_instructions = static_cast( - ::dlsym(handle, underscore_symbol_name.str().c_str())); - if (isolate_data == nullptr) { - // FML_LOG(ERROR) << "Could not resolve symbol in library: " - // << DartSnapshot::kIsolateInstructionsSymbol; - root_isolate_.lock()->LoadLoadingUnitFailure( - loading_unit_id, "Could not resolve instructions symbol in library", - true); - return; - } - } - - root_isolate_.lock()->LoadLoadingUnit(loading_unit_id, isolate_data, - isolate_instructions); + const uint8_t* snapshot_data, + const uint8_t* snapshot_instructions) { + root_isolate_.lock()->LoadLoadingUnit(loading_unit_id, snapshot_data, + snapshot_instructions); } -Dart_Handle RuntimeController::OnDartLoadLibrary(intptr_t loading_unit_id) { - return client_.OnDartLoadLibrary(loading_unit_id); +void RuntimeController::RequestDartDeferredLibrary(intptr_t loading_unit_id) { + return client_.RequestDartDeferredLibrary(loading_unit_id); } RuntimeController::Locale::Locale(std::string language_code_, diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 02fc925403ccc..481e4eb22fa01 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -474,26 +474,36 @@ class RuntimeController : public PlatformConfigurationClient { std::optional GetRootIsolateReturnCode(); //-------------------------------------------------------------------------- - /// @brief Loads the dart shared library from disk and into the dart VM - /// based off of the search parameters. When the dart library is - /// loaded successfully, the dart future returned by the - /// originating loadLibrary() call completes. + /// @brief Loads the Dart shared library into the Dart VM. When the + /// Dart library is loaded successfully, the Dart future + /// returned by the originating loadLibrary() call completes. /// - /// @param[in] loading_unit_id The unique id of the deferred library's - /// loading unit. + /// The Dart compiler may generate separate shared libraries + /// files called 'loading units' when libraries are imported + /// as deferred. Each of these shared libraries are identified + /// by a unique loading unit id. Callers should dlopen the + /// shared library file and use dlsym to resolve the dart + /// symbols. These symbols can then be passed to this method to + /// be dynamically loaded into the VM. + /// + /// This method is paired with a RequestDartDeferredLibrary + /// invocation that provides the embedder with the loading unit id + /// of the deferred library to load. /// - /// @param[in] lib_name The file name of the .so shared library - /// file. /// - /// @param[in] apkPaths The paths of the APKs that may or may not - /// contain the lib_name file. + /// @param[in] loading_unit_id The unique id of the deferred library's + /// loading unit, as passed in by + /// RequestDartDeferredLibrary. + /// + /// @param[in] snapshot_data Dart snapshot data of the loading unit's + /// shared library. /// - /// @param[in] abi The abi of the library, eg, arm64-v8a + /// @param[in] snapshot_data Dart snapshot instructions of the loading + /// unit's shared library. /// - void CompleteDartLoadLibrary(intptr_t loading_unit_id, - std::string lib_name, - std::vector& apkPaths, - std::string abi); + void LoadDartDeferredLibrary(intptr_t loading_unit_id, + const uint8_t* snapshot_data, + const uint8_t* snapshot_instructions); //-------------------------------------------------------------------------- /// @brief Invoked when the dart VM requests that a deferred library @@ -506,7 +516,7 @@ class RuntimeController : public PlatformConfigurationClient { /// @return A Dart_Handle that is Dart_Null on success, and a dart error /// on failure. /// - Dart_Handle OnDartLoadLibrary(intptr_t loading_unit_id); + void RequestDartDeferredLibrary(intptr_t loading_unit_id); protected: /// Constructor for Mocks. diff --git a/runtime/runtime_delegate.h b/runtime/runtime_delegate.h index e5a2113c4380d..d2e5e9ff14caf 100644 --- a/runtime/runtime_delegate.h +++ b/runtime/runtime_delegate.h @@ -43,7 +43,7 @@ class RuntimeDelegate { ComputePlatformResolvedLocale( const std::vector& supported_locale_data) = 0; - virtual Dart_Handle OnDartLoadLibrary(intptr_t loading_unit_id) = 0; + virtual void RequestDartDeferredLibrary(intptr_t loading_unit_id) = 0; protected: virtual ~RuntimeDelegate(); From 3a15e428153d49583cdc2ed4026b652c8d09f997 Mon Sep 17 00:00:00 2001 From: garyqian Date: Thu, 19 Nov 2020 20:43:58 -0800 Subject: [PATCH 03/15] Compiles --- runtime/dart_deferred_load_handler.cc | 4 ++-- shell/common/engine.cc | 15 ++++++--------- shell/common/engine.h | 5 +---- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/runtime/dart_deferred_load_handler.cc b/runtime/dart_deferred_load_handler.cc index 0f203ee4937b9..5ba967be3956d 100644 --- a/runtime/dart_deferred_load_handler.cc +++ b/runtime/dart_deferred_load_handler.cc @@ -25,8 +25,8 @@ void DartDeferredLoadHandler::RegisterRuntimeController( Dart_Handle DartDeferredLoadHandler::OnDartLoadLibrary( intptr_t loading_unit_id) { if (runtime_controller_ != nullptr) - return static_cast(runtime_controller_) - ->OnDartLoadLibrary(loading_unit_id); + static_cast(runtime_controller_) + ->RequestDartDeferredLibrary(loading_unit_id); return Dart_Null(); } diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 1b9c02413138b..baeae7f78f1a1 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -507,20 +507,17 @@ const std::string& Engine::GetLastEntrypointLibrary() const { return last_entry_point_library_; } -// The Following commented out code connects into part 2 of the split AOT -// feature. Left commented out until it lands: - -// // |RuntimeDelegate| -// void Engine::RequestDartDeferredLibrary(intptr_t loading_unit_id) { -// return delegate_.RequestDartDeferredLibrary(loading_unit_id); -// } +// |RuntimeDelegate| +void Engine::RequestDartDeferredLibrary(intptr_t loading_unit_id) { + return delegate_.RequestDartDeferredLibrary(loading_unit_id); +} void Engine::LoadDartDeferredLibrary(intptr_t loading_unit_id, const uint8_t* snapshot_data, const uint8_t* snapshot_instructions) { if (runtime_controller_->IsRootIsolateRunning()) { - // runtime_controller_->LoadDartDeferredLibrary(loading_unit_id, - // snapshot_data, snapshot_instructions); + runtime_controller_->LoadDartDeferredLibrary(loading_unit_id, snapshot_data, + snapshot_instructions); } } diff --git a/shell/common/engine.h b/shell/common/engine.h index 93a5f2367dbc3..c2bee1a0709ee 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -862,11 +862,8 @@ class Engine final : public RuntimeDelegate, std::unique_ptr> ComputePlatformResolvedLocale( const std::vector& supported_locale_data) override; - // The Following commented out code connects into part 2 of the split AOT - // feature. Left commented out until it lands: - // // |RuntimeDelegate| - // void RequestDartDeferredLibrary(intptr_t loading_unit_id) override; + void RequestDartDeferredLibrary(intptr_t loading_unit_id) override; void SetNeedsReportTimings(bool value) override; From a7026f98f484c38905a5a022259fce9d8a2a4351 Mon Sep 17 00:00:00 2001 From: garyqian Date: Mon, 23 Nov 2020 14:15:10 -0800 Subject: [PATCH 04/15] Remove deferred load handler from init --- runtime/dart_isolate.cc | 31 ++++-------- runtime/dart_isolate.h | 8 +-- runtime/dart_isolate_unittests.cc | 76 ++++++++++++++--------------- runtime/dart_lifecycle_unittests.cc | 26 +++++----- runtime/runtime_controller.cc | 35 +++++++------ 5 files changed, 76 insertions(+), 100 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index c40d897252ddc..8190b6e2fab97 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -87,7 +87,6 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags isolate_flags, - Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, std::optional dart_entrypoint, @@ -118,7 +117,6 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( advisory_script_uri, // advisory_script_entrypoint, // isolate_flags, // - deferred_load_handler, // isolate_create_callback, // isolate_shutdown_callback // ) @@ -189,7 +187,6 @@ std::weak_ptr DartIsolate::CreateRootIsolate( std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, - Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback) { TRACE_EVENT0("flutter", "DartIsolate::CreateRootIsolate"); @@ -226,7 +223,7 @@ std::weak_ptr DartIsolate::CreateRootIsolate( auto isolate_flags = flags.Get(); Dart_Isolate vm_isolate = CreateDartIsolateGroup( std::move(isolate_group_data), std::move(isolate_data), &isolate_flags, - deferred_load_handler, error.error()); + error.error()); if (error) { FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error.str(); @@ -292,8 +289,7 @@ std::string DartIsolate::GetServiceId() { return service_id; } -bool DartIsolate::Initialize(Dart_Isolate dart_isolate, - Dart_DeferredLoadHandler& deferred_load_handler) { +bool DartIsolate::Initialize(Dart_Isolate dart_isolate) { TRACE_EVENT0("flutter", "DartIsolate::Initialize"); if (phase_ != Phase::Uninitialized) { return false; @@ -325,8 +321,8 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate, return false; } - // Dart_SetDeferredLoadHandler(&DartDeferredLoadHandler); - Dart_SetDeferredLoadHandler(deferred_load_handler); + Dart_SetDeferredLoadHandler( + DartDeferredLoadHandler::dart_deferred_load_handler); if (!UpdateThreadPoolNames()) { return false; @@ -751,9 +747,8 @@ Dart_Isolate DartIsolate::DartCreateAndStartServiceIsolate( DART_VM_SERVICE_ISOLATE_NAME, // script uri DART_VM_SERVICE_ISOLATE_NAME, // script entrypoint DartIsolate::Flags{flags}, // flags - DartDeferredLoadHandler::empty_dart_deferred_load_handler, - nullptr, // isolate create callback - nullptr // isolate shutdown callback + nullptr, // isolate create callback + nullptr // isolate shutdown callback ); std::shared_ptr service_isolate = weak_service_isolate.lock(); @@ -863,8 +858,7 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( false))); // is_root_isolate Dart_Isolate vm_isolate = CreateDartIsolateGroup( - std::move(isolate_group_data), std::move(isolate_data), flags, - DartDeferredLoadHandler::empty_dart_deferred_load_handler, error); + std::move(isolate_group_data), std::move(isolate_data), flags, error); if (*error) { FML_LOG(ERROR) << "CreateDartIsolateGroup failed: " << error; @@ -909,9 +903,7 @@ bool DartIsolate::DartIsolateInitializeCallback(void** child_callback_data, false))); // is_root_isolate // root isolate should have been created via CreateRootIsolate - if (!InitializeIsolate( - *embedder_isolate, isolate, - DartDeferredLoadHandler::empty_dart_deferred_load_handler, error)) { + if (!InitializeIsolate(*embedder_isolate, isolate, error)) { return false; } @@ -927,7 +919,6 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( std::unique_ptr> isolate_group_data, std::unique_ptr> isolate_data, Dart_IsolateFlags* flags, - Dart_DeferredLoadHandler& deferred_load_handler, char** error) { TRACE_EVENT0("flutter", "DartIsolate::CreateDartIsolateGroup"); @@ -949,8 +940,7 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( isolate_group_data.release(); isolate_data.release(); - if (!InitializeIsolate(std::move(embedder_isolate), isolate, - deferred_load_handler, error)) { + if (!InitializeIsolate(std::move(embedder_isolate), isolate, error)) { return nullptr; } @@ -960,10 +950,9 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( bool DartIsolate::InitializeIsolate( std::shared_ptr embedder_isolate, Dart_Isolate isolate, - Dart_DeferredLoadHandler& deferred_load_handler, char** error) { TRACE_EVENT0("flutter", "DartIsolate::InitializeIsolate"); - if (!embedder_isolate->Initialize(isolate, deferred_load_handler)) { + if (!embedder_isolate->Initialize(isolate)) { *error = fml::strdup("Embedder could not initialize the Dart isolate."); FML_DLOG(ERROR) << *error; return false; diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index ca9ea2b80ad52..bf0388c51e9a1 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -225,7 +225,6 @@ class DartIsolate : public UIDartState { std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, - Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback, std::optional dart_entrypoint, @@ -427,7 +426,6 @@ class DartIsolate : public UIDartState { std::string advisory_script_uri, std::string advisory_script_entrypoint, Flags flags, - Dart_DeferredLoadHandler& deferred_load_handler, const fml::closure& isolate_create_callback, const fml::closure& isolate_shutdown_callback); @@ -442,9 +440,7 @@ class DartIsolate : public UIDartState { std::string advisory_script_entrypoint, bool is_root_isolate); - [[nodiscard]] bool Initialize( - Dart_Isolate isolate, - Dart_DeferredLoadHandler& deferred_load_handler); + [[nodiscard]] bool Initialize(Dart_Isolate isolate); void SetMessageHandlingTaskRunner(fml::RefPtr runner); @@ -484,12 +480,10 @@ class DartIsolate : public UIDartState { std::unique_ptr> isolate_group_data, std::unique_ptr> isolate_data, Dart_IsolateFlags* flags, - Dart_DeferredLoadHandler& deferred_load_handler, char** error); static bool InitializeIsolate(std::shared_ptr embedder_isolate, Dart_Isolate isolate, - Dart_DeferredLoadHandler& deferred_load_handler, char** error); // |Dart_IsolateShutdownCallback| diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index be62acbabd811..a90599fd8a3fb 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -8,7 +8,6 @@ #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/fml/thread.h" -#include "flutter/runtime/dart_deferred_load_handler.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/isolate_configuration.h" @@ -53,19 +52,18 @@ TEST_F(DartIsolateTest, RootIsolateCreationAndShutdown) { IsolateConfiguration::InferFromSettings(settings); auto weak_isolate = DartIsolate::CreateRunningRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - std::move(task_runners), // task runners - nullptr, // window - {}, // snapshot delegate - {}, // hint freed delegate - {}, // io manager - {}, // unref queue - {}, // image decoder - "main.dart", // advisory uri - "main", // advisory entrypoint, - DartIsolate::Flags{}, // flags - DartDeferredLoadHandler::empty_dart_deferred_load_handler, + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint, + DartIsolate::Flags{}, // flags settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint @@ -94,19 +92,18 @@ TEST_F(DartIsolateTest, IsolateShutdownCallbackIsInIsolateScope) { auto isolate_configuration = IsolateConfiguration::InferFromSettings(settings); auto weak_isolate = DartIsolate::CreateRunningRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - std::move(task_runners), // task runners - nullptr, // window - {}, // snapshot delegate - {}, // hint freed delegate - {}, // io manager - {}, // unref queue - {}, // image decoder - "main.dart", // advisory uri - "main", // advisory entrypoint - DartIsolate::Flags{}, // flags - DartDeferredLoadHandler::empty_dart_deferred_load_handler, + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint + DartIsolate::Flags{}, // flags settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint @@ -353,19 +350,18 @@ TEST_F(DartIsolateTest, CanCreateServiceIsolate) { auto isolate_configuration = IsolateConfiguration::InferFromSettings(settings); auto weak_isolate = DartIsolate::CreateRunningRootIsolate( - vm_data->GetSettings(), // settings - vm_data->GetIsolateSnapshot(), // isolate snapshot - std::move(task_runners), // task runners - nullptr, // window - {}, // snapshot delegate - {}, // hint freed delegate - {}, // io manager - {}, // unref queue - {}, // image decoder - "main.dart", // advisory uri - "main", // advisory entrypoint, - DartIsolate::Flags{}, // flags - DartDeferredLoadHandler::empty_dart_deferred_load_handler, + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint, + DartIsolate::Flags{}, // flags settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback "main", // dart entrypoint diff --git a/runtime/dart_lifecycle_unittests.cc b/runtime/dart_lifecycle_unittests.cc index 793247654a47d..8576b3fb11292 100644 --- a/runtime/dart_lifecycle_unittests.cc +++ b/runtime/dart_lifecycle_unittests.cc @@ -6,7 +6,6 @@ #include "flutter/fml/paths.h" #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" -#include "flutter/runtime/dart_deferred_load_handler.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/isolate_configuration.h" @@ -58,19 +57,18 @@ static std::shared_ptr CreateAndRunRootIsolate( auto isolate = DartIsolate::CreateRunningRootIsolate( - vm.GetSettings(), // settings - vm.GetIsolateSnapshot(), // isolate_snapshot - runners, // task_runners - {}, // window - {}, // snapshot_delegate - {}, // hint_freed_delegate - {}, // io_manager - {}, // unref_queue - {}, // image_decoder - "main.dart", // advisory_script_uri - entrypoint.c_str(), // advisory_script_entrypoint - DartIsolate::Flags{}, // flags - DartDeferredLoadHandler::empty_dart_deferred_load_handler, + vm.GetSettings(), // settings + vm.GetIsolateSnapshot(), // isolate_snapshot + runners, // task_runners + {}, // window + {}, // snapshot_delegate + {}, // hint_freed_delegate + {}, // io_manager + {}, // unref_queue + {}, // image_decoder + "main.dart", // advisory_script_uri + entrypoint.c_str(), // advisory_script_entrypoint + DartIsolate::Flags{}, // flags settings.isolate_create_callback, // isolate create callback settings.isolate_shutdown_callback, // isolate shutdown callback, entrypoint, // dart entrypoint diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 97e0b040d4e17..e2b317ce5abcc 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -359,24 +359,23 @@ bool RuntimeController::LaunchRootIsolate( auto strong_root_isolate = DartIsolate::CreateRunningRootIsolate( - settings, // - isolate_snapshot_, // - task_runners_, // - std::make_unique(this), // - snapshot_delegate_, // - hint_freed_delegate_, // - io_manager_, // - unref_queue_, // - image_decoder_, // - advisory_script_uri_, // - advisory_script_entrypoint_, // - DartIsolate::Flags{}, // - DartDeferredLoadHandler::dart_deferred_load_handler, // - isolate_create_callback_, // - isolate_shutdown_callback_, // - dart_entrypoint, // - dart_entrypoint_library, // - std::move(isolate_configuration) // + settings, // + isolate_snapshot_, // + task_runners_, // + std::make_unique(this), // + snapshot_delegate_, // + hint_freed_delegate_, // + io_manager_, // + unref_queue_, // + image_decoder_, // + advisory_script_uri_, // + advisory_script_entrypoint_, // + DartIsolate::Flags{}, // + isolate_create_callback_, // + isolate_shutdown_callback_, // + dart_entrypoint, // + dart_entrypoint_library, // + std::move(isolate_configuration) // ) .lock(); From ebaa564bf1e7755f0b979c1b8cdd551b681dc3ad Mon Sep 17 00:00:00 2001 From: garyqian Date: Mon, 23 Nov 2020 20:14:40 -0800 Subject: [PATCH 05/15] Refactor handler into platformconfiguration --- runtime/dart_isolate.cc | 9 +++++++-- runtime/dart_isolate.h | 3 +++ runtime/runtime_controller.h | 3 ++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 8190b6e2fab97..fbd66f0ad6d6a 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -321,8 +321,7 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate) { return false; } - Dart_SetDeferredLoadHandler( - DartDeferredLoadHandler::dart_deferred_load_handler); + Dart_SetDeferredLoadHandler(OnDartLoadLibrary); if (!UpdateThreadPoolNames()) { return false; @@ -1036,6 +1035,12 @@ void DartIsolate::OnShutdownCallback() { } } +Dart_Handle DartIsolate::OnDartLoadLibrary(intptr_t loading_unit_id) { + Current()->platform_configuration()->client()->RequestDartDeferredLibrary( + loading_unit_id); + return Dart_Null(); +} + DartIsolate::AutoFireClosure::AutoFireClosure(const fml::closure& closure) : closure_(closure) {} diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index bf0388c51e9a1..ff2582076cf42 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -500,6 +500,9 @@ class DartIsolate : public UIDartState { static void DartIsolateGroupCleanupCallback( std::shared_ptr* isolate_group_data); + // |Dart_DeferredLoadHandler| + static Dart_Handle OnDartLoadLibrary(intptr_t loading_unit_id); + FML_DISALLOW_COPY_AND_ASSIGN(DartIsolate); }; diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 481e4eb22fa01..6a51ad7375b5d 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -505,6 +505,7 @@ class RuntimeController : public PlatformConfigurationClient { const uint8_t* snapshot_data, const uint8_t* snapshot_instructions); + // |PlatformConfigurationClient| //-------------------------------------------------------------------------- /// @brief Invoked when the dart VM requests that a deferred library /// be loaded. Notifies the engine that the requested loading @@ -516,7 +517,7 @@ class RuntimeController : public PlatformConfigurationClient { /// @return A Dart_Handle that is Dart_Null on success, and a dart error /// on failure. /// - void RequestDartDeferredLibrary(intptr_t loading_unit_id); + void RequestDartDeferredLibrary(intptr_t loading_unit_id) override; protected: /// Constructor for Mocks. From 913a39ec20af1a7a81279d707cb4e5855c7a961e Mon Sep 17 00:00:00 2001 From: garyqian Date: Mon, 23 Nov 2020 20:17:12 -0800 Subject: [PATCH 06/15] Remove dartdeferredloadhandler --- lib/ui/window/platform_configuration.h | 2 ++ runtime/BUILD.gn | 2 -- runtime/dart_deferred_load_handler.cc | 38 -------------------------- runtime/dart_deferred_load_handler.h | 33 ---------------------- runtime/dart_isolate.cc | 1 - runtime/runtime_controller.cc | 5 +--- 6 files changed, 3 insertions(+), 78 deletions(-) delete mode 100644 runtime/dart_deferred_load_handler.cc delete mode 100644 runtime/dart_deferred_load_handler.h diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index 693ec18bcbd95..c20a68d9653b1 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -174,6 +174,8 @@ class PlatformConfigurationClient { ComputePlatformResolvedLocale( const std::vector& supported_locale_data) = 0; + virtual void RequestDartDeferredLibrary(intptr_t loading_unit_id) = 0; + protected: virtual ~PlatformConfigurationClient(); }; diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index 09207b330c3c4..9788794f7da83 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -37,8 +37,6 @@ group("libdart") { source_set("runtime") { sources = [ - "dart_deferred_load_handler.cc", - "dart_deferred_load_handler.h", "dart_isolate.cc", "dart_isolate.h", "dart_isolate_group_data.cc", diff --git a/runtime/dart_deferred_load_handler.cc b/runtime/dart_deferred_load_handler.cc deleted file mode 100644 index 5ba967be3956d..0000000000000 --- a/runtime/dart_deferred_load_handler.cc +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2020 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/runtime/dart_deferred_load_handler.h" - -#include "flutter/runtime/runtime_controller.h" - -namespace flutter { - -void* DartDeferredLoadHandler::runtime_controller_ = nullptr; - -Dart_DeferredLoadHandler - DartDeferredLoadHandler::empty_dart_deferred_load_handler = - &DartDeferredLoadHandler::EmptyDartLoadLibrary; - -Dart_DeferredLoadHandler DartDeferredLoadHandler::dart_deferred_load_handler = - &DartDeferredLoadHandler::OnDartLoadLibrary; - -void DartDeferredLoadHandler::RegisterRuntimeController( - void* runtime_controller) { - runtime_controller_ = runtime_controller; -} - -Dart_Handle DartDeferredLoadHandler::OnDartLoadLibrary( - intptr_t loading_unit_id) { - if (runtime_controller_ != nullptr) - static_cast(runtime_controller_) - ->RequestDartDeferredLibrary(loading_unit_id); - return Dart_Null(); -} - -Dart_Handle DartDeferredLoadHandler::EmptyDartLoadLibrary( - intptr_t loading_unit_id) { - return Dart_Null(); -} - -} // namespace flutter diff --git a/runtime/dart_deferred_load_handler.h b/runtime/dart_deferred_load_handler.h deleted file mode 100644 index a6947570ea132..0000000000000 --- a/runtime/dart_deferred_load_handler.h +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2020 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_RUNTIME_DART_DEFERRED_LOAD_HANDLER_H_ -#define FLUTTER_RUNTIME_DART_DEFERRED_LOAD_HANDLER_H_ - -#include -#include - -#include "third_party/dart/runtime/include/dart_api.h" - -namespace flutter { - -class DartDeferredLoadHandler { - public: - // TODO: Fix deps to not use void* - static void RegisterRuntimeController(void* runtime_controller); - - static Dart_DeferredLoadHandler empty_dart_deferred_load_handler; - static Dart_DeferredLoadHandler dart_deferred_load_handler; - - private: - static Dart_Handle OnDartLoadLibrary(intptr_t loading_unit_id); - // No-op function that returns Dart_Null() for when the isolate is not - // expected to handle deferred libraries. - static Dart_Handle EmptyDartLoadLibrary(intptr_t loading_unit_id); - static void* runtime_controller_; -}; - -} // namespace flutter - -#endif // FLUTTER_RUNTIME_DART_DEFERRED_LOAD_HANDLER_H_ diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index fbd66f0ad6d6a..8d67bf4d794ff 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "flutter/runtime/dart_isolate.h" -#include "flutter/runtime/dart_deferred_load_handler.h" #include #include diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index e2b317ce5abcc..869a04389f7a6 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -14,7 +14,6 @@ #include "flutter/lib/ui/window/platform_configuration.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/lib/ui/window/window.h" -#include "flutter/runtime/dart_deferred_load_handler.h" #include "flutter/runtime/isolate_configuration.h" #include "flutter/runtime/runtime_delegate.h" #include "third_party/tonic/dart_message_handler.h" @@ -57,9 +56,7 @@ RuntimeController::RuntimeController( platform_data_(std::move(p_platform_data)), isolate_create_callback_(p_isolate_create_callback), isolate_shutdown_callback_(p_isolate_shutdown_callback), - persistent_isolate_data_(std::move(p_persistent_isolate_data)) { - DartDeferredLoadHandler::RegisterRuntimeController(this); -} + persistent_isolate_data_(std::move(p_persistent_isolate_data)) {} RuntimeController::~RuntimeController() { FML_DCHECK(Dart_CurrentIsolate() == nullptr); From 81c927a3a29dabe09513a4846144d0d8c210d9dd Mon Sep 17 00:00:00 2001 From: garyqian Date: Mon, 23 Nov 2020 21:20:45 -0800 Subject: [PATCH 07/15] Remove dlfcn, revert accidentqal comment changes Revert accidental comment changes --- runtime/dart_isolate.cc | 22 ++++++++++------------ runtime/runtime_controller.cc | 1 - 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 8d67bf4d794ff..3dc6465f29888 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -149,8 +149,8 @@ std::weak_ptr DartIsolate::CreateRunningRootIsolate( } if (settings.root_isolate_create_callback) { - // Isolate callbacks always occur in isolate scope and before user code - // has had a chance to run. + // Isolate callbacks always occur in isolate scope and before user code has + // had a chance to run. tonic::DartState::Scope scope(isolate.get()); settings.root_isolate_create_callback(*isolate.get()); } @@ -678,8 +678,8 @@ bool DartIsolate::RunFromLibrary(std::optional library_name, bool DartIsolate::Shutdown() { TRACE_EVENT0("flutter", "DartIsolate::Shutdown"); // This call may be re-entrant since Dart_ShutdownIsolate can invoke the - // cleanup callback which deletes the embedder side object of the dart - // isolate (a.k.a. this). + // cleanup callback which deletes the embedder side object of the dart isolate + // (a.k.a. this). if (phase_ == Phase::Shutdown) { return false; } @@ -809,9 +809,8 @@ Dart_Isolate DartIsolate::DartIsolateGroupCreateCallback( // The VM attempts to start the VM service for us on |Dart_Initialize|. In // such a case, the callback data will be null and the script URI will be // DART_VM_SERVICE_ISOLATE_NAME. In such cases, we just create the service - // isolate like normal but dont hold a reference to it at all. We also - // start this isolate since we will never again reference it from the - // engine. + // isolate like normal but dont hold a reference to it at all. We also start + // this isolate since we will never again reference it from the engine. return DartCreateAndStartServiceIsolate(package_root, // package_config, // flags, // @@ -932,8 +931,7 @@ Dart_Isolate DartIsolate::CreateDartIsolateGroup( return nullptr; } - // Ownership of the isolate data objects has been transferred to the Dart - // VM. + // Ownership of the isolate data objects has been transferred to the Dart VM. std::shared_ptr embedder_isolate(*isolate_data); isolate_group_data.release(); isolate_data.release(); @@ -963,9 +961,9 @@ bool DartIsolate::InitializeIsolate( return false; } - // Root isolates will be setup by the engine and the service isolate (which - // is also a root isolate) by the utility routines in the VM. However, - // secondary isolates will be run by the VM if they are marked as runnable. + // Root isolates will be setup by the engine and the service isolate (which is + // also a root isolate) by the utility routines in the VM. However, secondary + // isolates will be run by the VM if they are marked as runnable. if (!embedder_isolate->IsRootIsolate()) { auto child_isolate_preparer = embedder_isolate->GetIsolateGroupData().GetChildIsolatePreparer(); diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 869a04389f7a6..58c38270348ba 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -4,7 +4,6 @@ #include "flutter/runtime/runtime_controller.h" -#include #include #include "flutter/fml/message_loop.h" From 2e905256e1dc64aad661aa7629384b1b1cc71e7d Mon Sep 17 00:00:00 2001 From: garyqian Date: Tue, 24 Nov 2020 18:50:32 -0800 Subject: [PATCH 08/15] Address comments, fix playstoredynamicfeaturemanager: --- runtime/dart_isolate.cc | 5 ++--- .../dynamicfeatures/PlayStoreDynamicFeatureManager.java | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 3dc6465f29888..f86190c12648e 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -320,7 +320,7 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate) { return false; } - Dart_SetDeferredLoadHandler(OnDartLoadLibrary); + tonic::LogIfError(Dart_SetDeferredLoadHandler(OnDartLoadLibrary)); if (!UpdateThreadPoolNames()) { return false; @@ -340,8 +340,7 @@ bool DartIsolate::LoadLoadingUnit(intptr_t loading_unit_id, tonic::DartState::Scope scope(this); Dart_Handle result = Dart_DeferredLoadComplete(loading_unit_id, snapshot_data, snapshot_instructions); - if (Dart_IsApiError(result)) { - FML_LOG(ERROR) << "LOADING FAILED " << loading_unit_id; + if (tonic::LogIfError(result)) { result = Dart_DeferredLoadCompleteError(loading_unit_id, Dart_GetError(result), /*transient*/ true); diff --git a/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java b/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java index 0ae2fd4961c0e..4f14f6e68eccf 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java +++ b/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java @@ -195,13 +195,13 @@ private String loadingUnitIdToModuleName(int loadingUnitId) { public void downloadDynamicFeature(int loadingUnitId, String moduleName) { String resolvedModuleName = - moduleName == null ? moduleName : loadingUnitIdToModuleName(loadingUnitId); + moduleName != null ? moduleName : loadingUnitIdToModuleName(loadingUnitId); if (resolvedModuleName == null) { Log.d(TAG, "Dynamic feature module name was null."); return; } - SplitInstallRequest request = SplitInstallRequest.newBuilder().addModule(moduleName).build(); + SplitInstallRequest request = SplitInstallRequest.newBuilder().addModule(resolvedModuleName).build(); splitInstallManager // Submits the request to install the module through the @@ -212,7 +212,7 @@ public void downloadDynamicFeature(int loadingUnitId, String moduleName) { // install which is handled in FeatureInstallStateUpdatedListener. .addOnSuccessListener( sessionId -> { - this.sessionIdToName.put(sessionId, moduleName); + this.sessionIdToName.put(sessionId, resolvedModuleName); this.sessionIdToLoadingUnitId.put(sessionId, loadingUnitId); }) .addOnFailureListener( From 1442b6e04f97fc3b9461dc928495439eeb87dccc Mon Sep 17 00:00:00 2001 From: garyqian Date: Tue, 24 Nov 2020 21:24:13 -0800 Subject: [PATCH 09/15] Use native_library Pass as mapping cleanup Docs More docs Cleanup --- runtime/dart_isolate.cc | 12 +++-- runtime/dart_isolate.h | 4 +- runtime/runtime_controller.cc | 9 ++-- runtime/runtime_controller.h | 38 +++++++++----- shell/common/engine.cc | 12 +++-- shell/common/engine.h | 24 ++++++--- shell/common/platform_view.cc | 4 +- shell/common/platform_view.h | 31 +++++++---- shell/common/shell.cc | 11 ++-- shell/common/shell.h | 7 +-- .../platform/android/platform_view_android.cc | 8 +-- .../platform/android/platform_view_android.h | 7 +-- .../android/platform_view_android_jni_impl.cc | 52 +++++-------------- .../Source/FlutterEnginePlatformViewTest.mm | 4 +- .../Source/FlutterPlatformViewsTest.mm | 4 +- .../Source/accessibility_bridge_test.mm | 4 +- .../fuchsia/flutter/platform_view_unittest.cc | 7 +-- 17 files changed, 127 insertions(+), 111 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index f86190c12648e..c0c79f7c68fd2 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -334,12 +334,14 @@ fml::RefPtr DartIsolate::GetMessageHandlingTaskRunner() const { return message_handling_task_runner_; } -bool DartIsolate::LoadLoadingUnit(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) { +bool DartIsolate::LoadLoadingUnit( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { tonic::DartState::Scope scope(this); - Dart_Handle result = Dart_DeferredLoadComplete(loading_unit_id, snapshot_data, - snapshot_instructions); + Dart_Handle result = + Dart_DeferredLoadComplete(loading_unit_id, snapshot_data->GetMapping(), + snapshot_instructions->GetMapping()); if (tonic::LogIfError(result)) { result = Dart_DeferredLoadCompleteError(loading_unit_id, Dart_GetError(result), diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index ff2582076cf42..b5f154915986e 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -385,8 +385,8 @@ class DartIsolate : public UIDartState { fml::RefPtr GetMessageHandlingTaskRunner() const; bool LoadLoadingUnit(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); void LoadLoadingUnitFailure(intptr_t loading_unit_id, const std::string error_message, diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 58c38270348ba..f77853bf62797 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -419,10 +419,11 @@ std::optional RuntimeController::GetRootIsolateReturnCode() { void RuntimeController::LoadDartDeferredLibrary( intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) { - root_isolate_.lock()->LoadLoadingUnit(loading_unit_id, snapshot_data, - snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { + root_isolate_.lock()->LoadLoadingUnit(loading_unit_id, + std::move(snapshot_data), + std::move(snapshot_instructions)); } void RuntimeController::RequestDartDeferredLibrary(intptr_t loading_unit_id) { diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 6a51ad7375b5d..1679451ddc3d8 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -11,6 +11,7 @@ #include "flutter/common/task_runners.h" #include "flutter/flow/layers/layer_tree.h" #include "flutter/fml/macros.h" +#include "flutter/fml/mapping.h" #include "flutter/lib/ui/io_manager.h" #include "flutter/lib/ui/text/font_collection.h" #include "flutter/lib/ui/ui_dart_state.h" @@ -481,10 +482,10 @@ class RuntimeController : public PlatformConfigurationClient { /// The Dart compiler may generate separate shared libraries /// files called 'loading units' when libraries are imported /// as deferred. Each of these shared libraries are identified - /// by a unique loading unit id. Callers should dlopen the - /// shared library file and use dlsym to resolve the dart - /// symbols. These symbols can then be passed to this method to - /// be dynamically loaded into the VM. + /// by a unique loading unit id. Callers should open and resolve + /// a SymbolMapping from the shared library. The symbols can + /// then be moved into the dart isolate to be dynamically loaded + /// into the VM via this method. /// /// This method is paired with a RequestDartDeferredLibrary /// invocation that provides the embedder with the loading unit id @@ -501,21 +502,30 @@ class RuntimeController : public PlatformConfigurationClient { /// @param[in] snapshot_data Dart snapshot instructions of the loading /// unit's shared library. /// - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions); + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); // |PlatformConfigurationClient| //-------------------------------------------------------------------------- - /// @brief Invoked when the dart VM requests that a deferred library - /// be loaded. Notifies the engine that the requested loading - /// unit should be downloaded and loaded. + /// @brief Invoked when the Dart VM requests that a deferred library + /// be loaded. Notifies the engine that the deferred library + /// identified by the specified loading unit id should be + /// downloaded and loaded into the Dart VM via + /// `LoadDartDeferredLibrary` + /// + /// Upon encountering errors or otherwise failing to load a + /// loading unit with the specified id, the failure should be + /// directly reported to dart by calling + /// `LoadDartDeferredLibraryFailure` to ensure the waiting dart + /// future completes with an error. /// /// @param[in] loading_unit_id The unique id of the deferred library's - /// loading unit. - /// - /// @return A Dart_Handle that is Dart_Null on success, and a dart error - /// on failure. + /// loading unit. This id is to be passed + /// back into LoadDartDeferredLibrary + /// in order to identify which deferred + /// library to load. /// void RequestDartDeferredLibrary(intptr_t loading_unit_id) override; diff --git a/shell/common/engine.cc b/shell/common/engine.cc index baeae7f78f1a1..4d64528d0a45e 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -512,12 +512,14 @@ void Engine::RequestDartDeferredLibrary(intptr_t loading_unit_id) { return delegate_.RequestDartDeferredLibrary(loading_unit_id); } -void Engine::LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) { +void Engine::LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { if (runtime_controller_->IsRootIsolateRunning()) { - runtime_controller_->LoadDartDeferredLibrary(loading_unit_id, snapshot_data, - snapshot_instructions); + runtime_controller_->LoadDartDeferredLibrary( + loading_unit_id, std::move(snapshot_data), + std::move(snapshot_instructions)); } } diff --git a/shell/common/engine.h b/shell/common/engine.h index c2bee1a0709ee..a5c248f401cbe 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -11,6 +11,7 @@ #include "flutter/assets/asset_manager.h" #include "flutter/common/task_runners.h" #include "flutter/fml/macros.h" +#include "flutter/fml/mapping.h" #include "flutter/fml/memory/weak_ptr.h" #include "flutter/lib/ui/hint_freed_delegate.h" #include "flutter/lib/ui/painting/image_decoder.h" @@ -268,6 +269,12 @@ class Engine final : public RuntimeDelegate, /// downloaded and loaded into the Dart VM via /// `LoadDartDeferredLibrary` /// + /// Upon encountering errors or otherwise failing to load a + /// loading unit with the specified id, the failure should be + /// directly reported to dart by calling + /// `LoadDartDeferredLibraryFailure` to ensure the waiting dart + /// future completes with an error. + /// /// @param[in] loading_unit_id The unique id of the deferred library's /// loading unit. This id is to be passed /// back into LoadDartDeferredLibrary @@ -790,10 +797,10 @@ class Engine final : public RuntimeDelegate, /// The Dart compiler may generate separate shared libraries /// files called 'loading units' when libraries are imported /// as deferred. Each of these shared libraries are identified - /// by a unique loading unit id. Callers should dlopen the - /// shared library file and use dlsym to resolve the dart - /// symbols. These symbols can then be passed to this method to - /// be dynamically loaded into the VM. + /// by a unique loading unit id. Callers should open and resolve + /// a SymbolMapping from the shared library. The symbols can + /// then be moved into the dart isolate to be dynamically loaded + /// into the VM via this method. /// /// This method is paired with a RequestDartDeferredLibrary /// invocation that provides the embedder with the loading unit id @@ -810,9 +817,10 @@ class Engine final : public RuntimeDelegate, /// @param[in] snapshot_data Dart snapshot instructions of the loading /// unit's shared library. /// - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions); + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); private: Engine::Delegate& delegate_; @@ -862,7 +870,7 @@ class Engine final : public RuntimeDelegate, std::unique_ptr> ComputePlatformResolvedLocale( const std::vector& supported_locale_data) override; - // // |RuntimeDelegate| + // |RuntimeDelegate| void RequestDartDeferredLibrary(intptr_t loading_unit_id) override; void SetNeedsReportTimings(bool value) override; diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index 55d3ebbcc7147..47d89fdb1258a 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -163,8 +163,8 @@ void PlatformView::RequestDartDeferredLibrary(intptr_t loading_unit_id) {} void PlatformView::LoadDartDeferredLibrary( intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) {} void PlatformView::UpdateAssetManager( std::shared_ptr asset_manager) {} diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index dd200b5948fdb..4f909b337a12a 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -12,6 +12,7 @@ #include "flutter/common/task_runners.h" #include "flutter/flow/surface.h" #include "flutter/fml/macros.h" +#include "flutter/fml/mapping.h" #include "flutter/fml/memory/weak_ptr.h" #include "flutter/lib/ui/semantics/custom_accessibility_action.h" #include "flutter/lib/ui/semantics/semantics_node.h" @@ -216,12 +217,17 @@ class PlatformView { /// dart library is loaded successfully, the dart future /// returned by the originating loadLibrary() call completes. /// - /// The Dart compiler may generate separate shared library .so + /// The Dart compiler may generate separate shared libraries /// files called 'loading units' when libraries are imported /// as deferred. Each of these shared libraries are identified - /// by a unique loading unit id and can be dynamically loaded - /// into the VM by dlopen-ing and resolving the data and - /// instructions symbols. + /// by a unique loading unit id. Callers should open and resolve + /// a SymbolMapping from the shared library. The symbols can + /// then be moved into the dart isolate to be dynamically loaded + /// into the VM via this method. + /// + /// This method is paired with a RequestDartDeferredLibrary + /// invocation that provides the embedder with the loading unit + /// id of the deferred library to load. /// /// /// @param[in] loading_unit_id The unique id of the deferred library's @@ -235,8 +241,8 @@ class PlatformView { /// virtual void LoadDartDeferredLibrary( intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) = 0; + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) = 0; // TODO(garyq): Implement a proper asset_resolver replacement instead of // overwriting the entire asset manager. @@ -609,6 +615,12 @@ class PlatformView { /// downloaded and loaded into the Dart VM via /// `LoadDartDeferredLibrary` /// + /// Upon encountering errors or otherwise failing to load a + /// loading unit with the specified id, the failure should be + /// directly reported to dart by calling + /// `LoadDartDeferredLibraryFailure` to ensure the waiting dart + /// future completes with an error. + /// /// @param[in] loading_unit_id The unique id of the deferred library's /// loading unit. This id is to be passed /// back into LoadDartDeferredLibrary @@ -645,9 +657,10 @@ class PlatformView { /// @param[in] snapshot_data Dart snapshot instructions of the loading /// unit's shared library. /// - virtual void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions); + virtual void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); // TODO(garyq): Implement a proper asset_resolver replacement instead of // overwriting the entire asset manager. diff --git a/shell/common/shell.cc b/shell/common/shell.cc index aaf3c3c2242ae..99ac295438801 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1185,11 +1185,12 @@ std::unique_ptr> Shell::ComputePlatformResolvedLocale( return platform_view_->ComputePlatformResolvedLocales(supported_locale_data); } -void Shell::LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) { - engine_->LoadDartDeferredLibrary(loading_unit_id, snapshot_data, - snapshot_instructions); +void Shell::LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { + engine_->LoadDartDeferredLibrary(loading_unit_id, std::move(snapshot_data), + std::move(snapshot_instructions)); } void Shell::UpdateAssetManager(std::shared_ptr asset_manager) { diff --git a/shell/common/shell.h b/shell/common/shell.h index 7ac297e5d07c6..7d315a33bd7dd 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -508,9 +508,10 @@ class Shell final : public PlatformView::Delegate, void OnPlatformViewSetNextFrameCallback(const fml::closure& closure) override; // |PlatformView::Delegate| - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) override; + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override; // |PlatformView::Delegate| void UpdateAssetManager(std::shared_ptr asset_manager) override; diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index f5049b23223d6..9340bdfc60a68 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -347,10 +347,10 @@ void PlatformViewAndroid::RequestDartDeferredLibrary(intptr_t loading_unit_id) { // |PlatformView| void PlatformViewAndroid::LoadDartDeferredLibrary( intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) { - delegate_.LoadDartDeferredLibrary(loading_unit_id, snapshot_data, - snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { + delegate_.LoadDartDeferredLibrary(loading_unit_id, std::move(snapshot_data), + std::move(snapshot_instructions)); } // |PlatformView| diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index ff2ac1353f2b3..10feabc7c8b82 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -94,9 +94,10 @@ class PlatformViewAndroid final : public PlatformView { const fml::jni::JavaObjectWeakGlobalRef& surface_texture); // |PlatformView| - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) override; + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override; // |PlatformView| void UpdateAssetManager(std::shared_ptr asset_manager) override; diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index a252d167b07c3..233ae0de14125 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -15,6 +15,8 @@ #include "flutter/assets/directory_asset_bundle.h" #include "flutter/common/settings.h" #include "flutter/fml/file.h" +#include "flutter/fml/mapping.h" +#include "flutter/fml/native_library.h" #include "flutter/fml/platform/android/jni_util.h" #include "flutter/fml/platform/android/jni_weak_ref.h" #include "flutter/fml/platform/android/scoped_java_ref.h" @@ -539,12 +541,8 @@ static void LoadDartDeferredLibrary(JNIEnv* env, std::vector search_paths = fml::jni::StringArrayToVector(env, jSearchPaths); - // TODO: Switch to using the NativeLibrary class, eg: - // - // fml::RefPtr native_lib = - // fml::NativeLibrary::Create(lib_name.c_str()); - // - // Find and open the shared library. + // Use dlopen here to directly check if handle is nullptr before creating a + // NativeLibrary. void* handle = nullptr; while (handle == nullptr && !search_paths.empty()) { std::string path = search_paths.back(); @@ -556,42 +554,20 @@ static void LoadDartDeferredLibrary(JNIEnv* env, "No lib .so found for provided search paths.", true); return; } + fml::RefPtr native_lib = + fml::NativeLibrary::CreateWithHandle(handle, false); // Resolve symbols. - uint8_t* isolate_data = - static_cast(::dlsym(handle, DartSnapshot::kIsolateDataSymbol)); - if (isolate_data == nullptr) { - // Mac sometimes requires an underscore prefix. - std::stringstream underscore_symbol_name; - underscore_symbol_name << "_" << DartSnapshot::kIsolateDataSymbol; - isolate_data = static_cast( - ::dlsym(handle, underscore_symbol_name.str().c_str())); - if (isolate_data == nullptr) { - LoadLoadingUnitFailure(loading_unit_id, - "Could not resolve data symbol in library", true); - return; - } - } - uint8_t* isolate_instructions = static_cast( - ::dlsym(handle, DartSnapshot::kIsolateInstructionsSymbol)); - if (isolate_instructions == nullptr) { - // Mac sometimes requires an underscore prefix. - std::stringstream underscore_symbol_name; - underscore_symbol_name << "_" << DartSnapshot::kIsolateInstructionsSymbol; - isolate_instructions = static_cast( - ::dlsym(handle, underscore_symbol_name.str().c_str())); - if (isolate_data == nullptr) { - LoadLoadingUnitFailure(loading_unit_id, - "Could not resolve instructions symbol in library", - true); - return; - } - } + std::unique_ptr data_mapping = + std::make_unique(native_lib, + DartSnapshot::kIsolateDataSymbol); + std::unique_ptr instructions_mapping = + std::make_unique( + native_lib, DartSnapshot::kIsolateInstructionsSymbol); ANDROID_SHELL_HOLDER->GetPlatformView()->LoadDartDeferredLibrary( - loading_unit_id, isolate_data, isolate_instructions); - - // TODO(garyq): fallback on soPath. + loading_unit_id, std::move(data_mapping), + std::move(instructions_mapping)); } // TODO(garyq): persist additional asset resolvers by updating instead of diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm index fbccf6677b02f..06ba7f6e6cc3f 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm @@ -35,8 +35,8 @@ void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) override {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override {} void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index d9abef5a093ef..1bb5109a91a66 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -105,8 +105,8 @@ void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) override {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override {} void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm index b027b20e915b4..74266fe5ad0a9 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm @@ -88,8 +88,8 @@ void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) override {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override {} void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index a54504a92ea74..fc1e38b2264c5 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -105,9 +105,10 @@ class MockPlatformViewDelegate : public flutter::PlatformView::Delegate { return nullptr; } // |flutter::PlatformView::Delegate| - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - const uint8_t* snapshot_data, - const uint8_t* snapshot_instructions) {} + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) {} // |flutter::PlatformView::Delegate| void UpdateAssetManager( std::shared_ptr asset_manager) {} From a43817f873d98a818ecc223490bd1b63d0d972ea Mon Sep 17 00:00:00 2001 From: garyqian Date: Wed, 25 Nov 2020 11:14:52 -0800 Subject: [PATCH 10/15] Formatting SymbolMapping --- runtime/dart_isolate.cc | 4 ++-- runtime/dart_isolate.h | 7 ++++--- runtime/runtime_controller.cc | 4 ++-- runtime/runtime_controller.h | 4 ++-- shell/common/engine.cc | 4 ++-- shell/common/engine.h | 4 ++-- shell/common/platform_view.cc | 4 ++-- shell/common/platform_view.h | 8 ++++---- shell/common/shell.cc | 4 ++-- shell/common/shell.h | 4 ++-- .../PlayStoreDynamicFeatureManager.java | 3 ++- shell/platform/android/platform_view_android.cc | 4 ++-- shell/platform/android/platform_view_android.h | 4 ++-- .../platform/android/platform_view_android_jni_impl.cc | 10 +++++----- .../framework/Source/FlutterEnginePlatformViewTest.mm | 7 ++++--- .../ios/framework/Source/FlutterPlatformViewsTest.mm | 7 ++++--- .../ios/framework/Source/accessibility_bridge_test.mm | 7 ++++--- .../platform/fuchsia/flutter/platform_view_unittest.cc | 4 ++-- 18 files changed, 49 insertions(+), 44 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index c0c79f7c68fd2..d8d54e47ee2af 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -336,8 +336,8 @@ fml::RefPtr DartIsolate::GetMessageHandlingTaskRunner() const { bool DartIsolate::LoadLoadingUnit( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { tonic::DartState::Scope scope(this); Dart_Handle result = Dart_DeferredLoadComplete(loading_unit_id, snapshot_data->GetMapping(), diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index b5f154915986e..0cfa802318b83 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -384,9 +384,10 @@ class DartIsolate : public UIDartState { /// fml::RefPtr GetMessageHandlingTaskRunner() const; - bool LoadLoadingUnit(intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + bool LoadLoadingUnit( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); void LoadLoadingUnitFailure(intptr_t loading_unit_id, const std::string error_message, diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index f77853bf62797..4043bd1807e61 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -419,8 +419,8 @@ std::optional RuntimeController::GetRootIsolateReturnCode() { void RuntimeController::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { root_isolate_.lock()->LoadLoadingUnit(loading_unit_id, std::move(snapshot_data), std::move(snapshot_instructions)); diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 1679451ddc3d8..48e0e800a6575 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -504,8 +504,8 @@ class RuntimeController : public PlatformConfigurationClient { /// void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); // |PlatformConfigurationClient| //-------------------------------------------------------------------------- diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 4d64528d0a45e..11f929c583077 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -514,8 +514,8 @@ void Engine::RequestDartDeferredLibrary(intptr_t loading_unit_id) { void Engine::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { if (runtime_controller_->IsRootIsolateRunning()) { runtime_controller_->LoadDartDeferredLibrary( loading_unit_id, std::move(snapshot_data), diff --git a/shell/common/engine.h b/shell/common/engine.h index a5c248f401cbe..09da5bd1646fb 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -819,8 +819,8 @@ class Engine final : public RuntimeDelegate, /// void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); private: Engine::Delegate& delegate_; diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index 47d89fdb1258a..b24b22d9dc533 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -163,8 +163,8 @@ void PlatformView::RequestDartDeferredLibrary(intptr_t loading_unit_id) {} void PlatformView::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) {} void PlatformView::UpdateAssetManager( std::shared_ptr asset_manager) {} diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 4f909b337a12a..02f866385686b 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -241,8 +241,8 @@ class PlatformView { /// virtual void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) = 0; + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) = 0; // TODO(garyq): Implement a proper asset_resolver replacement instead of // overwriting the entire asset manager. @@ -659,8 +659,8 @@ class PlatformView { /// virtual void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); // TODO(garyq): Implement a proper asset_resolver replacement instead of // overwriting the entire asset manager. diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 99ac295438801..2ea3b3f24c363 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1187,8 +1187,8 @@ std::unique_ptr> Shell::ComputePlatformResolvedLocale( void Shell::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { engine_->LoadDartDeferredLibrary(loading_unit_id, std::move(snapshot_data), std::move(snapshot_instructions)); } diff --git a/shell/common/shell.h b/shell/common/shell.h index 7d315a33bd7dd..772a78afbeae9 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -510,8 +510,8 @@ class Shell final : public PlatformView::Delegate, // |PlatformView::Delegate| void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override; + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override; // |PlatformView::Delegate| void UpdateAssetManager(std::shared_ptr asset_manager) override; diff --git a/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java b/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java index 4f14f6e68eccf..406392cd28b53 100644 --- a/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java +++ b/shell/platform/android/io/flutter/embedding/engine/dynamicfeatures/PlayStoreDynamicFeatureManager.java @@ -201,7 +201,8 @@ public void downloadDynamicFeature(int loadingUnitId, String moduleName) { return; } - SplitInstallRequest request = SplitInstallRequest.newBuilder().addModule(resolvedModuleName).build(); + SplitInstallRequest request = + SplitInstallRequest.newBuilder().addModule(resolvedModuleName).build(); splitInstallManager // Submits the request to install the module through the diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 9340bdfc60a68..9445451053cfb 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -347,8 +347,8 @@ void PlatformViewAndroid::RequestDartDeferredLibrary(intptr_t loading_unit_id) { // |PlatformView| void PlatformViewAndroid::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { delegate_.LoadDartDeferredLibrary(loading_unit_id, std::move(snapshot_data), std::move(snapshot_instructions)); } diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 10feabc7c8b82..140868ae16eda 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -96,8 +96,8 @@ class PlatformViewAndroid final : public PlatformView { // |PlatformView| void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override; + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override; // |PlatformView| void UpdateAssetManager(std::shared_ptr asset_manager) override; diff --git a/shell/platform/android/platform_view_android_jni_impl.cc b/shell/platform/android/platform_view_android_jni_impl.cc index 233ae0de14125..ac3c50e83d75c 100644 --- a/shell/platform/android/platform_view_android_jni_impl.cc +++ b/shell/platform/android/platform_view_android_jni_impl.cc @@ -558,11 +558,11 @@ static void LoadDartDeferredLibrary(JNIEnv* env, fml::NativeLibrary::CreateWithHandle(handle, false); // Resolve symbols. - std::unique_ptr data_mapping = - std::make_unique(native_lib, - DartSnapshot::kIsolateDataSymbol); - std::unique_ptr instructions_mapping = - std::make_unique( + std::unique_ptr data_mapping = + std::make_unique( + native_lib, DartSnapshot::kIsolateDataSymbol); + std::unique_ptr instructions_mapping = + std::make_unique( native_lib, DartSnapshot::kIsolateInstructionsSymbol); ANDROID_SHELL_HOLDER->GetPlatformView()->LoadDartDeferredLibrary( diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm index 06ba7f6e6cc3f..afb350742fe29 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm @@ -34,9 +34,10 @@ void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override {} + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override {} void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 1bb5109a91a66..8b5a89dda4788 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -104,9 +104,10 @@ void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override {} + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override {} void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm index 74266fe5ad0a9..a64cbcca6e1db 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm @@ -87,9 +87,10 @@ void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} - void LoadDartDeferredLibrary(intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override {} + void LoadDartDeferredLibrary( + intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override {} void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index fc1e38b2264c5..fa50fd5da4e91 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -107,8 +107,8 @@ class MockPlatformViewDelegate : public flutter::PlatformView::Delegate { // |flutter::PlatformView::Delegate| void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) {} // |flutter::PlatformView::Delegate| void UpdateAssetManager( std::shared_ptr asset_manager) {} From f0274523aef896a647b2b66084963825f1ae6a77 Mon Sep 17 00:00:00 2001 From: garyqian Date: Thu, 26 Nov 2020 01:48:22 -0800 Subject: [PATCH 11/15] Store DartSnapshots Use unordered_set --- runtime/dart_isolate.cc | 19 ++++++++++++------- runtime/dart_isolate.h | 2 ++ runtime/dart_snapshot.cc | 11 +++++++++++ runtime/dart_snapshot.h | 12 ++++++++++++ 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index d8d54e47ee2af..777fb223b4b49 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -320,7 +320,9 @@ bool DartIsolate::Initialize(Dart_Isolate dart_isolate) { return false; } - tonic::LogIfError(Dart_SetDeferredLoadHandler(OnDartLoadLibrary)); + if (tonic::LogIfError(Dart_SetDeferredLoadHandler(OnDartLoadLibrary))) { + return false; + } if (!UpdateThreadPoolNames()) { return false; @@ -339,15 +341,20 @@ bool DartIsolate::LoadLoadingUnit( std::unique_ptr snapshot_data, std::unique_ptr snapshot_instructions) { tonic::DartState::Scope scope(this); - Dart_Handle result = - Dart_DeferredLoadComplete(loading_unit_id, snapshot_data->GetMapping(), - snapshot_instructions->GetMapping()); + fml::RefPtr dart_snapshot = + DartSnapshot::IsolateSnapshotFromMappings( + std::move(snapshot_data), std::move(snapshot_instructions)); + + Dart_Handle result = Dart_DeferredLoadComplete( + loading_unit_id, dart_snapshot->GetDataMapping(), + dart_snapshot->GetInstructionsMapping()); if (tonic::LogIfError(result)) { result = Dart_DeferredLoadCompleteError(loading_unit_id, Dart_GetError(result), /*transient*/ true); return false; } + loading_unit_snapshots_.insert(dart_snapshot); return true; } @@ -357,9 +364,7 @@ void DartIsolate::LoadLoadingUnitFailure(intptr_t loading_unit_id, tonic::DartState::Scope scope(this); Dart_Handle result = Dart_DeferredLoadCompleteError( loading_unit_id, error_message.c_str(), transient); - if (Dart_IsApiError(result)) { - FML_LOG(ERROR) << "Dart error: " << Dart_GetError(result); - } + tonic::LogIfError(result); } void DartIsolate::SetMessageHandlingTaskRunner( diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index 0cfa802318b83..c4740f4dd47a5 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -9,6 +9,7 @@ #include #include #include +#include #include "flutter/common/task_runners.h" #include "flutter/fml/compiler_specific.h" @@ -410,6 +411,7 @@ class DartIsolate : public UIDartState { Phase phase_ = Phase::Unknown; std::vector> kernel_buffers_; std::vector> shutdown_callbacks_; + std::unordered_set> loading_unit_snapshots_; fml::RefPtr message_handling_task_runner_; const bool may_insecurely_connect_to_all_domains_; std::string domain_network_policy_; diff --git a/runtime/dart_snapshot.cc b/runtime/dart_snapshot.cc index e9ed35418880a..aac3f28789fa6 100644 --- a/runtime/dart_snapshot.cc +++ b/runtime/dart_snapshot.cc @@ -181,6 +181,17 @@ fml::RefPtr DartSnapshot::IsolateSnapshotFromSettings( return nullptr; } +fml::RefPtr DartSnapshot::IsolateSnapshotFromMappings( + std::shared_ptr snapshot_data, + std::shared_ptr snapshot_instructions) { + auto snapshot = + fml::MakeRefCounted(snapshot_data, snapshot_instructions); + if (snapshot->IsValid()) { + return snapshot; + } + return nullptr; +} + DartSnapshot::DartSnapshot(std::shared_ptr data, std::shared_ptr instructions) : data_(std::move(data)), instructions_(std::move(instructions)) {} diff --git a/runtime/dart_snapshot.h b/runtime/dart_snapshot.h index 15ca157c7a273..e43b89027db1b 100644 --- a/runtime/dart_snapshot.h +++ b/runtime/dart_snapshot.h @@ -102,6 +102,18 @@ class DartSnapshot : public fml::RefCountedThreadSafe { static fml::RefPtr IsolateSnapshotFromSettings( const Settings& settings); + //---------------------------------------------------------------------------- + /// @brief Create an isolate snapshot from existing fml::Mappings. + /// + /// @param[in] snapshot_data The mapping for the heap snapshot. + /// @param[in] snapshot_instructions The mapping for the instructions + /// snapshot. + /// + /// @return A valid isolate snapshot or nullptr. + static fml::RefPtr IsolateSnapshotFromMappings( + std::shared_ptr snapshot_data, + std::shared_ptr snapshot_instructions); + //---------------------------------------------------------------------------- /// @brief Determines if this snapshot contains a heap component. Since /// the instructions component is optional, the method does not From e3655e21a4511e1ab89cf9670817a4eb1fa663dd Mon Sep 17 00:00:00 2001 From: garyqian Date: Mon, 30 Nov 2020 14:55:56 -0800 Subject: [PATCH 12/15] Add failure test Begin testing work Loading split aot tests part 1 Passing tests, but flaky due to race --- runtime/dart_isolate.cc | 20 ++++-- runtime/dart_isolate.h | 4 +- runtime/dart_isolate_unittests.cc | 97 ++++++++++++++++++++++++++++ runtime/fixtures/runtime_test.dart | 21 ++++++ runtime/fixtures/split_lib_test.dart | 9 +++ shell/common/engine.h | 8 ++- shell/common/platform_view.h | 10 +-- testing/elf_loader.cc | 52 +++++++++++++++ testing/elf_loader.h | 8 +++ testing/fixture_test.cc | 1 + testing/fixture_test.h | 3 + testing/testing.gni | 3 + 12 files changed, 220 insertions(+), 16 deletions(-) create mode 100644 runtime/fixtures/split_lib_test.dart diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 777fb223b4b49..96f756020dbbd 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -338,9 +338,10 @@ fml::RefPtr DartIsolate::GetMessageHandlingTaskRunner() const { bool DartIsolate::LoadLoadingUnit( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { tonic::DartState::Scope scope(this); + fml::RefPtr dart_snapshot = DartSnapshot::IsolateSnapshotFromMappings( std::move(snapshot_data), std::move(snapshot_instructions)); @@ -349,9 +350,8 @@ bool DartIsolate::LoadLoadingUnit( loading_unit_id, dart_snapshot->GetDataMapping(), dart_snapshot->GetInstructionsMapping()); if (tonic::LogIfError(result)) { - result = - Dart_DeferredLoadCompleteError(loading_unit_id, Dart_GetError(result), - /*transient*/ true); + LoadLoadingUnitFailure(loading_unit_id, Dart_GetError(result), + /*transient*/ true); return false; } loading_unit_snapshots_.insert(dart_snapshot); @@ -1039,8 +1039,14 @@ void DartIsolate::OnShutdownCallback() { } Dart_Handle DartIsolate::OnDartLoadLibrary(intptr_t loading_unit_id) { - Current()->platform_configuration()->client()->RequestDartDeferredLibrary( - loading_unit_id); + if (Current()->platform_configuration()) { + Current()->platform_configuration()->client()->RequestDartDeferredLibrary( + loading_unit_id); + } else { + FML_LOG(ERROR) << "Platform Configuration was null. Deferred library load " + "request was not sent for loading unit " + << loading_unit_id << "."; + } return Dart_Null(); } diff --git a/runtime/dart_isolate.h b/runtime/dart_isolate.h index c4740f4dd47a5..a45d138fa4690 100644 --- a/runtime/dart_isolate.h +++ b/runtime/dart_isolate.h @@ -387,8 +387,8 @@ class DartIsolate : public UIDartState { bool LoadLoadingUnit( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); void LoadLoadingUnitFailure(intptr_t loading_unit_id, const std::string error_message, diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index a90599fd8a3fb..806b596a4754b 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -430,5 +430,102 @@ TEST_F(DartIsolateTest, ASSERT_EQ(create_callback_count, 1u); } +TEST_F(DartIsolateTest, InvalidLoadingUnitFails) { + if (!DartVM::IsRunningPrecompiledCode()) { + FML_LOG(INFO) << "Split AOT does not work in JIT mode"; + return; + } + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + auto settings = CreateSettingsForFixture(); + auto vm_ref = DartVMRef::Create(settings); + ASSERT_TRUE(vm_ref); + auto vm_data = vm_ref.GetVMData(); + ASSERT_TRUE(vm_data); + TaskRunners task_runners(GetCurrentTestName(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner(), // + GetCurrentTaskRunner() // + ); + auto isolate_configuration = + IsolateConfiguration::InferFromSettings(settings); + auto weak_isolate = DartIsolate::CreateRunningRootIsolate( + vm_data->GetSettings(), // settings + vm_data->GetIsolateSnapshot(), // isolate snapshot + std::move(task_runners), // task runners + nullptr, // window + {}, // snapshot delegate + {}, // hint freed delegate + {}, // io manager + {}, // unref queue + {}, // image decoder + "main.dart", // advisory uri + "main", // advisory entrypoint + DartIsolate::Flags{}, // flags + settings.isolate_create_callback, // isolate create callback + settings.isolate_shutdown_callback, // isolate shutdown callback + "main", // dart entrypoint + std::nullopt, // dart entrypoint library + std::move(isolate_configuration) // isolate configuration + ); + auto root_isolate = weak_isolate.lock(); + ASSERT_TRUE(root_isolate); + ASSERT_EQ(root_isolate->GetPhase(), DartIsolate::Phase::Running); + + auto isolate_data = std::make_unique( + split_aot_symbols_.vm_isolate_data, 0); + auto isolate_instructions = std::make_unique( + split_aot_symbols_.vm_isolate_instrs, 0); + + // Invalid loading unit should fail gracefully with error message. + ASSERT_FALSE(root_isolate->LoadLoadingUnit(3, std::move(isolate_data), + std::move(isolate_instructions))); + ASSERT_TRUE(root_isolate->Shutdown()); +} + +TEST_F(DartIsolateTest, ValidLoadingUnitSucceeds) { + if (!DartVM::IsRunningPrecompiledCode()) { + FML_LOG(INFO) << "Split AOT does not work in JIT mode"; + return; + } + + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + AddNativeCallback("NotifyNative", + CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) { + FML_LOG(ERROR) << "Hello from Dart!"; + Signal(); + }))); + AddNativeCallback( + "NotifySuccess", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + auto bool_handle = Dart_GetNativeArgument(args, 0); + ASSERT_FALSE(tonic::LogIfError(bool_handle)); + ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); + Signal(); + })); + const auto settings = CreateSettingsForFixture(); + auto vm_ref = DartVMRef::Create(settings); + auto thread = CreateNewThread(); + TaskRunners task_runners(GetCurrentTestName(), // + thread, // + thread, // + thread, // + thread // + ); + auto isolate = + RunDartCodeInIsolate(vm_ref, settings, task_runners, + "canCallDeferredLibrary", {}, GetFixturesPath()); + ASSERT_TRUE(isolate); + ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); + Wait(); + + auto isolate_data = std::make_unique( + split_aot_symbols_.vm_isolate_data, 0); + auto isolate_instructions = std::make_unique( + split_aot_symbols_.vm_isolate_instrs, 0); + + ASSERT_TRUE(isolate->get()->LoadLoadingUnit(2, std::move(isolate_data), + std::move(isolate_instructions))); +} + } // namespace testing } // namespace flutter diff --git a/runtime/fixtures/runtime_test.dart b/runtime/fixtures/runtime_test.dart index 616e026a5ccce..4eff9563a5384 100644 --- a/runtime/fixtures/runtime_test.dart +++ b/runtime/fixtures/runtime_test.dart @@ -2,9 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:isolate'; import 'dart:ui'; +import 'split_lib_test.dart' deferred as splitlib; + + void main() { } @@ -25,6 +29,23 @@ void canRegisterNativeCallback() async { print('Called native method from canRegisterNativeCallback'); } +Future? splitLoadFuture = null; + +@pragma('vm:entry-point') +void canCallDeferredLibrary() async { + print('In function canCallDeferredLibrary'); + splitLoadFuture = splitlib.loadLibrary() + .then((_) { + print('Deferred load complete'); + notifySuccess(splitlib.splitAdd(10, 23) == 33); + }) + .catchError((_) { + print('Deferred load error'); + notifySuccess(false); + }); + notifyNative(); +} + void notifyNative() native 'NotifyNative'; @pragma('vm:entry-point') diff --git a/runtime/fixtures/split_lib_test.dart b/runtime/fixtures/split_lib_test.dart new file mode 100644 index 0000000000000..5d811fc14c374 --- /dev/null +++ b/runtime/fixtures/split_lib_test.dart @@ -0,0 +1,9 @@ +// 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. + +library splitlib; + +int splitAdd(int i, int j) { + return i + j; +} diff --git a/shell/common/engine.h b/shell/common/engine.h index 09da5bd1646fb..33ed4abfd3d87 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -798,9 +798,11 @@ class Engine final : public RuntimeDelegate, /// files called 'loading units' when libraries are imported /// as deferred. Each of these shared libraries are identified /// by a unique loading unit id. Callers should open and resolve - /// a SymbolMapping from the shared library. The symbols can - /// then be moved into the dart isolate to be dynamically loaded - /// into the VM via this method. + /// a SymbolMapping from the shared library. The Mappings should + /// be moved into this method, as ownership will be assumed by the + /// dart isolate after successful loading and released after + /// shutdown of the dart isolate. If loading fails, the mappings + /// will naturally go out of scope. /// /// This method is paired with a RequestDartDeferredLibrary /// invocation that provides the embedder with the loading unit id diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 02f866385686b..8d1c04a891592 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -637,10 +637,12 @@ class PlatformView { /// The Dart compiler may generate separate shared libraries /// files called 'loading units' when libraries are imported /// as deferred. Each of these shared libraries are identified - /// by a unique loading unit id. Callers should dlopen the - /// shared library file and use dlsym to resolve the dart - /// symbols. These symbols can then be passed to this method to - /// be dynamically loaded into the VM. + /// by a unique loading unit id. Callers should open and resolve + /// a SymbolMapping from the shared library. The Mappings should + /// be moved into this method, as ownership will be assumed by the + /// dart isolate after successful loading and released after + /// shutdown of the dart isolate. If loading fails, the mappings + /// will naturally go out of scope. /// /// This method is paired with a RequestDartDeferredLibrary /// invocation that provides the embedder with the loading unit id diff --git a/testing/elf_loader.cc b/testing/elf_loader.cc index 618859d00b2f1..a3d8a6fe69a2e 100644 --- a/testing/elf_loader.cc +++ b/testing/elf_loader.cc @@ -4,7 +4,11 @@ #include "flutter/testing/elf_loader.h" +#include + #include "flutter/fml/file.h" +#include "flutter/fml/mapping.h" +#include "flutter/fml/native_library.h" #include "flutter/fml/paths.h" #include "flutter/runtime/dart_vm.h" #include "flutter/testing/testing.h" @@ -60,6 +64,54 @@ ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary() { return symbols; } +ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary() { + if (!DartVM::IsRunningPrecompiledCode()) { + return {}; + } + + const auto elf_path = + fml::paths::JoinPaths({GetFixturesPath(), kAOTAppELFSplitFileName}); + + if (!fml::IsFile(elf_path)) { + FML_LOG(ERROR) << "App AOT file does not exist for this fixture. Attempts " + "to launch the Dart VM with these AOT symbols will fail."; + return {}; + } + + ELFAOTSymbols symbols; + + // Must not be freed. + const char* error = nullptr; + +#if OS_FUCHSIA + // TODO(gw280): https://github.com/flutter/flutter/issues/50285 + // Dart doesn't implement Dart_LoadELF on Fuchsia + auto loaded_elf = nullptr; +#else + auto loaded_elf = + Dart_LoadELF(elf_path.c_str(), // file path + 0, // file offset + &error, // error (out) + &symbols.vm_snapshot_data, // vm snapshot data (out) + &symbols.vm_snapshot_instrs, // vm snapshot instrs (out) + &symbols.vm_isolate_data, // vm isolate data (out) + &symbols.vm_isolate_instrs // vm isolate instr (out) + ); +#endif + + if (loaded_elf == nullptr) { + FML_LOG(ERROR) + << "Could not fetch AOT symbols from loaded ELF. Attempts " + "to launch the Dart VM with these AOT symbols will fail. Error: " + << error; + return {}; + } + + symbols.loaded_elf.reset(loaded_elf); + + return symbols; +} + bool PrepareSettingsForAOTWithSymbols(Settings& settings, const ELFAOTSymbols& symbols) { if (!DartVM::IsRunningPrecompiledCode()) { diff --git a/testing/elf_loader.h b/testing/elf_loader.h index a8c37c92172d9..eb9f619f9347d 100644 --- a/testing/elf_loader.h +++ b/testing/elf_loader.h @@ -9,12 +9,15 @@ #include "flutter/common/settings.h" #include "flutter/fml/macros.h" +#include "flutter/fml/mapping.h" #include "third_party/dart/runtime/bin/elf_loader.h" namespace flutter { namespace testing { inline constexpr const char* kAOTAppELFFileName = "app_elf_snapshot.so"; +inline constexpr const char* kAOTAppELFSplitFileName = + "app_elf_snapshot.so-2.part.so"; struct LoadedELFDeleter { void operator()(Dart_LoadedElf* elf) { ::Dart_UnloadELF(elf); } @@ -40,6 +43,11 @@ struct ELFAOTSymbols { /// ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(); +ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary(); + +std::unique_ptr LoadELFSplitSymbolFromFixturesIfNeccessary( + std::string symbol_name); + //------------------------------------------------------------------------------ /// @brief Prepare the settings objects various AOT mappings resolvers with /// the symbols already loaded. This method does nothing in non-AOT diff --git a/testing/fixture_test.cc b/testing/fixture_test.cc index 3a8a94c138805..5e8f369b4ae3c 100644 --- a/testing/fixture_test.cc +++ b/testing/fixture_test.cc @@ -9,6 +9,7 @@ namespace testing { FixtureTest::FixtureTest() : native_resolver_(std::make_shared()), + split_aot_symbols_(LoadELFSplitSymbolFromFixturesIfNeccessary()), assets_dir_(fml::OpenDirectory(GetFixturesPath(), false, fml::FilePermission::kRead)), diff --git a/testing/fixture_test.h b/testing/fixture_test.h index f48c94319fc99..589f748ee136d 100644 --- a/testing/fixture_test.h +++ b/testing/fixture_test.h @@ -8,6 +8,7 @@ #include #include "flutter/common/settings.h" +#include "flutter/fml/mapping.h" #include "flutter/runtime/dart_vm.h" #include "flutter/testing/elf_loader.h" #include "flutter/testing/test_dart_native_resolver.h" @@ -30,6 +31,8 @@ class FixtureTest : public ThreadTest { std::shared_ptr native_resolver_; + ELFAOTSymbols split_aot_symbols_; + private: fml::UniqueFD assets_dir_; ELFAOTSymbols aot_symbols_; diff --git a/testing/testing.gni b/testing/testing.gni index 37a8302702bd8..2e52afff98b82 100644 --- a/testing/testing.gni +++ b/testing/testing.gni @@ -126,6 +126,8 @@ template("dart_snapshot_aot") { # Custom ELF loader is used for Mac and Windows. elf_object = "$target_gen_dir/assets/app_elf_snapshot.so" + loading_unit_manifest = "$target_gen_dir/assets/loading_unit_manifest.json" + outputs = [ elf_object ] args = [ @@ -133,6 +135,7 @@ template("dart_snapshot_aot") { "--lazy_async_stacks", "--deterministic", "--snapshot_kind=app-aot-elf", + "--loading_unit_manifest=" + rebase_path(loading_unit_manifest), "--elf=" + rebase_path(elf_object), rebase_path(invoker.dart_kernel), ] From 1f62be44b3f7c2f97121ca3b00515254f76f6e84 Mon Sep 17 00:00:00 2001 From: garyqian Date: Tue, 1 Dec 2020 22:42:28 -0800 Subject: [PATCH 13/15] Docs, fix test flakiness, use general Mapping Docs for testing changes Licenses --- ci/licenses_golden/licenses_flutter | 1 + runtime/dart_isolate_unittests.cc | 2 +- runtime/fixtures/runtime_test.dart | 3 +-- runtime/runtime_controller.cc | 4 ++-- runtime/runtime_controller.h | 13 ++++++++----- shell/common/engine.cc | 4 ++-- shell/common/engine.h | 11 ++++++----- shell/common/platform_view.cc | 4 ++-- shell/common/platform_view.h | 17 ++++++++++------- shell/common/shell.cc | 4 ++-- shell/common/shell.h | 4 ++-- shell/platform/android/platform_view_android.cc | 4 ++-- shell/platform/android/platform_view_android.h | 4 ++-- .../Source/FlutterEnginePlatformViewTest.mm | 8 ++++---- .../Source/FlutterPlatformViewsTest.mm | 8 ++++---- .../Source/accessibility_bridge_test.mm | 8 ++++---- .../fuchsia/flutter/platform_view_unittest.cc | 4 ++-- testing/elf_loader.h | 12 +++++++++--- 18 files changed, 64 insertions(+), 51 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index bc681ffb3ba48..e5e9ba58e321f 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -590,6 +590,7 @@ FILE: ../../../flutter/runtime/dart_vm_unittests.cc FILE: ../../../flutter/runtime/embedder_resources.cc FILE: ../../../flutter/runtime/embedder_resources.h FILE: ../../../flutter/runtime/fixtures/runtime_test.dart +FILE: ../../../flutter/runtime/fixtures/split_lib_test.dart FILE: ../../../flutter/runtime/isolate_configuration.cc FILE: ../../../flutter/runtime/isolate_configuration.h FILE: ../../../flutter/runtime/platform_data.cc diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 806b596a4754b..9856d22f03b1b 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -496,7 +496,7 @@ TEST_F(DartIsolateTest, ValidLoadingUnitSucceeds) { Signal(); }))); AddNativeCallback( - "NotifySuccess", CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + "NotifySuccess", CREATE_NATIVE_ENTRY([this](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); ASSERT_FALSE(tonic::LogIfError(bool_handle)); ASSERT_TRUE(tonic::DartConverter::FromDart(bool_handle)); diff --git a/runtime/fixtures/runtime_test.dart b/runtime/fixtures/runtime_test.dart index 4eff9563a5384..d559e6ec3d02b 100644 --- a/runtime/fixtures/runtime_test.dart +++ b/runtime/fixtures/runtime_test.dart @@ -8,7 +8,6 @@ import 'dart:ui'; import 'split_lib_test.dart' deferred as splitlib; - void main() { } @@ -32,7 +31,7 @@ void canRegisterNativeCallback() async { Future? splitLoadFuture = null; @pragma('vm:entry-point') -void canCallDeferredLibrary() async { +void canCallDeferredLibrary() { print('In function canCallDeferredLibrary'); splitLoadFuture = splitlib.loadLibrary() .then((_) { diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 4043bd1807e61..b0fc48ccd18b1 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -419,8 +419,8 @@ std::optional RuntimeController::GetRootIsolateReturnCode() { void RuntimeController::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { root_isolate_.lock()->LoadLoadingUnit(loading_unit_id, std::move(snapshot_data), std::move(snapshot_instructions)); diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 48e0e800a6575..ae9e93af4969d 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -483,9 +483,12 @@ class RuntimeController : public PlatformConfigurationClient { /// files called 'loading units' when libraries are imported /// as deferred. Each of these shared libraries are identified /// by a unique loading unit id. Callers should open and resolve - /// a SymbolMapping from the shared library. The symbols can - /// then be moved into the dart isolate to be dynamically loaded - /// into the VM via this method. + /// a SymbolMapping from the shared library. The Mappings should + /// be moved into this method, as ownership will be assumed by the + /// dart root isolate after successful loading and released after + /// shutdown of the root isolate. The loading unit may not be + /// used after isolate shutdown. If loading fails, the mappings + /// will be released. /// /// This method is paired with a RequestDartDeferredLibrary /// invocation that provides the embedder with the loading unit id @@ -504,8 +507,8 @@ class RuntimeController : public PlatformConfigurationClient { /// void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); // |PlatformConfigurationClient| //-------------------------------------------------------------------------- diff --git a/shell/common/engine.cc b/shell/common/engine.cc index 11f929c583077..7ca7ae9e28720 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -514,8 +514,8 @@ void Engine::RequestDartDeferredLibrary(intptr_t loading_unit_id) { void Engine::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { if (runtime_controller_->IsRootIsolateRunning()) { runtime_controller_->LoadDartDeferredLibrary( loading_unit_id, std::move(snapshot_data), diff --git a/shell/common/engine.h b/shell/common/engine.h index 33ed4abfd3d87..03e676b15b03b 100644 --- a/shell/common/engine.h +++ b/shell/common/engine.h @@ -800,9 +800,10 @@ class Engine final : public RuntimeDelegate, /// by a unique loading unit id. Callers should open and resolve /// a SymbolMapping from the shared library. The Mappings should /// be moved into this method, as ownership will be assumed by the - /// dart isolate after successful loading and released after - /// shutdown of the dart isolate. If loading fails, the mappings - /// will naturally go out of scope. + /// dart root isolate after successful loading and released after + /// shutdown of the root isolate. The loading unit may not be + /// used after isolate shutdown. If loading fails, the mappings + /// will be released. /// /// This method is paired with a RequestDartDeferredLibrary /// invocation that provides the embedder with the loading unit id @@ -821,8 +822,8 @@ class Engine final : public RuntimeDelegate, /// void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); private: Engine::Delegate& delegate_; diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index b24b22d9dc533..9981202f02c9b 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -163,8 +163,8 @@ void PlatformView::RequestDartDeferredLibrary(intptr_t loading_unit_id) {} void PlatformView::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) {} void PlatformView::UpdateAssetManager( std::shared_ptr asset_manager) {} diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index 8d1c04a891592..f2e72e36d2f7e 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -221,9 +221,12 @@ class PlatformView { /// files called 'loading units' when libraries are imported /// as deferred. Each of these shared libraries are identified /// by a unique loading unit id. Callers should open and resolve - /// a SymbolMapping from the shared library. The symbols can - /// then be moved into the dart isolate to be dynamically loaded - /// into the VM via this method. + /// a SymbolMapping from the shared library. The Mappings should + /// be moved into this method, as ownership will be assumed by + /// the dart root isolate after successful loading and released + /// after shutdown of the root isolate. The loading unit may not + /// be used after isolate shutdown. If loading fails, the + /// mappings will be released. /// /// This method is paired with a RequestDartDeferredLibrary /// invocation that provides the embedder with the loading unit @@ -241,8 +244,8 @@ class PlatformView { /// virtual void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) = 0; + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) = 0; // TODO(garyq): Implement a proper asset_resolver replacement instead of // overwriting the entire asset manager. @@ -661,8 +664,8 @@ class PlatformView { /// virtual void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions); + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions); // TODO(garyq): Implement a proper asset_resolver replacement instead of // overwriting the entire asset manager. diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 2ea3b3f24c363..2a23ee783af26 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1187,8 +1187,8 @@ std::unique_ptr> Shell::ComputePlatformResolvedLocale( void Shell::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { engine_->LoadDartDeferredLibrary(loading_unit_id, std::move(snapshot_data), std::move(snapshot_instructions)); } diff --git a/shell/common/shell.h b/shell/common/shell.h index 772a78afbeae9..c3a743778c8db 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -510,8 +510,8 @@ class Shell final : public PlatformView::Delegate, // |PlatformView::Delegate| void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override; + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override; // |PlatformView::Delegate| void UpdateAssetManager(std::shared_ptr asset_manager) override; diff --git a/shell/platform/android/platform_view_android.cc b/shell/platform/android/platform_view_android.cc index 9445451053cfb..2d13e567b8038 100644 --- a/shell/platform/android/platform_view_android.cc +++ b/shell/platform/android/platform_view_android.cc @@ -347,8 +347,8 @@ void PlatformViewAndroid::RequestDartDeferredLibrary(intptr_t loading_unit_id) { // |PlatformView| void PlatformViewAndroid::LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) { + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) { delegate_.LoadDartDeferredLibrary(loading_unit_id, std::move(snapshot_data), std::move(snapshot_instructions)); } diff --git a/shell/platform/android/platform_view_android.h b/shell/platform/android/platform_view_android.h index 140868ae16eda..87ff5bcd917dc 100644 --- a/shell/platform/android/platform_view_android.h +++ b/shell/platform/android/platform_view_android.h @@ -96,8 +96,8 @@ class PlatformViewAndroid final : public PlatformView { // |PlatformView| void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override; + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override; // |PlatformView| void UpdateAssetManager(std::shared_ptr asset_manager) override; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm index afb350742fe29..ed0e5544900ee 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEnginePlatformViewTest.mm @@ -34,10 +34,10 @@ void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} - void LoadDartDeferredLibrary( - intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override {} + void LoadDartDeferredLibrary(intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override { + } void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm index 8b5a89dda4788..5b11fd3eae409 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterPlatformViewsTest.mm @@ -104,10 +104,10 @@ void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} - void LoadDartDeferredLibrary( - intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override {} + void LoadDartDeferredLibrary(intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override { + } void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm index a64cbcca6e1db..677f4eac350bf 100644 --- a/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm +++ b/shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm @@ -87,10 +87,10 @@ void OnPlatformViewRegisterTexture(std::shared_ptr texture) override {} void OnPlatformViewUnregisterTexture(int64_t texture_id) override {} void OnPlatformViewMarkTextureFrameAvailable(int64_t texture_id) override {} - void LoadDartDeferredLibrary( - intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) override {} + void LoadDartDeferredLibrary(intptr_t loading_unit_id, + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) override { + } void UpdateAssetManager(std::shared_ptr asset_manager) override {} }; diff --git a/shell/platform/fuchsia/flutter/platform_view_unittest.cc b/shell/platform/fuchsia/flutter/platform_view_unittest.cc index fa50fd5da4e91..9e05013045a50 100644 --- a/shell/platform/fuchsia/flutter/platform_view_unittest.cc +++ b/shell/platform/fuchsia/flutter/platform_view_unittest.cc @@ -107,8 +107,8 @@ class MockPlatformViewDelegate : public flutter::PlatformView::Delegate { // |flutter::PlatformView::Delegate| void LoadDartDeferredLibrary( intptr_t loading_unit_id, - std::unique_ptr snapshot_data, - std::unique_ptr snapshot_instructions) {} + std::unique_ptr snapshot_data, + std::unique_ptr snapshot_instructions) {} // |flutter::PlatformView::Delegate| void UpdateAssetManager( std::shared_ptr asset_manager) {} diff --git a/testing/elf_loader.h b/testing/elf_loader.h index eb9f619f9347d..7aa6f74272ea3 100644 --- a/testing/elf_loader.h +++ b/testing/elf_loader.h @@ -43,11 +43,17 @@ struct ELFAOTSymbols { /// ELFAOTSymbols LoadELFSymbolFromFixturesIfNeccessary(); +//------------------------------------------------------------------------------ +/// @brief Attempts to resolve split loading unit AOT symbols from the +/// portable ELF loader. If the dart code does not make use of +/// deferred libraries, then there will be no split .so to load. +/// This only returns valid symbols when the VM is configured for +/// AOT. +/// +/// @return The loaded ELF symbols. +/// ELFAOTSymbols LoadELFSplitSymbolFromFixturesIfNeccessary(); -std::unique_ptr LoadELFSplitSymbolFromFixturesIfNeccessary( - std::string symbol_name); - //------------------------------------------------------------------------------ /// @brief Prepare the settings objects various AOT mappings resolvers with /// the symbols already loaded. This method does nothing in non-AOT From c9ca40b4b3d6e0346e9653b8804c36226a026ef4 Mon Sep 17 00:00:00 2001 From: garyqian Date: Wed, 2 Dec 2020 00:07:32 -0800 Subject: [PATCH 14/15] Clean imports Additional cleanup --- runtime/dart_isolate.cc | 4 ---- runtime/runtime_controller.cc | 2 -- testing/elf_loader.cc | 4 ---- testing/elf_loader.h | 1 - 4 files changed, 11 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 96f756020dbbd..d918fef232ac8 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -1042,10 +1042,6 @@ Dart_Handle DartIsolate::OnDartLoadLibrary(intptr_t loading_unit_id) { if (Current()->platform_configuration()) { Current()->platform_configuration()->client()->RequestDartDeferredLibrary( loading_unit_id); - } else { - FML_LOG(ERROR) << "Platform Configuration was null. Deferred library load " - "request was not sent for loading unit " - << loading_unit_id << "."; } return Dart_Null(); } diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index b0fc48ccd18b1..04216f5606875 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -4,8 +4,6 @@ #include "flutter/runtime/runtime_controller.h" -#include - #include "flutter/fml/message_loop.h" #include "flutter/fml/trace_event.h" #include "flutter/lib/ui/compositing/scene.h" diff --git a/testing/elf_loader.cc b/testing/elf_loader.cc index a3d8a6fe69a2e..6d075ecfc15f7 100644 --- a/testing/elf_loader.cc +++ b/testing/elf_loader.cc @@ -4,11 +4,7 @@ #include "flutter/testing/elf_loader.h" -#include - #include "flutter/fml/file.h" -#include "flutter/fml/mapping.h" -#include "flutter/fml/native_library.h" #include "flutter/fml/paths.h" #include "flutter/runtime/dart_vm.h" #include "flutter/testing/testing.h" diff --git a/testing/elf_loader.h b/testing/elf_loader.h index 7aa6f74272ea3..b3dda191f99e8 100644 --- a/testing/elf_loader.h +++ b/testing/elf_loader.h @@ -9,7 +9,6 @@ #include "flutter/common/settings.h" #include "flutter/fml/macros.h" -#include "flutter/fml/mapping.h" #include "third_party/dart/runtime/bin/elf_loader.h" namespace flutter { From df145b932e8a1533d46c4bb3533d594ff6bb70ec Mon Sep 17 00:00:00 2001 From: garyqian Date: Wed, 2 Dec 2020 12:20:19 -0800 Subject: [PATCH 15/15] Address nits --- lib/ui/window/platform_configuration.h | 19 +++++++++++++++++++ runtime/dart_isolate.cc | 10 ++++++++-- runtime/runtime_controller.h | 19 ------------------- testing/elf_loader.h | 6 ++++++ testing/fixture_test.h | 1 - 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/lib/ui/window/platform_configuration.h b/lib/ui/window/platform_configuration.h index c20a68d9653b1..cd22a965b255a 100644 --- a/lib/ui/window/platform_configuration.h +++ b/lib/ui/window/platform_configuration.h @@ -174,6 +174,25 @@ class PlatformConfigurationClient { ComputePlatformResolvedLocale( const std::vector& supported_locale_data) = 0; + //-------------------------------------------------------------------------- + /// @brief Invoked when the Dart VM requests that a deferred library + /// be loaded. Notifies the engine that the deferred library + /// identified by the specified loading unit id should be + /// downloaded and loaded into the Dart VM via + /// `LoadDartDeferredLibrary` + /// + /// Upon encountering errors or otherwise failing to load a + /// loading unit with the specified id, the failure should be + /// directly reported to dart by calling + /// `LoadDartDeferredLibraryFailure` to ensure the waiting dart + /// future completes with an error. + /// + /// @param[in] loading_unit_id The unique id of the deferred library's + /// loading unit. This id is to be passed + /// back into LoadDartDeferredLibrary + /// in order to identify which deferred + /// library to load. + /// virtual void RequestDartDeferredLibrary(intptr_t loading_unit_id) = 0; protected: diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index d918fef232ac8..2a14f6afbdd5e 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -1042,8 +1042,14 @@ Dart_Handle DartIsolate::OnDartLoadLibrary(intptr_t loading_unit_id) { if (Current()->platform_configuration()) { Current()->platform_configuration()->client()->RequestDartDeferredLibrary( loading_unit_id); - } - return Dart_Null(); + return Dart_Null(); + } + const std::string error_message = + "Platform Configuration was null. Deferred library load request" + "for loading unit id " + + std::to_string(loading_unit_id) + " was not sent."; + FML_LOG(ERROR) << error_message; + return Dart_NewApiError(error_message.c_str()); } DartIsolate::AutoFireClosure::AutoFireClosure(const fml::closure& closure) diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index ae9e93af4969d..3b4ef073fc411 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -511,25 +511,6 @@ class RuntimeController : public PlatformConfigurationClient { std::unique_ptr snapshot_instructions); // |PlatformConfigurationClient| - //-------------------------------------------------------------------------- - /// @brief Invoked when the Dart VM requests that a deferred library - /// be loaded. Notifies the engine that the deferred library - /// identified by the specified loading unit id should be - /// downloaded and loaded into the Dart VM via - /// `LoadDartDeferredLibrary` - /// - /// Upon encountering errors or otherwise failing to load a - /// loading unit with the specified id, the failure should be - /// directly reported to dart by calling - /// `LoadDartDeferredLibraryFailure` to ensure the waiting dart - /// future completes with an error. - /// - /// @param[in] loading_unit_id The unique id of the deferred library's - /// loading unit. This id is to be passed - /// back into LoadDartDeferredLibrary - /// in order to identify which deferred - /// library to load. - /// void RequestDartDeferredLibrary(intptr_t loading_unit_id) override; protected: diff --git a/testing/elf_loader.h b/testing/elf_loader.h index b3dda191f99e8..d5861f04dee43 100644 --- a/testing/elf_loader.h +++ b/testing/elf_loader.h @@ -15,6 +15,12 @@ namespace flutter { namespace testing { inline constexpr const char* kAOTAppELFFileName = "app_elf_snapshot.so"; + +// This file name is what gen_snapshot defaults to. It is based off of the +// name of the base file, with the `2` indicating that this split corresponds +// to loading unit id of 2. The base module id is 1 and is omitted as it is not +// considered a split. If dart changes the naming convention, this should be +// changed to match, however, this is considered unlikely to happen. inline constexpr const char* kAOTAppELFSplitFileName = "app_elf_snapshot.so-2.part.so"; diff --git a/testing/fixture_test.h b/testing/fixture_test.h index 589f748ee136d..82a82cb8e2f66 100644 --- a/testing/fixture_test.h +++ b/testing/fixture_test.h @@ -8,7 +8,6 @@ #include #include "flutter/common/settings.h" -#include "flutter/fml/mapping.h" #include "flutter/runtime/dart_vm.h" #include "flutter/testing/elf_loader.h" #include "flutter/testing/test_dart_native_resolver.h"