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

Commit 72edd1e4 authored by Mark Punzalan's avatar Mark Punzalan
Browse files

aapt2: Ensure sparse encoding isn't used on unsupported platforms

Current behavior:
- When --enable-sparse-encoding is used, sparse encode when minSdkVersion is missing or is >= 32 (S_v2)
- When --force-sparse-encoding is used, always sparse encode regardless of minSdkVersion

New behavior:
- When --enable-sparse-encoding is used, sparse encode when minSdkVersion is >= 32 (S_v2)
- When --force-sparse-encoding is used, sparse encode when minSdkVersion is missing or is >= 32 (S_v2)

Bug: 398187461
Test: build + boot on Pixel 6 Pro
Flag: EXEMPT build tool
Change-Id: Ide4e37f565107a33ecafc269ed81633e616b1a19
parent dbe3505d
Loading
Loading
Loading
Loading
+7 −7
Original line number Diff line number Diff line
@@ -38,13 +38,13 @@ class ConvertCommand : public Command {
        "--enable-sparse-encoding",
        "Enables encoding sparse entries using a binary search tree.\n"
        "This decreases APK size at the cost of resource retrieval performance.\n"
        "Only applies sparse encoding to Android O+ resources or all resources if minSdk of "
        "the APK is O+",
        "Only applies sparse encoding if minSdk of the APK is >= 32",
        &enable_sparse_encoding_);
    AddOptionalSwitch("--force-sparse-encoding",
    AddOptionalSwitch(
        "--force-sparse-encoding",
        "Enables encoding sparse entries using a binary search tree.\n"
        "This decreases APK size at the cost of resource retrieval performance.\n"
                      "Applies sparse encoding to all resources regardless of minSdk.",
        "Only applies sparse encoding if minSdk of the APK is >= 32 or is not set",
        &force_sparse_encoding_);
    AddOptionalSwitch(
        "--enable-compact-entries",
+6 −3
Original line number Diff line number Diff line
@@ -164,8 +164,11 @@ class LinkCommand : public Command {
    AddOptionalSwitch("--no-resource-removal", "Disables automatic removal of resources without\n"
            "defaults. Use this only when building runtime resource overlay packages.",
        &options_.no_resource_removal);
    AddOptionalSwitch("--enable-sparse-encoding",
                      "This decreases APK size at the cost of resource retrieval performance.",
    AddOptionalSwitch(
        "--enable-sparse-encoding",
        "Enables encoding sparse entries using a binary search tree.\n"
        "This decreases APK size at the cost of resource retrieval performance.\n"
        "Only applies sparse encoding if minSdk of the APK is >= 32",
        &options_.use_sparse_encoding);
    AddOptionalSwitch("--enable-compact-entries",
        "This decreases APK size by using compact resource entries for simple data types.",
+7 −7
Original line number Diff line number Diff line
@@ -108,13 +108,13 @@ class OptimizeCommand : public Command {
        "--enable-sparse-encoding",
        "Enables encoding sparse entries using a binary search tree.\n"
        "This decreases APK size at the cost of resource retrieval performance.\n"
        "Only applies sparse encoding to Android O+ resources or all resources if minSdk of "
        "the APK is O+",
        "Only applies sparse encoding if minSdk of the APK is >= 32",
        &options_.enable_sparse_encoding);
    AddOptionalSwitch("--force-sparse-encoding",
    AddOptionalSwitch(
        "--force-sparse-encoding",
        "Enables encoding sparse entries using a binary search tree.\n"
        "This decreases APK size at the cost of resource retrieval performance.\n"
                      "Applies sparse encoding to all resources regardless of minSdk.",
        "Only applies sparse encoding if minSdk of the APK is >= 32 or is not set",
        &options_.force_sparse_encoding);
    AddOptionalSwitch(
        "--enable-compact-entries",
+10 −7
Original line number Diff line number Diff line
@@ -197,13 +197,16 @@ class PackageFlattener {
    bool sparse_encode = sparse_entries_ == SparseEntriesMode::Enabled ||
                         sparse_entries_ == SparseEntriesMode::Forced;

    if (sparse_entries_ == SparseEntriesMode::Forced ||
        (context_->GetMinSdkVersion() == 0 && config.sdkVersion == 0)) {
      // Sparse encode if forced or sdk version is not set in context and config.
    } else {
      // Otherwise, only sparse encode if the entries will be read on platforms S_V2+.
      sparse_encode = sparse_encode && (context_->GetMinSdkVersion() >= SDK_S_V2);
    }
    // Only sparse encode if the entries will be read on platforms S_V2+. Sparse encoding
    // is not supported on older platforms (b/197642721, b/197976367).
    //
    // We also allow sparse encoding for minSdk is 0 (not set) if sparse encoding is forced,
    // in order to support Bundletool's usage of aapt2 where minSdk is not set in splits.
    bool meets_min_sdk_requirement_for_sparse_encoding =
        (context_->GetMinSdkVersion() >= SDK_S_V2) ||
        (context_->GetMinSdkVersion() == 0 && sparse_entries_ == SparseEntriesMode::Forced);

    sparse_encode = sparse_encode && meets_min_sdk_requirement_for_sparse_encoding;

    // Only sparse encode if the offsets are representable in 2 bytes.
    sparse_encode = sparse_encode && short_offsets;
+86 −36
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@
 */

#include "format/binary/TableFlattener.h"
#include <string>

#include "android-base/stringprintf.h"
#include "androidfw/TypeWrappers.h"
@@ -326,6 +327,28 @@ static std::unique_ptr<ResourceTable> BuildTableWithSparseEntries(
  return table;
}

static void CheckSparseEntries(IAaptContext* context, const ConfigDescription& sparse_config,
                               const std::string& sparse_contents) {
  ResourceTable sparse_table;
  BinaryResourceParser parser(context->GetDiagnostics(), &sparse_table, Source("test.arsc"),
                              sparse_contents.data(), sparse_contents.size());
  ASSERT_TRUE(parser.Parse());

  auto value = test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_0",
                                                        sparse_config);
  ASSERT_THAT(value, NotNull());
  EXPECT_EQ(0u, value->value.data);

  ASSERT_THAT(test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_1",
                                                       sparse_config),
              IsNull());

  value = test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_4",
                                                   sparse_config);
  ASSERT_THAT(value, NotNull());
  EXPECT_EQ(4u, value->value.data);
}

TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkSV2) {
  std::unique_ptr<IAaptContext> context = test::ContextBuilder()
                                              .SetCompilationPackage("android")
@@ -347,29 +370,56 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkSV2) {

  EXPECT_GT(no_sparse_contents.size(), sparse_contents.size());

  // Attempt to parse the sparse contents.
  CheckSparseEntries(context.get(), sparse_config, sparse_contents);
}

  ResourceTable sparse_table;
  BinaryResourceParser parser(context->GetDiagnostics(), &sparse_table, Source("test.arsc"),
                              sparse_contents.data(), sparse_contents.size());
  ASSERT_TRUE(parser.Parse());
TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkSV2AndForced) {
  std::unique_ptr<IAaptContext> context = test::ContextBuilder()
                                              .SetCompilationPackage("android")
                                              .SetPackageId(0x01)
                                              .SetMinSdkVersion(SDK_S_V2)
                                              .Build();

  auto value = test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_0",
                                                        sparse_config);
  ASSERT_THAT(value, NotNull());
  EXPECT_EQ(0u, value->value.data);
  const ConfigDescription sparse_config = test::ParseConfigOrDie("en-rGB");
  auto table_in = BuildTableWithSparseEntries(context.get(), sparse_config, 0.25f);

  ASSERT_THAT(test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_1",
                                                       sparse_config),
              IsNull());
  TableFlattenerOptions options;
  options.sparse_entries = SparseEntriesMode::Forced;

  value = test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_4",
                                                   sparse_config);
  ASSERT_THAT(value, NotNull());
  EXPECT_EQ(4u, value->value.data);
  std::string no_sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &no_sparse_contents));

  std::string sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents));

  EXPECT_GT(no_sparse_contents.size(), sparse_contents.size());

  CheckSparseEntries(context.get(), sparse_config, sparse_contents);
}

TEST_F(TableFlattenerTest, FlattenSparseEntryWithConfigSdkVersionSV2) {
TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkBeforeSV2) {
  std::unique_ptr<IAaptContext> context = test::ContextBuilder()
                                              .SetCompilationPackage("android")
                                              .SetPackageId(0x01)
                                              .SetMinSdkVersion(SDK_LOLLIPOP)
                                              .Build();

  const ConfigDescription sparse_config = test::ParseConfigOrDie("en-rGB");
  auto table_in = BuildTableWithSparseEntries(context.get(), sparse_config, 0.25f);

  TableFlattenerOptions options;
  options.sparse_entries = SparseEntriesMode::Enabled;

  std::string no_sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &no_sparse_contents));

  std::string sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents));

  EXPECT_EQ(no_sparse_contents.size(), sparse_contents.size());
}

TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkBeforeSV2AndConfigSdkVersionSV2) {
  std::unique_ptr<IAaptContext> context = test::ContextBuilder()
                                              .SetCompilationPackage("android")
                                              .SetPackageId(0x01)
@@ -391,7 +441,7 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryWithConfigSdkVersionSV2) {
  EXPECT_EQ(no_sparse_contents.size(), sparse_contents.size());
}

TEST_F(TableFlattenerTest, FlattenSparseEntryRegardlessOfMinSdkWhenForced) {
TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkBeforeSV2AndForced) {
  std::unique_ptr<IAaptContext> context = test::ContextBuilder()
                                              .SetCompilationPackage("android")
                                              .SetPackageId(0x01)
@@ -410,7 +460,7 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryRegardlessOfMinSdkWhenForced) {
  std::string sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents));

  EXPECT_GT(no_sparse_contents.size(), sparse_contents.size());
  EXPECT_EQ(no_sparse_contents.size(), sparse_contents.size());
}

TEST_F(TableFlattenerTest, FlattenSparseEntryWithSdkVersionNotSet) {
@@ -429,28 +479,28 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryWithSdkVersionNotSet) {
  std::string sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents));

  EXPECT_GT(no_sparse_contents.size(), sparse_contents.size());
  EXPECT_EQ(no_sparse_contents.size(), sparse_contents.size());
}

  // Attempt to parse the sparse contents.
TEST_F(TableFlattenerTest, FlattenSparseEntryWithSdkVersionNotSetAndForced) {
  std::unique_ptr<IAaptContext> context =
      test::ContextBuilder().SetCompilationPackage("android").SetPackageId(0x01).Build();

  ResourceTable sparse_table;
  BinaryResourceParser parser(context->GetDiagnostics(), &sparse_table, Source("test.arsc"),
                              sparse_contents.data(), sparse_contents.size());
  ASSERT_TRUE(parser.Parse());
  const ConfigDescription sparse_config = test::ParseConfigOrDie("en-rGB");
  auto table_in = BuildTableWithSparseEntries(context.get(), sparse_config, 0.25f);

  auto value = test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_0",
                                                        sparse_config);
  ASSERT_THAT(value, NotNull());
  EXPECT_EQ(0u, value->value.data);
  TableFlattenerOptions options;
  options.sparse_entries = SparseEntriesMode::Forced;

  ASSERT_THAT(test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_1",
                                                       sparse_config),
              IsNull());
  std::string no_sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &no_sparse_contents));

  value = test::GetValueForConfig<BinaryPrimitive>(&sparse_table, "android:string/foo_4",
                                                   sparse_config);
  ASSERT_THAT(value, NotNull());
  EXPECT_EQ(4u, value->value.data);
  std::string sparse_contents;
  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents));

  EXPECT_GT(no_sparse_contents.size(), sparse_contents.size());

  CheckSparseEntries(context.get(), sparse_config, sparse_contents);
}

TEST_F(TableFlattenerTest, DoNotUseSparseEntryForDenseConfig) {
Loading