From f8594e47bfbf6f77359627e84f30347a0f8f90dd Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Oct 2023 10:09:26 -0700 Subject: [PATCH 1/3] Impl --- shell/common/shell.cc | 33 ++++++++++++++++++++------------- shell/common/shell.h | 38 +++++++++++++++++++++++++++----------- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index a7f7c9ae4c782..138b3971dafb1 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -144,6 +144,24 @@ void PerformInitializationTasks(Settings& settings) { } // namespace +DartVMRef Shell::InferVmInitDataFromSettings( + fml::RefPtr& isolate_snapshot, + Settings& settings) { + // Always use the `vm_snapshot` and `isolate_snapshot` provided by the + // settings to launch the VM. If the VM is already running, the snapshot + // arguments are ignored. + auto vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings); + isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); + auto vm = DartVMRef::Create(settings, vm_snapshot, isolate_snapshot); + + // If the settings did not specify an `isolate_snapshot`, fall back to the + // one the VM was launched with. + if (!isolate_snapshot) { + isolate_snapshot = vm->GetVMData()->GetIsolateSnapshot(); + } + return vm; +} + std::unique_ptr Shell::Create( const PlatformData& platform_data, const TaskRunners& task_runners, @@ -156,19 +174,8 @@ std::unique_ptr Shell::Create( TRACE_EVENT0("flutter", "Shell::Create"); - // Always use the `vm_snapshot` and `isolate_snapshot` provided by the - // settings to launch the VM. If the VM is already running, the snapshot - // arguments are ignored. - auto vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings); - auto isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); - auto vm = DartVMRef::Create(settings, vm_snapshot, isolate_snapshot); - FML_CHECK(vm) << "Must be able to initialize the VM."; - - // If the settings did not specify an `isolate_snapshot`, fall back to the - // one the VM was launched with. - if (!isolate_snapshot) { - isolate_snapshot = vm->GetVMData()->GetIsolateSnapshot(); - } + fml::RefPtr isolate_snapshot; + auto vm = InferVmInitDataFromSettings(isolate_snapshot, settings); auto resource_cache_limit_calculator = std::make_shared( settings.resource_cache_max_bytes_threshold); diff --git a/shell/common/shell.h b/shell/common/shell.h index 039eb653a05e7..74a7cfc54ecbe 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -438,15 +438,29 @@ class Shell final : public PlatformView::Delegate, const std::shared_ptr GetConcurrentWorkerTaskRunner() const; + // Infer the VM ref and the isolate snapshot based on the settings. + // + // The inferred VM ref is returned, while the inferred isolate snapshot + // is assigned through the reference parameter. + // + // If the VM is already running, the settings are ignored, but the resulting + // isolate snapshot always prioritize what is specified by the settings, and + // falls back to the one VM was launched with. + // + // This function is what Shell::Create uses to infer snapshot settings. + static DartVMRef InferVmInitDataFromSettings( + fml::RefPtr& out_isolate_snapshot, + Settings& settings); + private: using ServiceProtocolHandler = std::function; /// A collection of message channels (by name) that have sent at least one - /// message from a non-platform thread. Used to prevent printing the error log - /// more than once per channel, as a badly behaving plugin may send multiple - /// messages per second indefinitely. + /// message from a non-platform thread. Used to prevent printing the error + /// log more than once per channel, as a badly behaving plugin may send + /// multiple messages per second indefinitely. std::mutex misbehaving_message_channels_mutex_; std::set misbehaving_message_channels_; const TaskRunners task_runners_; @@ -497,19 +511,20 @@ class Shell final : public PlatformView::Delegate, bool frame_timings_report_scheduled_ = false; // Vector of FrameTiming::kCount * n timestamps for n frames whose timings - // have not been reported yet. Vector of ints instead of FrameTiming is stored - // here for easier conversions to Dart objects. + // have not been reported yet. Vector of ints instead of FrameTiming is + // stored here for easier conversions to Dart objects. std::vector unreported_timings_; - /// Manages the displays. This class is thread safe, can be accessed from any - /// of the threads. + /// Manages the displays. This class is thread safe, can be accessed from + /// any of the threads. std::unique_ptr display_manager_; // protects expected_frame_size_ which is set on platform thread and read on // raster thread std::mutex resize_mutex_; - // used to discard wrong size layer tree produced during interactive resizing + // used to discard wrong size layer tree produced during interactive + // resizing std::unordered_map expected_frame_sizes_; // Used to communicate the right frame bounds via service protocol. @@ -746,7 +761,8 @@ class Shell final : public PlatformView::Delegate, // Service protocol handler // - // The returned SkSLs are base64 encoded. Decode before storing them to files. + // The returned SkSLs are base64 encoded. Decode before storing them to + // files. bool OnServiceProtocolGetSkSLs( const ServiceProtocol::Handler::ServiceProtocolMap& params, rapidjson::Document* response); @@ -767,8 +783,8 @@ class Shell final : public PlatformView::Delegate, // Service protocol handler // - // Forces the FontCollection to reload the font manifest. Used to support hot - // reload for fonts. + // Forces the FontCollection to reload the font manifest. Used to support + // hot reload for fonts. bool OnServiceProtocolReloadAssetFonts( const ServiceProtocol::Handler::ServiceProtocolMap& params, rapidjson::Document* response); From ec703df15b410b1f7512e3dd923c24c3fc7f5636 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Oct 2023 10:20:09 -0700 Subject: [PATCH 2/3] TODO --- shell/common/shell.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/shell/common/shell.h b/shell/common/shell.h index 74a7cfc54ecbe..c08d06b473a59 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -448,6 +448,10 @@ class Shell final : public PlatformView::Delegate, // falls back to the one VM was launched with. // // This function is what Shell::Create uses to infer snapshot settings. + // + // TODO(dkwingsmt): Extracting this method is part of a bigger change. If the + // entire change is not eventually landed, we should merge this method back + // to Create. https://github.com/flutter/flutter/issues/136826 static DartVMRef InferVmInitDataFromSettings( fml::RefPtr& out_isolate_snapshot, Settings& settings); From a608fd2a828d421e9277e61eb8388039047c1c01 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 18 Oct 2023 12:34:45 -0700 Subject: [PATCH 3/3] Revert to pair return value --- shell/common/shell.cc | 12 +++++------- shell/common/shell.h | 10 +++------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 138b3971dafb1..a6ba487b581f7 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -144,14 +144,13 @@ void PerformInitializationTasks(Settings& settings) { } // namespace -DartVMRef Shell::InferVmInitDataFromSettings( - fml::RefPtr& isolate_snapshot, - Settings& settings) { +std::pair> +Shell::InferVmInitDataFromSettings(Settings& settings) { // Always use the `vm_snapshot` and `isolate_snapshot` provided by the // settings to launch the VM. If the VM is already running, the snapshot // arguments are ignored. auto vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings); - isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); + auto isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); auto vm = DartVMRef::Create(settings, vm_snapshot, isolate_snapshot); // If the settings did not specify an `isolate_snapshot`, fall back to the @@ -159,7 +158,7 @@ DartVMRef Shell::InferVmInitDataFromSettings( if (!isolate_snapshot) { isolate_snapshot = vm->GetVMData()->GetIsolateSnapshot(); } - return vm; + return {std::move(vm), isolate_snapshot}; } std::unique_ptr Shell::Create( @@ -174,8 +173,7 @@ std::unique_ptr Shell::Create( TRACE_EVENT0("flutter", "Shell::Create"); - fml::RefPtr isolate_snapshot; - auto vm = InferVmInitDataFromSettings(isolate_snapshot, settings); + auto [vm, isolate_snapshot] = InferVmInitDataFromSettings(settings); auto resource_cache_limit_calculator = std::make_shared( settings.resource_cache_max_bytes_threshold); diff --git a/shell/common/shell.h b/shell/common/shell.h index c08d06b473a59..9eb5bd1e9b226 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -440,10 +440,7 @@ class Shell final : public PlatformView::Delegate, // Infer the VM ref and the isolate snapshot based on the settings. // - // The inferred VM ref is returned, while the inferred isolate snapshot - // is assigned through the reference parameter. - // - // If the VM is already running, the settings are ignored, but the resulting + // If the VM is already running, the settings are ignored, but the returned // isolate snapshot always prioritize what is specified by the settings, and // falls back to the one VM was launched with. // @@ -452,9 +449,8 @@ class Shell final : public PlatformView::Delegate, // TODO(dkwingsmt): Extracting this method is part of a bigger change. If the // entire change is not eventually landed, we should merge this method back // to Create. https://github.com/flutter/flutter/issues/136826 - static DartVMRef InferVmInitDataFromSettings( - fml::RefPtr& out_isolate_snapshot, - Settings& settings); + static std::pair> + InferVmInitDataFromSettings(Settings& settings); private: using ServiceProtocolHandler =