Revert "arc: Expand 3 more .prop files at runtime"

This reverts commit 67a9b3fb.

Reason for revert: This seems to cause SEGV in R's init sometimes.

Original change's description:
> 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>.
> 
> BUG=b:162555733,b:162024174
> TEST=components_unittests
> 
> Change-Id: I98e1a5118657b91171b63c6bcbe9f71d9d1fe6da
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333580
> Commit-Queue: Yusuke Sato <yusukes@chromium.org>
> Reviewed-by: Yury Khmel <khmel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#794771}

TBR=yusukes@chromium.org,khmel@google.com,khmel@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: b:162555733
Bug: b:162024174
Change-Id: I05510404e9fa5a77d67596e5843af3b4cb7182ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343322Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Auto-Submit: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796105}

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

@@ -22,8 +22,7 @@ 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",
-        "system_ext_build.prop", "product_build.prop", "odm_build.prop"}) {
+       {"default.prop", "build.prop", "vendor_build.prop"}) {
     if (base::WriteFile(source_dir->Append(filename), "", 1) != 1)
       return false;
   }

components/arc/session/arc_property_util.cc

@@ -15,7 +15,6 @@
 #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 {
@@ -41,12 +40,11 @@ constexpr char kOEMKey1PropertyPrefix[] = "ro.oem.key1=";
 constexpr char kPAIRegionsPropertyName[] = "pai-regions";
 
 // Properties related to dynamically adding native bridge 64 bit support.
-constexpr char kAbilistPropertyPrefixTemplate[] = "ro.%sproduct.cpu.abilist=";
+constexpr char kAbilistPropertyPrefix[] = "ro.product.cpu.abilist=";
 constexpr char kAbilistPropertyExpected[] = "x86_64,x86,armeabi-v7a,armeabi";
 constexpr char kAbilistPropertyReplacement[] =
     "x86_64,x86,arm64-v8a,armeabi-v7a,armeabi";
-constexpr char kAbilist64PropertyPrefixTemplate[] =
-    "ro.%sproduct.cpu.abilist64=";
+constexpr char kAbilist64PropertyPrefix[] = "ro.product.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,9 +158,7 @@ std::string ComputeOEMKey(brillo::CrosConfigInterface* config,
 bool ExpandPropertyContents(const std::string& content,
                             brillo::CrosConfigInterface* config,
                             std::string* expanded_content,
-                            bool add_native_bridge_64bit_support,
-                            bool append_dalvik_isa,
-                            const std::string& partition_name) {
+                            bool add_native_bridge_64bit_support) {
   const std::vector<std::string> lines = base::SplitString(
       content, "\n", base::WhitespaceHandling::KEEP_WHITESPACE,
       base::SplitResult::SPLIT_WANT_ALL);
@@ -208,30 +204,27 @@ bool ExpandPropertyContents(const std::string& content,
         expanded += line.substr(prev_match);
       line = expanded;
     } while (inserted);
-
     if (add_native_bridge_64bit_support) {
-      // 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());
+      // Special-case ro.product.cpu.abilist / ro.product.cpu.abilist64 to add
+      // ARM64.
       std::string value;
-      if (FindProperty(prefix, &value, line)) {
+      if (FindProperty(kAbilistPropertyPrefix, &value, line)) {
         if (value == kAbilistPropertyExpected) {
-          line = prefix + std::string(kAbilistPropertyReplacement);
+          line = std::string(kAbilistPropertyPrefix) +
+                 std::string(kAbilistPropertyReplacement);
         } else {
-          LOG(ERROR) << "Found unexpected value for " << prefix << ", value "
-                     << value;
+          LOG(ERROR) << "Found unexpected value for " << kAbilistPropertyPrefix
+                     << ", value " << value;
           return false;
         }
       }
-      prefix = base::StringPrintf(kAbilist64PropertyPrefixTemplate,
-                                  partition_name.c_str());
-      if (FindProperty(prefix, &value, line)) {
+      if (FindProperty(kAbilist64PropertyPrefix, &value, line)) {
         if (value == kAbilist64PropertyExpected) {
-          line = prefix + std::string(kAbilist64PropertyReplacement);
+          line = std::string(kAbilist64PropertyPrefix) +
+                 std::string(kAbilist64PropertyReplacement);
         } else {
-          LOG(ERROR) << "Found unexpected value for " << prefix << ", value "
-                     << value;
+          LOG(ERROR) << "Found unexpected value for "
+                     << kAbilist64PropertyPrefix << ", value " << value;
           return false;
         }
       }
@@ -255,7 +248,7 @@ bool ExpandPropertyContents(const std::string& content,
     }
   }
 
-  if (append_dalvik_isa) {
+  if (add_native_bridge_64bit_support) {
     // Special-case to add ro.dalvik.vm.isa.arm64.
     new_properties += std::string(kDalvikVmIsaArm64) + "\n";
   }
@@ -268,9 +261,7 @@ bool ExpandPropertyFile(const base::FilePath& input,
                         const base::FilePath& output,
                         CrosConfig* config,
                         bool append,
-                        bool add_native_bridge_64bit_support,
-                        bool append_dalvik_isa,
-                        const std::string& partition_name) {
+                        bool add_native_bridge_64bit_support) {
   std::string content;
   std::string expanded;
   if (!base::ReadFileToString(input, &content)) {
@@ -278,8 +269,7 @@ bool ExpandPropertyFile(const base::FilePath& input,
     return false;
   }
   if (!ExpandPropertyContents(content, config, &expanded,
-                              add_native_bridge_64bit_support,
-                              append_dalvik_isa, partition_name))
+                              add_native_bridge_64bit_support))
     return false;
   if (append && base::PathExists(output)) {
     if (!base::AppendToFile(output, expanded.data(), expanded.size())) {
@@ -340,8 +330,7 @@ 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,
-                                false, "");
+                                /*add_native_bridge_64bit_support=*/false);
 }
 
 bool TruncateAndroidPropertyForTesting(const std::string& line,
@@ -353,8 +342,7 @@ 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, false,
-                            "");
+                            /*add_native_bridge_64bit_support=*/false);
 }
 
 bool ExpandPropertyFiles(const base::FilePath& source_path,
@@ -367,24 +355,13 @@ bool ExpandPropertyFiles(const base::FilePath& source_path,
 
   // default.prop may not exist. Silently skip it if not found.
   for (const auto& tuple :
-       // 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."}}) {
+       {std::tuple<const char*, bool, bool>{"default.prop", true, false},
+        {"build.prop", false, true},
+        {"vendor_build.prop", false, false}}) {
     const char* file = std::get<0>(tuple);
     const bool is_optional = std::get<1>(tuple);
-    // When true, unconditionally add |kDalvikVmIsaArm64| property.
-    const bool append_dalvik_isa =
+    const bool add_native_bridge_properties =
         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;
@@ -392,8 +369,7 @@ 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_64bit_support,
-            append_dalvik_isa, partition_name)) {
+            /*append=*/single_file, add_native_bridge_properties)) {
       LOG(ERROR) << "Failed to expand " << source_path.Append(file);
       return false;
     }

components/arc/session/arc_property_util_unittest.cc

@@ -379,88 +379,54 @@ 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.
-  const base::FilePath default_prop = source_dir.Append("default.prop");
+  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.
-  const base::FilePath build_prop = source_dir.Append("build.prop");
+  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.
-  const base::FilePath vendor_build_prop =
-      source_dir.Append("vendor_build.prop");
+  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.
   // Note: ExpandPropertyFileForTesting() adds a trailing LF.
   std::string content;
   EXPECT_TRUE(base::ReadFileToString(dest_prop_file, &content));
-  EXPECT_EQ(
-      base::StringPrintf("%s\n%s\n%s\n%s\n%s\n%s\n", kDefaultProp, kBuildProp,
-                         kSystemExtBuildProp, kVendorBuildProp, kOdmBuildProp,
-                         kProductBuildProp),
-      content);
+  EXPECT_EQ(base::StringPrintf("%s\n%s\n%s\n", kDefaultProp, kBuildProp,
+                               kVendorBuildProp),
+            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\n%s\n%s\n%s\n%s\n%s\n", kDefaultProp, kBuildProp,
-                         kSystemExtBuildProp, kVendorBuildProp, kOdmBuildProp,
-                         kProductBuildProp),
-      content);
+  EXPECT_EQ(base::StringPrintf("%s\n%s\n%s\n", kDefaultProp, kBuildProp,
+                               kVendorBuildProp),
+            content);
 
-  // If optional ones e.g. default.prop does not exist in the source path, it
-  // should still process the other files.
+  // If 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\n%s\n%s\n%s\n", kBuildProp, kSystemExtBuildProp,
-                         kVendorBuildProp, kProductBuildProp),
-      content);
+  EXPECT_EQ(base::StringPrintf("%s\n%s\n", kBuildProp, kVendorBuildProp),
+            content);
 
   // Finally, test the case where source is valid but the dest is not.
   EXPECT_FALSE(ExpandPropertyFiles(source_dir, base::FilePath("/nonexistent"),
@@ -488,10 +454,7 @@ 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"
-      "ro.vendor.product.cpu.abilist=x86_64,x86,armeabi-v7a,armeabi\n"
-      "ro.vendor.product.cpu.abilist64=x86_64\n";
+  constexpr const char kVendorBuildProp[] = "ro.a=b\n";
   base::WriteFile(vendor_build_prop, kVendorBuildProp,
                   strlen(kVendorBuildProp));
 
@@ -522,11 +485,7 @@ TEST_F(ArcPropertyUtilTest, TestNativeBridge64Support) {
   EXPECT_EQ(std::string(kBuildPropModified) + "\n", 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) + "\n", content);
+  EXPECT_EQ(std::string(kVendorBuildProp) + "\n", content);
 
   // Expand to a single file with experiment on, verify properties are added /
   // modified as expected.
@@ -539,7 +498,7 @@ TEST_F(ArcPropertyUtilTest, TestNativeBridge64Support) {
   // Verify the contents.
   EXPECT_TRUE(base::ReadFileToString(dest_prop_file, &content));
   EXPECT_EQ(base::StringPrintf("%s\n%s\n%s\n", kDefaultProp, kBuildPropModified,
-                               kVendorBuildPropModified),
+                               kVendorBuildProp),
             content);
 
   // Verify that unexpected property values generate an error.