From deee07150c80d8d7f614f8a898e2ef4a33fe68a1 Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Wed, 7 Sep 2016 00:22:27 -0700 Subject: [PATCH] Introduce UnzipperProvider (flutter/engine#3004) This patch abstracts where the content of the zip file is stored. Currently, zip files are stored in the file system, but in Fuchsia, we're going to store them in memory (at least for the time being). Rather than represent a zip file as a path in the file system, we instead use an UnzipperProvider, which can create zip::UniqueUnzipper objects on demand. --- engine/src/flutter/assets/BUILD.gn | 5 ++-- engine/src/flutter/assets/unique_unzipper.cc | 15 ----------- engine/src/flutter/assets/unique_unzipper.h | 22 --------------- engine/src/flutter/assets/unzip_job.cc | 19 +++++-------- engine/src/flutter/assets/unzip_job.h | 7 +++-- .../src/flutter/assets/unzipper_provider.cc | 21 +++++++++++++++ engine/src/flutter/assets/unzipper_provider.h | 20 ++++++++++++++ engine/src/flutter/assets/zip_asset_store.cc | 27 ++++++++++--------- engine/src/flutter/assets/zip_asset_store.h | 6 +++-- engine/src/flutter/runtime/dart_init.cc | 6 +++-- 10 files changed, 76 insertions(+), 72 deletions(-) delete mode 100644 engine/src/flutter/assets/unique_unzipper.cc delete mode 100644 engine/src/flutter/assets/unique_unzipper.h create mode 100644 engine/src/flutter/assets/unzipper_provider.cc create mode 100644 engine/src/flutter/assets/unzipper_provider.h diff --git a/engine/src/flutter/assets/BUILD.gn b/engine/src/flutter/assets/BUILD.gn index 8ded253f04..7080d19d4f 100644 --- a/engine/src/flutter/assets/BUILD.gn +++ b/engine/src/flutter/assets/BUILD.gn @@ -6,10 +6,10 @@ source_set("assets") { sources = [ "directory_asset_bundle.cc", "directory_asset_bundle.h", - "unique_unzipper.cc", - "unique_unzipper.h", "unzip_job.cc", "unzip_job.h", + "unzipper_provider.cc", + "unzipper_provider.h", "zip_asset_bundle.cc", "zip_asset_bundle.h", "zip_asset_store.cc", @@ -19,6 +19,7 @@ source_set("assets") { deps = [ "//flutter/glue", "//lib/ftl", + "//lib/zip", "//mojo/public/cpp/bindings:callback", "//mojo/public/cpp/system", "//mojo/public/cpp/bindings:utility", diff --git a/engine/src/flutter/assets/unique_unzipper.cc b/engine/src/flutter/assets/unique_unzipper.cc deleted file mode 100644 index 2507e3d25b..0000000000 --- a/engine/src/flutter/assets/unique_unzipper.cc +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2016 The Chromium 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/assets/unique_unzipper.h" - -#include "third_party/zlib/contrib/minizip/unzip.h" - -namespace blink { - -void UniqueUnzipperTraits::Free(void* file) { - unzClose(file); -} - -} // namespace blink diff --git a/engine/src/flutter/assets/unique_unzipper.h b/engine/src/flutter/assets/unique_unzipper.h deleted file mode 100644 index 7c592fb61d..0000000000 --- a/engine/src/flutter/assets/unique_unzipper.h +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2016 The Chromium 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_ASSETS_UNIQUE_UNZIPPER_H_ -#define FLUTTER_ASSETS_UNIQUE_UNZIPPER_H_ - -#include "lib/ftl/memory/unique_object.h" - -namespace blink { - -struct UniqueUnzipperTraits { - static inline void* InvalidValue() { return nullptr; } - static inline bool IsValid(void* value) { return value != InvalidValue(); } - static void Free(void* file); -}; - -using UniqueUnzipper = ftl::UniqueObject; - -} // namespace blink - -#endif // FLUTTER_ASSETS_UNIQUE_UNZIPPER_H_ diff --git a/engine/src/flutter/assets/unzip_job.cc b/engine/src/flutter/assets/unzip_job.cc index 39475156fc..ad8107ade7 100644 --- a/engine/src/flutter/assets/unzip_job.cc +++ b/engine/src/flutter/assets/unzip_job.cc @@ -11,31 +11,24 @@ namespace blink { -UnzipJob::UnzipJob(std::string zip_path, +UnzipJob::UnzipJob(zip::UniqueUnzipper unzipper, std::string asset_name, mojo::ScopedDataPipeProducerHandle producer, ftl::RefPtr task_runner) - : zip_path_(std::move(zip_path)), + : unzipper_(std::move(unzipper)), asset_name_(std::move(asset_name)), producer_(std::move(producer)), task_runner_(std::move(task_runner)), waiter_(mojo::Environment::GetDefaultAsyncWaiter()), wait_id_(0) { + FTL_DCHECK(unzipper_.is_valid()); task_runner_->PostTask([this]() { Start(); }); } UnzipJob::~UnzipJob() {} void UnzipJob::Start() { - zip_file_.reset(unzOpen2(zip_path_.c_str(), nullptr)); - - if (!zip_file_.is_valid()) { - FTL_LOG(ERROR) << "Unable to open ZIP file: " << zip_path_; - delete this; - return; - } - - int result = unzLocateFile(zip_file_.get(), asset_name_.c_str(), 0); + int result = unzLocateFile(unzipper_.get(), asset_name_.c_str(), 0); if (result != UNZ_OK) { FTL_LOG(WARNING) << "Requested asset '" << asset_name_ << "' does not exist."; @@ -43,7 +36,7 @@ void UnzipJob::Start() { return; } - result = unzOpenCurrentFile(zip_file_.get()); + result = unzOpenCurrentFile(unzipper_.get()); if (result != UNZ_OK) { FTL_LOG(WARNING) << "unzOpenCurrentFile failed, error=" << result; delete this; @@ -61,7 +54,7 @@ void UnzipJob::OnHandleReady(MojoResult result) { MOJO_WRITE_DATA_FLAG_NONE); if (result == MOJO_RESULT_OK) { FTL_DCHECK(size < static_cast(std::numeric_limits::max())); - ssize_t bytes_read = unzReadCurrentFile(zip_file_.get(), buffer, size); + ssize_t bytes_read = unzReadCurrentFile(unzipper_.get(), buffer, size); result = mojo::EndWriteDataRaw(producer_.get(), std::max(0l, bytes_read)); if (bytes_read < 0) { diff --git a/engine/src/flutter/assets/unzip_job.h b/engine/src/flutter/assets/unzip_job.h index 36dfbebb1a..dfe42fe61f 100644 --- a/engine/src/flutter/assets/unzip_job.h +++ b/engine/src/flutter/assets/unzip_job.h @@ -8,10 +8,10 @@ #include #include -#include "flutter/assets/unique_unzipper.h" #include "lib/ftl/macros.h" #include "lib/ftl/memory/ref_counted.h" #include "lib/ftl/tasks/task_runner.h" +#include "lib/zip/unique_unzipper.h" #include "mojo/public/cpp/environment/async_waiter.h" #include "mojo/services/asset_bundle/interfaces/asset_bundle.mojom.h" @@ -19,7 +19,7 @@ namespace blink { class UnzipJob { public: - UnzipJob(std::string zip_path, + UnzipJob(zip::UniqueUnzipper unzipper, std::string asset_name, mojo::ScopedDataPipeProducerHandle producer, ftl::RefPtr task_runner); @@ -30,11 +30,10 @@ class UnzipJob { void OnHandleReady(MojoResult result); static void WaitComplete(void* context, MojoResult result); - const std::string zip_path_; + zip::UniqueUnzipper unzipper_; const std::string asset_name_; mojo::ScopedDataPipeProducerHandle producer_; ftl::RefPtr task_runner_; - UniqueUnzipper zip_file_; const MojoAsyncWaiter* waiter_; MojoAsyncWaitID wait_id_; diff --git a/engine/src/flutter/assets/unzipper_provider.cc b/engine/src/flutter/assets/unzipper_provider.cc new file mode 100644 index 0000000000..dc93d721cf --- /dev/null +++ b/engine/src/flutter/assets/unzipper_provider.cc @@ -0,0 +1,21 @@ +// Copyright 2016 The Chromium 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/assets/unzipper_provider.h" + +#include "lib/ftl/logging.h" +#include "third_party/zlib/contrib/minizip/unzip.h" + +namespace blink { + +UnzipperProvider GetUnzipperProviderForPath(std::string zip_path) { + return [zip_path]() { + zip::UniqueUnzipper unzipper(unzOpen2(zip_path.c_str(), nullptr)); + if (!unzipper.is_valid()) + FTL_LOG(ERROR) << "Unable to open zip file: " << zip_path; + return unzipper; + }; +} + +} // namespace blink diff --git a/engine/src/flutter/assets/unzipper_provider.h b/engine/src/flutter/assets/unzipper_provider.h new file mode 100644 index 0000000000..f0f8d9597d --- /dev/null +++ b/engine/src/flutter/assets/unzipper_provider.h @@ -0,0 +1,20 @@ +// Copyright 2016 The Chromium 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_ASSETS_UNZIP_PROVIDER_H_ +#define FLUTTER_ASSETS_UNZIP_PROVIDER_H_ + +#include + +#include "lib/zip/unique_unzipper.h" + +namespace blink { + +using UnzipperProvider = std::function; + +UnzipperProvider GetUnzipperProviderForPath(std::string zip_path); + +} // namespace blink + +#endif // FLUTTER_ASSETS_UNZIP_PROVIDER_H_ diff --git a/engine/src/flutter/assets/zip_asset_store.cc b/engine/src/flutter/assets/zip_asset_store.cc index fff0085a31..c7218a7a5f 100644 --- a/engine/src/flutter/assets/zip_asset_store.cc +++ b/engine/src/flutter/assets/zip_asset_store.cc @@ -9,18 +9,19 @@ #include -#include "flutter/assets/unique_unzipper.h" #include "flutter/assets/unzip_job.h" #include "flutter/glue/data_pipe_utils.h" #include "lib/ftl/files/eintr_wrapper.h" #include "lib/ftl/files/unique_fd.h" +#include "lib/zip/unique_unzipper.h" #include "third_party/zlib/contrib/minizip/unzip.h" namespace blink { -ZipAssetStore::ZipAssetStore(std::string zip_path, +ZipAssetStore::ZipAssetStore(UnzipperProvider unzipper_provider, ftl::RefPtr task_runner) - : zip_path_(std::move(zip_path)), task_runner_(std::move(task_runner)) {} + : unzipper_provider_(std::move(unzipper_provider)), + task_runner_(std::move(task_runner)) {} ZipAssetStore::~ZipAssetStore() {} @@ -40,32 +41,34 @@ void ZipAssetStore::GetAsStream(const std::string& asset_name, glue::CopyFromFileDescriptor(std::move(fd), std::move(producer), task_runner_.get(), [](bool ignored) {}); } else { - new UnzipJob(zip_path_, asset_name, std::move(producer), task_runner_); + zip::UniqueUnzipper unzipper = unzipper_provider_(); + if (!unzipper.is_valid()) + return; + new UnzipJob(std::move(unzipper), asset_name, std::move(producer), + task_runner_); } } bool ZipAssetStore::GetAsBuffer(const std::string& asset_name, std::vector* data) { - UniqueUnzipper zip_file(unzOpen2(zip_path_.c_str(), nullptr)); - if (!zip_file.is_valid()) { - FTL_LOG(ERROR) << "Unable to open ZIP file: " << zip_path_; + zip::UniqueUnzipper unzipper = unzipper_provider_(); + if (!unzipper.is_valid()) return false; - } - int result = unzLocateFile(zip_file.get(), asset_name.c_str(), 0); + int result = unzLocateFile(unzipper.get(), asset_name.c_str(), 0); if (result != UNZ_OK) { return false; } unz_file_info file_info; - result = unzGetCurrentFileInfo(zip_file.get(), &file_info, nullptr, 0, + result = unzGetCurrentFileInfo(unzipper.get(), &file_info, nullptr, 0, nullptr, 0, nullptr, 0); if (result != UNZ_OK) { FTL_LOG(WARNING) << "unzGetCurrentFileInfo failed, error=" << result; return false; } - result = unzOpenCurrentFile(zip_file.get()); + result = unzOpenCurrentFile(unzipper.get()); if (result != UNZ_OK) { FTL_LOG(WARNING) << "unzOpenCurrentFile failed, error=" << result; return false; @@ -75,7 +78,7 @@ bool ZipAssetStore::GetAsBuffer(const std::string& asset_name, int total_read = 0; while (total_read < static_cast(data->size())) { int bytes_read = unzReadCurrentFile( - zip_file.get(), data->data() + total_read, data->size() - total_read); + unzipper.get(), data->data() + total_read, data->size() - total_read); if (bytes_read <= 0) return false; total_read += bytes_read; diff --git a/engine/src/flutter/assets/zip_asset_store.h b/engine/src/flutter/assets/zip_asset_store.h index 5bc7132149..1a10d0e53e 100644 --- a/engine/src/flutter/assets/zip_asset_store.h +++ b/engine/src/flutter/assets/zip_asset_store.h @@ -8,6 +8,7 @@ #include #include +#include "flutter/assets/unzipper_provider.h" #include "lib/ftl/macros.h" #include "lib/ftl/memory/ref_counted.h" #include "lib/ftl/tasks/task_runner.h" @@ -17,7 +18,8 @@ namespace blink { class ZipAssetStore : public ftl::RefCountedThreadSafe { public: - ZipAssetStore(std::string zip_path, ftl::RefPtr task_runner); + ZipAssetStore(UnzipperProvider unzipper_provider, + ftl::RefPtr task_runner); ~ZipAssetStore(); // Serve this asset from another file instead of using the ZIP contents. @@ -28,7 +30,7 @@ class ZipAssetStore : public ftl::RefCountedThreadSafe { bool GetAsBuffer(const std::string& asset_name, std::vector* data); private: - const std::string zip_path_; + UnzipperProvider unzipper_provider_; ftl::RefPtr task_runner_; std::map overlay_files_; diff --git a/engine/src/flutter/runtime/dart_init.cc b/engine/src/flutter/runtime/dart_init.cc index 4542b9cf75..e14970ecec 100644 --- a/engine/src/flutter/runtime/dart_init.cc +++ b/engine/src/flutter/runtime/dart_init.cc @@ -19,6 +19,7 @@ #include "dart/runtime/bin/embedded_dart_io.h" #include "dart/runtime/include/dart_mirrors_api.h" +#include "flutter/assets/unzipper_provider.h" #include "flutter/assets/zip_asset_store.h" #include "flutter/common/settings.h" #include "flutter/glue/trace_event.h" @@ -247,8 +248,9 @@ Dart_Isolate IsolateCreateCallback(const char* script_uri, FTL_CHECK(uri.find(kFileUriPrefix) == 0u); std::string bundle_path(script_uri + strlen(kFileUriPrefix)); ftl::RefPtr zip_asset_store = - ftl::MakeRefCounted(std::move(bundle_path), - ftl::RefPtr()); + ftl::MakeRefCounted( + GetUnzipperProviderForPath(std::move(bundle_path)), + ftl::RefPtr()); FTL_CHECK(zip_asset_store->GetAsBuffer(kSnapshotAssetKey, &snapshot_data)); }