Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2146265f authored by Yurii Zubrytskyi's avatar Yurii Zubrytskyi
Browse files

[res] Properly exclude system overlays when requested 

Getting non-system locales / configurations was broken for the
case when there are no non-system overlays present - it used to
assume all overlays are non-system instead of the opposite. This
was causing the wrong list of locales returned in the API

Bug: 421654198
Test: atest libandroidfw_tests
Flag: EXEMPT minor bugfix
Change-Id: I612a8526ae09175ea337faa2d37ea47bfbc08aa9
parent 8d900756

libs/androidfw/AssetManager2.cpp

+26 −27
Original line number Diff line number Diff line
@@ -492,8 +492,8 @@ void AssetManager2::SetOverlayConstraints(int32_t display_id, int32_t device_id) 
  }

}



std::set<AssetManager2::ApkAssetsPtr> AssetManager2::GetNonSystemOverlays() const {

  std::set<ApkAssetsPtr> non_system_overlays;

AssetManager2::AssetsSet AssetManager2::GetNonSystemOverlays() const {

  AssetManager2::AssetsSet non_system_overlays;

  for (const PackageGroup& package_group : package_groups_) {

    bool found_system_package = false;

    for (const ConfiguredPackage& package : package_group.packages_) {
@@ -518,6 +518,24 @@ std::set<AssetManager2::ApkAssetsPtr> AssetManager2::GetNonSystemOverlays() cons 
  return non_system_overlays;

}



bool AssetManager2::IsSystemPackage(const ConfiguredPackage& package, ApkAssetsCookie cookie,

                                    const AssetsSet& non_system_overlays) const {

  if (package.loaded_package_->IsSystem()) {

    return true;

  }

  // Also check for the overlays that target only system resources.

  if (package.loaded_package_->IsOverlay()) {

    if (non_system_overlays.empty()) {

      return true;  // No non-system overlays: all overlay packages are system-only.

    }

    const auto& apk_assets = GetApkAssets(cookie);

    if (apk_assets && !non_system_overlays.contains(apk_assets)) {

      return true;

    }

  }

  return false;

}



base::expected<std::set<ResTable_config>, IOError> AssetManager2::GetResourceConfigurations(

    bool exclude_system, bool exclude_mipmap) const {

  ATRACE_NAME("AssetManager::GetResourceConfigurations");
@@ -530,20 +548,10 @@ base::expected<std::set<ResTable_config>, IOError> AssetManager2::GetResourceCon 
  for (const PackageGroup& package_group : package_groups_) {

    for (size_t i = 0; i < package_group.packages_.size(); i++) {

      const ConfiguredPackage& package = package_group.packages_[i];

      if (exclude_system) {

        if (package.loaded_package_->IsSystem()) {

      if (exclude_system &&

          IsSystemPackage(package, package_group.cookies_[i], non_system_overlays)) {

        continue;

      }

        if (!non_system_overlays.empty()) {

          // Exclude overlays that target only system resources.

          const auto& apk_assets = GetApkAssets(package_group.cookies_[i]);

          if (apk_assets && apk_assets->IsOverlay() &&

              non_system_overlays.find(apk_assets) == non_system_overlays.end()) {

            continue;

          }

        }

      }



      auto result = package.loaded_package_->CollectConfigurations(exclude_mipmap, &configurations);

      if (UNLIKELY(!result.has_value())) {

        return base::unexpected(result.error());
@@ -565,19 +573,10 @@ LoadedPackage::Locales AssetManager2::GetResourceLocales( 
  for (const PackageGroup& package_group : package_groups_) {

    for (size_t i = 0; i < package_group.packages_.size(); i++) {

      const ConfiguredPackage& package = package_group.packages_[i];

      if (exclude_system) {

        if (package.loaded_package_->IsSystem()) {

      if (exclude_system &&

          IsSystemPackage(package, package_group.cookies_[i], non_system_overlays)) {

        continue;

      }

        if (!non_system_overlays.empty()) {

          // Exclude overlays that target only system resources.

          const auto& apk_assets = GetApkAssets(package_group.cookies_[i]);

          if (apk_assets && apk_assets->IsOverlay() && !non_system_overlays.contains(apk_assets)) {

            continue;

          }

        }

      }



      package.loaded_package_->CollectLocales(merge_equivalent_languages, &locales);

    }

  }

libs/androidfw/include/androidfw/AssetManager2.h

+8 −1
Original line number Diff line number Diff line
@@ -449,8 +449,15 @@ class AssetManager2 { 
  // This should always be called when mutating the AssetManager's configuration or ApkAssets set.

  void RebuildFilterList();



  using AssetsSet = std::set<ApkAssetsPtr>;



  // Retrieves the APK paths of overlays that overlay non-system packages.

  std::set<ApkAssetsPtr> GetNonSystemOverlays() const;

  AssetsSet GetNonSystemOverlays() const;



  // Checks if the package is a system-only package (a system package itself, or and overlay

  // that only targets resources in system packages).

  bool IsSystemPackage(const ConfiguredPackage& package, ApkAssetsCookie cookie,

                       const AssetsSet& non_system_overlays) const;



  // AssetManager2::GetBag(resid) wraps this function to track which resource ids have already

  // been seen while traversing bag parents.

libs/androidfw/tests/AssetManager2_test.cpp

+18 −0
Original line number Diff line number Diff line
@@ -90,6 +90,10 @@ class AssetManager2Test : public ::testing::Test { 
    overlayable_assets_ = ApkAssets::Load("overlayable/overlayable.apk");

    ASSERT_THAT(overlayable_assets_, NotNull());



    overlayable_system_assets_ =

        ApkAssets::Load("overlayable/overlayable.apk", nullptr, PROPERTY_SYSTEM);

    ASSERT_THAT(overlayable_system_assets_, NotNull());



    flagged_assets_ = ApkAssets::Load("flagged/flagged.apk");

    ASSERT_THAT(app_assets_, NotNull());
@@ -110,6 +114,7 @@ class AssetManager2Test : public ::testing::Test { 
  AssetManager2::ApkAssetsPtr app_assets_;

  AssetManager2::ApkAssetsPtr overlay_assets_;

  AssetManager2::ApkAssetsPtr overlayable_assets_;

  AssetManager2::ApkAssetsPtr overlayable_system_assets_;

  AssetManager2::ApkAssetsPtr flagged_assets_;

};
@@ -643,6 +648,19 @@ TEST_F(AssetManager2Test, GetResourceLocales) { 
  EXPECT_GT(locales.count("fr"), 0u);

}



TEST_F(AssetManager2Test, GetResourceLocalesSystemOverlay) {

  AssetManager2 assetmanager;

  // overlay_assets_ doesn't have the system flag set, testing the non-system overlay for system

  // assets case.

  assetmanager.SetApkAssets({overlayable_system_assets_, basic_de_fr_assets_, overlay_assets_});



  auto locales = assetmanager.GetResourceLocales(true /*exclude_system*/);

  // We expect only the de and fr locales from basic_de_fr assets.

  EXPECT_EQ(2u, locales.size());

  EXPECT_TRUE(locales.contains("de"));

  EXPECT_TRUE(locales.contains("fr"));

}



TEST_F(AssetManager2Test, GetResourceId) {

  AssetManager2 assetmanager;

  assetmanager.SetApkAssets({basic_assets_});