AAPT2: Add --package-id flag for feature-split suppport

Bug: 35928935
Change-Id: Ia8496505e61cfcfdb8f9f62366d2f36e453db111
Test: make aapt2_tests

13 files changed, 220 insertions, 152 deletions

diff --git a/tests/FeatureSplit/base/Android.mk b/tests/FeatureSplit/base/Android.mk
index 7c0fc04a445a..93f6d7a5f52b 100644
--- a/tests/FeatureSplit/base/Android.mk
+++ b/tests/FeatureSplit/base/Android.mk
@@ -19,6 +19,7 @@ include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := $(call all-subdir-java-files)
 LOCAL_PACKAGE_NAME := FeatureSplitBase
+LOCAL_EXPORT_PACKAGE_RESOURCES := true
 
 LOCAL_MODULE_TAGS := tests
 
diff --git a/tests/FeatureSplit/feature1/Android.mk b/tests/FeatureSplit/feature1/Android.mk
index aa222dd6aa66..e6ba5c2d04c9 100644
--- a/tests/FeatureSplit/feature1/Android.mk
+++ b/tests/FeatureSplit/feature1/Android.mk
@@ -17,17 +17,15 @@
 LOCAL_PATH:= $(call my-dir)
 include $(CLEAR_VARS)
 
+LOCAL_USE_AAPT2 := true
 LOCAL_SRC_FILES := $(call all-subdir-java-files)
 LOCAL_PACKAGE_NAME := FeatureSplit1
 LOCAL_MODULE_TAGS := tests
 
-featureOf := FeatureSplitBase
+LOCAL_APK_LIBRARIES := FeatureSplitBase
+LOCAL_RES_LIBRARIES := FeatureSplitBase
 
-LOCAL_APK_LIBRARIES := $(featureOf)
-featureOfApk := $(call intermediates-dir-for,APPS,$(featureOf))/package.apk
-localRStamp := $(call intermediates-dir-for,APPS,$(LOCAL_PACKAGE_NAME),,COMMON)/src/R.stamp
-$(localRStamp): $(featureOfApk)
-
-LOCAL_AAPT_FLAGS := --feature-of $(featureOfApk) --custom-package com.android.test.split.feature.one
+LOCAL_AAPT_FLAGS += --package-id 0x80
+LOCAL_AAPT_FLAGS += --custom-package com.android.test.split.feature.one
 
 include $(BUILD_PACKAGE)
diff --git a/tests/FeatureSplit/feature2/Android.mk b/tests/FeatureSplit/feature2/Android.mk
index 1a0322bf686e..c8e860942fa3 100644
--- a/tests/FeatureSplit/feature2/Android.mk
+++ b/tests/FeatureSplit/feature2/Android.mk
@@ -17,22 +17,15 @@
 LOCAL_PATH:= $(call my-dir)
 include $(CLEAR_VARS)
 
+LOCAL_USE_AAPT2 := true
 LOCAL_SRC_FILES := $(call all-subdir-java-files)
 LOCAL_PACKAGE_NAME := FeatureSplit2
 LOCAL_MODULE_TAGS := tests
 
-featureOf := FeatureSplitBase
-featureAfter := FeatureSplit1
+LOCAL_APK_LIBRARIES := FeatureSplitBase
+LOCAL_RES_LIBRARIES := FeatureSplitBase
 
-LOCAL_APK_LIBRARIES := $(featureOf)
-
-featureOfApk := $(call intermediates-dir-for,APPS,$(featureOf))/package.apk
-featureAfterApk := $(call intermediates-dir-for,APPS,$(featureAfter))/package.apk
-localRStamp := $(call intermediates-dir-for,APPS,$(LOCAL_PACKAGE_NAME),,COMMON)/src/R.stamp
-$(localRStamp): $(featureOfApk) $(featureAfterApk)
-
-LOCAL_AAPT_FLAGS := --feature-of $(featureOfApk)
-LOCAL_AAPT_FLAGS += --feature-after $(featureAfterApk)
+LOCAL_AAPT_FLAGS += --package-id 0x81
 LOCAL_AAPT_FLAGS += --custom-package com.android.test.split.feature.two
 
 include $(BUILD_PACKAGE)
diff --git a/tools/aapt2/Main.cpp b/tools/aapt2/Main.cpp
index 38d9ef84f44b..456f68635705 100644
--- a/tools/aapt2/Main.cpp
+++ b/tools/aapt2/Main.cpp
@@ -25,7 +25,7 @@ namespace aapt {
 static const char* sMajorVersion = "2";
 
 // Update minor version whenever a feature or flag is added.
-static const char* sMinorVersion = "9";
+static const char* sMinorVersion = "10";
 
 int PrintVersion() {
   std::cerr << "Android Asset Packaging Tool (aapt) " << sMajorVersion << "."
diff --git a/tools/aapt2/diff/Diff.cpp b/tools/aapt2/diff/Diff.cpp
index c8774687944c..dacf8d9f1e96 100644
--- a/tools/aapt2/diff/Diff.cpp
+++ b/tools/aapt2/diff/Diff.cpp
@@ -331,7 +331,7 @@ class ZeroingReferenceVisitor : public ValueVisitor {
 
   void Visit(Reference* ref) override {
     if (ref->name && ref->id) {
-      if (ref->id.value().package_id() == 0x7f) {
+      if (ref->id.value().package_id() == kAppPackageId) {
         ref->id = {};
       }
     }
diff --git a/tools/aapt2/flatten/XmlFlattener_test.cpp b/tools/aapt2/flatten/XmlFlattener_test.cpp
index ec3d75e354c7..494d9d25b008 100644
--- a/tools/aapt2/flatten/XmlFlattener_test.cpp
+++ b/tools/aapt2/flatten/XmlFlattener_test.cpp
@@ -30,23 +30,20 @@ namespace aapt {
 class XmlFlattenerTest : public ::testing::Test {
  public:
   void SetUp() override {
-    context_ =
-        test::ContextBuilder()
-            .SetCompilationPackage("com.app.test")
-            .SetNameManglerPolicy(NameManglerPolicy{"com.app.test"})
-            .AddSymbolSource(
-                test::StaticSymbolSourceBuilder()
-                    .AddSymbol("android:attr/id", ResourceId(0x010100d0),
-                               test::AttributeBuilder().Build())
-                    .AddSymbol("com.app.test:id/id", ResourceId(0x7f020000))
-                    .AddSymbol("android:attr/paddingStart",
-                               ResourceId(0x010103b3),
-                               test::AttributeBuilder().Build())
-                    .AddSymbol("android:attr/colorAccent",
-                               ResourceId(0x01010435),
-                               test::AttributeBuilder().Build())
-                    .Build())
-            .Build();
+    context_ = test::ContextBuilder()
+                   .SetCompilationPackage("com.app.test")
+                   .SetNameManglerPolicy(NameManglerPolicy{"com.app.test"})
+                   .AddSymbolSource(
+                       test::StaticSymbolSourceBuilder()
+                           .AddSymbol("android:attr/id", ResourceId(0x010100d0),
+                                      test::AttributeBuilder().Build())
+                           .AddSymbol("com.app.test:id/id", ResourceId(0x7f020000))
+                           .AddPublicSymbol("android:attr/paddingStart", ResourceId(0x010103b3),
+                                            test::AttributeBuilder().Build())
+                           .AddPublicSymbol("android:attr/colorAccent", ResourceId(0x01010435),
+                                            test::AttributeBuilder().Build())
+                           .Build())
+                   .Build();
   }
 
   ::testing::AssertionResult Flatten(xml::XmlResource* doc,
diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp
index dd8e14b40fa8..c8f02171bfd3 100644
--- a/tools/aapt2/link/Link.cpp
+++ b/tools/aapt2/link/Link.cpp
@@ -23,6 +23,7 @@
 
 #include "android-base/errors.h"
 #include "android-base/file.h"
+#include "android-base/stringprintf.h"
 #include "androidfw/StringPiece.h"
 #include "google/protobuf/io/coded_stream.h"
 
@@ -57,6 +58,7 @@
 #include "xml/XmlDom.h"
 
 using android::StringPiece;
+using android::base::StringPrintf;
 using ::google::protobuf::io::CopyingOutputStreamAdaptor;
 
 namespace aapt {
@@ -705,11 +707,17 @@ class LinkCommand {
         }
 
         // If we are using --no-static-lib-packages, we need to rename the
-        // package of this
-        // table to our compilation package.
+        // package of this table to our compilation package.
         if (options_.no_static_lib_packages) {
-          if (ResourceTablePackage* pkg = include_static->FindPackageById(0x7f)) {
+          // Since package names can differ, and multiple packages can exist in a ResourceTable,
+          // we place the requirement that all static libraries are built with the package
+          // ID 0x7f. So if one is not found, this is an error.
+          if (ResourceTablePackage* pkg = include_static->FindPackageById(kAppPackageId)) {
             pkg->name = context_->GetCompilationPackage();
+          } else {
+            context_->GetDiagnostics()->Error(DiagMessage(path)
+                                              << "no package with ID 0x7f found in static library");
+            return false;
           }
         }
 
@@ -733,7 +741,7 @@ class LinkCommand {
     // Capture the shared libraries so that the final resource table can be properly flattened
     // with support for shared libraries.
     for (auto& entry : asset_source->GetAssignedPackageIds()) {
-      if (entry.first > 0x01 && entry.first < 0x7f) {
+      if (entry.first > kFrameworkPackageId && entry.first < kAppPackageId) {
         final_table_.included_packages_[entry.first] = entry.second;
       }
     }
@@ -860,10 +868,9 @@ class LinkCommand {
     for (const auto& package : final_table_.packages) {
       for (const auto& type : package->types) {
         if (type->id) {
-          context_->GetDiagnostics()->Error(
-              DiagMessage() << "type " << type->type << " has ID " << std::hex
-                            << (int)type->id.value() << std::dec
-                            << " assigned");
+          context_->GetDiagnostics()->Error(DiagMessage() << "type " << type->type << " has ID "
+                                                          << StringPrintf("%02x", type->id.value())
+                                                          << " assigned");
           return false;
         }
 
@@ -871,9 +878,8 @@ class LinkCommand {
           if (entry->id) {
             ResourceNameRef res_name(package->name, type->type, entry->name);
             context_->GetDiagnostics()->Error(
-                DiagMessage() << "entry " << res_name << " has ID " << std::hex
-                              << (int)entry->id.value() << std::dec
-                              << " assigned");
+                DiagMessage() << "entry " << res_name << " has ID "
+                              << StringPrintf("%02x", entry->id.value()) << " assigned");
             return false;
           }
         }
@@ -1103,7 +1109,7 @@ class LinkCommand {
       return false;
     }
 
-    ResourceTablePackage* pkg = table->FindPackageById(0x7f);
+    ResourceTablePackage* pkg = table->FindPackageById(kAppPackageId);
     if (!pkg) {
       context_->GetDiagnostics()->Error(DiagMessage(input) << "static library has no package");
       return false;
@@ -1490,12 +1496,17 @@ class LinkCommand {
 
     context_->SetNameManglerPolicy(
         NameManglerPolicy{context_->GetCompilationPackage()});
-    if (options_.package_type == PackageType::kSharedLib) {
-      context_->SetPackageId(0x00);
-    } else if (context_->GetCompilationPackage() == "android") {
+
+    // Override the package ID when it is "android".
+    if (context_->GetCompilationPackage() == "android") {
       context_->SetPackageId(0x01);
-    } else {
-      context_->SetPackageId(0x7f);
+
+      // Verify we're building a regular app.
+      if (options_.package_type != PackageType::kApp) {
+        context_->GetDiagnostics()->Error(
+            DiagMessage() << "package 'android' can only be built as a regular app");
+        return 1;
+      }
     }
 
     if (!LoadSymbolsFromIncludePaths()) {
@@ -1509,10 +1520,9 @@ class LinkCommand {
 
     if (context_->IsVerbose()) {
       context_->GetDiagnostics()->Note(DiagMessage()
-                                       << "linking package '"
-                                       << context_->GetCompilationPackage()
-                                       << "' with package ID " << std::hex
-                                       << (int)context_->GetPackageId());
+                                       << StringPrintf("linking package '%s' using package ID %02x",
+                                                       context_->GetCompilationPackage().data(),
+                                                       context_->GetPackageId()));
     }
 
     for (const std::string& input : input_files) {
@@ -1889,6 +1899,7 @@ int Link(const std::vector<StringPiece>& args) {
   LinkOptions options;
   std::vector<std::string> overlay_arg_list;
   std::vector<std::string> extra_java_packages;
+  Maybe<std::string> package_id;
   Maybe<std::string> configs;
   Maybe<std::string> preferred_density;
   Maybe<std::string> product_list;
@@ -1909,6 +1920,10 @@ int Link(const std::vector<StringPiece>& args) {
                             "Compilation unit to link, using `overlay` semantics.\n"
                             "The last conflicting resource given takes precedence.",
                             &overlay_arg_list)
+          .OptionalFlag("--package-id",
+                        "Specify the package ID to use for this app. Must be greater or equal to\n"
+                        "0x7f and can't be used with --static-lib or --shared-lib.",
+                        &package_id)
           .OptionalFlag("--java", "Directory in which to generate R.java",
                         &options.generate_java_class_path)
           .OptionalFlag("--proguard", "Output file for generated Proguard rules",
@@ -2062,6 +2077,47 @@ int Link(const std::vector<StringPiece>& args) {
     context.SetVerbose(verbose);
   }
 
+  if (shared_lib && static_lib) {
+    context.GetDiagnostics()->Error(DiagMessage()
+                                    << "only one of --shared-lib and --static-lib can be defined");
+    return 1;
+  }
+
+  if (shared_lib) {
+    options.package_type = PackageType::kSharedLib;
+    context.SetPackageId(0x00);
+  } else if (static_lib) {
+    options.package_type = PackageType::kStaticLib;
+    context.SetPackageId(kAppPackageId);
+  } else {
+    options.package_type = PackageType::kApp;
+    context.SetPackageId(kAppPackageId);
+  }
+
+  if (package_id) {
+    if (options.package_type != PackageType::kApp) {
+      context.GetDiagnostics()->Error(
+          DiagMessage() << "can't specify --package-id when not building a regular app");
+      return 1;
+    }
+
+    const Maybe<uint32_t> maybe_package_id_int = ResourceUtils::ParseInt(package_id.value());
+    if (!maybe_package_id_int) {
+      context.GetDiagnostics()->Error(DiagMessage() << "package ID '" << package_id.value()
+                                                    << "' is not a valid integer");
+      return 1;
+    }
+
+    const uint32_t package_id_int = maybe_package_id_int.value();
+    if (package_id_int < kAppPackageId || package_id_int > std::numeric_limits<uint8_t>::max()) {
+      context.GetDiagnostics()->Error(
+          DiagMessage() << StringPrintf(
+              "invalid package ID 0x%02x. Must be in the range 0x7f-0xff.", package_id_int));
+      return 1;
+    }
+    context.SetPackageId(static_cast<uint8_t>(package_id_int));
+  }
+
   // Populate the set of extra packages for which to generate R.java.
   for (std::string& extra_package : extra_java_packages) {
     // A given package can actually be a colon separated list of packages.
@@ -2128,18 +2184,6 @@ int Link(const std::vector<StringPiece>& args) {
     options.table_splitter_options.preferred_densities.push_back(preferred_density_config.density);
   }
 
-  if (shared_lib && static_lib) {
-    context.GetDiagnostics()->Error(DiagMessage()
-                                    << "only one of --shared-lib and --static-lib can be defined");
-    return 1;
-  }
-
-  if (shared_lib) {
-    options.package_type = PackageType::kSharedLib;
-  } else if (static_lib) {
-    options.package_type = PackageType::kStaticLib;
-  }
-
   if (options.package_type != PackageType::kStaticLib && stable_id_file_path) {
     if (!LoadStableIdMap(context.GetDiagnostics(), stable_id_file_path.value(),
                          &options.stable_id_map)) {
diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp
index 0331313563c8..833ae69a0317 100644
--- a/tools/aapt2/link/ReferenceLinker.cpp
+++ b/tools/aapt2/link/ReferenceLinker.cpp
@@ -51,18 +51,16 @@ class ReferenceLinkerVisitor : public ValueVisitor {
  public:
   using ValueVisitor::Visit;
 
-  ReferenceLinkerVisitor(IAaptContext* context, SymbolTable* symbols,
-                         StringPool* string_pool, xml::IPackageDeclStack* decl,
-                         CallSite* callsite)
-      : context_(context),
+  ReferenceLinkerVisitor(const CallSite& callsite, IAaptContext* context, SymbolTable* symbols,
+                         StringPool* string_pool, xml::IPackageDeclStack* decl)
+      : callsite_(callsite),
+        context_(context),
         symbols_(symbols),
         package_decls_(decl),
-        string_pool_(string_pool),
-        callsite_(callsite) {}
+        string_pool_(string_pool) {}
 
   void Visit(Reference* ref) override {
-    if (!ReferenceLinker::LinkReference(ref, context_, symbols_, package_decls_,
-                                        callsite_)) {
+    if (!ReferenceLinker::LinkReference(callsite_, ref, context_, symbols_, package_decls_)) {
       error_ = true;
     }
   }
@@ -97,7 +95,7 @@ class ReferenceLinkerVisitor : public ValueVisitor {
       // Find the attribute in the symbol table and check if it is visible from
       // this callsite.
       const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveAttributeCheckVisibility(
-          transformed_reference, symbols_, callsite_, &err_str);
+          transformed_reference, callsite_, symbols_, &err_str);
       if (symbol) {
         // Assign our style key the correct ID.
         // The ID may not exist.
@@ -105,8 +103,7 @@ class ReferenceLinkerVisitor : public ValueVisitor {
 
         // Try to convert the value to a more specific, typed value based on the
         // attribute it is set to.
-        entry.value = ParseValueWithAttribute(std::move(entry.value),
-                                              symbol->attribute.get());
+        entry.value = ParseValueWithAttribute(std::move(entry.value), symbol->attribute.get());
 
         // Link/resolve the final value (mostly if it's a reference).
         entry.value->Accept(this);
@@ -131,8 +128,7 @@ class ReferenceLinkerVisitor : public ValueVisitor {
       } else {
         DiagMessage msg(entry.key.GetSource());
         msg << "style attribute '";
-        ReferenceLinker::WriteResourceName(&msg, entry.key,
-                                           transformed_reference);
+        ReferenceLinker::WriteResourceName(&msg, entry.key, transformed_reference);
         msg << "' " << err_str;
         context_->GetDiagnostics()->Error(msg);
         error_ = true;
@@ -158,28 +154,26 @@ class ReferenceLinkerVisitor : public ValueVisitor {
           ResourceUtils::TryParseItemForAttribute(*raw_string->value, attr);
 
       // If we could not parse as any specific type, try a basic STRING.
-      if (!transformed &&
-          (attr->type_mask & android::ResTable_map::TYPE_STRING)) {
+      if (!transformed && (attr->type_mask & android::ResTable_map::TYPE_STRING)) {
         util::StringBuilder string_builder;
         string_builder.Append(*raw_string->value);
         if (string_builder) {
-          transformed = util::make_unique<String>(
-              string_pool_->MakeRef(string_builder.ToString()));
+          transformed = util::make_unique<String>(string_pool_->MakeRef(string_builder.ToString()));
         }
       }
 
       if (transformed) {
         return transformed;
       }
-    };
+    }
     return value;
   }
 
+  const CallSite& callsite_;
   IAaptContext* context_;
   SymbolTable* symbols_;
   xml::IPackageDeclStack* package_decls_;
   StringPool* string_pool_;
-  CallSite* callsite_;
   bool error_ = false;
 };
 
@@ -234,8 +228,8 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbol(const Reference& refer
 }
 
 const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const Reference& reference,
+                                                                         const CallSite& callsite,
                                                                          SymbolTable* symbols,
-                                                                         CallSite* callsite,
                                                                          std::string* out_error) {
   const SymbolTable::Symbol* symbol = ResolveSymbol(reference, symbols);
   if (!symbol) {
@@ -243,7 +237,7 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const R
     return nullptr;
   }
 
-  if (!IsSymbolVisible(*symbol, reference, *callsite)) {
+  if (!IsSymbolVisible(*symbol, reference, callsite)) {
     if (out_error) *out_error = "is private";
     return nullptr;
   }
@@ -251,9 +245,10 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const R
 }
 
 const SymbolTable::Symbol* ReferenceLinker::ResolveAttributeCheckVisibility(
-    const Reference& reference, SymbolTable* symbols, CallSite* callsite, std::string* out_error) {
+    const Reference& reference, const CallSite& callsite, SymbolTable* symbols,
+    std::string* out_error) {
   const SymbolTable::Symbol* symbol =
-      ResolveSymbolCheckVisibility(reference, symbols, callsite, out_error);
+      ResolveSymbolCheckVisibility(reference, callsite, symbols, out_error);
   if (!symbol) {
     return nullptr;
   }
@@ -266,12 +261,12 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveAttributeCheckVisibility(
 }
 
 Maybe<xml::AaptAttribute> ReferenceLinker::CompileXmlAttribute(const Reference& reference,
+                                                               const CallSite& callsite,
                                                                SymbolTable* symbols,
-                                                               CallSite* callsite,
                                                                std::string* out_error) {
-  const SymbolTable::Symbol* symbol = ResolveSymbol(reference, symbols);
+  const SymbolTable::Symbol* symbol =
+      ResolveAttributeCheckVisibility(reference, callsite, symbols, out_error);
   if (!symbol) {
-    if (out_error) *out_error = "not found";
     return {};
   }
 
@@ -297,10 +292,9 @@ void ReferenceLinker::WriteResourceName(DiagMessage* out_msg,
   }
 }
 
-bool ReferenceLinker::LinkReference(Reference* reference, IAaptContext* context,
-                                    SymbolTable* symbols,
-                                    xml::IPackageDeclStack* decls,
-                                    CallSite* callsite) {
+bool ReferenceLinker::LinkReference(const CallSite& callsite, Reference* reference,
+                                    IAaptContext* context, SymbolTable* symbols,
+                                    xml::IPackageDeclStack* decls) {
   CHECK(reference != nullptr);
   CHECK(reference->name || reference->id);
 
@@ -309,7 +303,7 @@ bool ReferenceLinker::LinkReference(Reference* reference, IAaptContext* context,
 
   std::string err_str;
   const SymbolTable::Symbol* s =
-      ResolveSymbolCheckVisibility(transformed_reference, symbols, callsite, &err_str);
+      ResolveSymbolCheckVisibility(transformed_reference, callsite, symbols, &err_str);
   if (s) {
     // The ID may not exist. This is fine because of the possibility of building
     // against libraries without assigned IDs.
@@ -344,11 +338,9 @@ bool ReferenceLinker::Consume(IAaptContext* context, ResourceTable* table) {
           error = true;
         }
 
-        CallSite callsite = {
-            ResourceNameRef(package->name, type->type, entry->name)};
-        ReferenceLinkerVisitor visitor(context, context->GetExternalSymbols(),
-                                       &table->string_pool, &decl_stack,
-                                       &callsite);
+        CallSite callsite = {ResourceNameRef(package->name, type->type, entry->name)};
+        ReferenceLinkerVisitor visitor(callsite, context, context->GetExternalSymbols(),
+                                       &table->string_pool, &decl_stack);
 
         for (auto& config_value : entry->values) {
           config_value->value->Accept(&visitor);
diff --git a/tools/aapt2/link/ReferenceLinker.h b/tools/aapt2/link/ReferenceLinker.h
index 2d18c49d3687..b3d0196d9e7c 100644
--- a/tools/aapt2/link/ReferenceLinker.h
+++ b/tools/aapt2/link/ReferenceLinker.h
@@ -59,8 +59,8 @@ class ReferenceLinker : public IResourceTableConsumer {
    * returned. out_error holds the error message.
    */
   static const SymbolTable::Symbol* ResolveSymbolCheckVisibility(const Reference& reference,
+                                                                 const CallSite& callsite,
                                                                  SymbolTable* symbols,
-                                                                 CallSite* callsite,
                                                                  std::string* out_error);
 
   /**
@@ -70,8 +70,8 @@ class ReferenceLinker : public IResourceTableConsumer {
    * ISymbolTable::Symbol::attribute.
    */
   static const SymbolTable::Symbol* ResolveAttributeCheckVisibility(const Reference& reference,
+                                                                    const CallSite& callsite,
                                                                     SymbolTable* symbols,
-                                                                    CallSite* callsite,
                                                                     std::string* out_error);
 
   /**
@@ -80,7 +80,8 @@ class ReferenceLinker : public IResourceTableConsumer {
    * If resolution fails, outError holds the error message.
    */
   static Maybe<xml::AaptAttribute> CompileXmlAttribute(const Reference& reference,
-                                                       SymbolTable* symbols, CallSite* callsite,
+                                                       const CallSite& callsite,
+                                                       SymbolTable* symbols,
                                                        std::string* out_error);
 
   /**
@@ -99,9 +100,8 @@ class ReferenceLinker : public IResourceTableConsumer {
    * Returns false on failure, and an error message is logged to the
    * IDiagnostics in the context.
    */
-  static bool LinkReference(Reference* reference, IAaptContext* context,
-                            SymbolTable* symbols, xml::IPackageDeclStack* decls,
-                            CallSite* callsite);
+  static bool LinkReference(const CallSite& callsite, Reference* reference, IAaptContext* context,
+                            SymbolTable* symbols, xml::IPackageDeclStack* decls);
 
   /**
    * Links all references in the ResourceTable.
diff --git a/tools/aapt2/link/ReferenceLinker_test.cpp b/tools/aapt2/link/ReferenceLinker_test.cpp
index 4ca36a9e2b22..d8e33a42711a 100644
--- a/tools/aapt2/link/ReferenceLinker_test.cpp
+++ b/tools/aapt2/link/ReferenceLinker_test.cpp
@@ -53,21 +53,20 @@ TEST(ReferenceLinkerTest, LinkSimpleReferences) {
   ReferenceLinker linker;
   ASSERT_TRUE(linker.Consume(context.get(), table.get()));
 
-  Reference* ref =
-      test::GetValue<Reference>(table.get(), "com.app.test:string/foo");
-  ASSERT_NE(ref, nullptr);
+  Reference* ref = test::GetValue<Reference>(table.get(), "com.app.test:string/foo");
+  ASSERT_NE(nullptr, ref);
   AAPT_ASSERT_TRUE(ref->id);
-  EXPECT_EQ(ref->id.value(), ResourceId(0x7f020001));
+  EXPECT_EQ(ResourceId(0x7f020001), ref->id.value());
 
   ref = test::GetValue<Reference>(table.get(), "com.app.test:string/bar");
-  ASSERT_NE(ref, nullptr);
+  ASSERT_NE(nullptr, ref);
   AAPT_ASSERT_TRUE(ref->id);
-  EXPECT_EQ(ref->id.value(), ResourceId(0x7f020002));
+  EXPECT_EQ(ResourceId(0x7f020002), ref->id.value());
 
   ref = test::GetValue<Reference>(table.get(), "com.app.test:string/baz");
-  ASSERT_NE(ref, nullptr);
+  ASSERT_NE(nullptr, ref);
   AAPT_ASSERT_TRUE(ref->id);
-  EXPECT_EQ(ref->id.value(), ResourceId(0x01040034));
+  EXPECT_EQ(ResourceId(0x01040034), ref->id.value());
 }
 
 TEST(ReferenceLinkerTest, LinkStyleAttributes) {
@@ -87,9 +86,8 @@ TEST(ReferenceLinkerTest, LinkStyleAttributes) {
     // We need to fill in the value for the attribute android:attr/bar after we
     // build the
     // table, because we need access to the string pool.
-    Style* style =
-        test::GetValue<Style>(table.get(), "com.app.test:style/Theme");
-    ASSERT_NE(style, nullptr);
+    Style* style = test::GetValue<Style>(table.get(), "com.app.test:style/Theme");
+    ASSERT_NE(nullptr, style);
     style->entries.back().value =
         util::make_unique<RawString>(table->string_pool.MakeRef("one|two"));
   }
@@ -120,20 +118,20 @@ TEST(ReferenceLinkerTest, LinkStyleAttributes) {
   ASSERT_TRUE(linker.Consume(context.get(), table.get()));
 
   Style* style = test::GetValue<Style>(table.get(), "com.app.test:style/Theme");
-  ASSERT_NE(style, nullptr);
+  ASSERT_NE(nullptr, style);
   AAPT_ASSERT_TRUE(style->parent);
   AAPT_ASSERT_TRUE(style->parent.value().id);
-  EXPECT_EQ(style->parent.value().id.value(), ResourceId(0x01060000));
+  EXPECT_EQ(ResourceId(0x01060000), style->parent.value().id.value());
 
   ASSERT_EQ(2u, style->entries.size());
 
   AAPT_ASSERT_TRUE(style->entries[0].key.id);
-  EXPECT_EQ(style->entries[0].key.id.value(), ResourceId(0x01010001));
-  ASSERT_NE(ValueCast<BinaryPrimitive>(style->entries[0].value.get()), nullptr);
+  EXPECT_EQ(ResourceId(0x01010001), style->entries[0].key.id.value());
+  ASSERT_NE(nullptr, ValueCast<BinaryPrimitive>(style->entries[0].value.get()));
 
   AAPT_ASSERT_TRUE(style->entries[1].key.id);
-  EXPECT_EQ(style->entries[1].key.id.value(), ResourceId(0x01010002));
-  ASSERT_NE(ValueCast<BinaryPrimitive>(style->entries[1].value.get()), nullptr);
+  EXPECT_EQ(ResourceId(0x01010002), style->entries[1].key.id.value());
+  ASSERT_NE(nullptr, ValueCast<BinaryPrimitive>(style->entries[1].value.get()));
 }
 
 TEST(ReferenceLinkerTest, LinkMangledReferencesAndAttributes) {
@@ -167,10 +165,10 @@ TEST(ReferenceLinkerTest, LinkMangledReferencesAndAttributes) {
   ASSERT_TRUE(linker.Consume(context.get(), table.get()));
 
   Style* style = test::GetValue<Style>(table.get(), "com.app.test:style/Theme");
-  ASSERT_NE(style, nullptr);
+  ASSERT_NE(nullptr, style);
   ASSERT_EQ(1u, style->entries.size());
   AAPT_ASSERT_TRUE(style->entries.front().key.id);
-  EXPECT_EQ(style->entries.front().key.id.value(), ResourceId(0x7f010000));
+  EXPECT_EQ(ResourceId(0x7f010000), style->entries.front().key.id.value());
 }
 
 TEST(ReferenceLinkerTest, FailToLinkPrivateSymbols) {
@@ -257,4 +255,42 @@ TEST(ReferenceLinkerTest, FailToLinkPrivateStyleAttributes) {
   ASSERT_FALSE(linker.Consume(context.get(), table.get()));
 }
 
+TEST(ReferenceLinkerTest, AppsWithSamePackageButDifferentIdAreVisibleNonPublic) {
+  NameMangler mangler(NameManglerPolicy{"com.app.test"});
+  SymbolTable table(&mangler);
+  table.AppendSource(test::StaticSymbolSourceBuilder()
+                         .AddSymbol("com.app.test:string/foo", ResourceId(0x7f010000))
+                         .Build());
+
+  std::string error;
+  const CallSite call_site{ResourceNameRef("com.app.test", ResourceType::kString, "foo")};
+  const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveSymbolCheckVisibility(
+      *test::BuildReference("com.app.test:string/foo"), call_site, &table, &error);
+  ASSERT_NE(nullptr, symbol);
+  EXPECT_TRUE(error.empty());
+}
+
+TEST(ReferenceLinkerTest, AppsWithDifferentPackageCanNotUseEachOthersAttribute) {
+  NameMangler mangler(NameManglerPolicy{"com.app.ext"});
+  SymbolTable table(&mangler);
+  table.AppendSource(test::StaticSymbolSourceBuilder()
+                         .AddSymbol("com.app.test:attr/foo", ResourceId(0x7f010000),
+                                    test::AttributeBuilder().Build())
+                         .AddPublicSymbol("com.app.test:attr/public_foo", ResourceId(0x7f010001),
+                                          test::AttributeBuilder().Build())
+                         .Build());
+
+  std::string error;
+  const CallSite call_site{ResourceNameRef("com.app.ext", ResourceType::kLayout, "foo")};
+
+  AAPT_EXPECT_FALSE(ReferenceLinker::CompileXmlAttribute(
+      *test::BuildReference("com.app.test:attr/foo"), call_site, &table, &error));
+  EXPECT_FALSE(error.empty());
+
+  error = "";
+  AAPT_ASSERT_TRUE(ReferenceLinker::CompileXmlAttribute(
+      *test::BuildReference("com.app.test:attr/public_foo"), call_site, &table, &error));
+  EXPECT_TRUE(error.empty());
+}
+
 }  // namespace aapt
diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp
index b839862a0689..94bdccd5ab64 100644
--- a/tools/aapt2/link/XmlReferenceLinker.cpp
+++ b/tools/aapt2/link/XmlReferenceLinker.cpp
@@ -42,12 +42,12 @@ class ReferenceVisitor : public ValueVisitor {
  public:
   using ValueVisitor::Visit;
 
-  ReferenceVisitor(IAaptContext* context, SymbolTable* symbols, xml::IPackageDeclStack* decls,
-                   CallSite* callsite)
-      : context_(context), symbols_(symbols), decls_(decls), callsite_(callsite), error_(false) {}
+  ReferenceVisitor(const CallSite& callsite, IAaptContext* context, SymbolTable* symbols,
+                   xml::IPackageDeclStack* decls)
+      : callsite_(callsite), context_(context), symbols_(symbols), decls_(decls), error_(false) {}
 
   void Visit(Reference* ref) override {
-    if (!ReferenceLinker::LinkReference(ref, context_, symbols_, decls_, callsite_)) {
+    if (!ReferenceLinker::LinkReference(callsite_, ref, context_, symbols_, decls_)) {
       error_ = true;
     }
   }
@@ -57,10 +57,10 @@ class ReferenceVisitor : public ValueVisitor {
  private:
   DISALLOW_COPY_AND_ASSIGN(ReferenceVisitor);
 
+  const CallSite& callsite_;
   IAaptContext* context_;
   SymbolTable* symbols_;
   xml::IPackageDeclStack* decls_;
-  CallSite* callsite_;
   bool error_;
 };
 
@@ -71,14 +71,14 @@ class XmlVisitor : public xml::PackageAwareVisitor {
  public:
   using xml::PackageAwareVisitor::Visit;
 
-  XmlVisitor(IAaptContext* context, SymbolTable* symbols, const Source& source,
-             std::set<int>* sdk_levels_found, CallSite* callsite)
-      : context_(context),
+  XmlVisitor(const Source& source, const CallSite& callsite, IAaptContext* context,
+             SymbolTable* symbols, std::set<int>* sdk_levels_found)
+      : source_(source),
+        callsite_(callsite),
+        context_(context),
         symbols_(symbols),
-        source_(source),
         sdk_levels_found_(sdk_levels_found),
-        callsite_(callsite),
-        reference_visitor_(context, symbols, this, callsite) {}
+        reference_visitor_(callsite, context, symbols, this) {}
 
   void Visit(xml::Element* el) override {
     // The default Attribute allows everything except enums or flags.
@@ -108,7 +108,7 @@ class XmlVisitor : public xml::PackageAwareVisitor {
 
         std::string err_str;
         attr.compiled_attribute =
-            ReferenceLinker::CompileXmlAttribute(attr_ref, symbols_, callsite_, &err_str);
+            ReferenceLinker::CompileXmlAttribute(attr_ref, callsite_, symbols_, &err_str);
 
         if (!attr.compiled_attribute) {
           context_->GetDiagnostics()->Error(DiagMessage(source) << "attribute '"
@@ -159,11 +159,12 @@ class XmlVisitor : public xml::PackageAwareVisitor {
  private:
   DISALLOW_COPY_AND_ASSIGN(XmlVisitor);
 
+  Source source_;
+  const CallSite& callsite_;
   IAaptContext* context_;
   SymbolTable* symbols_;
-  Source source_;
+
   std::set<int>* sdk_levels_found_;
-  CallSite* callsite_;
   ReferenceVisitor reference_visitor_;
   bool error_ = false;
 };
@@ -172,9 +173,9 @@ class XmlVisitor : public xml::PackageAwareVisitor {
 
 bool XmlReferenceLinker::Consume(IAaptContext* context, xml::XmlResource* resource) {
   sdk_levels_found_.clear();
-  CallSite callsite = {resource->file.name};
-  XmlVisitor visitor(context, context->GetExternalSymbols(), resource->file.source,
-                     &sdk_levels_found_, &callsite);
+  const CallSite callsite = {resource->file.name};
+  XmlVisitor visitor(resource->file.source, callsite, context, context->GetExternalSymbols(),
+                     &sdk_levels_found_);
   if (resource->root) {
     resource->root->Accept(&visitor);
     return !visitor.HasError();
diff --git a/tools/aapt2/process/SymbolTable_test.cpp b/tools/aapt2/process/SymbolTable_test.cpp
index bba316f4b484..fd8a5080278b 100644
--- a/tools/aapt2/process/SymbolTable_test.cpp
+++ b/tools/aapt2/process/SymbolTable_test.cpp
@@ -30,10 +30,8 @@ TEST(ResourceTableSymbolSourceTest, FindSymbols) {
           .Build();
 
   ResourceTableSymbolSource symbol_source(table.get());
-  EXPECT_NE(nullptr,
-            symbol_source.FindByName(test::ParseNameOrDie("android:id/foo")));
-  EXPECT_NE(nullptr,
-            symbol_source.FindByName(test::ParseNameOrDie("android:id/bar")));
+  EXPECT_NE(nullptr, symbol_source.FindByName(test::ParseNameOrDie("android:id/foo")));
+  EXPECT_NE(nullptr, symbol_source.FindByName(test::ParseNameOrDie("android:id/bar")));
 
   std::unique_ptr<SymbolTable::Symbol> s =
       symbol_source.FindByName(test::ParseNameOrDie("android:attr/foo"));
diff --git a/tools/aapt2/readme.md b/tools/aapt2/readme.md
index fedd65c4e239..1c9a75d0547d 100644
--- a/tools/aapt2/readme.md
+++ b/tools/aapt2/readme.md
@@ -1,5 +1,13 @@
 # Android Asset Packaging Tool 2.0 (AAPT2) release notes
 
+## Version 2.10
+### `aapt2 link ...`
+- Add ability to specify package ID to compile with for regular apps (not shared or static libs).
+  This package ID is limited to the range 0x7f-0xff inclusive. Specified with the --package-id
+  flag.
+- Fixed issue with <plurals> resources being stripped for locales and other configuration.
+- Fixed issue with escaping strings in XML resources.
+
 ## Version 2.9
 ### `aapt2 link ...`
 - Added sparse resource type encoding, which encodes resource entries that are sparse with