Merge "Error correction: Use libfec in fs_mgr"
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index 0531cf9..bc5ba38 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -425,6 +425,7 @@
 // Used to pass multiple values to the stdin read thread.
 struct StdinReadArgs {
     int stdin_fd, write_fd;
+    bool raw_stdin;
     std::unique_ptr<ShellProtocol> protocol;
 };
 
@@ -452,26 +453,42 @@
         D("stdin_read_thread(): pre unix_read(fdi=%d,...)", args->stdin_fd);
         int r = unix_read(args->stdin_fd, buffer_ptr, buffer_size);
         D("stdin_read_thread(): post unix_read(fdi=%d,...)", args->stdin_fd);
-        if (r <= 0) break;
-        for (int n = 0; n < r; n++){
-            switch(buffer_ptr[n]) {
-            case '\n':
-                state = 1;
-                break;
-            case '\r':
-                state = 1;
-                break;
-            case '~':
-                if(state == 1) state++;
-                break;
-            case '.':
-                if(state == 2) {
-                    fprintf(stderr,"\n* disconnect *\n");
-                    stdin_raw_restore(args->stdin_fd);
-                    exit(0);
+        if (r <= 0) {
+            // Only devices using the shell protocol know to close subprocess
+            // stdin. For older devices we want to just leave the connection
+            // open, otherwise an unpredictable amount of return data could
+            // be lost due to the FD closing before all data has been received.
+            if (args->protocol) {
+                args->protocol->Write(ShellProtocol::kIdCloseStdin, 0);
+            }
+            break;
+        }
+        // If we made stdin raw, check input for the "~." escape sequence. In
+        // this situation signals like Ctrl+C are sent remotely rather than
+        // interpreted locally so this provides an emergency out if the remote
+        // process starts ignoring the signal. SSH also does this, see the
+        // "escape characters" section on the ssh man page for more info.
+        if (args->raw_stdin) {
+            for (int n = 0; n < r; n++){
+                switch(buffer_ptr[n]) {
+                case '\n':
+                    state = 1;
+                    break;
+                case '\r':
+                    state = 1;
+                    break;
+                case '~':
+                    if(state == 1) state++;
+                    break;
+                case '.':
+                    if(state == 2) {
+                        stdin_raw_restore(args->stdin_fd);
+                        fprintf(stderr,"\n* disconnect *\n");
+                        exit(0);
+                    }
+                default:
+                    state = 0;
                 }
-            default:
-                state = 0;
             }
         }
         if (args->protocol) {
@@ -488,8 +505,44 @@
     return nullptr;
 }
 
-static int interactive_shell(const std::string& service_string,
-                             bool use_shell_protocol) {
+// Returns a shell service string with the indicated arguments and command.
+static std::string ShellServiceString(bool use_shell_protocol,
+                                      const std::string& type_arg,
+                                      const std::string& command) {
+    std::vector<std::string> args;
+    if (use_shell_protocol) {
+        args.push_back(kShellServiceArgShellProtocol);
+    }
+    if (!type_arg.empty()) {
+        args.push_back(type_arg);
+    }
+
+    // Shell service string can look like: shell[,arg1,arg2,...]:[command].
+    return android::base::StringPrintf("shell%s%s:%s",
+                                       args.empty() ? "" : ",",
+                                       android::base::Join(args, ',').c_str(),
+                                       command.c_str());
+}
+
+// Connects to a shell on the device and read/writes data.
+//
+// Note: currently this function doesn't properly clean up resources; the
+// FD connected to the adb server is never closed and the stdin read thread
+// may never exit.
+//
+// On success returns the remote exit code if |use_shell_protocol| is true,
+// 0 otherwise. On failure returns 1.
+static int RemoteShell(bool use_shell_protocol, const std::string& type_arg,
+                       const std::string& command) {
+    std::string service_string = ShellServiceString(use_shell_protocol,
+                                                    type_arg, command);
+
+    // Make local stdin raw if the device allocates a PTY, which happens if:
+    //   1. We are explicitly asking for a PTY shell, or
+    //   2. We don't specify shell type and are starting an interactive session.
+    bool raw_stdin = (type_arg == kShellServiceArgPty ||
+                      (type_arg.empty() && command.empty()));
+
     std::string error;
     int fd = adb_connect(service_string, &error);
     if (fd < 0) {
@@ -502,13 +555,16 @@
         LOG(ERROR) << "couldn't allocate StdinReadArgs object";
         return 1;
     }
-    args->stdin_fd = 0;
+    args->stdin_fd = STDIN_FILENO;
     args->write_fd = fd;
+    args->raw_stdin = raw_stdin;
     if (use_shell_protocol) {
         args->protocol.reset(new ShellProtocol(args->write_fd));
     }
 
-    stdin_raw_init(args->stdin_fd);
+    if (raw_stdin) {
+        stdin_raw_init(STDIN_FILENO);
+    }
 
     int exit_code = 0;
     if (!adb_thread_create(stdin_read_thread, args)) {
@@ -519,7 +575,12 @@
         exit_code = read_and_dump(fd, use_shell_protocol);
     }
 
-    stdin_raw_restore(args->stdin_fd);
+    if (raw_stdin) {
+        stdin_raw_restore(STDIN_FILENO);
+    }
+
+    // TODO(dpursell): properly exit stdin_read_thread and close |fd|.
+
     return exit_code;
 }
 
@@ -795,25 +856,6 @@
     return adb_command(cmd);
 }
 
-// Returns a shell service string with the indicated arguments and command.
-static std::string ShellServiceString(bool use_shell_protocol,
-                                      const std::string& type_arg,
-                                      const std::string& command) {
-    std::vector<std::string> args;
-    if (use_shell_protocol) {
-        args.push_back(kShellServiceArgShellProtocol);
-    }
-    if (!type_arg.empty()) {
-        args.push_back(type_arg);
-    }
-
-    // Shell service string can look like: shell[,arg1,arg2,...]:[command].
-    return android::base::StringPrintf("shell%s%s:%s",
-                                       args.empty() ? "" : ",",
-                                       android::base::Join(args, ',').c_str(),
-                                       command.c_str());
-}
-
 // Connects to the device "shell" service with |command| and prints the
 // resulting output.
 static int send_shell_command(TransportType transport_type, const char* serial,
@@ -1320,51 +1362,26 @@
             }
         }
 
+        std::string command;
+        if (argc) {
+            // We don't escape here, just like ssh(1). http://b/20564385.
+            command = android::base::Join(
+                    std::vector<const char*>(argv, argv + argc), ' ');
+        }
+
         if (h) {
             printf("\x1b[41;33m");
             fflush(stdout);
         }
 
-        if (!argc) {
-            D("starting interactive shell");
-            std::string service_string =
-                    ShellServiceString(use_shell_protocol, shell_type_arg, "");
-            r = interactive_shell(service_string, use_shell_protocol);
-            if (h) {
-                printf("\x1b[0m");
-                fflush(stdout);
-            }
-            return r;
+        r = RemoteShell(use_shell_protocol, shell_type_arg, command);
+
+        if (h) {
+            printf("\x1b[0m");
+            fflush(stdout);
         }
 
-        // We don't escape here, just like ssh(1). http://b/20564385.
-        std::string command = android::base::Join(
-                std::vector<const char*>(argv, argv + argc), ' ');
-        std::string service_string =
-                ShellServiceString(use_shell_protocol, shell_type_arg, command);
-
-        while (true) {
-            D("non-interactive shell loop. cmd=%s", service_string.c_str());
-            std::string error;
-            int fd = adb_connect(service_string, &error);
-            int r;
-            if (fd >= 0) {
-                D("about to read_and_dump(fd=%d)", fd);
-                r = read_and_dump(fd, use_shell_protocol);
-                D("read_and_dump() done.");
-                adb_close(fd);
-            } else {
-                fprintf(stderr,"error: %s\n", error.c_str());
-                r = -1;
-            }
-
-            if (h) {
-                printf("\x1b[0m");
-                fflush(stdout);
-            }
-            D("non-interactive shell loop. return r=%d", r);
-            return r;
-        }
+        return r;
     }
     else if (!strcmp(argv[0], "exec-in") || !strcmp(argv[0], "exec-out")) {
         int exec_in = !strcmp(argv[0], "exec-in");
diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp
index 8aeea81..544afce 100644
--- a/adb/shell_service.cpp
+++ b/adb/shell_service.cpp
@@ -382,7 +382,7 @@
     subprocess->PassDataStreams();
     subprocess->WaitForExit();
 
-    D("deleting Subprocess");
+    D("deleting Subprocess for PID %d", subprocess->pid());
     delete subprocess;
 
     return nullptr;
@@ -501,11 +501,31 @@
             return &protocol_sfd_;
         }
 
-        // We only care about stdin packets.
-        if (stdinout_sfd_.valid() && input_->id() == ShellProtocol::kIdStdin) {
-            input_bytes_left_ = input_->data_length();
-        } else {
-            input_bytes_left_ = 0;
+        if (stdinout_sfd_.valid()) {
+            switch (input_->id()) {
+                case ShellProtocol::kIdStdin:
+                    input_bytes_left_ = input_->data_length();
+                    break;
+                case ShellProtocol::kIdCloseStdin:
+                    if (type_ == SubprocessType::kRaw) {
+                        if (adb_shutdown(stdinout_sfd_.fd(), SHUT_WR) == 0) {
+                            return nullptr;
+                        }
+                        PLOG(ERROR) << "failed to shutdown writes to FD "
+                                    << stdinout_sfd_.fd();
+                        return &stdinout_sfd_;
+                    } else {
+                        // PTYs can't close just input, so rather than close the
+                        // FD and risk losing subprocess output, leave it open.
+                        // This only happens if the client starts a PTY shell
+                        // non-interactively which is rare and unsupported.
+                        // If necessary, the client can manually close the shell
+                        // with `exit` or by killing the adb client process.
+                        D("can't close input for PTY FD %d",
+                          stdinout_sfd_.fd());
+                    }
+                    break;
+            }
         }
     }
 
@@ -532,7 +552,9 @@
 ScopedFd* Subprocess::PassOutput(ScopedFd* sfd, ShellProtocol::Id id) {
     int bytes = adb_read(sfd->fd(), output_->data(), output_->data_capacity());
     if (bytes == 0 || (bytes < 0 && errno != EAGAIN)) {
-        if (bytes < 0) {
+        // read() returns EIO if a PTY closes; don't report this as an error,
+        // it just means the subprocess completed.
+        if (bytes < 0 && !(type_ == SubprocessType::kPty && errno == EIO)) {
             PLOG(ERROR) << "error reading output FD " << sfd->fd();
         }
         return sfd;
diff --git a/adb/shell_service.h b/adb/shell_service.h
index 8868f10..01410a9 100644
--- a/adb/shell_service.h
+++ b/adb/shell_service.h
@@ -50,11 +50,12 @@
   public:
     // This is an unscoped enum to make it easier to compare against raw bytes.
     enum Id : uint8_t {
-        kIdStdin  = 0,
+        kIdStdin = 0,
         kIdStdout = 1,
         kIdStderr = 2,
-        kIdExit   = 3,
-        kIdInvalid  = 255,  // Indicates an invalid or unknown packet.
+        kIdExit = 3,
+        kIdCloseStdin = 4,  // Close subprocess stdin if possible.
+        kIdInvalid = 255,  // Indicates an invalid or unknown packet.
     };
 
     // ShellPackets will probably be too large to allocate on the stack so they
diff --git a/adb/shell_service_test.cpp b/adb/shell_service_test.cpp
index 20efd84..e18f905 100644
--- a/adb/shell_service_test.cpp
+++ b/adb/shell_service_test.cpp
@@ -245,6 +245,25 @@
     EXPECT_FALSE(stdout.find("--abc123--") == std::string::npos);
 }
 
+// Tests closing raw subprocess stdin.
+TEST_F(ShellServiceTest, CloseClientStdin) {
+    ASSERT_NO_FATAL_FAILURE(StartTestSubprocess(
+            "cat; echo TEST_DONE",
+            SubprocessType::kRaw, SubprocessProtocol::kShell));
+
+    std::string input = "foo\nbar";
+    ShellProtocol* protocol = new ShellProtocol(subprocess_fd_);
+    memcpy(protocol->data(), input.data(), input.length());
+    ASSERT_TRUE(protocol->Write(ShellProtocol::kIdStdin, input.length()));
+    ASSERT_TRUE(protocol->Write(ShellProtocol::kIdCloseStdin, 0));
+    delete protocol;
+
+    std::string stdout, stderr;
+    EXPECT_EQ(0, ReadShellProtocol(subprocess_fd_, &stdout, &stderr));
+    ExpectLinesEqual(stdout, {"foo", "barTEST_DONE"});
+    ExpectLinesEqual(stderr, {});
+}
+
 // Tests that nothing breaks when the stdin/stdout pipe closes.
 TEST_F(ShellServiceTest, CloseStdinStdoutSubprocess) {
     ASSERT_NO_FATAL_FAILURE(StartTestSubprocess(
diff --git a/adb/sysdeps.h b/adb/sysdeps.h
index 5918a94..501a75a 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -488,6 +488,10 @@
 {
     return shutdown(fd, SHUT_RDWR);
 }
+static __inline__ int  adb_shutdown(int fd, int direction)
+{
+    return shutdown(fd, direction);
+}
 #undef   shutdown
 #define  shutdown   ____xxx_shutdown
 
diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp
index 42f6d9b..994b851 100644
--- a/adb/sysdeps_win32.cpp
+++ b/adb/sysdeps_win32.cpp
@@ -3265,6 +3265,15 @@
         // terminal.
         return _console_read(_console_handle, buf, len);
     } else {
+        // On older versions of Windows (definitely 7, definitely not 10),
+        // ReadConsole() with a size >= 31367 fails, so if |fd| is a console
+        // we need to limit the read size. This may also catch devices like NUL,
+        // but that is OK as we just want to avoid capping pipes and files which
+        // don't need size limiting. This isatty() test is very simple and quick
+        // and doesn't call the OS.
+        if (isatty(fd) && len > 4096) {
+            len = 4096;
+        }
         // Just call into C Runtime which can read from pipes/files and which
         // can do LF/CR translation (which is overridable with _setmode()).
         // Undefine the macro that is set in sysdeps.h which bans calls to
diff --git a/crash_reporter/crash_reporter.rc b/crash_reporter/crash_reporter.rc
index 7bfe0c2..57c1d40 100644
--- a/crash_reporter/crash_reporter.rc
+++ b/crash_reporter/crash_reporter.rc
@@ -5,7 +5,7 @@
 on property:crash_reporter.coredump.enabled=0
     write /proc/sys/kernel/core_pattern "core"
 
-on boot
+on post-fs-data
     # Allow catching multiple unrelated concurrent crashes, but use a finite
     # number to prevent infinitely recursing on crash handling.
     write /proc/sys/kernel/core_pipe_limit 4
diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender
index e38d410..1f58792 100755
--- a/crash_reporter/crash_sender
+++ b/crash_reporter/crash_sender
@@ -298,8 +298,8 @@
   local log="$(get_key_value "${meta_path}" "log")"
   local sig="$(get_key_value "${meta_path}" "sig")"
   local send_payload_size="$(stat -c "%s" "${report_payload}" 2>/dev/null)"
-  local product="$(get_key_value "${meta_path}" "upload_var_prod")"
-  local version="$(get_key_value "${meta_path}" "upload_var_ver")"
+  local product="$(get_key_value "${meta_path}" "product_id")"
+  local version="$(get_key_value "${meta_path}" "product_version")"
   local upload_prefix="$(get_key_value "${meta_path}" "upload_prefix")"
   local guid
   local model_manifest_id="$(get_key_value "${WEAVE_CONF_FILE}" "model_id")"
@@ -350,25 +350,6 @@
     esac
   done
 
-  # When uploading Chrome reports we need to report the right product and
-  # version. If the meta file does not specify it, use GOOGLE_CRASH_ID
-  # as the product and GOOGLE_CRASH_VERSION_ID as the version.
-  if [ "${product}" = "undefined" ]; then
-    product="$(get_key_value /etc/os-release 'GOOGLE_CRASH_ID')"
-  fi
-  if [ "${version}" = "undefined" ]; then
-    version="$(get_key_value /etc/os-release 'GOOGLE_CRASH_VERSION_ID')"
-  fi
-
-  # If GOOGLE_CRASH_* is undefined, we look for ID and VERSION_ID in
-  # /etc/os-release.
-  if [ "${product}" = "undefined" ]; then
-    product="$(get_key_value /etc/os-release 'ID')"
-  fi
-  if [ "${version}" = "undefined" ]; then
-    version="$(get_key_value /etc/os-release 'VERSION_ID')"
-  fi
-
   # If ID or VERSION_ID is undefined, we use the default product name
   # and bdk_version from /etc/os-release.d.
   if [ "${product}" = "undefined" ]; then
@@ -408,6 +389,7 @@
   lecho "  Metadata: ${meta_path} (${kind})"
   lecho "  Payload: ${report_payload}"
   lecho "  Version: ${version}"
+  lecho "  Bdk Version: ${bdk_version}"
   [ -n "${image_type}" ] && lecho "  Image type: ${image_type}"
   [ -n "${boot_mode}" ] && lecho "  Boot mode: ${boot_mode}"
   if is_mock; then
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index 53fbb14..56e7bb9 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -59,13 +59,10 @@
 const char *UserCollector::kUserId = "Uid:\t";
 const char *UserCollector::kGroupId = "Gid:\t";
 
-// The property containing the OS version.
-const char kVersionProperty[] = "ro.build.id";
-
-// The property containing the product id.
-const char kProductIDProperty[] = "ro.product.product_id";
-
+// Product information keys in the /etc/os-release.d folder.
 static const char kBdkVersionKey[] = "bdk_version";
+static const char kProductIDKey[] = "product_id";
+static const char kProductVersionKey[] = "product_version";
 
 
 using base::FilePath;
@@ -508,20 +505,28 @@
   if (GetLogContents(FilePath(log_config_path_), exec, log_path))
     AddCrashMetaData("log", log_path.value());
 
-  char value[PROPERTY_VALUE_MAX];
-  property_get(kVersionProperty, value, "undefined");
-  AddCrashMetaUploadData("ver", value);
-  property_get(kProductIDProperty, value, "undefined");
-  AddCrashMetaUploadData("prod", value);
-
   brillo::OsReleaseReader reader;
   reader.Load();
-  std::string bdk_version = "undefined";
-  if (!reader.GetString(kBdkVersionKey, &bdk_version)) {
+  std::string value = "undefined";
+  if (!reader.GetString(kBdkVersionKey, &value)) {
     LOG(ERROR) << "Could not read " << kBdkVersionKey
                << " from /etc/os-release.d/";
   }
-  AddCrashMetaData(kBdkVersionKey, bdk_version);
+  AddCrashMetaData(kBdkVersionKey, value);
+
+  value = "undefined";
+  if (!reader.GetString(kProductIDKey, &value)) {
+    LOG(ERROR) << "Could not read " << kProductIDKey
+               << " from /etc/os-release.d/";
+  }
+  AddCrashMetaData(kProductIDKey, value);
+
+  value = "undefined";
+  if (!reader.GetString(kProductVersionKey, &value)) {
+    LOG(ERROR) << "Could not read " << kProductVersionKey
+               << " from /etc/os-release.d/";
+  }
+  AddCrashMetaData(kProductVersionKey, value);
 
   ErrorType error_type =
       ConvertCoreToMinidump(pid, container_dir, core_path, minidump_path);
diff --git a/metricsd/Android.mk b/metricsd/Android.mk
index 3deb7d3..8dbaad9 100644
--- a/metricsd/Android.mk
+++ b/metricsd/Android.mk
@@ -69,11 +69,11 @@
   libchrome-dbus \
   libchromeos-http \
   libchromeos-dbus \
-  libcutils \
   libdbus \
   libmetrics \
   libprotobuf-cpp-lite \
   librootdev \
+  libupdate_engine_client \
   libweaved \
 
 # Shared library for metrics.
diff --git a/metricsd/constants.h b/metricsd/constants.h
index 8a00fc4..7e1e116 100644
--- a/metricsd/constants.h
+++ b/metricsd/constants.h
@@ -27,10 +27,9 @@
 static const char kFailedUploadCountName[] = "failed_upload_count";
 static const char kDefaultVersion[] = "0.0.0.0";
 
-// System properties used.
-static const char kProductIdProperty[] = "ro.product.product_id";
-static const char kChannelProperty[] = "ro.product.channel";
-static const char kProductVersionProperty[] = "ro.product.version";
+// Build time properties name.
+static const char kProductId[] = "product_id";
+static const char kProductVersion[] = "product_version";
 }  // namespace metrics
 
 #endif  // METRICS_CONSTANTS_H_
diff --git a/metricsd/metrics_daemon.cc b/metricsd/metrics_daemon.cc
index 2b7e98c..ed786e1 100644
--- a/metricsd/metrics_daemon.cc
+++ b/metricsd/metrics_daemon.cc
@@ -28,7 +28,7 @@
 #include <base/strings/string_split.h>
 #include <base/strings/string_util.h>
 #include <base/strings/stringprintf.h>
-#include <cutils/properties.h>
+#include <brillo/osrelease_reader.h>
 #include <dbus/dbus.h>
 #include <dbus/message.h>
 
@@ -153,23 +153,19 @@
 }
 
 uint32_t MetricsDaemon::GetOsVersionHash() {
-  static uint32_t cached_version_hash = 0;
-  static bool version_hash_is_cached = false;
-  if (version_hash_is_cached)
-    return cached_version_hash;
-  version_hash_is_cached = true;
-
-  char version[PROPERTY_VALUE_MAX];
-  // The version might not be set for development devices. In this case, use the
-  // zero version.
-  property_get(metrics::kProductVersionProperty, version,
-               metrics::kDefaultVersion);
-
-  cached_version_hash = base::Hash(version);
-  if (testing_) {
-    cached_version_hash = 42;  // return any plausible value for the hash
+  brillo::OsReleaseReader reader;
+  reader.Load();
+  string version;
+  if (!reader.GetString(metrics::kProductVersion, &version)) {
+    LOG(ERROR) << "failed to read the product version.";
+    version = metrics::kDefaultVersion;
   }
-  return cached_version_hash;
+
+  uint32_t version_hash = base::Hash(version);
+  if (testing_) {
+    version_hash = 42;  // return any plausible value for the hash
+  }
+  return version_hash;
 }
 
 void MetricsDaemon::Init(bool testing,
diff --git a/metricsd/metrics_daemon.rc b/metricsd/metrics_daemon.rc
index 0e1fcd5..0ee577e 100644
--- a/metricsd/metrics_daemon.rc
+++ b/metricsd/metrics_daemon.rc
@@ -1,4 +1,4 @@
-on boot
+on post-fs-data
     mkdir /data/misc/metrics 0770 system system
 
 service metrics_daemon /system/bin/metrics_daemon --uploader -nodaemon
diff --git a/metricsd/uploader/system_profile_cache.cc b/metricsd/uploader/system_profile_cache.cc
index 1d87be5..1995510 100644
--- a/metricsd/uploader/system_profile_cache.cc
+++ b/metricsd/uploader/system_profile_cache.cc
@@ -21,8 +21,9 @@
 #include <base/logging.h>
 #include <base/strings/string_number_conversions.h>
 #include <base/strings/string_util.h>
-#include <cutils/properties.h>
+#include <brillo/osrelease_reader.h>
 #include <string>
+#include <update_engine/client.h>
 #include <vector>
 
 #include "constants.h"
@@ -73,16 +74,27 @@
   CHECK(!initialized_)
       << "this should be called only once in the metrics_daemon lifetime.";
 
-  profile_.product_id = GetProperty(metrics::kProductIdProperty);
+  brillo::OsReleaseReader reader;
+  std::string channel;
+  if (testing_) {
+    reader.LoadTestingOnly(metrics_directory_);
+    channel = "unknown";
+  } else {
+    reader.Load();
+    auto client = update_engine::UpdateEngineClient::CreateInstance();
+    if (!client->GetChannel(&channel)) {
+      LOG(ERROR) << "failed to read the current channel from update engine.";
+    }
+  }
 
-  if (profile_.product_id.empty()) {
-    LOG(ERROR) << "System property " << metrics::kProductIdProperty
-               << " is not set.";
+  if (!reader.GetString(metrics::kProductId, &profile_.product_id)) {
+    LOG(ERROR) << "product_id is not set.";
     return false;
   }
 
-  std::string channel = GetProperty(metrics::kChannelProperty);
-  profile_.version = GetProperty(metrics::kProductVersionProperty);
+  if (!reader.GetString(metrics::kProductVersion, &profile_.version)) {
+    LOG(ERROR) << "failed to read the product version";
+  }
 
   if (channel.empty() || profile_.version.empty()) {
     // If the channel or version is missing, the image is not official.
@@ -154,18 +166,6 @@
   return guid;
 }
 
-std::string SystemProfileCache::GetProperty(const std::string& name) {
-  if (testing_) {
-    std::string content;
-    base::ReadFileToString(metrics_directory_.Append(name), &content);
-    return content;
-  } else {
-    char value[PROPERTY_VALUE_MAX];
-    property_get(name.data(), value, "");
-    return std::string(value);
-  }
-}
-
 metrics::SystemProfileProto_Channel SystemProfileCache::ProtoChannelFromString(
     const std::string& channel) {
   if (channel == "stable") {
diff --git a/metricsd/uploader/system_profile_cache.h b/metricsd/uploader/system_profile_cache.h
index 3410157..97fb33a 100644
--- a/metricsd/uploader/system_profile_cache.h
+++ b/metricsd/uploader/system_profile_cache.h
@@ -76,11 +76,6 @@
   // Initializes |profile_| only if it has not been yet initialized.
   bool InitializeOrCheck();
 
-  // Gets a system property as a string.
-  // When |testing_| is true, reads the value from |metrics_directory_|/|name|
-  // instead.
-  std::string GetProperty(const std::string& name);
-
   bool initialized_;
   bool testing_;
   base::FilePath metrics_directory_;
diff --git a/metricsd/uploader/upload_service_test.cc b/metricsd/uploader/upload_service_test.cc
index 305fd0c..236376a 100644
--- a/metricsd/uploader/upload_service_test.cc
+++ b/metricsd/uploader/upload_service_test.cc
@@ -58,9 +58,11 @@
   }
 
   void SetTestingProperty(const std::string& name, const std::string& value) {
+    base::FilePath filepath = dir_.path().Append("etc/os-release.d").Append(name);
+    ASSERT_TRUE(base::CreateDirectory(filepath.DirName()));
     ASSERT_EQ(
         value.size(),
-        base::WriteFile(dir_.path().Append(name), value.data(), value.size()));
+        base::WriteFile(filepath, value.data(), value.size()));
   }
 
   base::ScopedTempDir dir_;
@@ -225,9 +227,8 @@
   SenderMock* sender = new SenderMock();
   upload_service_->sender_.reset(sender);
 
-  SetTestingProperty(metrics::kChannelProperty, "beta");
-  SetTestingProperty(metrics::kProductIdProperty, "hello");
-  SetTestingProperty(metrics::kProductVersionProperty, "1.2.3.4");
+  SetTestingProperty(metrics::kProductId, "hello");
+  SetTestingProperty(metrics::kProductVersion, "1.2.3.4");
 
   scoped_ptr<metrics::MetricSample> histogram =
       metrics::MetricSample::SparseHistogramSample("myhistogram", 1);
@@ -242,8 +243,6 @@
   EXPECT_TRUE(sender->is_good_proto());
   EXPECT_EQ(1, sender->last_message_proto().histogram_event().size());
 
-  EXPECT_EQ(metrics::SystemProfileProto::CHANNEL_BETA,
-            sender->last_message_proto().system_profile().channel());
   EXPECT_NE(0, sender->last_message_proto().client_id());
   EXPECT_NE(0, sender->last_message_proto().system_profile().build_timestamp());
   EXPECT_NE(0, sender->last_message_proto().session_id());
@@ -269,7 +268,7 @@
 }
 
 TEST_F(UploadServiceTest, SessionIdIncrementedAtInitialization) {
-  SetTestingProperty(metrics::kProductIdProperty, "hello");
+  SetTestingProperty(metrics::kProductId, "hello");
   SystemProfileCache cache(true, dir_.path());
   cache.Initialize();
   int session_id = cache.profile_.session_id;