Revert "Always enable sparse encoding if minSdk >= 32"

This reverts commit c9c3150896e1b637df6c8ecc485cb0e3384448c7.

Reason for revert: Sparse encoding got enabled for minSdk < 32
(b/395135057). Keeping the minor version bump to 2.20.

Change-Id: I8e143038a2e2a59be84669787764a9bf69210cd0

12 files changed, 43 insertions, 58 deletions

diff --git a/tools/aapt2/cmd/Command.cpp b/tools/aapt2/cmd/Command.cpp
index f00a6cad6b46..20315561cceb 100644
--- a/tools/aapt2/cmd/Command.cpp
+++ b/tools/aapt2/cmd/Command.cpp
@@ -54,9 +54,7 @@ std::string GetSafePath(StringPiece arg) {
 void Command::AddRequiredFlag(StringPiece name, StringPiece description, std::string* value,
                               uint32_t flags) {
   auto func = [value, flags](StringPiece arg, std::ostream*) -> bool {
-    if (value) {
-      *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg);
-    }
+    *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg);
     return true;
   };
 
@@ -67,9 +65,7 @@ void Command::AddRequiredFlag(StringPiece name, StringPiece description, std::st
 void Command::AddRequiredFlagList(StringPiece name, StringPiece description,
                                   std::vector<std::string>* value, uint32_t flags) {
   auto func = [value, flags](StringPiece arg, std::ostream*) -> bool {
-    if (value) {
-      value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg));
-    }
+    value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg));
     return true;
   };
 
@@ -80,9 +76,7 @@ void Command::AddRequiredFlagList(StringPiece name, StringPiece description,
 void Command::AddOptionalFlag(StringPiece name, StringPiece description,
                               std::optional<std::string>* value, uint32_t flags) {
   auto func = [value, flags](StringPiece arg, std::ostream*) -> bool {
-    if (value) {
-      *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg);
-    }
+    *value = (flags & Command::kPath) ? GetSafePath(arg) : std::string(arg);
     return true;
   };
 
@@ -93,9 +87,7 @@ void Command::AddOptionalFlag(StringPiece name, StringPiece description,
 void Command::AddOptionalFlagList(StringPiece name, StringPiece description,
                                   std::vector<std::string>* value, uint32_t flags) {
   auto func = [value, flags](StringPiece arg, std::ostream*) -> bool {
-    if (value) {
-      value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg));
-    }
+    value->push_back((flags & Command::kPath) ? GetSafePath(arg) : std::string(arg));
     return true;
   };
 
@@ -106,9 +98,7 @@ void Command::AddOptionalFlagList(StringPiece name, StringPiece description,
 void Command::AddOptionalFlagList(StringPiece name, StringPiece description,
                                   std::unordered_set<std::string>* value) {
   auto func = [value](StringPiece arg, std::ostream* out_error) -> bool {
-    if (value) {
-      value->emplace(arg);
-    }
+    value->emplace(arg);
     return true;
   };
 
@@ -118,9 +108,7 @@ void Command::AddOptionalFlagList(StringPiece name, StringPiece description,
 
 void Command::AddOptionalSwitch(StringPiece name, StringPiece description, bool* value) {
   auto func = [value](StringPiece arg, std::ostream* out_error) -> bool {
-    if (value) {
-      *value = true;
-    }
+    *value = true;
     return true;
   };
 
diff --git a/tools/aapt2/cmd/Command_test.cpp b/tools/aapt2/cmd/Command_test.cpp
index ad167c979662..2a3cb2a0c65d 100644
--- a/tools/aapt2/cmd/Command_test.cpp
+++ b/tools/aapt2/cmd/Command_test.cpp
@@ -159,22 +159,4 @@ TEST(CommandTest, ShortOptions) {
   ASSERT_NE(0, command.Execute({"-w"s, "2"s}, &std::cerr));
 }
 
-TEST(CommandTest, OptionsWithNullptrToAcceptValues) {
-  TestCommand command;
-  command.AddRequiredFlag("--rflag", "", nullptr);
-  command.AddRequiredFlagList("--rlflag", "", nullptr);
-  command.AddOptionalFlag("--oflag", "", nullptr);
-  command.AddOptionalFlagList("--olflag", "", (std::vector<std::string>*)nullptr);
-  command.AddOptionalFlagList("--olflag2", "", (std::unordered_set<std::string>*)nullptr);
-  command.AddOptionalSwitch("--switch", "", nullptr);
-
-  ASSERT_EQ(0, command.Execute({
-    "--rflag"s, "1"s,
-    "--rlflag"s, "1"s,
-    "--oflag"s, "1"s,
-    "--olflag"s, "1"s,
-    "--olflag2"s, "1"s,
-    "--switch"s}, &std::cerr));
-}
-
 }  // namespace aapt
\ No newline at end of file
diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp
index 060bc5fa2242..6c3eae11eab9 100644
--- a/tools/aapt2/cmd/Convert.cpp
+++ b/tools/aapt2/cmd/Convert.cpp
@@ -425,6 +425,9 @@ int ConvertCommand::Action(const std::vector<std::string>& args) {
                                     << output_format_.value());
     return 1;
   }
+  if (enable_sparse_encoding_) {
+    table_flattener_options_.sparse_entries = SparseEntriesMode::Enabled;
+  }
   if (force_sparse_encoding_) {
     table_flattener_options_.sparse_entries = SparseEntriesMode::Forced;
   }
diff --git a/tools/aapt2/cmd/Convert.h b/tools/aapt2/cmd/Convert.h
index 98c8f5ff89c0..9452e588953e 100644
--- a/tools/aapt2/cmd/Convert.h
+++ b/tools/aapt2/cmd/Convert.h
@@ -36,9 +36,11 @@ class ConvertCommand : public Command {
         kOutputFormatProto, kOutputFormatBinary, kOutputFormatBinary), &output_format_);
     AddOptionalSwitch(
         "--enable-sparse-encoding",
-        "[DEPRECATED] This flag is a no-op as of aapt2 v2.20. Sparse encoding is always\n"
-        "enabled if minSdk of the APK is >= 32.",
-        nullptr);
+        "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+",
+        &enable_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"
@@ -85,6 +87,7 @@ class ConvertCommand : public Command {
   std::string output_path_;
   std::optional<std::string> output_format_;
   bool verbose_ = false;
+  bool enable_sparse_encoding_ = false;
   bool force_sparse_encoding_ = false;
   bool enable_compact_entries_ = false;
   std::optional<std::string> resources_config_path_;
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index eb71189ffc46..232b4024abd2 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -2504,6 +2504,9 @@ int LinkCommand::Action(const std::vector<std::string>& args) {
                 << "the --merge-only flag can be only used when building a static library");
     return 1;
   }
+  if (options_.use_sparse_encoding) {
+    options_.table_flattener_options.sparse_entries = SparseEntriesMode::Enabled;
+  }
 
   // The default build type.
   context.SetPackageType(PackageType::kApp);
diff --git a/tools/aapt2/cmd/Link.h b/tools/aapt2/cmd/Link.h
index b5bd905c02be..2f17853718ec 100644
--- a/tools/aapt2/cmd/Link.h
+++ b/tools/aapt2/cmd/Link.h
@@ -75,6 +75,7 @@ struct LinkOptions {
   bool no_resource_removal = false;
   bool no_xml_namespaces = false;
   bool do_not_compress_anything = false;
+  bool use_sparse_encoding = false;
   std::unordered_set<std::string> extensions_to_not_compress;
   std::optional<std::regex> regex_to_not_compress;
   FeatureFlagValues feature_flag_values;
@@ -162,11 +163,9 @@ 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",
-        "[DEPRECATED] This flag is a no-op as of aapt2 v2.20. Sparse encoding is always\n"
-        "enabled if minSdk of the APK is >= 32.",
-        nullptr);
+    AddOptionalSwitch("--enable-sparse-encoding",
+                      "This decreases APK size at the cost of resource retrieval performance.",
+                      &options_.use_sparse_encoding);
     AddOptionalSwitch("--enable-compact-entries",
         "This decreases APK size by using compact resource entries for simple data types.",
         &options_.table_flattener_options.use_compact_entries);
diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp
index f218307af578..762441ee1872 100644
--- a/tools/aapt2/cmd/Optimize.cpp
+++ b/tools/aapt2/cmd/Optimize.cpp
@@ -406,6 +406,9 @@ int OptimizeCommand::Action(const std::vector<std::string>& args) {
     return 1;
   }
 
+  if (options_.enable_sparse_encoding) {
+    options_.table_flattener_options.sparse_entries = SparseEntriesMode::Enabled;
+  }
   if (options_.force_sparse_encoding) {
     options_.table_flattener_options.sparse_entries = SparseEntriesMode::Forced;
   }
diff --git a/tools/aapt2/cmd/Optimize.h b/tools/aapt2/cmd/Optimize.h
index e3af584cbbd9..012b0f230ca2 100644
--- a/tools/aapt2/cmd/Optimize.h
+++ b/tools/aapt2/cmd/Optimize.h
@@ -61,6 +61,9 @@ struct OptimizeOptions {
   // TODO(b/246489170): keep the old option and format until transform to the new one
   std::optional<std::string> shortened_paths_map_path;
 
+  // Whether sparse encoding should be used for O+ resources.
+  bool enable_sparse_encoding = false;
+
   // Whether sparse encoding should be used for all resources.
   bool force_sparse_encoding = false;
 
@@ -103,9 +106,11 @@ class OptimizeCommand : public Command {
         &kept_artifacts_);
     AddOptionalSwitch(
         "--enable-sparse-encoding",
-        "[DEPRECATED] This flag is a no-op as of aapt2 v2.20. Sparse encoding is always\n"
-        "enabled if minSdk of the APK is >= 32.",
-        nullptr);
+        "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+",
+        &options_.enable_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"
diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp
index b8ac7925d44e..1a82021bce71 100644
--- a/tools/aapt2/format/binary/TableFlattener.cpp
+++ b/tools/aapt2/format/binary/TableFlattener.cpp
@@ -201,7 +201,7 @@ class PackageFlattener {
         (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+ (32).
+      // Otherwise, only sparse encode if the entries will be read on platforms S_V2+.
       sparse_encode = sparse_encode && (context_->GetMinSdkVersion() >= SDK_S_V2);
     }
 
diff --git a/tools/aapt2/format/binary/TableFlattener.h b/tools/aapt2/format/binary/TableFlattener.h
index f1c4c3512ed3..0633bc81cb25 100644
--- a/tools/aapt2/format/binary/TableFlattener.h
+++ b/tools/aapt2/format/binary/TableFlattener.h
@@ -37,7 +37,8 @@ constexpr const size_t kSparseEncodingThreshold = 60;
 enum class SparseEntriesMode {
   // Disables sparse encoding for entries.
   Disabled,
-  // Enables sparse encoding for all entries for APKs with minSdk >= 32 (S_V2).
+  // Enables sparse encoding for all entries for APKs with O+ minSdk. For APKs with minSdk less
+  // than O only applies sparse encoding for resource configuration available on O+.
   Enabled,
   // Enables sparse encoding for all entries regardless of minSdk.
   Forced,
@@ -46,7 +47,7 @@ enum class SparseEntriesMode {
 struct TableFlattenerOptions {
   // When enabled, types for configurations with a sparse set of entries are encoded
   // as a sparse map of entry ID and offset to actual data.
-  SparseEntriesMode sparse_entries = SparseEntriesMode::Enabled;
+  SparseEntriesMode sparse_entries = SparseEntriesMode::Disabled;
 
   // When true, use compact entries for simple data
   bool use_compact_entries = false;
diff --git a/tools/aapt2/format/binary/TableFlattener_test.cpp b/tools/aapt2/format/binary/TableFlattener_test.cpp
index e3d589eb078b..0f1168514c4a 100644
--- a/tools/aapt2/format/binary/TableFlattener_test.cpp
+++ b/tools/aapt2/format/binary/TableFlattener_test.cpp
@@ -337,13 +337,13 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryWithMinSdkSV2) {
   auto table_in = BuildTableWithSparseEntries(context.get(), sparse_config, 0.25f);
 
   TableFlattenerOptions options;
-  options.sparse_entries = SparseEntriesMode::Disabled;
+  options.sparse_entries = SparseEntriesMode::Enabled;
 
   std::string no_sparse_contents;
-  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &no_sparse_contents));
+  ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &no_sparse_contents));
 
   std::string sparse_contents;
-  ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &sparse_contents));
+  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents));
 
   EXPECT_GT(no_sparse_contents.size(), sparse_contents.size());
 
@@ -421,13 +421,13 @@ TEST_F(TableFlattenerTest, FlattenSparseEntryWithSdkVersionNotSet) {
   auto table_in = BuildTableWithSparseEntries(context.get(), sparse_config, 0.25f);
 
   TableFlattenerOptions options;
-  options.sparse_entries = SparseEntriesMode::Disabled;
+  options.sparse_entries = SparseEntriesMode::Enabled;
 
   std::string no_sparse_contents;
-  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &no_sparse_contents));
+  ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &no_sparse_contents));
 
   std::string sparse_contents;
-  ASSERT_TRUE(Flatten(context.get(), {}, table_in.get(), &sparse_contents));
+  ASSERT_TRUE(Flatten(context.get(), options, table_in.get(), &sparse_contents));
 
   EXPECT_GT(no_sparse_contents.size(), sparse_contents.size());
 
diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index 664d8412a3be..5c3dfdcadfec 100644
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -3,8 +3,6 @@
 ## Version 2.20
 - Too many features, bug fixes, and improvements to list since the last minor version update in
   2017. This README will be updated more frequently in the future.
-- Sparse encoding is now always enabled by default if the minSdkVersion is >= 32 (S_V2). The
-  `--enable-sparse-encoding` flag still exists, but is a no-op.
 
 ## Version 2.19
 - Added navigation resource type.