From 68c553a22b92693dd49adf5030ab9df8be549114 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Fri, 19 Apr 2019 12:12:29 -0700 Subject: [PATCH] Add factory methods to FileMapping that make it easy to create common mappings. The GetMapping calls removed in this patch had the same code and had to be repeated across different test harnesses as well as in dart_snapshot.cc. Just make this a factory method so the code is less verbose. --- fml/mapping.cc | 49 +++++++++++++++++++ fml/mapping.h | 13 +++++ runtime/dart_snapshot.cc | 26 ++-------- runtime/runtime_test.cc | 39 +++------------ shell/common/shell_test.cc | 39 +++------------ .../embedder/tests/embedder_context.cc | 37 +++----------- 6 files changed, 87 insertions(+), 116 deletions(-) diff --git a/fml/mapping.cc b/fml/mapping.cc index 75ca1b396912d..7ffacfb493890 100644 --- a/fml/mapping.cc +++ b/fml/mapping.cc @@ -14,6 +14,55 @@ uint8_t* FileMapping::GetMutableMapping() { return mutable_mapping_; } +std::unique_ptr FileMapping::CreateReadOnly( + const std::string& path) { + return CreateReadOnly(OpenFile(path.c_str(), false, FilePermission::kRead), + ""); +} + +std::unique_ptr FileMapping::CreateReadOnly( + const fml::UniqueFD& base_fd, + const std::string& sub_path) { + if (sub_path.size() != 0) { + return CreateReadOnly( + OpenFile(base_fd, sub_path.c_str(), false, FilePermission::kRead), ""); + } + + auto mapping = std::make_unique( + base_fd, std::initializer_list{Protection::kRead}); + + if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) { + return nullptr; + } + + return mapping; +} + +std::unique_ptr FileMapping::CreateReadExecute( + const std::string& path) { + return CreateReadExecute( + OpenFile(path.c_str(), false, FilePermission::kRead)); +} + +std::unique_ptr FileMapping::CreateReadExecute( + const fml::UniqueFD& base_fd, + const std::string& sub_path) { + if (sub_path.size() != 0) { + return CreateReadExecute( + OpenFile(base_fd, sub_path.c_str(), false, FilePermission::kRead), ""); + } + + auto mapping = std::make_unique( + base_fd, std::initializer_list{Protection::kRead, + Protection::kExecute}); + + if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) { + return nullptr; + } + + return mapping; +} + // Data Mapping DataMapping::DataMapping(std::vector data) : data_(std::move(data)) {} diff --git a/fml/mapping.h b/fml/mapping.h index f7e6bbd4231d6..90b88cc15840c 100644 --- a/fml/mapping.h +++ b/fml/mapping.h @@ -46,6 +46,19 @@ class FileMapping final : public Mapping { ~FileMapping() override; + static std::unique_ptr CreateReadOnly(const std::string& path); + + static std::unique_ptr CreateReadOnly( + const fml::UniqueFD& base_fd, + const std::string& sub_path = ""); + + static std::unique_ptr CreateReadExecute( + const std::string& path); + + static std::unique_ptr CreateReadExecute( + const fml::UniqueFD& base_fd, + const std::string& sub_path = ""); + // |Mapping| size_t GetSize() const override; diff --git a/runtime/dart_snapshot.cc b/runtime/dart_snapshot.cc index fb49f990a4f3c..c666d1c236e62 100644 --- a/runtime/dart_snapshot.cc +++ b/runtime/dart_snapshot.cc @@ -21,33 +21,13 @@ const char* DartSnapshot::kIsolateInstructionsSymbol = "kDartIsolateSnapshotInstructions"; static std::unique_ptr GetFileMapping( - const std::string path, + const std::string& path, bool executable) { - fml::UniqueFD file = - fml::OpenFile(path.c_str(), // file path - false, // create file if necessary - fml::FilePermission::kRead // file permissions - ); - - if (!file.is_valid()) { - return nullptr; - } - - using Prot = fml::FileMapping::Protection; - std::unique_ptr mapping; if (executable) { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead, Prot::kExecute}); + return fml::FileMapping::CreateReadExecute(path); } else { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead}); + return fml::FileMapping::CreateReadOnly(path); } - - if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) { - return nullptr; - } - - return mapping; } // The first party embedders don't yet use the stable embedder API and depend on diff --git a/runtime/runtime_test.cc b/runtime/runtime_test.cc index 6f56b2f743f26..97d42bc431418 100644 --- a/runtime/runtime_test.cc +++ b/runtime/runtime_test.cc @@ -15,32 +15,6 @@ RuntimeTest::RuntimeTest() RuntimeTest::~RuntimeTest() = default; -static std::unique_ptr GetMapping(const fml::UniqueFD& directory, - const char* path, - bool executable) { - fml::UniqueFD file = fml::OpenFile(directory, path, false /* create */, - fml::FilePermission::kRead); - if (!file.is_valid()) { - return nullptr; - } - - using Prot = fml::FileMapping::Protection; - std::unique_ptr mapping; - if (executable) { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead, Prot::kExecute}); - } else { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead}); - } - - if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) { - return nullptr; - } - - return mapping; -} - void RuntimeTest::SetSnapshotsAndAssets(Settings& settings) { if (!assets_dir_.is_valid()) { return; @@ -52,27 +26,30 @@ void RuntimeTest::SetSnapshotsAndAssets(Settings& settings) { // don't need to be explicitly suppiled by the embedder. if (DartVM::IsRunningPrecompiledCode()) { settings.vm_snapshot_data = [this]() { - return GetMapping(assets_dir_, "vm_snapshot_data", false); + return fml::FileMapping::CreateReadOnly(assets_dir_, "vm_snapshot_data"); }; settings.isolate_snapshot_data = [this]() { - return GetMapping(assets_dir_, "isolate_snapshot_data", false); + return fml::FileMapping::CreateReadOnly(assets_dir_, + "isolate_snapshot_data"); }; if (DartVM::IsRunningPrecompiledCode()) { settings.vm_snapshot_instr = [this]() { - return GetMapping(assets_dir_, "vm_snapshot_instr", true); + return fml::FileMapping::CreateReadExecute(assets_dir_, + "vm_snapshot_instr"); }; settings.isolate_snapshot_instr = [this]() { - return GetMapping(assets_dir_, "isolate_snapshot_instr", true); + return fml::FileMapping::CreateReadExecute(assets_dir_, + "isolate_snapshot_instr"); }; } } else { settings.application_kernels = [this]() { std::vector> kernel_mappings; kernel_mappings.emplace_back( - GetMapping(assets_dir_, "kernel_blob.bin", false)); + fml::FileMapping::CreateReadOnly(assets_dir_, "kernel_blob.bin")); return kernel_mappings; }; } diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 9a9c16db41239..bf70163ad3dd8 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -18,32 +18,6 @@ ShellTest::ShellTest() ShellTest::~ShellTest() = default; -static std::unique_ptr GetMapping(const fml::UniqueFD& directory, - const char* path, - bool executable) { - fml::UniqueFD file = fml::OpenFile(directory, path, false /* create */, - fml::FilePermission::kRead); - if (!file.is_valid()) { - return nullptr; - } - - using Prot = fml::FileMapping::Protection; - std::unique_ptr mapping; - if (executable) { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead, Prot::kExecute}); - } else { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead}); - } - - if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) { - return nullptr; - } - - return mapping; -} - void ShellTest::SetSnapshotsAndAssets(Settings& settings) { if (!assets_dir_.is_valid()) { return; @@ -55,27 +29,30 @@ void ShellTest::SetSnapshotsAndAssets(Settings& settings) { // don't need to be explicitly suppiled by the embedder. if (DartVM::IsRunningPrecompiledCode()) { settings.vm_snapshot_data = [this]() { - return GetMapping(assets_dir_, "vm_snapshot_data", false); + return fml::FileMapping::CreateReadOnly(assets_dir_, "vm_snapshot_data"); }; settings.isolate_snapshot_data = [this]() { - return GetMapping(assets_dir_, "isolate_snapshot_data", false); + return fml::FileMapping::CreateReadOnly(assets_dir_, + "isolate_snapshot_data"); }; if (DartVM::IsRunningPrecompiledCode()) { settings.vm_snapshot_instr = [this]() { - return GetMapping(assets_dir_, "vm_snapshot_instr", true); + return fml::FileMapping::CreateReadExecute(assets_dir_, + "vm_snapshot_instr"); }; settings.isolate_snapshot_instr = [this]() { - return GetMapping(assets_dir_, "isolate_snapshot_instr", true); + return fml::FileMapping::CreateReadExecute(assets_dir_, + "isolate_snapshot_instr"); }; } } else { settings.application_kernels = [this]() { std::vector> kernel_mappings; kernel_mappings.emplace_back( - GetMapping(assets_dir_, "kernel_blob.bin", false)); + fml::FileMapping::CreateReadOnly(assets_dir_, "kernel_blob.bin")); return kernel_mappings; }; } diff --git a/shell/platform/embedder/tests/embedder_context.cc b/shell/platform/embedder/tests/embedder_context.cc index 870c18d4d8a13..eecec06fe8c9a 100644 --- a/shell/platform/embedder/tests/embedder_context.cc +++ b/shell/platform/embedder/tests/embedder_context.cc @@ -9,46 +9,21 @@ namespace flutter { namespace testing { -static std::unique_ptr GetMapping(const fml::UniqueFD& directory, - const char* path, - bool executable) { - fml::UniqueFD file = fml::OpenFile(directory, path, false /* create */, - fml::FilePermission::kRead); - if (!file.is_valid()) { - return nullptr; - } - - using Prot = fml::FileMapping::Protection; - std::unique_ptr mapping; - if (executable) { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead, Prot::kExecute}); - } else { - mapping = std::make_unique( - file, std::initializer_list{Prot::kRead}); - } - - if (mapping->GetSize() == 0 || mapping->GetMapping() == nullptr) { - return nullptr; - } - - return mapping; -} - EmbedderContext::EmbedderContext(std::string assets_path) : assets_path_(std::move(assets_path)), native_resolver_(std::make_shared<::testing::TestDartNativeResolver>()) { auto assets_dir = fml::OpenDirectory(assets_path_.c_str(), false, fml::FilePermission::kRead); - vm_snapshot_data_ = GetMapping(assets_dir, "vm_snapshot_data", false); + vm_snapshot_data_ = + fml::FileMapping::CreateReadOnly(assets_dir, "vm_snapshot_data"); isolate_snapshot_data_ = - GetMapping(assets_dir, "isolate_snapshot_data", false); + fml::FileMapping::CreateReadOnly(assets_dir, "isolate_snapshot_data"); if (flutter::DartVM::IsRunningPrecompiledCode()) { vm_snapshot_instructions_ = - GetMapping(assets_dir, "vm_snapshot_instr", true); - isolate_snapshot_instructions_ = - GetMapping(assets_dir, "isolate_snapshot_instr", true); + fml::FileMapping::CreateReadExecute(assets_dir, "vm_snapshot_instr"); + isolate_snapshot_instructions_ = fml::FileMapping::CreateReadExecute( + assets_dir, "isolate_snapshot_instr"); } isolate_create_callbacks_.push_back(