AAPT2: Workaround for feature splits without namespacing

Android Instant Apps in its current iteration does not use namespaces,
but due to limitations on the Android resource runtime, needs to make
it look like it does.

This is due to a bug that treats any package ID that's not 0x01 or 0x7F
as a shared library. Shared libraries require unique package names.

As a workaround, and since Android Instant Apps can not have a feature
depend on another feature, we can alter the resource package name of a
feature split just before writing it out to disk. This avoids using a
unique package name while linking, thereby avoiding namespace issues.

Bug: 68820737
Test: manual
Change-Id: Ic553ed42656436bbb949393d0248ee7bb9d37860
diff --git a/tests/FeatureSplit/feature1/Android.mk b/tests/FeatureSplit/feature1/Android.mk
index b3ea97b..e6ba5c2 100644
--- a/tests/FeatureSplit/feature1/Android.mk
+++ b/tests/FeatureSplit/feature1/Android.mk
@@ -26,6 +26,6 @@
 LOCAL_RES_LIBRARIES := FeatureSplitBase
 
 LOCAL_AAPT_FLAGS += --package-id 0x80
-LOCAL_AAPT_FLAGS += --rename-manifest-package com.android.test.split.feature
+LOCAL_AAPT_FLAGS += --custom-package com.android.test.split.feature.one
 
 include $(BUILD_PACKAGE)
diff --git a/tests/FeatureSplit/feature1/AndroidManifest.xml b/tests/FeatureSplit/feature1/AndroidManifest.xml
index 4e7d151..b87361f 100644
--- a/tests/FeatureSplit/feature1/AndroidManifest.xml
+++ b/tests/FeatureSplit/feature1/AndroidManifest.xml
@@ -15,13 +15,13 @@
 -->
 
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
-    package="com.android.test.split.feature.one"
+    package="com.android.test.split.feature"
     featureSplit="feature1">
 
     <uses-sdk android:minSdkVersion="21" />
 
     <application>
-        <activity android:name=".One" android:label="Feature One">
+        <activity android:name=".one.One" android:label="Feature One">
             <intent-filter>
                 <action android:name="android.intent.action.MAIN" />
                 <category android:name="android.intent.category.LAUNCHER" />
diff --git a/tests/FeatureSplit/feature1/res/values/values.xml b/tests/FeatureSplit/feature1/res/values/values.xml
index 0e3e73c..7d58865 100644
--- a/tests/FeatureSplit/feature1/res/values/values.xml
+++ b/tests/FeatureSplit/feature1/res/values/values.xml
@@ -20,7 +20,7 @@
     <integer name="test_integer2">200</integer>
     <color name="test_color2">#00ff00</color>
     <string-array name="string_array2">
-        <item>@*com.android.test.split.feature:string/app_title</item>
+        <item>@string/app_title</item>
     </string-array>
 </resources>
 
diff --git a/tests/FeatureSplit/feature2/Android.mk b/tests/FeatureSplit/feature2/Android.mk
index e2fd903..c8e8609 100644
--- a/tests/FeatureSplit/feature2/Android.mk
+++ b/tests/FeatureSplit/feature2/Android.mk
@@ -26,6 +26,6 @@
 LOCAL_RES_LIBRARIES := FeatureSplitBase
 
 LOCAL_AAPT_FLAGS += --package-id 0x81
-LOCAL_AAPT_FLAGS += --rename-manifest-package com.android.test.split.feature
+LOCAL_AAPT_FLAGS += --custom-package com.android.test.split.feature.two
 
 include $(BUILD_PACKAGE)
diff --git a/tests/FeatureSplit/feature2/AndroidManifest.xml b/tests/FeatureSplit/feature2/AndroidManifest.xml
index bfe6f38..abd0b5e 100644
--- a/tests/FeatureSplit/feature2/AndroidManifest.xml
+++ b/tests/FeatureSplit/feature2/AndroidManifest.xml
@@ -15,7 +15,7 @@
 -->
 
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
-    package="com.android.test.split.feature.two"
+    package="com.android.test.split.feature"
     featureSplit="feature2">
 
     <uses-sdk android:minSdkVersion="21" />
diff --git a/tests/FeatureSplit/feature2/res/values/values.xml b/tests/FeatureSplit/feature2/res/values/values.xml
index 2fa6f90..af5ed1b 100644
--- a/tests/FeatureSplit/feature2/res/values/values.xml
+++ b/tests/FeatureSplit/feature2/res/values/values.xml
@@ -18,7 +18,7 @@
     <integer name="test_integer3">300</integer>
     <color name="test_color3">#0000ff</color>
     <string-array name="string_array3">
-        <item>@*com.android.test.split.feature:string/app_title</item>
+        <item>@string/app_title</item>
     </string-array>
 </resources>
 
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index b38d259..a782cd3 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -118,7 +118,7 @@
 bool Reference::Flatten(android::Res_value* out_value) const {
   const ResourceId resid = id.value_or_default(ResourceId(0));
   const bool dynamic = resid.is_valid_dynamic() && resid.package_id() != kFrameworkPackageId &&
-                       resid.package_id() != kAppPackageId;
+                       resid.package_id() < kAppPackageId;
 
   if (reference_type == Reference::Type::kResource) {
     if (dynamic) {
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index b372923..66b5a7a 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -795,6 +795,9 @@
     for (auto& entry : asset_source->GetAssignedPackageIds()) {
       if (entry.first > kFrameworkPackageId && entry.first < kAppPackageId) {
         final_table_.included_packages_[entry.first] = entry.second;
+      } else if (entry.first == kAppPackageId) {
+        // Capture the included base feature package.
+        included_feature_base_ = entry.second;
       }
     }
 
@@ -1465,11 +1468,48 @@
       return false;
     }
 
-    if (!FlattenTable(table, options_.output_format, writer)) {
-      context_->GetDiagnostics()->Error(DiagMessage() << "failed to write resource table");
-      return false;
+    // Hack to fix b/68820737.
+    // We need to modify the ResourceTable's package name, but that should NOT affect
+    // 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.
+    ResourceTablePackage* package_to_rewrite = nullptr;
+    if (context_->GetPackageId() > kAppPackageId &&
+        included_feature_base_ == make_value(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.
+      package_to_rewrite = table->FindPackage(context_->GetCompilationPackage());
+      if (package_to_rewrite != nullptr) {
+        CHECK_EQ(1u, table->packages.size()) << "can't change name of package when > 1 package";
+
+        std::string new_package_name =
+            StringPrintf("%s.%s", package_to_rewrite->name.c_str(),
+                         app_info_.split_name.value_or_default("feature").c_str());
+
+        if (context_->IsVerbose()) {
+          context_->GetDiagnostics()->Note(
+              DiagMessage() << "rewriting resource package name for feature split to '"
+                            << new_package_name << "'");
+        }
+        package_to_rewrite->name = new_package_name;
+      }
     }
-    return true;
+
+    bool success = FlattenTable(table, options_.output_format, writer);
+
+    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());
+      }
+    }
+
+    if (!success) {
+      context_->GetDiagnostics()->Error(DiagMessage() << "failed to write resource table");
+    }
+    return success;
   }
 
   int Run(const std::vector<std::string>& input_files) {
@@ -1498,8 +1538,8 @@
       return 1;
     }
 
-    const AppInfo& app_info = maybe_app_info.value();
-    context_->SetMinSdkVersion(app_info.min_sdk_version.value_or_default(0));
+    app_info_ = maybe_app_info.value();
+    context_->SetMinSdkVersion(app_info_.min_sdk_version.value_or_default(0));
 
     context_->SetNameManglerPolicy(NameManglerPolicy{context_->GetCompilationPackage()});
 
@@ -1699,7 +1739,7 @@
 
         // Generate an AndroidManifest.xml for each split.
         std::unique_ptr<xml::XmlResource> split_manifest =
-            GenerateSplitManifest(app_info, *split_constraints_iter);
+            GenerateSplitManifest(app_info_, *split_constraints_iter);
 
         XmlReferenceLinker linker;
         if (!linker.Consume(context_, split_manifest.get())) {
@@ -1799,6 +1839,8 @@
   LinkContext* context_;
   ResourceTable final_table_;
 
+  AppInfo app_info_;
+
   std::unique_ptr<TableMerger> table_merger_;
 
   // A pointer to the FileCollection representing the filesystem (not archives).
@@ -1816,6 +1858,9 @@
 
   // The set of shared libraries being used, mapping their assigned package ID to package name.
   std::map<size_t, std::string> shared_libs_;
+
+  // The package name of the base application, if it is included.
+  Maybe<std::string> included_feature_base_;
 };
 
 int Link(const std::vector<StringPiece>& args, IDiagnostics* diagnostics) {
diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp
index 4a1b46c..2a51df3 100644
--- a/tools/aapt2/format/binary/TableFlattener.cpp
+++ b/tools/aapt2/format/binary/TableFlattener.cpp
@@ -23,6 +23,7 @@
 
 #include "android-base/logging.h"
 #include "android-base/macros.h"
+#include "android-base/stringprintf.h"
 
 #include "ResourceTable.h"
 #include "ResourceValues.h"
@@ -579,7 +580,15 @@
       // Write a self mapping entry for this package if the ID is non-standard (0x7f).
       const uint8_t package_id = package->id.value();
       if (package_id != kFrameworkPackageId && package_id != kAppPackageId) {
-        table->included_packages_[package_id] = package->name;
+        auto result = table->included_packages_.insert({package_id, package->name});
+        if (!result.second && result.first->second != package->name) {
+          // A mapping for this package ID already exists, and is a different package. Error!
+          context->GetDiagnostics()->Error(
+              DiagMessage() << android::base::StringPrintf(
+                  "can't map package ID %02x to '%s'. Already mapped to '%s'", package_id,
+                  package->name.c_str(), result.first->second.c_str()));
+          return false;
+        }
       }
     }