AAPT2: Update versionCode in multi-APKs.

Set versionCode in the AndroidManifest.xml so each artifact has a unique
version code. This will allow the play store to serve up the correct
APKs.

The versionCode is determined by taking the versionCode from the input
APK and adding the configured version to that. If an artifact does not
have a version, the next version is used based on the order in the
configuration file. If the firs artifact does not have a version, it
starts at 1, so that all artifacts have a different version code to the
base APK.

Bug: 64972562

Test: unit tests
Test: manually optimized an APK into multiple APKs and confirmed versions:
  aapt dump badging ${apk} | grep versionCode

Change-Id: Ie12b3e42bbd8ce9a2819712c2c56b24c0a901e6b

6 files changed, 208 insertions, 64 deletions

diff --git a/tools/aapt2/configuration/ConfigurationParser.cpp b/tools/aapt2/configuration/ConfigurationParser.cpp
index d2b4d725c416..a79a577c663b 100644
--- a/tools/aapt2/configuration/ConfigurationParser.cpp
+++ b/tools/aapt2/configuration/ConfigurationParser.cpp
@@ -330,15 +330,32 @@ Maybe<PostProcessingConfiguration> ConfigurationParser::Parse() {
   // TODO: Validate all references in the configuration are valid. It should be safe to assume from
   // this point on that any references from one section to another will be present.
 
+  // TODO: Automatically arrange artifacts so that they match Play Store multi-APK requirements.
+  // see: https://developer.android.com/google/play/publishing/multiple-apks.html
+  //
+  // For now, make sure the version codes are unique.
+  std::vector<Artifact>& artifacts = config.artifacts;
+  std::sort(artifacts.begin(), artifacts.end());
+  if (std::adjacent_find(artifacts.begin(), artifacts.end()) != artifacts.end()) {
+    diag_->Error(DiagMessage() << "Configuration has duplicate versions");
+    return {};
+  }
+
   return {config};
 }
 
 ConfigurationParser::ActionHandler ConfigurationParser::artifact_handler_ =
     [](PostProcessingConfiguration* config, Element* root_element, IDiagnostics* diag) -> bool {
+  // This will be incremented later so the first version will always be different to the base APK.
+  int current_version = (config->artifacts.empty()) ? 0 : config->artifacts.back().version;
+
   Artifact artifact{};
+  Maybe<int> version;
   for (const auto& attr : root_element->attributes) {
     if (attr.name == "name") {
       artifact.name = attr.value;
+    } else if (attr.name == "version") {
+      version = std::stoi(attr.value);
     } else if (attr.name == "abi-group") {
       artifact.abi_group = {attr.value};
     } else if (attr.name == "screen-density-group") {
@@ -356,6 +373,9 @@ ConfigurationParser::ActionHandler ConfigurationParser::artifact_handler_ =
                                << attr.value);
     }
   }
+
+  artifact.version = (version) ? version.value() : current_version + 1;
+
   config->artifacts.push_back(artifact);
   return true;
 };
diff --git a/tools/aapt2/configuration/ConfigurationParser.h b/tools/aapt2/configuration/ConfigurationParser.h
index ebf1c986a980..c5d3284c33f4 100644
--- a/tools/aapt2/configuration/ConfigurationParser.h
+++ b/tools/aapt2/configuration/ConfigurationParser.h
@@ -17,6 +17,7 @@
 #ifndef AAPT2_CONFIGURATION_H
 #define AAPT2_CONFIGURATION_H
 
+#include <set>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -41,6 +42,12 @@ using Entry = std::unordered_map<std::string, T>;
 struct Artifact {
   /** Name to use for output of processing foo.apk -> foo.<name>.apk. */
   Maybe<std::string> name;
+  /**
+   * Value to add to the base Android manifest versionCode. If it is not present in the
+   * configuration file, it is set to the previous artifact + 1. If the first artifact does not have
+   * a value, artifacts are a 1 based index.
+   */
+  int version;
   /** If present, uses the ABI group with this name. */
   Maybe<std::string> abi_group;
   /** If present, uses the screen density group with this name. */
@@ -60,6 +67,15 @@ struct Artifact {
 
   /** Convert an artifact name template into a name string based on configuration contents. */
   Maybe<std::string> Name(const android::StringPiece& apk_name, IDiagnostics* diag) const;
+
+  bool operator<(const Artifact& rhs) const {
+    // TODO(safarmer): Order by play store multi-APK requirements.
+    return version < rhs.version;
+  }
+
+  bool operator==(const Artifact& rhs) const  {
+    return version == rhs.version;
+  }
 };
 
 /** Enumeration of currently supported ABIs. */
diff --git a/tools/aapt2/configuration/ConfigurationParser_test.cpp b/tools/aapt2/configuration/ConfigurationParser_test.cpp
index 6bb168f15b1e..3654901e3e02 100644
--- a/tools/aapt2/configuration/ConfigurationParser_test.cpp
+++ b/tools/aapt2/configuration/ConfigurationParser_test.cpp
@@ -183,55 +183,117 @@ TEST_F(ConfigurationParserTest, InvalidNamespace) {
 }
 
 TEST_F(ConfigurationParserTest, ArtifactAction) {
-  static constexpr const char* xml = R"xml(
+  PostProcessingConfiguration config;
+  {
+    const auto doc = test::BuildXmlDom(R"xml(
+      <artifact
+          abi-group="arm"
+          screen-density-group="large"
+          locale-group="europe"
+          android-sdk-group="v19"
+          gl-texture-group="dxt1"
+          device-feature-group="low-latency"/>)xml");
+
+    ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc->root.get()), &diag_));
+
+    EXPECT_EQ(1ul, config.artifacts.size());
+
+    auto& artifact = config.artifacts.back();
+    EXPECT_FALSE(artifact.name);  // TODO: make this fail.
+    EXPECT_EQ(1, artifact.version);
+    EXPECT_EQ("arm", artifact.abi_group.value());
+    EXPECT_EQ("large", artifact.screen_density_group.value());
+    EXPECT_EQ("europe", artifact.locale_group.value());
+    EXPECT_EQ("v19", artifact.android_sdk_group.value());
+    EXPECT_EQ("dxt1", artifact.gl_texture_group.value());
+    EXPECT_EQ("low-latency", artifact.device_feature_group.value());
+  }
+
+  {
+    // Perform a second action to ensure we get 2 artifacts.
+    const auto doc = test::BuildXmlDom(R"xml(
+      <artifact
+          abi-group="other"
+          screen-density-group="large"
+          locale-group="europe"
+          android-sdk-group="v19"
+          gl-texture-group="dxt1"
+          device-feature-group="low-latency"/>)xml");
+
+    ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_));
+    EXPECT_EQ(2ul, config.artifacts.size());
+    EXPECT_EQ(2, config.artifacts.back().version);
+  }
+
+  {
+    // Perform a third action with a set version code.
+    const auto doc = test::BuildXmlDom(R"xml(
     <artifact
-        abi-group="arm"
+        version="5"
+        abi-group="other"
         screen-density-group="large"
         locale-group="europe"
         android-sdk-group="v19"
         gl-texture-group="dxt1"
-        device-feature-group="low-latency"/>)xml";
+        device-feature-group="low-latency"/>)xml");
 
-  auto doc = test::BuildXmlDom(xml);
-
-  PostProcessingConfiguration config;
-  bool ok = artifact_handler_(&config, NodeCast<Element>(doc->root.get()), &diag_);
-  ASSERT_TRUE(ok);
-
-  EXPECT_EQ(1ul, config.artifacts.size());
-
-  auto& artifact = config.artifacts.front();
-  EXPECT_FALSE(artifact.name);  // TODO: make this fail.
-  EXPECT_EQ("arm", artifact.abi_group.value());
-  EXPECT_EQ("large", artifact.screen_density_group.value());
-  EXPECT_EQ("europe", artifact.locale_group.value());
-  EXPECT_EQ("v19", artifact.android_sdk_group.value());
-  EXPECT_EQ("dxt1", artifact.gl_texture_group.value());
-  EXPECT_EQ("low-latency", artifact.device_feature_group.value());
+    ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_));
+    EXPECT_EQ(3ul, config.artifacts.size());
+    EXPECT_EQ(5, config.artifacts.back().version);
+  }
 
-  // Perform a second action to ensure we get 2 artifacts.
-  static constexpr const char* second = R"xml(
+  {
+    // Perform a fourth action to ensure the version code still increments.
+    const auto doc = test::BuildXmlDom(R"xml(
     <artifact
         abi-group="other"
         screen-density-group="large"
         locale-group="europe"
         android-sdk-group="v19"
         gl-texture-group="dxt1"
-        device-feature-group="low-latency"/>)xml";
-  doc = test::BuildXmlDom(second);
+        device-feature-group="low-latency"/>)xml");
 
-  ok = artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_);
-  ASSERT_TRUE(ok);
-  EXPECT_EQ(2ul, config.artifacts.size());
+    ASSERT_TRUE(artifact_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_));
+    EXPECT_EQ(4ul, config.artifacts.size());
+    EXPECT_EQ(6, config.artifacts.back().version);
+  }
+}
+
+TEST_F(ConfigurationParserTest, DuplicateArtifactVersion) {
+  static constexpr const char* configuration = R"xml(<?xml version="1.0" encoding="utf-8" ?>
+      <pst-process xmlns="http://schemas.android.com/tools/aapt">>
+        <artifacts>
+          <artifact-format>
+            ${base}.${abi}.${screen-density}.${locale}.${sdk}.${gl}.${feature}.release
+          </artifact-format>
+          <artifact
+              name="art1"
+              abi-group="arm"
+              screen-density-group="large"
+              locale-group="europe"
+              android-sdk-group="v19"
+              gl-texture-group="dxt1"
+              device-feature-group="low-latency"/>
+          <artifact
+              name="art2"
+              version = "1"
+              abi-group="other"
+              screen-density-group="alldpi"
+              locale-group="north-america"
+              android-sdk-group="v19"
+              gl-texture-group="dxt1"
+              device-feature-group="low-latency"/>
+        </artifacts>
+      </post-process>)xml";
+  auto result = ConfigurationParser::ForContents(configuration).Parse();
+  ASSERT_FALSE(result);
 }
 
 TEST_F(ConfigurationParserTest, ArtifactFormatAction) {
-  static constexpr const char* xml = R"xml(
+  const auto doc = test::BuildXmlDom(R"xml(
     <artifact-format>
       ${base}.${abi}.${screen-density}.${locale}.${sdk}.${gl}.${feature}.release
-    </artifact-format>)xml";
-
-  auto doc = test::BuildXmlDom(xml);
+    </artifact-format>)xml");
 
   PostProcessingConfiguration config;
   bool ok = artifact_format_handler_(&config, NodeCast<Element>(doc.get()->root.get()), &diag_);
diff --git a/tools/aapt2/configuration/aapt2.xsd b/tools/aapt2/configuration/aapt2.xsd
index 47bf99e28089..134153a017f8 100644
--- a/tools/aapt2/configuration/aapt2.xsd
+++ b/tools/aapt2/configuration/aapt2.xsd
@@ -39,6 +39,7 @@
   <!-- Groups output artifacts together by dimension labels. -->
   <xsd:complexType name="artifact">
     <xsd:attribute name="name" type="xsd:string"/>
+    <xsd:attribute name="version" type="xsd:integer"/>
     <xsd:attribute name="abi-group" type="xsd:string"/>
     <xsd:attribute name="android-sdk-group" type="xsd:string"/>
     <xsd:attribute name="device-feature-group" type="xsd:string"/>
diff --git a/tools/aapt2/optimize/MultiApkGenerator.cpp b/tools/aapt2/optimize/MultiApkGenerator.cpp
index 563c46df26d1..fb150e204167 100644
--- a/tools/aapt2/optimize/MultiApkGenerator.cpp
+++ b/tools/aapt2/optimize/MultiApkGenerator.cpp
@@ -23,6 +23,7 @@
 
 #include "LoadedApk.h"
 #include "ResourceUtils.h"
+#include "ValueVisitor.h"
 #include "configuration/ConfigurationParser.h"
 #include "filter/AbiFilter.h"
 #include "filter/Filter.h"
@@ -33,12 +34,14 @@
 #include "split/TableSplitter.h"
 #include "util/Files.h"
 #include "xml/XmlDom.h"
+#include "xml/XmlUtil.h"
 
 namespace aapt {
 
 using ::aapt::configuration::AndroidSdk;
 using ::aapt::configuration::Artifact;
 using ::aapt::configuration::PostProcessingConfiguration;
+using ::aapt::xml::kSchemaAndroid;
 using ::aapt::xml::XmlResource;
 using ::android::StringPiece;
 
@@ -153,40 +156,10 @@ bool MultiApkGenerator::FromBaseApk(const MultiApkGeneratorOptions& options) {
     }
 
     std::unique_ptr<XmlResource> manifest;
-
-    Maybe<AndroidSdk> maybe_sdk = GetAndroidSdk(artifact, config, diag);
-    if (maybe_sdk) {
-      // TODO(safarmer): Handle the rest of the Android SDK.
-      const AndroidSdk& android_sdk = maybe_sdk.value();
-
-      manifest = apk_->InflateManifest(context_);
-      if (!manifest) {
-        return false;
-      }
-
-      // Make sure the first element is <manifest> with package attribute.
-      xml::Element* manifest_el = manifest->root.get();
-      if (manifest_el == nullptr) {
-        return {};
-      }
-
-      if (!manifest_el->namespace_uri.empty() || manifest_el->name != "manifest") {
-        diag->Error(DiagMessage(manifest->file.source) << "root tag must be <manifest>");
-        return {};
-      }
-
-      if (xml::Element* uses_sdk_el = manifest_el->FindChild({}, "uses-sdk")) {
-        if (xml::Attribute* min_sdk_attr =
-                uses_sdk_el->FindAttribute(xml::kSchemaAndroid, "minSdkVersion")) {
-          if (min_sdk_attr == nullptr) {
-            diag->Error(DiagMessage(manifest->file.source.WithLine(uses_sdk_el->line_number))
-                        << "missing android:minSdkVersion");
-            return {};
-          }
-          const std::string& min_sdk_str = std::to_string(android_sdk.min_sdk_version.value());
-          min_sdk_attr->compiled_value = ResourceUtils::TryParseInt(min_sdk_str);
-        }
-      }
+    if (!UpdateManifest(artifact, config, &manifest, diag)) {
+      diag->Error(DiagMessage() << "could not update AndroidManifest.xml for "
+                                << artifact_name.value());
+      return false;
     }
 
     std::string out = options.out_dir;
@@ -288,4 +261,72 @@ std::unique_ptr<ResourceTable> MultiApkGenerator::FilterTable(
   return table;
 }
 
+bool MultiApkGenerator::UpdateManifest(const Artifact& artifact,
+                                       const PostProcessingConfiguration& config,
+                                       std::unique_ptr<XmlResource>* updated_manifest,
+                                       IDiagnostics* diag) {
+  *updated_manifest = apk_->InflateManifest(context_);
+  XmlResource* manifest = updated_manifest->get();
+  if (manifest == nullptr) {
+    return false;
+  }
+
+  // Make sure the first element is <manifest> with package attribute.
+  xml::Element* manifest_el = manifest->root.get();
+  if (manifest_el == nullptr) {
+    return false;
+  }
+
+  if (!manifest_el->namespace_uri.empty() || manifest_el->name != "manifest") {
+    diag->Error(DiagMessage(manifest->file.source) << "root tag must be <manifest>");
+    return false;
+  }
+
+  // Update the versionCode attribute.
+  xml::Attribute* versionCode = manifest_el->FindAttribute(kSchemaAndroid, "versionCode");
+  if (versionCode == nullptr) {
+    diag->Error(DiagMessage(manifest->file.source) << "manifest must have a versionCode attribute");
+    return false;
+  }
+
+  auto* compiled_version = ValueCast<BinaryPrimitive>(versionCode->compiled_value.get());
+  if (compiled_version == nullptr) {
+    diag->Error(DiagMessage(manifest->file.source) << "versionCode is invalid");
+    return false;
+  }
+
+  int new_version = compiled_version->value.data + artifact.version;
+  versionCode->compiled_value = ResourceUtils::TryParseInt(std::to_string(new_version));
+
+  // Check to see if the minSdkVersion needs to be updated.
+  Maybe<AndroidSdk> maybe_sdk = GetAndroidSdk(artifact, config, diag);
+  if (maybe_sdk) {
+    // TODO(safarmer): Handle the rest of the Android SDK.
+    const AndroidSdk& android_sdk = maybe_sdk.value();
+
+    if (xml::Element* uses_sdk_el = manifest_el->FindChild({}, "uses-sdk")) {
+      if (xml::Attribute* min_sdk_attr =
+              uses_sdk_el->FindAttribute(xml::kSchemaAndroid, "minSdkVersion")) {
+        // Populate with a pre-compiles attribute to we don't need to relink etc.
+        const std::string& min_sdk_str = std::to_string(android_sdk.min_sdk_version.value());
+        min_sdk_attr->compiled_value = ResourceUtils::TryParseInt(min_sdk_str);
+      } else {
+        // There was no minSdkVersion. This is strange since at this point we should have been
+        // through the manifest fixer which sets the default minSdkVersion.
+        diag->Error(DiagMessage(manifest->file.source) << "missing minSdkVersion from <uses-sdk>");
+        return false;
+      }
+    } else {
+      // No uses-sdk present. This is strange since at this point we should have been
+      // through the manifest fixer which should have added it.
+      diag->Error(DiagMessage(manifest->file.source) << "missing <uses-sdk> from <manifest>");
+      return false;
+    }
+  }
+
+  // TODO(safarmer): Check if we changed the supported screens and update the manifest.
+
+  return true;
+}
+
 }  // namespace aapt
diff --git a/tools/aapt2/optimize/MultiApkGenerator.h b/tools/aapt2/optimize/MultiApkGenerator.h
index b06440014be6..e6546eec584e 100644
--- a/tools/aapt2/optimize/MultiApkGenerator.h
+++ b/tools/aapt2/optimize/MultiApkGenerator.h
@@ -54,6 +54,10 @@ class MultiApkGenerator {
     return context_->GetDiagnostics();
   }
 
+  bool UpdateManifest(const configuration::Artifact& artifact,
+                      const configuration::PostProcessingConfiguration& config,
+                      std::unique_ptr<xml::XmlResource>* updated_manifest, IDiagnostics* diag);
+
   LoadedApk* apk_;
   IAaptContext* context_;
 };