[res] Refactor AssetManager + Providers
- More moves where possible
- Better interface for PathOrDebugName creation
- function_ref as a callback
- Get rid of some obsolete Util.h code
- Shut up the logging in host mode, and ignore all calls
Bug: 237583012
Test: build + boot + UTs
Change-Id: Ia71fc1c83f17ab5ce3cac1179f74534f7ad2a3cb
diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp
index cc7e871..68f5e4a 100644
--- a/libs/androidfw/AssetManager2.cpp
+++ b/libs/androidfw/AssetManager2.cpp
@@ -1562,11 +1562,11 @@
std::unordered_map<ApkAssetsCookie, SourceToDestinationRuntimePackageMap> src_asset_cookie_id_map;
// Determine which ApkAssets are loaded in both theme AssetManagers.
- const auto src_assets = source.asset_manager_->GetApkAssets();
+ const auto& src_assets = source.asset_manager_->GetApkAssets();
for (size_t i = 0; i < src_assets.size(); i++) {
const ApkAssets* src_asset = src_assets[i];
- const auto dest_assets = asset_manager_->GetApkAssets();
+ const auto& dest_assets = asset_manager_->GetApkAssets();
for (size_t j = 0; j < dest_assets.size(); j++) {
const ApkAssets* dest_asset = dest_assets[j];
if (src_asset != dest_asset) {
diff --git a/libs/androidfw/AssetsProvider.cpp b/libs/androidfw/AssetsProvider.cpp
index b9264c5..2d3c065 100644
--- a/libs/androidfw/AssetsProvider.cpp
+++ b/libs/androidfw/AssetsProvider.cpp
@@ -73,9 +73,6 @@
(path != nullptr) ? base::unique_fd(-1) : std::move(fd));
}
-ZipAssetsProvider::PathOrDebugName::PathOrDebugName(std::string&& value, bool is_path)
- : value_(std::forward<std::string>(value)), is_path_(is_path) {}
-
const std::string* ZipAssetsProvider::PathOrDebugName::GetPath() const {
return is_path_ ? &value_ : nullptr;
}
@@ -84,10 +81,14 @@
return value_;
}
+void ZipAssetsProvider::ZipCloser::operator()(ZipArchive* a) const {
+ ::CloseArchive(a);
+}
+
ZipAssetsProvider::ZipAssetsProvider(ZipArchiveHandle handle, PathOrDebugName&& path,
package_property_t flags, time_t last_mod_time)
- : zip_handle_(handle, ::CloseArchive),
- name_(std::forward<PathOrDebugName>(path)),
+ : zip_handle_(handle),
+ name_(std::move(path)),
flags_(flags),
last_mod_time_(last_mod_time) {}
@@ -110,14 +111,12 @@
// Stat requires execute permissions on all directories path to the file. If the process does
// not have execute permissions on this file, allow the zip to be opened but IsUpToDate() will
// always have to return true.
- LOG(WARNING) << "Failed to stat file '" << path << "': "
- << base::SystemErrorCodeToString(errno);
+ PLOG(WARNING) << "Failed to stat file '" << path << "'";
}
}
return std::unique_ptr<ZipAssetsProvider>(
- new ZipAssetsProvider(handle, PathOrDebugName{std::move(path),
- true /* is_path */}, flags, sb.st_mtime));
+ new ZipAssetsProvider(handle, PathOrDebugName::Path(std::move(path)), flags, sb.st_mtime));
}
std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(base::unique_fd fd,
@@ -150,9 +149,8 @@
}
}
- return std::unique_ptr<ZipAssetsProvider>(
- new ZipAssetsProvider(handle, PathOrDebugName{std::move(friendly_name),
- false /* is_path */}, flags, sb.st_mtime));
+ return std::unique_ptr<ZipAssetsProvider>(new ZipAssetsProvider(
+ handle, PathOrDebugName::DebugName(std::move(friendly_name)), flags, sb.st_mtime));
}
std::unique_ptr<Asset> ZipAssetsProvider::OpenInternal(const std::string& path,
@@ -219,8 +217,9 @@
return asset;
}
-bool ZipAssetsProvider::ForEachFile(const std::string& root_path,
- const std::function<void(StringPiece, FileType)>& f) const {
+bool ZipAssetsProvider::ForEachFile(
+ const std::string& root_path,
+ base::function_ref<void(StringPiece, FileType)> f) const {
std::string root_path_full = root_path;
if (root_path_full.back() != '/') {
root_path_full += '/';
@@ -297,7 +296,7 @@
}
DirectoryAssetsProvider::DirectoryAssetsProvider(std::string&& path, time_t last_mod_time)
- : dir_(std::forward<std::string>(path)), last_mod_time_(last_mod_time) {}
+ : dir_(std::move(path)), last_mod_time_(last_mod_time) {}
std::unique_ptr<DirectoryAssetsProvider> DirectoryAssetsProvider::Create(std::string path) {
struct stat sb;
@@ -312,7 +311,7 @@
return nullptr;
}
- if (path[path.size() - 1] != OS_PATH_SEPARATOR) {
+ if (path.back() != OS_PATH_SEPARATOR) {
path += OS_PATH_SEPARATOR;
}
@@ -335,7 +334,7 @@
bool DirectoryAssetsProvider::ForEachFile(
const std::string& /* root_path */,
- const std::function<void(StringPiece, FileType)>& /* f */) const {
+ base::function_ref<void(StringPiece, FileType)> /* f */) const {
return true;
}
@@ -362,8 +361,7 @@
MultiAssetsProvider::MultiAssetsProvider(std::unique_ptr<AssetsProvider>&& primary,
std::unique_ptr<AssetsProvider>&& secondary)
- : primary_(std::forward<std::unique_ptr<AssetsProvider>>(primary)),
- secondary_(std::forward<std::unique_ptr<AssetsProvider>>(secondary)) {
+ : primary_(std::move(primary)), secondary_(std::move(secondary)) {
debug_name_ = primary_->GetDebugName() + " and " + secondary_->GetDebugName();
path_ = (primary_->GetDebugName() != kEmptyDebugString) ? primary_->GetPath()
: secondary_->GetPath();
@@ -385,8 +383,9 @@
return (asset) ? std::move(asset) : secondary_->Open(path, mode, file_exists);
}
-bool MultiAssetsProvider::ForEachFile(const std::string& root_path,
- const std::function<void(StringPiece, FileType)>& f) const {
+bool MultiAssetsProvider::ForEachFile(
+ const std::string& root_path,
+ base::function_ref<void(StringPiece, FileType)> f) const {
return primary_->ForEachFile(root_path, f) && secondary_->ForEachFile(root_path, f);
}
@@ -424,7 +423,7 @@
bool EmptyAssetsProvider::ForEachFile(
const std::string& /* root_path */,
- const std::function<void(StringPiece, FileType)>& /* f */) const {
+ base::function_ref<void(StringPiece, FileType)> /* f */) const {
return true;
}
@@ -447,4 +446,4 @@
return true;
}
-} // namespace android
+} // namespace android
diff --git a/libs/androidfw/Idmap.cpp b/libs/androidfw/Idmap.cpp
index f3d2443..8983574 100644
--- a/libs/androidfw/Idmap.cpp
+++ b/libs/androidfw/Idmap.cpp
@@ -201,7 +201,7 @@
const auto& config = configurations_[value.config_index];
values_map[config] = value.value;
}
- return Result(values_map);
+ return Result(std::move(values_map));
}
return {};
}
@@ -250,8 +250,7 @@
}
} // namespace
-LoadedIdmap::LoadedIdmap(std::string&& idmap_path,
- const Idmap_header* header,
+LoadedIdmap::LoadedIdmap(std::string&& idmap_path, const Idmap_header* header,
const Idmap_data_header* data_header,
const Idmap_target_entry* target_entries,
const Idmap_target_entry_inline* target_inline_entries,
@@ -259,20 +258,19 @@
const ConfigDescription* configs,
const Idmap_overlay_entry* overlay_entries,
std::unique_ptr<ResStringPool>&& string_pool,
- std::string_view overlay_apk_path,
- std::string_view target_apk_path)
- : header_(header),
- data_header_(data_header),
- target_entries_(target_entries),
- target_inline_entries_(target_inline_entries),
- inline_entry_values_(inline_entry_values),
- configurations_(configs),
- overlay_entries_(overlay_entries),
- string_pool_(std::move(string_pool)),
- idmap_path_(std::move(idmap_path)),
- overlay_apk_path_(overlay_apk_path),
- target_apk_path_(target_apk_path),
- idmap_last_mod_time_(getFileModDate(idmap_path_.data())) {}
+ std::string_view overlay_apk_path, std::string_view target_apk_path)
+ : header_(header),
+ data_header_(data_header),
+ target_entries_(target_entries),
+ target_inline_entries_(target_inline_entries),
+ inline_entry_values_(inline_entry_values),
+ configurations_(configs),
+ overlay_entries_(overlay_entries),
+ string_pool_(std::move(string_pool)),
+ idmap_path_(std::move(idmap_path)),
+ overlay_apk_path_(overlay_apk_path),
+ target_apk_path_(target_apk_path),
+ idmap_last_mod_time_(getFileModDate(idmap_path_.data())) {}
std::unique_ptr<LoadedIdmap> LoadedIdmap::Load(StringPiece idmap_path, StringPiece idmap_data) {
ATRACE_CALL();
diff --git a/libs/androidfw/include/androidfw/AssetManager2.h b/libs/androidfw/include/androidfw/AssetManager2.h
index e4d1218..f10cb9b 100644
--- a/libs/androidfw/include/androidfw/AssetManager2.h
+++ b/libs/androidfw/include/androidfw/AssetManager2.h
@@ -105,7 +105,7 @@
// new resource IDs.
bool SetApkAssets(std::vector<const ApkAssets*> apk_assets, bool invalidate_caches = true);
- inline const std::vector<const ApkAssets*> GetApkAssets() const {
+ inline const std::vector<const ApkAssets*>& GetApkAssets() const {
return apk_assets_;
}
diff --git a/libs/androidfw/include/androidfw/AssetsProvider.h b/libs/androidfw/include/androidfw/AssetsProvider.h
index 7891194..d33c325 100644
--- a/libs/androidfw/include/androidfw/AssetsProvider.h
+++ b/libs/androidfw/include/androidfw/AssetsProvider.h
@@ -20,6 +20,7 @@
#include <memory>
#include <string>
+#include "android-base/function_ref.h"
#include "android-base/macros.h"
#include "android-base/unique_fd.h"
@@ -46,7 +47,7 @@
// Iterate over all files and directories provided by the interface. The order of iteration is
// stable.
virtual bool ForEachFile(const std::string& path,
- const std::function<void(StringPiece, FileType)>& f) const = 0;
+ base::function_ref<void(StringPiece, FileType)> f) const = 0;
// Retrieves the path to the contents of the AssetsProvider on disk. The path could represent an
// APk, a directory, or some other file type.
@@ -90,7 +91,7 @@
off64_t len = kUnknownLength);
bool ForEachFile(const std::string& root_path,
- const std::function<void(StringPiece, FileType)>& f) const override;
+ base::function_ref<void(StringPiece, FileType)> f) const override;
WARN_UNUSED std::optional<std::string_view> GetPath() const override;
WARN_UNUSED const std::string& GetDebugName() const override;
@@ -108,7 +109,12 @@
time_t last_mod_time);
struct PathOrDebugName {
- PathOrDebugName(std::string&& value, bool is_path);
+ static PathOrDebugName Path(std::string value) {
+ return {std::move(value), true};
+ }
+ static PathOrDebugName DebugName(std::string value) {
+ return {std::move(value), false};
+ }
// Retrieves the path or null if this class represents a debug name.
WARN_UNUSED const std::string* GetPath() const;
@@ -117,11 +123,16 @@
WARN_UNUSED const std::string& GetDebugName() const;
private:
+ PathOrDebugName(std::string value, bool is_path) : value_(std::move(value)), is_path_(is_path) {
+ }
std::string value_;
bool is_path_;
};
- std::unique_ptr<ZipArchive, void (*)(ZipArchive*)> zip_handle_;
+ struct ZipCloser {
+ void operator()(ZipArchive* a) const;
+ };
+ std::unique_ptr<ZipArchive, ZipCloser> zip_handle_;
PathOrDebugName name_;
package_property_t flags_;
time_t last_mod_time_;
@@ -132,7 +143,7 @@
static std::unique_ptr<DirectoryAssetsProvider> Create(std::string root_dir);
bool ForEachFile(const std::string& path,
- const std::function<void(StringPiece, FileType)>& f) const override;
+ base::function_ref<void(StringPiece, FileType)> f) const override;
WARN_UNUSED std::optional<std::string_view> GetPath() const override;
WARN_UNUSED const std::string& GetDebugName() const override;
@@ -157,7 +168,7 @@
std::unique_ptr<AssetsProvider>&& secondary);
bool ForEachFile(const std::string& root_path,
- const std::function<void(StringPiece, FileType)>& f) const override;
+ base::function_ref<void(StringPiece, FileType)> f) const override;
WARN_UNUSED std::optional<std::string_view> GetPath() const override;
WARN_UNUSED const std::string& GetDebugName() const override;
@@ -184,7 +195,7 @@
static std::unique_ptr<AssetsProvider> Create(std::string path);
bool ForEachFile(const std::string& path,
- const std::function<void(StringPiece, FileType)>& f) const override;
+ base::function_ref<void(StringPiece, FileType)> f) const override;
WARN_UNUSED std::optional<std::string_view> GetPath() const override;
WARN_UNUSED const std::string& GetDebugName() const override;
diff --git a/libs/androidfw/include/androidfw/Idmap.h b/libs/androidfw/include/androidfw/Idmap.h
index f173e6e..6068912 100644
--- a/libs/androidfw/include/androidfw/Idmap.h
+++ b/libs/androidfw/include/androidfw/Idmap.h
@@ -93,8 +93,8 @@
public:
Result() = default;
explicit Result(uint32_t value) : data_(value) {};
- explicit Result(const std::map<ConfigDescription, Res_value> &value)
- : data_(value) { };
+ explicit Result(std::map<ConfigDescription, Res_value> value) : data_(std::move(value)) {
+ }
// Returns `true` if the resource is overlaid.
explicit operator bool() const {
diff --git a/libs/androidfw/include/androidfw/Util.h b/libs/androidfw/include/androidfw/Util.h
index ae7c65f..a188abb 100644
--- a/libs/androidfw/include/androidfw/Util.h
+++ b/libs/androidfw/include/androidfw/Util.h
@@ -22,7 +22,6 @@
#include <cstdlib>
#include <memory>
-#include <sstream>
#include <vector>
#include "androidfw/BigBuffer.h"
@@ -33,7 +32,14 @@
#ifdef __ANDROID__
#define ANDROID_LOG(x) LOG(x)
#else
-#define ANDROID_LOG(x) std::stringstream()
+namespace android {
+// No default logging for aapt2, as it's too noisy for a command line dev tool.
+struct NullLogger {
+ template <class T>
+ friend const NullLogger& operator<<(const NullLogger& l, const T&) { return l; }
+};
+}
+#define ANDROID_LOG(x) (android::NullLogger{})
#endif
namespace android {
@@ -49,76 +55,14 @@
return std::unique_ptr<T>(new T{std::forward<Args>(args)...});
}
-// Based on std::unique_ptr, but uses free() to release malloc'ed memory
-// without incurring the size increase of holding on to a custom deleter.
-template <typename T>
-class unique_cptr {
- public:
- using pointer = typename std::add_pointer<T>::type;
-
- constexpr unique_cptr() : ptr_(nullptr) {}
- constexpr explicit unique_cptr(std::nullptr_t) : ptr_(nullptr) {}
- explicit unique_cptr(pointer ptr) : ptr_(ptr) {}
- unique_cptr(unique_cptr&& o) noexcept : ptr_(o.ptr_) { o.ptr_ = nullptr; }
-
- ~unique_cptr() { std::free(reinterpret_cast<void*>(ptr_)); }
-
- inline unique_cptr& operator=(unique_cptr&& o) noexcept {
- if (&o == this) {
- return *this;
- }
-
- std::free(reinterpret_cast<void*>(ptr_));
- ptr_ = o.ptr_;
- o.ptr_ = nullptr;
- return *this;
+// Based on std::unique_ptr, but uses free() to release malloc'ed memory.
+struct FreeDeleter {
+ void operator()(void* ptr) const {
+ ::free(ptr);
}
-
- inline unique_cptr& operator=(std::nullptr_t) {
- std::free(reinterpret_cast<void*>(ptr_));
- ptr_ = nullptr;
- return *this;
- }
-
- pointer release() {
- pointer result = ptr_;
- ptr_ = nullptr;
- return result;
- }
-
- inline pointer get() const { return ptr_; }
-
- void reset(pointer ptr = pointer()) {
- if (ptr == ptr_) {
- return;
- }
-
- pointer old_ptr = ptr_;
- ptr_ = ptr;
- std::free(reinterpret_cast<void*>(old_ptr));
- }
-
- inline void swap(unique_cptr& o) { std::swap(ptr_, o.ptr_); }
-
- inline explicit operator bool() const { return ptr_ != nullptr; }
-
- inline typename std::add_lvalue_reference<T>::type operator*() const { return *ptr_; }
-
- inline pointer operator->() const { return ptr_; }
-
- inline bool operator==(const unique_cptr& o) const { return ptr_ == o.ptr_; }
-
- inline bool operator!=(const unique_cptr& o) const { return ptr_ != o.ptr_; }
-
- inline bool operator==(std::nullptr_t) const { return ptr_ == nullptr; }
-
- inline bool operator!=(std::nullptr_t) const { return ptr_ != nullptr; }
-
- private:
- DISALLOW_COPY_AND_ASSIGN(unique_cptr);
-
- pointer ptr_;
};
+template <typename T>
+using unique_cptr = std::unique_ptr<T, FreeDeleter>;
void ReadUtf16StringFromDevice(const uint16_t* src, size_t len, std::string* out);
@@ -152,13 +96,13 @@
std::vector<std::string> SplitAndLowercase(android::StringPiece str, char sep);
-template <typename T>
-inline bool IsFourByteAligned(const incfs::map_ptr<T>& data) {
- return ((size_t)data.unsafe_ptr() & 0x3U) == 0;
+inline bool IsFourByteAligned(const void* data) {
+ return ((uintptr_t)data & 0x3U) == 0;
}
-inline bool IsFourByteAligned(const void* data) {
- return ((size_t)data & 0x3U) == 0;
+template <typename T>
+inline bool IsFourByteAligned(const incfs::map_ptr<T>& data) {
+ return IsFourByteAligned(data.unsafe_ptr());
}
// Helper method to extract a UTF-16 string from a StringPool. If the string is stored as UTF-8,