From 87d30ddcc05a9fbbb3e38eb9383fc14495529104 Mon Sep 17 00:00:00 2001
From: Ryan Mitchell <rtmitchell@google.com>
Date: Tue, 30 Mar 2021 08:22:39 -0700
Subject: [PATCH] Fix aapt2 feature split package name generation

Before flattening the resource table of a feature split, if the package
name of the feature split is the same as the base package name, the
split name is appended to the end of the feature's package name.

A bug was causing the rewritten package name to not be applied to the
feature package which resulted in the feature split being flattened
under the same package name as the base.

Bug: 184034454
Test: `m CtsSplitAppFeatureWarm_v23` && `aapt2 dump resources`
      to observe that package name is correct
Change-Id: I0a0ac57764f599c4dd05326a222056a3c52a4ae3
---
 tools/aapt2/Android.bp   | 11 +++++++----
 tools/aapt2/cmd/Link.cpp | 24 ++++++++++++------------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp
index df727e0f883e2..3937cee99e3b4 100644
--- a/tools/aapt2/Android.bp
+++ b/tools/aapt2/Android.bp
@@ -196,9 +196,9 @@ cc_test_host {
     ],
     defaults: ["aapt2_defaults"],
     data: [
-         "integration-tests/CompileTest/**/*",
-         "integration-tests/CommandTests/**/*",
-         "integration-tests/ConvertTest/**/*"
+        "integration-tests/CompileTest/**/*",
+        "integration-tests/CommandTests/**/*",
+        "integration-tests/ConvertTest/**/*",
     ],
 }
 
@@ -232,6 +232,9 @@ genrule {
         "cp $(in) $(genDir)/protos && " +
         "$(location :soong_zip) -o $(out) -C $(genDir)/protos -D $(genDir)/protos",
     dist: {
-        targets: ["sdk_repo", "aapt2_artifacts"],
+        targets: [
+            "sdk_repo",
+            "aapt2_artifacts",
+        ],
     },
 }
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index ee6a764966f0f..2a8923d927f68 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -1764,7 +1764,7 @@ class Linker {
     // anything else being generated, which includes the Java classes.
     // If required, the package name is modifed before flattening, and then modified back
     // to its original name.
-    std::optional<ResourceTablePackageView> package_to_rewrite;
+    ResourceTablePackage* package_to_rewrite = nullptr;
     // Pre-O, the platform treats negative resource IDs [those with a package ID of 0x80
     // or higher] as invalid. In order to work around this limitation, we allow the use
     // of traditionally reserved resource IDs [those between 0x02 and 0x7E]. Allow the
@@ -1773,16 +1773,13 @@ class Linker {
           context_->GetPackageId() != kAppPackageId &&
           context_->GetPackageId() != kFrameworkPackageId)
         || (!options_.allow_reserved_package_id && context_->GetPackageId() > kAppPackageId);
-    if (isSplitPackage &&
-        included_feature_base_ == make_value(context_->GetCompilationPackage())) {
+    if (isSplitPackage && included_feature_base_ == context_->GetCompilationPackage()) {
       // The base APK is included, and this is a feature split. If the base package is
       // the same as this package, then we are building an old style Android Instant Apps feature
       // split and must apply this workaround to avoid requiring namespaces support.
-      auto table_view = table->GetPartitionedView();
-      if (!table_view.packages.empty() &&
-          table_view.packages.back().name == context_->GetCompilationPackage()) {
-        package_to_rewrite = std::move(table_view.packages.back());
-        CHECK_EQ(1u, table->packages.size()) << "can't change name of package when > 1 package";
+      if (!table->packages.empty() &&
+          table->packages.back()->name == context_->GetCompilationPackage()) {
+        package_to_rewrite = table->packages.back().get();
         std::string new_package_name =
             StringPrintf("%s.%s", package_to_rewrite->name.c_str(),
                          app_info_.split_name.value_or_default("feature").c_str());
@@ -1798,12 +1795,15 @@ class Linker {
 
     bool success = FlattenTable(table, options_.output_format, writer);
 
-    if (package_to_rewrite.has_value()) {
+    if (package_to_rewrite != nullptr) {
       // Change the name back.
       package_to_rewrite->name = context_->GetCompilationPackage();
-      if (package_to_rewrite->id) {
-        table->included_packages_.erase(package_to_rewrite->id.value());
-      }
+
+      // TableFlattener creates an `included_packages_` mapping entry for each package with a
+      // non-standard package id (not 0x01 or 0x7f). Since this is a feature split and not a shared
+      // library, do not include a mapping from the feature package name to the feature package id
+      // in the feature's dynamic reference table.
+      table->included_packages_.erase(context_->GetPackageId());
     }
 
     if (!success) {