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

Commit 6b0cf87d authored by Mark Punzalan's avatar Mark Punzalan Committed by Android (Google) Code Review
Browse files

Merge "Update VersionCollapser to consider minorVersion" into main

parents a17a5171 bcbc12d9

libs/androidfw/ResourceTypes.cpp

+10 −4
Original line number Diff line number Diff line
@@ -2202,8 +2202,11 @@ int ResTable_config::compare(const ResTable_config& o) const { 
    if (screenSize != o.screenSize) {

        return (screenSize > o.screenSize) ? 1 : -1;

    }

    if (version != o.version) {

        return (version > o.version) ? 1 : -1;

    if (sdkVersion != o.sdkVersion) {

        return (sdkVersion > o.sdkVersion) ? 1 : -1;

    }

    if (minorVersion != o.minorVersion) {

        return (minorVersion > o.minorVersion) ? 1 : -1;

    }

    if (screenLayout != o.screenLayout) {

        return (screenLayout > o.screenLayout) ? 1 : -1;
@@ -2286,8 +2289,11 @@ int ResTable_config::compareLogical(const ResTable_config& o) const { 
    if (uiMode != o.uiMode) {

        return uiMode < o.uiMode ? -1 : 1;

    }

    if (version != o.version) {

        return version < o.version ? -1 : 1;

    if (sdkVersion != o.sdkVersion) {

        return sdkVersion < o.sdkVersion ? -1 : 1;

    }

    if (minorVersion != o.minorVersion) {

        return minorVersion < o.minorVersion ? -1 : 1;

    }

    return 0;

}

tools/aapt2/optimize/VersionCollapser.cpp

+13 −8
Original line number Diff line number Diff line
@@ -72,6 +72,8 @@ FilterIterator<Iterator, Pred> make_filter_iterator(Iterator begin, 
 */

static void CollapseVersions(IAaptContext* context, int min_sdk, ResourceEntry* entry) {

  // First look for all sdks less than minSdk.

  // Note that we iterate in reverse order, meaning we will first encounter entries with the

  // highest SDK level and work our way down.

  for (auto iter = entry->values.rbegin(); iter != entry->values.rend();

       ++iter) {

    // Check if the item was already marked for removal.
@@ -80,10 +82,11 @@ static void CollapseVersions(IAaptContext* context, int min_sdk, ResourceEntry* 
    }



    const ConfigDescription& config = (*iter)->config;

    if (config.sdkVersion <= min_sdk) {

    if (config.sdkVersion != 0 && (config.sdkVersion < min_sdk ||

                                   (config.sdkVersion == min_sdk && config.minorVersion == 0))) {

      // This is the first configuration we've found with a smaller or equal SDK level to the

      // minimum. We MUST keep this one, but remove all others we find, which get overridden by this

      // one.

      // minimum. We MUST keep this one, but remove all others we find, which will be smaller and

      // therefore get overridden by this one.



      ConfigDescription config_without_sdk = config.CopyWithoutSdkVersion();

      auto pred = [&](const std::unique_ptr<ResourceConfigValue>& val) -> bool {
@@ -93,12 +96,12 @@ static void CollapseVersions(IAaptContext* context, int min_sdk, ResourceEntry* 
        }



        // Only return Configs that differ in SDK version.

        config_without_sdk.sdkVersion = val->config.sdkVersion;

        config_without_sdk.version = val->config.version;

        return config_without_sdk == val->config &&

               val->config.sdkVersion <= min_sdk;

      };



      // Remove the rest that match.

      // Remove the rest that match; all of them will be overridden by this one.

      auto filter_iter =

          make_filter_iterator(iter + 1, entry->values.rend(), pred);

      while (filter_iter.HasNext()) {
@@ -107,7 +110,8 @@ static void CollapseVersions(IAaptContext* context, int min_sdk, ResourceEntry* 
          context->GetDiagnostics()->Note(android::DiagMessage()

                                          << "removing configuration " << next->config.to_string()

                                          << " for entry: " << entry->name

                                          << ", because its SDK version is smaller than minSdk");

                                          << ", because its SDK version is smaller than minSdk "

                                          << min_sdk);

        }

        next = {};

      }
@@ -126,8 +130,9 @@ static void CollapseVersions(IAaptContext* context, int min_sdk, ResourceEntry* 
  // space in the resources.arsc table.

  bool modified = false;

  for (std::unique_ptr<ResourceConfigValue>& config_value : entry->values) {

    if (config_value->config.sdkVersion != 0 &&

        config_value->config.sdkVersion <= min_sdk) {

    const auto& config = config_value->config;

    if (config.sdkVersion != 0 && (config.sdkVersion < min_sdk ||

                                   (config.sdkVersion == min_sdk && config.minorVersion == 0))) {

      // Override the resource with a Configuration without an SDK.

      std::unique_ptr<ResourceConfigValue> new_value =

          util::make_unique<ResourceConfigValue>(

tools/aapt2/optimize/VersionCollapser_test.cpp

+147 −66
Original line number Diff line number Diff line
@@ -18,102 +18,183 @@ 


#include "test/Test.h"



using android::StringPiece;

using ::android::ConfigDescription;

using ::android::StringPiece;

using ::testing::IsNull;

using ::testing::NotNull;

using ::testing::StrEq;



namespace aapt {



static std::unique_ptr<ResourceTable> BuildTableWithConfigs(

    StringPiece name, std::initializer_list<std::string> list) {

    StringPiece name, std::initializer_list<std::string> configs) {

  test::ResourceTableBuilder builder;

  for (const std::string& item : list) {

    builder.AddSimple(name, test::ParseConfigOrDie(item));

  for (const std::string& config : configs) {

    // Use the config as the string value, so we can verify the correct config was renamed.

    builder.AddString(name, {}, test::ParseConfigOrDie(config), config);

  }

  return builder.Build();

}



TEST(VersionCollapserTest, CollapseVersions) {

  std::unique_ptr<IAaptContext> context =

      test::ContextBuilder().SetMinSdkVersion(7).Build();

static constexpr auto kResName = "@android:string/foo";



  const StringPiece res_name = "@android:string/foo";

static void ExpectConfigValueIsNull(ResourceTable* table, StringPiece config_str) {

  EXPECT_THAT(test::GetValueForConfig<String>(table, kResName, test::ParseConfigOrDie(config_str)),

              IsNull());

}



  std::unique_ptr<ResourceTable> table = BuildTableWithConfigs(

      res_name,

      {"land-v4", "land-v5", "sw600dp", "land-v6", "land-v14", "land-v21"});

static void ExpectConfigValueIs(ResourceTable* table, const ConfigDescription& config,

                                StringPiece expected_value_str) {

  auto value = test::GetValueForConfig<String>(table, kResName, config);

  ASSERT_THAT(value, NotNull());

  EXPECT_THAT(*value->value, StrEq(expected_value_str));

}



static void ExpectConfigValueIs(ResourceTable* table, StringPiece config_str,

                                StringPiece expected_value_str) {

  ExpectConfigValueIs(table, test::ParseConfigOrDie(config_str), expected_value_str);

}



TEST(VersionCollapserTest, CollapseVersions) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(7).Build();

  std::unique_ptr<ResourceTable> table = BuildTableWithConfigs(

      kResName, {"land-v4", "land-v5", "sw600dp", "land-v6", "land-v14", "land-v21"});

  VersionCollapser collapser;

  ASSERT_TRUE(collapser.Consume(context.get(), table.get()));



  // These should be removed.

  EXPECT_EQ(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v4")));

  EXPECT_EQ(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v5")));

  // This one should be removed because it was renamed to 'land', with the

  // version dropped.

  EXPECT_EQ(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v6")));

  ExpectConfigValueIsNull(table.get(), "land-v4");

  ExpectConfigValueIsNull(table.get(), "land-v5");



  // land-v6 should have been converted to land.

  ExpectConfigValueIsNull(table.get(), "land-v6");

  ExpectConfigValueIs(table.get(), "land", "land-v6");



  // These should remain.

  EXPECT_NE(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("sw600dp")));



  // 'land' should be present because it was renamed from 'land-v6'.

  EXPECT_NE(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land")));

  EXPECT_NE(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v14")));

  EXPECT_NE(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v21")));

  ExpectConfigValueIs(table.get(), "sw600dp", "sw600dp");

  ExpectConfigValueIs(table.get(), "land-v14", "land-v14");

  ExpectConfigValueIs(table.get(), "land-v21", "land-v21");

}



TEST(VersionCollapserTest, CollapseVersionsWhenMinSdkIsHighest) {

  std::unique_ptr<IAaptContext> context =

      test::ContextBuilder().SetMinSdkVersion(21).Build();

TEST(VersionCollapserTest, CollapseVersionsWhenMinSdkMatchesConfigVersionExactly) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(14).Build();

  std::unique_ptr<ResourceTable> table = BuildTableWithConfigs(

      kResName, {"land-v4", "land-v5", "sw600dp", "land-v6", "land-v14", "land-v21", "land-v22"});

  VersionCollapser collapser;

  ASSERT_TRUE(collapser.Consume(context.get(), table.get()));



  const StringPiece res_name = "@android:string/foo";

  // These should all be removed.

  ExpectConfigValueIsNull(table.get(), "land-v4");

  ExpectConfigValueIsNull(table.get(), "land-v5");

  ExpectConfigValueIsNull(table.get(), "land-v6");



  // land-v14 should have been converted to land.

  ExpectConfigValueIsNull(table.get(), "land-v14");

  ExpectConfigValueIs(table.get(), "land", "land-v14");



  // These should remain.

  ExpectConfigValueIs(table.get(), test::ParseConfigOrDie("sw600dp").CopyWithoutSdkVersion(),

                      "sw600dp");

  ExpectConfigValueIs(table.get(), "land-v21", "land-v21");

  ExpectConfigValueIs(table.get(), "land-v22", "land-v22");

}



TEST(VersionCollapserTest, CollapseVersionsWhenMinSdkIsHighest) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(21).Build();

  std::unique_ptr<ResourceTable> table = BuildTableWithConfigs(

      res_name, {"land-v4", "land-v5", "sw600dp", "land-v6", "land-v14",

                 "land-v21", "land-v22"});

      kResName, {"land-v4", "land-v5", "sw600dp", "land-v6", "land-v14", "land-v21"});

  VersionCollapser collapser;

  ASSERT_TRUE(collapser.Consume(context.get(), table.get()));



  // These should all be removed.

  EXPECT_EQ(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v4")));

  EXPECT_EQ(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v5")));

  EXPECT_EQ(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v6")));

  EXPECT_EQ(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v14")));

  ExpectConfigValueIsNull(table.get(), "land-v4");

  ExpectConfigValueIsNull(table.get(), "land-v5");

  ExpectConfigValueIsNull(table.get(), "land-v6");

  ExpectConfigValueIsNull(table.get(), "land-v14");



  // land-v21 should have been converted to land.

  ExpectConfigValueIsNull(table.get(), "land-v21");

  ExpectConfigValueIs(table.get(), "land", "land-v21");



  // These should remain.

  EXPECT_NE(nullptr,

            test::GetValueForConfig<Id>(

                table.get(), res_name,

                test::ParseConfigOrDie("sw600dp").CopyWithoutSdkVersion()));

  ExpectConfigValueIs(table.get(), test::ParseConfigOrDie("sw600dp").CopyWithoutSdkVersion(),

                      "sw600dp");

}



  // land-v21 should have been converted to land.

  EXPECT_NE(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land")));

  // land-v22 should remain as-is.

  EXPECT_NE(nullptr,

            test::GetValueForConfig<Id>(table.get(), res_name,

                                        test::ParseConfigOrDie("land-v22")));

TEST(VersionCollapserTest, CollapseVersionsWithMinorVersion) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(47).Build();

  std::unique_ptr<ResourceTable> table =

      BuildTableWithConfigs(kResName, {"land-v4", "land-v45.5", "sw600dp", "land-v46", "land-v46.1",

                                       "land-v54.5", "land-v61", "land-v63.2"});

  VersionCollapser collapser;

  ASSERT_TRUE(collapser.Consume(context.get(), table.get()));



  // These should be removed.

  ExpectConfigValueIsNull(table.get(), "land-v4");

  ExpectConfigValueIsNull(table.get(), "land-v45.5");

  ExpectConfigValueIsNull(table.get(), "land-v46");



  // land-v46.1 should have been converted to land.

  ExpectConfigValueIsNull(table.get(), "land-v46.1");

  ExpectConfigValueIs(table.get(), "land", "land-v46.1");



  // These should remain.

  ExpectConfigValueIs(table.get(), test::ParseConfigOrDie("sw600dp").CopyWithoutSdkVersion(),

                      "sw600dp");

  ExpectConfigValueIs(table.get(), "land-v54.5", "land-v54.5");

  ExpectConfigValueIs(table.get(), "land-v61", "land-v61");

  ExpectConfigValueIs(table.get(), "land-v63.2", "land-v63.2");

}



TEST(VersionCollapserTest, CollapseVersionsWithMinorVersionAndMinSdkMatchesConfigVersionExactly) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(46).Build();

  std::unique_ptr<ResourceTable> table =

      BuildTableWithConfigs(kResName, {"land-v4", "land-v45.5", "sw600dp", "land-v46", "land-v46.1",

                                       "land-v54.5", "land-v61", "land-v63.2"});

  VersionCollapser collapser;

  ASSERT_TRUE(collapser.Consume(context.get(), table.get()));



  // These should be removed.

  ExpectConfigValueIsNull(table.get(), "land-v4");

  ExpectConfigValueIsNull(table.get(), "land-v45.5");



  // land-v46 should have been converted to land.

  ExpectConfigValueIsNull(table.get(), "land-v46");

  ExpectConfigValueIs(table.get(), "land", "land-v46");



  // These should remain.

  ExpectConfigValueIs(table.get(), test::ParseConfigOrDie("sw600dp").CopyWithoutSdkVersion(),

                      "sw600dp");

  ExpectConfigValueIs(table.get(), "land-v46.1", "land-v46.1");

  ExpectConfigValueIs(table.get(), "land-v54.5", "land-v54.5");

  ExpectConfigValueIs(table.get(), "land-v61", "land-v61");

  ExpectConfigValueIs(table.get(), "land-v63.2", "land-v63.2");

}



TEST(VersionCollapserTest, CollapseVersionsWithMinorVersionAndMinSdkIsHighest) {

  std::unique_ptr<IAaptContext> context = test::ContextBuilder().SetMinSdkVersion(64).Build();

  std::unique_ptr<ResourceTable> table =

      BuildTableWithConfigs(kResName, {"land-v4", "land-v45.5", "sw600dp", "land-v46", "land-v46.1",

                                       "land-v54.5", "land-v61", "land-v63.2"});



  VersionCollapser collapser;

  ASSERT_TRUE(collapser.Consume(context.get(), table.get()));



  // These should be removed.

  ExpectConfigValueIsNull(table.get(), "land-v4");

  ExpectConfigValueIsNull(table.get(), "land-v45.5");

  ExpectConfigValueIsNull(table.get(), "land-v46");

  ExpectConfigValueIsNull(table.get(), "land-v46.1");

  ExpectConfigValueIsNull(table.get(), "land-v54.5");

  ExpectConfigValueIsNull(table.get(), "land-v61");



  // land-v63.2 should have been converted to land.

  ExpectConfigValueIsNull(table.get(), "land-v63.2");

  ExpectConfigValueIs(table.get(), "land", "land-v63.2");



  // These should remain.

  ExpectConfigValueIs(table.get(), test::ParseConfigOrDie("sw600dp").CopyWithoutSdkVersion(),

                      "sw600dp");

}



}  // namespace aapt