From 671cd0f32a3208d7436f2c0528a6ed2f0a54d2e9 Mon Sep 17 00:00:00 2001 From: James Robinson Date: Thu, 11 Jun 2020 17:03:44 -0700 Subject: [PATCH] [fuchsia] Add ability to configure separate data and asset dirs This allows Fuchsia components executed by the Flutter runner to specify a directory containing assets if they wish to store assets separate from program data. This is specified in the program metadata field within the component's specification with the new "assets" attribute. If this attribute is absent, assets are loaded relative to the path specified in the "data" attribute as before. This is useful in the short term to use a location in the package where we can store small files more efficiently. It is also potentially useful longer term to enforce a stronger separatation between executable program data and non-executable assets. This commit adds some basic unit testing for the data parsing to the flutter_runner_tests suite. --- ci/licenses_golden/licenses_flutter | 1 + shell/platform/fuchsia/flutter/BUILD.gn | 4 + shell/platform/fuchsia/flutter/component.cc | 76 +++++++++++++------ shell/platform/fuchsia/flutter/component.h | 7 +- .../fuchsia/flutter/component_unittest.cc | 51 +++++++++++++ 5 files changed, 113 insertions(+), 26 deletions(-) create mode 100644 shell/platform/fuchsia/flutter/component_unittest.cc diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 2770d2953015b..8550f0ae525a4 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -1101,6 +1101,7 @@ FILE: ../../../flutter/shell/platform/fuchsia/flutter/accessibility_bridge_unitt FILE: ../../../flutter/shell/platform/fuchsia/flutter/collect_traces.dart FILE: ../../../flutter/shell/platform/fuchsia/flutter/component.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/component.h +FILE: ../../../flutter/shell/platform/fuchsia/flutter/component_unittest.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/compositor_context.cc FILE: ../../../flutter/shell/platform/fuchsia/flutter/compositor_context.h FILE: ../../../flutter/shell/platform/fuchsia/flutter/engine.cc diff --git a/shell/platform/fuchsia/flutter/BUILD.gn b/shell/platform/fuchsia/flutter/BUILD.gn index 53ffc29c0f8b8..9b53c3943e53e 100644 --- a/shell/platform/fuchsia/flutter/BUILD.gn +++ b/shell/platform/fuchsia/flutter/BUILD.gn @@ -271,6 +271,9 @@ executable("flutter_runner_unittests") { "accessibility_bridge.cc", "accessibility_bridge.h", "accessibility_bridge_unittest.cc", + "component.cc", + "component.h", + "component_unittest.cc", "flutter_runner_fakes.h", "flutter_runner_product_configuration.cc", "flutter_runner_product_configuration.h", @@ -315,6 +318,7 @@ executable("flutter_runner_unittests") { "//build/fuchsia/pkg:async-loop-default", "//build/fuchsia/pkg:scenic_cpp", "//build/fuchsia/pkg:sys_cpp_testing", + "//flutter/common", "//flutter/flow", "//flutter/lib/ui", "//flutter/runtime", diff --git a/shell/platform/fuchsia/flutter/component.cc b/shell/platform/fuchsia/flutter/component.cc index 2cf27a6a60976..00cd9d318ae83 100644 --- a/shell/platform/fuchsia/flutter/component.cc +++ b/shell/platform/fuchsia/flutter/component.cc @@ -51,9 +51,33 @@ constexpr uint32_t OPEN_RIGHT_EXECUTABLE = 8u; namespace flutter_runner { constexpr char kDataKey[] = "data"; +constexpr char kAssetsKey[] = "assets"; constexpr char kTmpPath[] = "/tmp"; constexpr char kServiceRootPath[] = "/svc"; +// static +void Application::ParseProgramMetadata( + const fidl::VectorPtr& program_metadata, + std::string* data_path, + std::string* assets_path) { + if (!program_metadata.has_value()) { + return; + } + for (const auto& pg : *program_metadata) { + if (pg.key.compare(kDataKey) == 0) { + *data_path = "pkg/" + pg.value; + } else if (pg.key.compare(kAssetsKey) == 0) { + *assets_path = "pkg/" + pg.value; + } + } + + // assets_path defaults to the same as data_path if omitted. + if (assets_path->empty()) { + *assets_path = *data_path; + } +} + +// static ActiveApplication Application::Create( TerminationCallback termination_callback, fuchsia::sys::Package package, @@ -149,14 +173,11 @@ Application::Application( settings_.dart_entrypoint_args = arguments.value(); } - // Determine /pkg/data directory from StartupInfo. + // Determine where data and assets are stored within /pkg. std::string data_path; - for (size_t i = 0; i < startup_info.program_metadata->size(); ++i) { - auto pg = startup_info.program_metadata->at(i); - if (pg.key.compare(kDataKey) == 0) { - data_path = "pkg/" + pg.value; - } - } + std::string assets_path; + ParseProgramMetadata(startup_info.program_metadata, &data_path, &assets_path); + if (data_path.empty()) { FML_DLOG(ERROR) << "Could not find a /pkg/data directory for " << package.resolved_url; @@ -189,11 +210,20 @@ Application::Application( } } - application_directory_.reset(fdio_ns_opendir(fdio_ns_.get())); - FML_DCHECK(application_directory_.is_valid()); + { + fml::UniqueFD ns_fd(fdio_ns_opendir(fdio_ns_.get())); + FML_DCHECK(ns_fd.is_valid()); + + constexpr mode_t mode = O_RDONLY | O_DIRECTORY; - application_assets_directory_.reset(openat( - application_directory_.get(), data_path.c_str(), O_RDONLY | O_DIRECTORY)); + application_assets_directory_.reset( + openat(ns_fd.get(), assets_path.c_str(), mode)); + FML_DCHECK(application_assets_directory_.is_valid()); + + application_data_directory_.reset( + openat(ns_fd.get(), data_path.c_str(), mode)); + FML_DCHECK(application_data_directory_.is_valid()); + } // TODO: LaunchInfo::out. @@ -294,7 +324,7 @@ Application::Application( std::string app_framework; if (dart_utils::ReadFileToString("pkg/data/runner.frameworkversion", &runner_framework) && - dart_utils::ReadFileToStringAt(application_assets_directory_.get(), + dart_utils::ReadFileToStringAt(application_data_directory_.get(), "app.frameworkversion", &app_framework) && (runner_framework.compare(app_framework) == 0)) { @@ -509,7 +539,7 @@ void Application::AttemptVMLaunchWithCurrentSettings( std::shared_ptr snapshot = std::make_shared(); - if (snapshot->Load(application_assets_directory_.get(), + if (snapshot->Load(application_data_directory_.get(), "app_aot_snapshot.so")) { const uint8_t* isolate_data = snapshot->IsolateData(); const uint8_t* isolate_instructions = snapshot->IsolateInstrs(); @@ -531,20 +561,16 @@ void Application::AttemptVMLaunchWithCurrentSettings( hold_snapshot)); } else { vm_snapshot = fml::MakeRefCounted( - CreateWithContentsOfFile( - application_assets_directory_.get() /* /pkg/data */, - "vm_snapshot_data.bin", false), - CreateWithContentsOfFile( - application_assets_directory_.get() /* /pkg/data */, - "vm_snapshot_instructions.bin", true)); + CreateWithContentsOfFile(application_data_directory_.get(), + "vm_snapshot_data.bin", false), + CreateWithContentsOfFile(application_data_directory_.get(), + "vm_snapshot_instructions.bin", true)); isolate_snapshot_ = fml::MakeRefCounted( - CreateWithContentsOfFile( - application_assets_directory_.get() /* /pkg/data */, - "isolate_snapshot_data.bin", false), - CreateWithContentsOfFile( - application_assets_directory_.get() /* /pkg/data */, - "isolate_snapshot_instructions.bin", true)); + CreateWithContentsOfFile(application_data_directory_.get(), + "isolate_snapshot_data.bin", false), + CreateWithContentsOfFile(application_data_directory_.get(), + "isolate_snapshot_instructions.bin", true)); } auto vm = flutter::DartVMRef::Create(settings_, // diff --git a/shell/platform/fuchsia/flutter/component.h b/shell/platform/fuchsia/flutter/component.h index aa0dd1ddf547a..7c4cc12110dfb 100644 --- a/shell/platform/fuchsia/flutter/component.h +++ b/shell/platform/fuchsia/flutter/component.h @@ -70,6 +70,11 @@ class Application final : public Engine::Delegate, // may be collected after. ~Application(); + static void ParseProgramMetadata( + const fidl::VectorPtr& program_metadata, + std::string* data_path, + std::string* assets_path); + const std::string& GetDebugLabel() const; #if !defined(DART_PRODUCT) @@ -82,7 +87,7 @@ class Application final : public Engine::Delegate, TerminationCallback termination_callback_; const std::string debug_label_; UniqueFDIONS fdio_ns_ = UniqueFDIONSCreate(); - fml::UniqueFD application_directory_; + fml::UniqueFD application_data_directory_; fml::UniqueFD application_assets_directory_; fidl::Binding application_controller_; diff --git a/shell/platform/fuchsia/flutter/component_unittest.cc b/shell/platform/fuchsia/flutter/component_unittest.cc new file mode 100644 index 0000000000000..c0183d477bb6a --- /dev/null +++ b/shell/platform/fuchsia/flutter/component_unittest.cc @@ -0,0 +1,51 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/shell/platform/fuchsia/flutter/component.h" + +#include + +namespace flutter_runner { +namespace { + +TEST(Component, ParseProgramMetadata) { + std::string data_path; + std::string assets_path; + + // The ProgramMetadata field may be null. We should parse this as if no + // fields were specified. + Application::ParseProgramMetadata(nullptr, &data_path, &assets_path); + + EXPECT_EQ(data_path, ""); + EXPECT_EQ(assets_path, ""); + + // The ProgramMetadata field may be empty. Treat this the same as null. + fidl::VectorPtr program_metadata(size_t{0}); + + Application::ParseProgramMetadata(program_metadata, &data_path, &assets_path); + + EXPECT_EQ(data_path, ""); + EXPECT_EQ(assets_path, ""); + + // The assets_path defaults to the "data" value if unspecified + program_metadata = {{"data", "foobar"}}; + + Application::ParseProgramMetadata(program_metadata, &data_path, &assets_path); + + EXPECT_EQ(data_path, "pkg/foobar"); + EXPECT_EQ(assets_path, "pkg/foobar"); + + data_path = ""; + assets_path = ""; + + program_metadata = {{"not_data", "foo"}, {"data", "bar"}, {"assets", "baz"}}; + + Application::ParseProgramMetadata(program_metadata, &data_path, &assets_path); + + EXPECT_EQ(data_path, "pkg/bar"); + EXPECT_EQ(assets_path, "pkg/baz"); +} + +} // anonymous namespace +} // namespace flutter_runner