[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

8 files changed, 80 insertions, 128 deletions

diff --git a/core/jni/android_content_res_ApkAssets.cpp b/core/jni/android_content_res_ApkAssets.cpp
index 29560dce1cd8..e9ada235b388 100644
--- a/core/jni/android_content_res_ApkAssets.cpp
+++ b/core/jni/android_content_res_ApkAssets.cpp
@@ -96,7 +96,7 @@ class LoaderAssetsProvider : public AssetsProvider {
   }
 
   bool ForEachFile(const std::string& /* root_path */,
-                   const std::function<void(StringPiece, FileType)>& /* f */) const {
+                   android::base::function_ref<void(StringPiece, FileType)> /* f */) const {
     return true;
   }
 
diff --git a/libs/androidfw/AssetManager2.cpp b/libs/androidfw/AssetManager2.cpp
index cc7e8714c0ac..68f5e4a88c7e 100644
--- a/libs/androidfw/AssetManager2.cpp
+++ b/libs/androidfw/AssetManager2.cpp
@@ -1562,11 +1562,11 @@ base::expected<std::monostate, IOError> Theme::SetTo(const Theme& source) {
     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 b9264c5d0f2d..2d3c06506a1f 100644
--- a/libs/androidfw/AssetsProvider.cpp
+++ b/libs/androidfw/AssetsProvider.cpp
@@ -73,9 +73,6 @@ std::unique_ptr<Asset> AssetsProvider::CreateAssetFromFd(base::unique_fd fd,
                                           (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 @@ const std::string& ZipAssetsProvider::PathOrDebugName::GetDebugName() const {
   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 @@ std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(std::string path,
       // 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 @@ std::unique_ptr<ZipAssetsProvider> ZipAssetsProvider::Create(base::unique_fd fd,
     }
   }
 
-  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 @@ std::unique_ptr<Asset> ZipAssetsProvider::OpenInternal(const std::string& path,
     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 @@ bool ZipAssetsProvider::IsUpToDate() const {
 }
 
 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 @@ std::unique_ptr<DirectoryAssetsProvider> DirectoryAssetsProvider::Create(std::st
     return nullptr;
   }
 
-  if (path[path.size() - 1] != OS_PATH_SEPARATOR) {
+  if (path.back() != OS_PATH_SEPARATOR) {
     path += OS_PATH_SEPARATOR;
   }
 
@@ -335,7 +334,7 @@ std::unique_ptr<Asset> DirectoryAssetsProvider::OpenInternal(const std::string&
 
 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 @@ bool DirectoryAssetsProvider::IsUpToDate() const {
 
 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 @@ std::unique_ptr<Asset> MultiAssetsProvider::OpenInternal(const std::string& path
   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 @@ std::unique_ptr<Asset> EmptyAssetsProvider::OpenInternal(const std::string& /* p
 
 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 @@ bool EmptyAssetsProvider::IsUpToDate() const {
   return true;
 }
 
-} // namespace android
+}  // namespace android
diff --git a/libs/androidfw/Idmap.cpp b/libs/androidfw/Idmap.cpp
index f3d244342b55..89835742c8ff 100644
--- a/libs/androidfw/Idmap.cpp
+++ b/libs/androidfw/Idmap.cpp
@@ -201,7 +201,7 @@ IdmapResMap::Result IdmapResMap::Lookup(uint32_t target_res_id) const {
       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 @@ std::optional<std::string_view> ReadString(const uint8_t** in_out_data_ptr, size
 }
 } // 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 @@ LoadedIdmap::LoadedIdmap(std::string&& idmap_path,
                          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 e4d1218debe6..f10cb9bf480a 100644
--- a/libs/androidfw/include/androidfw/AssetManager2.h
+++ b/libs/androidfw/include/androidfw/AssetManager2.h
@@ -105,7 +105,7 @@ class AssetManager2 {
   // 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 7891194a65bd..d33c325ff369 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 @@ struct AssetsProvider {
   // 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 @@ struct ZipAssetsProvider : public AssetsProvider {
                                                    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 @@ struct ZipAssetsProvider : public AssetsProvider {
                     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 @@ struct ZipAssetsProvider : public AssetsProvider {
     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 @@ struct DirectoryAssetsProvider : public AssetsProvider {
   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 @@ struct MultiAssetsProvider : public AssetsProvider {
                                                 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 @@ struct EmptyAssetsProvider : public AssetsProvider {
   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 f173e6ef9c16..60689128dffb 100644
--- a/libs/androidfw/include/androidfw/Idmap.h
+++ b/libs/androidfw/include/androidfw/Idmap.h
@@ -93,8 +93,8 @@ class IdmapResMap {
    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 ae7c65f2d4e1..a188abb7ecb5 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 @@ std::unique_ptr<T> make_unique(Args&&... args) {
   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;
-  }
-
-  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;
+// Based on std::unique_ptr, but uses free() to release malloc'ed memory.
+struct FreeDeleter {
+  void operator()(void* ptr) const {
+    ::free(ptr);
   }
-
-  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 @@ inline uint32_t DeviceToHost32(uint32_t value) {
 
 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,