Correctly insert platformBuildVersionCode/Name

AAPT(1) encodes platformBuildVersionCode and
platformBuildVersionName into the manifest along with
compileSdkVersion and compileSdkVersionCodename. AAPT2 was extracting
the platform attributes from the app being created rather than the
framework.

This change inserts the correct platform attributes.

Bug: 124227171
Test: aapt2_tests
Change-Id: I4bc7986d25f065cecf01b9e988de4d37283ef7de

2 files changed, 48 insertions, 69 deletions

diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index 85bf6f218ba0..582a5b869c65 100644
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -294,22 +294,6 @@ bool ManifestFixer::BuildRules(xml::XmlActionExecutor* executor,
       }
     }
 
-    if (el->FindAttribute("", "platformBuildVersionCode") == nullptr) {
-      auto versionCode = el->FindAttribute(xml::kSchemaAndroid, "versionCode");
-      if (versionCode != nullptr) {
-        el->attributes.push_back(xml::Attribute{"", "platformBuildVersionCode",
-                                                versionCode->value});
-      }
-    }
-
-    if (el->FindAttribute("", "platformBuildVersionName") == nullptr) {
-      auto versionName = el->FindAttribute(xml::kSchemaAndroid, "versionName");
-      if (versionName != nullptr) {
-        el->attributes.push_back(xml::Attribute{"", "platformBuildVersionName",
-                                                versionName->value});
-      }
-    }
-
     return true;
   });
 
@@ -489,8 +473,14 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) {
 
     // Make sure we un-compile the value if it was set to something else.
     attr->compiled_value = {};
+    attr->value = options_.compile_sdk_version.value();
+
+    attr = root->FindOrCreateAttribute("", "platformBuildVersionCode");
 
+    // Make sure we un-compile the value if it was set to something else.
+    attr->compiled_value = {};
     attr->value = options_.compile_sdk_version.value();
+
   }
 
   if (options_.compile_sdk_version_codename) {
@@ -499,7 +489,12 @@ bool ManifestFixer::Consume(IAaptContext* context, xml::XmlResource* doc) {
 
     // Make sure we un-compile the value if it was set to something else.
     attr->compiled_value = {};
+    attr->value = options_.compile_sdk_version_codename.value();
 
+    attr = root->FindOrCreateAttribute("", "platformBuildVersionName");
+
+    // Make sure we un-compile the value if it was set to something else.
+    attr->compiled_value = {};
     attr->value = options_.compile_sdk_version_codename.value();
   }
 
diff --git a/tools/aapt2/link/ManifestFixer_test.cpp b/tools/aapt2/link/ManifestFixer_test.cpp
index adea6273bc8b..4842f62e53b5 100644
--- a/tools/aapt2/link/ManifestFixer_test.cpp
+++ b/tools/aapt2/link/ManifestFixer_test.cpp
@@ -725,6 +725,43 @@ TEST_F(ManifestFixerTest, InsertCompileSdkVersions) {
   attr = manifest->root->FindAttribute(xml::kSchemaAndroid, "compileSdkVersionCodename");
   ASSERT_THAT(attr, NotNull());
   EXPECT_THAT(attr->value, StrEq("P"));
+
+  attr = manifest->root->FindAttribute("", "platformBuildVersionCode");
+  ASSERT_THAT(attr, NotNull());
+  EXPECT_THAT(attr->value, StrEq("28"));
+
+  attr = manifest->root->FindAttribute("", "platformBuildVersionName");
+  ASSERT_THAT(attr, NotNull());
+  EXPECT_THAT(attr->value, StrEq("P"));
+}
+
+TEST_F(ManifestFixerTest, OverrideCompileSdkVersions) {
+  std::string input = R"(
+      <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android"
+          compileSdkVersion="27" compileSdkVersionCodename="O"
+          platformBuildVersionCode="27" platformBuildVersionName="O"/>)";
+  ManifestFixerOptions options;
+  options.compile_sdk_version = {"28"};
+  options.compile_sdk_version_codename = {"P"};
+
+  std::unique_ptr<xml::XmlResource> manifest = VerifyWithOptions(input, options);
+  ASSERT_THAT(manifest, NotNull());
+
+  xml::Attribute* attr = manifest->root->FindAttribute(xml::kSchemaAndroid, "compileSdkVersion");
+  ASSERT_THAT(attr, NotNull());
+  EXPECT_THAT(attr->value, StrEq("28"));
+
+  attr = manifest->root->FindAttribute(xml::kSchemaAndroid, "compileSdkVersionCodename");
+  ASSERT_THAT(attr, NotNull());
+  EXPECT_THAT(attr->value, StrEq("P"));
+
+  attr = manifest->root->FindAttribute("", "platformBuildVersionCode");
+  ASSERT_THAT(attr, NotNull());
+  EXPECT_THAT(attr->value, StrEq("28"));
+
+  attr = manifest->root->FindAttribute("", "platformBuildVersionName");
+  ASSERT_THAT(attr, NotNull());
+  EXPECT_THAT(attr->value, StrEq("P"));
 }
 
 TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) {
@@ -750,59 +787,6 @@ TEST_F(ManifestFixerTest, UnexpectedElementsInManifest) {
   ASSERT_THAT(manifest, IsNull());
 }
 
-TEST_F(ManifestFixerTest, InsertPlatformBuildVersions) {
-  // Test for insertion when versionCode and versionName are included in the manifest
-  {
-    std::string input = R"(
-        <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android"
-          android:versionCode="27" android:versionName="O"/>)";
-    std::unique_ptr<xml::XmlResource> manifest = Verify(input);
-    ASSERT_THAT(manifest, NotNull());
-
-    xml::Attribute* attr = manifest->root->FindAttribute("", "platformBuildVersionCode");
-    ASSERT_THAT(attr, NotNull());
-    EXPECT_THAT(attr->value, StrEq("27"));
-    attr = manifest->root->FindAttribute("", "platformBuildVersionName");
-    ASSERT_THAT(attr, NotNull());
-    EXPECT_THAT(attr->value, StrEq("O"));
-  }
-
-  // Test for insertion when versionCode and versionName defaults are specified
-  {
-    std::string input = R"(
-      <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android"/>)";
-    ManifestFixerOptions options;
-    options.version_code_default = {"27"};
-    options.version_name_default = {"O"};
-    std::unique_ptr<xml::XmlResource> manifest = VerifyWithOptions(input, options);
-    ASSERT_THAT(manifest, NotNull());
-
-    xml::Attribute* attr = manifest->root->FindAttribute("", "platformBuildVersionCode");
-    ASSERT_THAT(attr, NotNull());
-    EXPECT_THAT(attr->value, StrEq("27"));
-    attr = manifest->root->FindAttribute("", "platformBuildVersionName");
-    ASSERT_THAT(attr, NotNull());
-    EXPECT_THAT(attr->value, StrEq("O"));
-  }
-
-  // Test that the platform build version attributes are not changed if they are currently present
-  {
-    std::string input = R"(
-        <manifest xmlns:android="http://schemas.android.com/apk/res/android" package="android"
-          android:versionCode="28" android:versionName="P"
-          platformBuildVersionCode="27" platformBuildVersionName="O"/>)";
-    std::unique_ptr<xml::XmlResource> manifest = Verify(input);
-    ASSERT_THAT(manifest, NotNull());
-
-    xml::Attribute* attr = manifest->root->FindAttribute("", "platformBuildVersionCode");
-    ASSERT_THAT(attr, NotNull());
-    EXPECT_THAT(attr->value, StrEq("27"));
-    attr = manifest->root->FindAttribute("", "platformBuildVersionName");
-    ASSERT_THAT(attr, NotNull());
-    EXPECT_THAT(attr->value, StrEq("O"));
-  }
-}
-
 TEST_F(ManifestFixerTest, UsesLibraryMustHaveNonEmptyName) {
   std::string input = R"(
       <manifest xmlns:android="http://schemas.android.com/apk/res/android"