Merge "Correctly insert platformBuildVersionCode/Name"
diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index 85bf6f2..582a5b8 100644
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -294,22 +294,6 @@
       }
     }
 
-    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 @@
 
     // 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 @@
 
     // 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 adea627..4842f62 100644
--- a/tools/aapt2/link/ManifestFixer_test.cpp
+++ b/tools/aapt2/link/ManifestFixer_test.cpp
@@ -725,6 +725,43 @@
   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 @@
   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"