Reland: arc: Expand 3 more .prop files at runtime

ARCVM boards will have {system_ext,odm,product}_build.prop files in
/usr/share/arcvm/properties. This CL let Chrome process these files
in addition to the existing ones (default.prop, build.prop, and
vendor_build.prop).

When processing the files, this files also let Chrome replace
ro.<partition>.product.cpu.abilist* so that all abilist properties
have a single consistent value regardless of the <partition>.

We can reland this because chromium:2343505 has fixed the
infinite import issue.

BUG=b:162555733,b:162024174
TEST=components_unittests

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

chrome/browser/chromeos/arc/test/test_arc_session_manager.cc

@@ -22,7 +22,8 @@ bool CreateFilesAndDirectories(const base::FilePath& temp_dir,
   // Create empty prop files so ArcSessionManager's property expansion code
   // works like production.
   for (const char* filename :
-       {"default.prop", "build.prop", "vendor_build.prop"}) {
+       {"default.prop", "build.prop", "vendor_build.prop",
+        "system_ext_build.prop", "product_build.prop", "odm_build.prop"}) {
     if (base::WriteFile(source_dir->Append(filename), "", 1) != 1)
       return false;
   }

components/arc/session/arc_property_util.cc

@@ -15,6 +15,7 @@
 #include "base/process/launch.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "chromeos/constants/chromeos_switches.h"
 
 namespace brillo {
@@ -40,11 +41,12 @@ constexpr char kOEMKey1PropertyPrefix[] = "ro.oem.key1=";
 constexpr char kPAIRegionsPropertyName[] = "pai-regions";
 
 // Properties related to dynamically adding native bridge 64 bit support.
-constexpr char kAbilistPropertyPrefix[] = "ro.product.cpu.abilist=";
+constexpr char kAbilistPropertyPrefixTemplate[] = "ro.%sproduct.cpu.abilist=";
 constexpr char kAbilistPropertyExpected[] = "x86_64,x86,armeabi-v7a,armeabi";
 constexpr char kAbilistPropertyReplacement[] =
     "x86_64,x86,arm64-v8a,armeabi-v7a,armeabi";
-constexpr char kAbilist64PropertyPrefix[] = "ro.product.cpu.abilist64=";
+constexpr char kAbilist64PropertyPrefixTemplate[] =
+    "ro.%sproduct.cpu.abilist64=";
 constexpr char kAbilist64PropertyExpected[] = "x86_64";
 constexpr char kAbilist64PropertyReplacement[] = "x86_64,arm64-v8a";
 constexpr char kDalvikVmIsaArm64[] = "ro.dalvik.vm.isa.arm64=x86_64";
@@ -160,7 +162,9 @@ bool IsComment(const std::string& line) {
 bool ExpandPropertyContents(const std::string& content,
                             brillo::CrosConfigInterface* config,
                             std::string* expanded_content,
-                            bool add_native_bridge_64bit_support) {
+                            bool add_native_bridge_64bit_support,
+                            bool append_dalvik_isa,
+                            const std::string& partition_name) {
   const std::vector<std::string> lines = base::SplitString(
       content, "\n", base::WhitespaceHandling::KEEP_WHITESPACE,
       base::SplitResult::SPLIT_WANT_ALL);
@@ -216,27 +220,30 @@ bool ExpandPropertyContents(const std::string& content,
         expanded += line.substr(prev_match);
       line = expanded;
     } while (inserted);
+
     if (add_native_bridge_64bit_support) {
-      // Special-case ro.product.cpu.abilist / ro.product.cpu.abilist64 to add
-      // ARM64.
+      // Special-case ro.<partition>.product.cpu.abilist and
+      // ro.<partition>.product.cpu.abilist64 to add ARM64.
+      std::string prefix = base::StringPrintf(kAbilistPropertyPrefixTemplate,
+                                              partition_name.c_str());
       std::string value;
-      if (FindProperty(kAbilistPropertyPrefix, &value, line)) {
+      if (FindProperty(prefix, &value, line)) {
         if (value == kAbilistPropertyExpected) {
-          line = std::string(kAbilistPropertyPrefix) +
-                 std::string(kAbilistPropertyReplacement);
+          line = prefix + std::string(kAbilistPropertyReplacement);
         } else {
-          LOG(ERROR) << "Found unexpected value for " << kAbilistPropertyPrefix
-                     << ", value " << value;
+          LOG(ERROR) << "Found unexpected value for " << prefix << ", value "
+                     << value;
           return false;
         }
       }
-      if (FindProperty(kAbilist64PropertyPrefix, &value, line)) {
+      prefix = base::StringPrintf(kAbilist64PropertyPrefixTemplate,
+                                  partition_name.c_str());
+      if (FindProperty(prefix, &value, line)) {
         if (value == kAbilist64PropertyExpected) {
-          line = std::string(kAbilist64PropertyPrefix) +
-                 std::string(kAbilist64PropertyReplacement);
+          line = prefix + std::string(kAbilist64PropertyReplacement);
         } else {
-          LOG(ERROR) << "Found unexpected value for "
-                     << kAbilist64PropertyPrefix << ", value " << value;
+          LOG(ERROR) << "Found unexpected value for " << prefix << ", value "
+                     << value;
           return false;
         }
       }
@@ -260,7 +267,7 @@ bool ExpandPropertyContents(const std::string& content,
     }
   }
 
-  if (add_native_bridge_64bit_support) {
+  if (append_dalvik_isa) {
     // Special-case to add ro.dalvik.vm.isa.arm64.
     new_properties += std::string(kDalvikVmIsaArm64) + "\n";
   }
@@ -273,7 +280,9 @@ bool ExpandPropertyFile(const base::FilePath& input,
                         const base::FilePath& output,
                         CrosConfig* config,
                         bool append,
-                        bool add_native_bridge_64bit_support) {
+                        bool add_native_bridge_64bit_support,
+                        bool append_dalvik_isa,
+                        const std::string& partition_name) {
   std::string content;
   std::string expanded;
   if (!base::ReadFileToString(input, &content)) {
@@ -281,7 +290,8 @@ bool ExpandPropertyFile(const base::FilePath& input,
     return false;
   }
   if (!ExpandPropertyContents(content, config, &expanded,
-                              add_native_bridge_64bit_support))
+                              add_native_bridge_64bit_support,
+                              append_dalvik_isa, partition_name))
     return false;
   if (append && base::PathExists(output)) {
     if (!base::AppendToFile(output, expanded.data(), expanded.size())) {
@@ -342,7 +352,8 @@ bool ExpandPropertyContentsForTesting(const std::string& content,
                                       brillo::CrosConfigInterface* config,
                                       std::string* expanded_content) {
   return ExpandPropertyContents(content, config, expanded_content,
-                                /*add_native_bridge_64bit_support=*/false);
+                                /*add_native_bridge_64bit_support=*/false,
+                                false, "");
 }
 
 bool TruncateAndroidPropertyForTesting(const std::string& line,
@@ -354,7 +365,8 @@ bool ExpandPropertyFileForTesting(const base::FilePath& input,
                                   const base::FilePath& output,
                                   CrosConfig* config) {
   return ExpandPropertyFile(input, output, config, /*append=*/false,
-                            /*add_native_bridge_64bit_support=*/false);
+                            /*add_native_bridge_64bit_support=*/false, false,
+                            "");
 }
 
 bool ExpandPropertyFiles(const base::FilePath& source_path,
@@ -367,13 +379,24 @@ bool ExpandPropertyFiles(const base::FilePath& source_path,
 
   // default.prop may not exist. Silently skip it if not found.
   for (const auto& tuple :
-       {std::tuple<const char*, bool, bool>{"default.prop", true, false},
-        {"build.prop", false, true},
-        {"vendor_build.prop", false, false}}) {
+       // The order has to match the one in PropertyLoadBootDefaults() in
+       // system/core/init/property_service.cpp.
+       // Note: Our vendor image doesn't have /vendor/default.prop although
+       // PropertyLoadBootDefaults() tries to open it.
+       {std::tuple<const char*, bool, bool, const char*>{"default.prop", true,
+                                                         false, ""},
+        {"build.prop", false, true, ""},
+        {"system_ext_build.prop", true, false, "system_ext."},
+        {"vendor_build.prop", false, false, "vendor."},
+        {"odm_build.prop", true, false, "odm."},
+        {"product_build.prop", true, false, "product."}}) {
     const char* file = std::get<0>(tuple);
     const bool is_optional = std::get<1>(tuple);
-    const bool add_native_bridge_properties =
+    // When true, unconditionally add |kDalvikVmIsaArm64| property.
+    const bool append_dalvik_isa =
         std::get<2>(tuple) && add_native_bridge_64bit_support;
+    // Search for ro.<partition_name>product.cpu.abilist* properties.
+    const char* partition_name = std::get<3>(tuple);
 
     if (is_optional && !base::PathExists(source_path.Append(file)))
       continue;
@@ -381,7 +404,8 @@ bool ExpandPropertyFiles(const base::FilePath& source_path,
     if (!ExpandPropertyFile(
             source_path.Append(file),
             single_file ? dest_path : dest_path.Append(file), &config,
-            /*append=*/single_file, add_native_bridge_properties)) {
+            /*append=*/single_file, add_native_bridge_64bit_support,
+            append_dalvik_isa, partition_name)) {
       LOG(ERROR) << "Failed to expand " << source_path.Append(file);
       return false;
     }

components/arc/session/arc_property_util_unittest.cc

@@ -379,52 +379,84 @@ TEST_F(ArcPropertyUtilTest, ExpandPropertyFiles_SingleFile) {
   EXPECT_FALSE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
 
   // Add default.prop to the source, but not build.prop.
-  base::FilePath default_prop = source_dir.Append("default.prop");
+  const base::FilePath default_prop = source_dir.Append("default.prop");
   constexpr const char kDefaultProp[] = "ro.foo=bar\n";
   base::WriteFile(default_prop, kDefaultProp, strlen(kDefaultProp));
   EXPECT_FALSE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
 
   // Add build.prop too. The call should not succeed still.
-  base::FilePath build_prop = source_dir.Append("build.prop");
+  const base::FilePath build_prop = source_dir.Append("build.prop");
   constexpr const char kBuildProp[] = "ro.baz=boo\n";
   base::WriteFile(build_prop, kBuildProp, strlen(kBuildProp));
   EXPECT_FALSE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
 
   // Add vendor_build.prop too. Then the call should succeed.
-  base::FilePath vendor_build_prop = source_dir.Append("vendor_build.prop");
+  const base::FilePath vendor_build_prop =
+      source_dir.Append("vendor_build.prop");
   constexpr const char kVendorBuildProp[] = "ro.a=b\n";
   base::WriteFile(vendor_build_prop, kVendorBuildProp,
                   strlen(kVendorBuildProp));
   EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
 
+  // Add other optional files too. Then the call should succeed.
+  const base::FilePath system_ext_build_prop =
+      source_dir.Append("system_ext_build.prop");
+  constexpr const char kSystemExtBuildProp[] = "ro.c=d\n";
+  base::WriteFile(system_ext_build_prop, kSystemExtBuildProp,
+                  strlen(kSystemExtBuildProp));
+  EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
+
+  const base::FilePath odm_build_prop = source_dir.Append("odm_build.prop");
+  constexpr const char kOdmBuildProp[] = "ro.e=f\n";
+  base::WriteFile(odm_build_prop, kOdmBuildProp, strlen(kOdmBuildProp));
+  EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
+
+  const base::FilePath product_build_prop =
+      source_dir.Append("product_build.prop");
+  constexpr const char kProductBuildProp[] = "ro.g=h\n";
+  base::WriteFile(product_build_prop, kProductBuildProp,
+                  strlen(kProductBuildProp));
+  EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
+
   // Verify only one dest file exists.
   EXPECT_FALSE(
       base::PathExists(dest_prop_file.DirName().Append("default.prop")));
   EXPECT_FALSE(base::PathExists(dest_prop_file.DirName().Append("build.prop")));
   EXPECT_FALSE(
       base::PathExists(dest_prop_file.DirName().Append("vendor_build.prop")));
+  EXPECT_FALSE(base::PathExists(
+      dest_prop_file.DirName().Append("system_ext_build.prop")));
+  EXPECT_FALSE(
+      base::PathExists(dest_prop_file.DirName().Append("odm_build.prop")));
+  EXPECT_FALSE(
+      base::PathExists(dest_prop_file.DirName().Append("product_build.prop")));
   EXPECT_TRUE(base::PathExists(dest_prop_file));
 
   // Verify the content.
   std::string content;
   EXPECT_TRUE(base::ReadFileToString(dest_prop_file, &content));
-  EXPECT_EQ(
-      base::StringPrintf("%s%s%s", kDefaultProp, kBuildProp, kVendorBuildProp),
-      content);
+  EXPECT_EQ(base::StringPrintf("%s%s%s%s%s%s", kDefaultProp, kBuildProp,
+                               kSystemExtBuildProp, kVendorBuildProp,
+                               kOdmBuildProp, kProductBuildProp),
+            content);
 
   // Expand it again, verify the previous result is cleared.
   EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
   EXPECT_TRUE(base::ReadFileToString(dest_prop_file, &content));
-  EXPECT_EQ(
-      base::StringPrintf("%s%s%s", kDefaultProp, kBuildProp, kVendorBuildProp),
-      content);
+  EXPECT_EQ(base::StringPrintf("%s%s%s%s%s%s", kDefaultProp, kBuildProp,
+                               kSystemExtBuildProp, kVendorBuildProp,
+                               kOdmBuildProp, kProductBuildProp),
+            content);
 
-  // If default.prop does not exist in the source path, it should still process
-  // the other files.
+  // If optional ones e.g. default.prop does not exist in the source path, it
+  // should still process the other files.
   base::DeleteFile(source_dir.Append("default.prop"));
+  base::DeleteFile(source_dir.Append("odm_build.prop"));
   EXPECT_TRUE(ExpandPropertyFiles(source_dir, dest_prop_file, true, false));
   EXPECT_TRUE(base::ReadFileToString(dest_prop_file, &content));
-  EXPECT_EQ(base::StringPrintf("%s%s", kBuildProp, kVendorBuildProp), content);
+  EXPECT_EQ(base::StringPrintf("%s%s%s%s", kBuildProp, kSystemExtBuildProp,
+                               kVendorBuildProp, kProductBuildProp),
+            content);
 
   // Finally, test the case where source is valid but the dest is not.
   EXPECT_FALSE(ExpandPropertyFiles(source_dir, base::FilePath("/nonexistent"),
@@ -452,7 +484,10 @@ TEST_F(ArcPropertyUtilTest, TestNativeBridge64Support) {
   base::WriteFile(build_prop, kBuildProp, strlen(kBuildProp));
 
   base::FilePath vendor_build_prop = source_dir.Append("vendor_build.prop");
-  constexpr const char kVendorBuildProp[] = "ro.a=b\n";
+  constexpr const char kVendorBuildProp[] =
+      "ro.a=b\n"
+      "ro.vendor.product.cpu.abilist=x86_64,x86,armeabi-v7a,armeabi\n"
+      "ro.vendor.product.cpu.abilist64=x86_64\n";
   base::WriteFile(vendor_build_prop, kVendorBuildProp,
                   strlen(kVendorBuildProp));
 
@@ -483,7 +518,11 @@ TEST_F(ArcPropertyUtilTest, TestNativeBridge64Support) {
   EXPECT_EQ(std::string(kBuildPropModified), content);
   EXPECT_TRUE(
       base::ReadFileToString(dest_dir.Append("vendor_build.prop"), &content));
-  EXPECT_EQ(std::string(kVendorBuildProp), 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);
 
   // Expand to a single file with experiment on, verify properties are added /
   // modified as expected.
@@ -496,7 +535,7 @@ TEST_F(ArcPropertyUtilTest, TestNativeBridge64Support) {
   // Verify the contents.
   EXPECT_TRUE(base::ReadFileToString(dest_prop_file, &content));
   EXPECT_EQ(base::StringPrintf("%s%s%s", kDefaultProp, kBuildPropModified,
-                               kVendorBuildProp),
+                               kVendorBuildPropModified),
             content);
 
   // Verify that unexpected property values generate an error.