arc: Fix P builds

crrev.com/796583 broke post-submit ARC tests on P boards.

BUG=1114931
TEST=tast run DUT_EVE arc.Boot

Change-Id: Id2bed1c14707763023ee145f6bc8931ecf5b847b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2348197
Reviewed-by: Yury Khmel <khmel@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796729}

Author: yusukes

components/arc/session/arc_property_util.cc

@@ -164,6 +164,7 @@ bool IsComment(const std::string& line) {
 bool ExpandPropertyContents(const std::string& content,
                             brillo::CrosConfigInterface* config,
                             std::string* expanded_content,
+                            bool filter_non_ro_props,
                             bool add_native_bridge_64bit_support,
                             bool append_dalvik_isa,
                             const std::string& partition_name) {
@@ -173,8 +174,12 @@ bool ExpandPropertyContents(const std::string& content,
 
   std::string new_properties;
   for (std::string line : lines) {
-    // Chrome only expands ro. properties at runtime.
-    if (!base::StartsWith(line, "ro.", base::CompareCase::SENSITIVE)) {
+    // Since Chrome only expands ro. properties at runtime, skip processing
+    // non-ro lines here for R+. For P, we cannot do that because the
+    // expanded property files will directly replace the original ones via
+    // bind mounts.
+    if (filter_non_ro_props &&
+        !base::StartsWith(line, "ro.", base::CompareCase::SENSITIVE)) {
       if (!IsComment(line) && line.find('{') != std::string::npos) {
         // The non-ro property has substitution(s).
         LOG(ERROR) << "Found substitution(s) in a non-ro property: " << line;
@@ -294,6 +299,7 @@ bool ExpandPropertyFile(const base::FilePath& input,
     return false;
   }
   if (!ExpandPropertyContents(content, config, &expanded,
+                              /*filter_non_ro_props=*/append,
                               add_native_bridge_64bit_support,
                               append_dalvik_isa, partition_name))
     return false;
@@ -356,6 +362,7 @@ bool ExpandPropertyContentsForTesting(const std::string& content,
                                       brillo::CrosConfigInterface* config,
                                       std::string* expanded_content) {
   return ExpandPropertyContents(content, config, expanded_content,
+                                /*filter_non_ro_props=*/true,
                                 /*add_native_bridge_64bit_support=*/false,
                                 false, std::string());
 }

components/arc/session/arc_property_util_unittest.cc

@@ -330,18 +330,18 @@ TEST_F(ArcPropertyUtilTest, ExpandPropertyFiles) {
   std::string content;
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("default.prop"), &content));
-  EXPECT_EQ(std::string(kDefaultProp), content);
+  EXPECT_EQ(std::string(kDefaultProp) + "\n", content);
   EXPECT_TRUE(base::ReadFileToString(dest_dir.Append("build.prop"), &content));
-  EXPECT_EQ(std::string(kBuildProp), content);
+  EXPECT_EQ(std::string(kBuildProp) + "\n", content);
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("vendor_build.prop"), &content));
-  EXPECT_EQ(std::string(kVendorBuildProp), content);
+  EXPECT_EQ(std::string(kVendorBuildProp) + "\n", content);
 
   // Expand it again, verify the previous result is cleared.
   EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_dir, false, false));
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("default.prop"), &content));
-  EXPECT_EQ(std::string(kDefaultProp), content);
+  EXPECT_EQ(std::string(kDefaultProp) + "\n", content);
 
   // If default.prop does not exist in the source path, it should still process
   // the other files, while also ensuring that default.prop is removed from the
@@ -351,10 +351,10 @@ TEST_F(ArcPropertyUtilTest, ExpandPropertyFiles) {
   EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_dir, false, false));
 
   EXPECT_TRUE(base::ReadFileToString(dest_dir.Append("build.prop"), &content));
-  EXPECT_EQ(std::string(kBuildProp), content);
+  EXPECT_EQ(std::string(kBuildProp) + "\n", content);
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("vendor_build.prop"), &content));
-  EXPECT_EQ(std::string(kVendorBuildProp), content);
+  EXPECT_EQ(std::string(kVendorBuildProp) + "\n", content);
 
   // Finally, test the case where source is valid but the dest is not.
   EXPECT_FALSE(ExpandPropertyFiles(source_dir, base::FilePath("/nonexistent"),
@@ -496,33 +496,36 @@ TEST_F(ArcPropertyUtilTest, TestNativeBridge64Support) {
   EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_dir, false, false));
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("default.prop"), &content));
-  EXPECT_EQ(std::string(kDefaultProp), content);
+  EXPECT_EQ(std::string(kDefaultProp) + "\n", content);
   EXPECT_TRUE(base::ReadFileToString(dest_dir.Append("build.prop"), &content));
-  EXPECT_EQ(std::string(kBuildProp), content);
+  EXPECT_EQ(std::string(kBuildProp) + "\n", content);
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("vendor_build.prop"), &content));
-  EXPECT_EQ(std::string(kVendorBuildProp), content);
+  EXPECT_EQ(std::string(kVendorBuildProp) + "\n", content);
 
   // Expand with experiment on, verify properties are added / modified in
   // build.prop but not other files.
   EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_dir, false, true));
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("default.prop"), &content));
-  EXPECT_EQ(std::string(kDefaultProp), content);
+  EXPECT_EQ(std::string(kDefaultProp) + "\n", content);
   EXPECT_TRUE(base::ReadFileToString(dest_dir.Append("build.prop"), &content));
-  constexpr const char kBuildPropModified[] =
+  constexpr const char kBuildPropModifiedFirst[] =
       "ro.baz=boo\n"
       "ro.product.cpu.abilist=x86_64,x86,arm64-v8a,armeabi-v7a,armeabi\n"
-      "ro.product.cpu.abilist64=x86_64,arm64-v8a\n"
+      "ro.product.cpu.abilist64=x86_64,arm64-v8a\n";
+  constexpr const char kBuildPropModifiedSecond[] =
       "ro.dalvik.vm.isa.arm64=x86_64\n";
-  EXPECT_EQ(std::string(kBuildPropModified), content);
+  EXPECT_EQ(base::StringPrintf("%s\n%s", kBuildPropModifiedFirst,
+                               kBuildPropModifiedSecond),
+            content);
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("vendor_build.prop"), &content));
   constexpr const char kVendorBuildPropModified[] =
       "ro.a=b\n"
       "ro.vendor.product.cpu.abilist=x86_64,x86,arm64-v8a,armeabi-v7a,armeabi\n"
       "ro.vendor.product.cpu.abilist64=x86_64,arm64-v8a\n";
-  EXPECT_EQ(std::string(kVendorBuildPropModified), content);
+  EXPECT_EQ(std::string(kVendorBuildPropModified) + "\n", content);
 
   // Expand to a single file with experiment on, verify properties are added /
   // modified as expected.
@@ -534,9 +537,10 @@ TEST_F(ArcPropertyUtilTest, TestNativeBridge64Support) {
 
   // Verify the contents.
   EXPECT_TRUE(base::ReadFileToString(dest_prop_file, &content));
-  EXPECT_EQ(base::StringPrintf("%s%s%s", kDefaultProp, kBuildPropModified,
-                               kVendorBuildPropModified),
-            content);
+  EXPECT_EQ(
+      base::StringPrintf("%s%s%s%s", kDefaultProp, kBuildPropModifiedFirst,
+                         kBuildPropModifiedSecond, kVendorBuildPropModified),
+      content);
 
   // Verify that unexpected property values generate an error.
   constexpr const char kBuildPropUnexpected[] =