crash-reporter: Write out a magic string for Chrome crashes.

Also do some much needed code cleanup.

BUG=chromium:363660
TEST=emerge platform2

Change-Id: Ica9abfd854e2c77d970851805989c86a6a45fdee
Reviewed-on: https://chromium-review.googlesource.com/196764
Commit-Queue: Lei Zhang <thestig@chromium.org>
Tested-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
diff --git a/crash_reporter/chrome_collector.cc b/crash_reporter/chrome_collector.cc
index 896d0c3..9798ec5 100644
--- a/crash_reporter/chrome_collector.cc
+++ b/crash_reporter/chrome_collector.cc
@@ -128,11 +128,11 @@
 } //namespace
 
 
-ChromeCollector::ChromeCollector() {}
+ChromeCollector::ChromeCollector() : output_file_ptr_(stdout) {}
 
 ChromeCollector::~ChromeCollector() {}
 
-bool ChromeCollector::HandleCrash(const std::string &file_path,
+bool ChromeCollector::HandleCrash(const FilePath &file_path,
                                   const std::string &pid_string,
                                   const std::string &uid_string,
                                   const std::string &exe_name) {
@@ -158,8 +158,8 @@
   FilePath log_path = GetCrashPath(dir, dump_basename, "log.tar.xz");
 
   std::string data;
-  if (!base::ReadFileToString(FilePath(file_path), &data)) {
-    LOG(ERROR) << "Can't read crash log: " << file_path.c_str();
+  if (!base::ReadFileToString(file_path, &data)) {
+    LOG(ERROR) << "Can't read crash log: " << file_path.value();
     return false;
   }
 
@@ -186,6 +186,9 @@
   // We're done.
   WriteCrashMetaData(meta_path, exe_name, minidump_path.value());
 
+  fprintf(output_file_ptr_, "%s", kSuccessMagic);
+  fflush(output_file_ptr_);
+
   return true;
 }
 
@@ -288,3 +291,6 @@
 
   return at == data.size();
 }
+
+// static
+const char ChromeCollector::kSuccessMagic[] = "_sys_cr_finished";
diff --git a/crash_reporter/chrome_collector.h b/crash_reporter/chrome_collector.h
index ab59407..c6dbfc9 100644
--- a/crash_reporter/chrome_collector.h
+++ b/crash_reporter/chrome_collector.h
@@ -19,9 +19,14 @@
   ChromeCollector();
   virtual ~ChromeCollector();
 
+  // Magic string to let Chrome know the crash report succeeded.
+  static const char kSuccessMagic[];
+
   // Handle a specific chrome crash.  Returns true on success.
-  bool HandleCrash(const std::string &file_path, const std::string &pid_string,
-                   const std::string &uid_string, const std::string &exe_name);
+  bool HandleCrash(const base::FilePath &file_path,
+                   const std::string &pid_string,
+                   const std::string &uid_string,
+                   const std::string &exe_name);
 
  private:
   friend class ChromeCollectorTest;
@@ -29,6 +34,7 @@
   FRIEND_TEST(ChromeCollectorTest, BadValues);
   FRIEND_TEST(ChromeCollectorTest, Newlines);
   FRIEND_TEST(ChromeCollectorTest, File);
+  FRIEND_TEST(ChromeCollectorTest, HandleCrash);
 
   // Crashes are expected to be in a TLV-style format of:
   // <name>:<length>:<value>
@@ -39,6 +45,8 @@
   bool ParseCrashLog(const std::string &data, const base::FilePath &dir,
                      const base::FilePath &minidump,
                      const std::string &basename);
+
+  FILE *output_file_ptr_;
 };
 
 #endif
diff --git a/crash_reporter/chrome_collector_test.cc b/crash_reporter/chrome_collector_test.cc
index 8a82cb2..37a180b 100644
--- a/crash_reporter/chrome_collector_test.cc
+++ b/crash_reporter/chrome_collector_test.cc
@@ -2,13 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <bits/wordsize.h>
-#include <elf.h>
-#include <unistd.h>
+#include <stdio.h>
 
+#include <dbus/dbus-glib-lowlevel.h>
+
+#include "base/auto_reset.h"
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/strings/string_split.h"
 #include "chromeos/syslog_logging.h"
 #include "chromeos/test_helpers.h"
 #include "crash-reporter/chrome_collector.h"
@@ -30,37 +30,30 @@
     "value3:2:ok";
 
 void CountCrash() {
-  static int s_crashes = 0;
-  ++s_crashes;
 }
 
+static bool s_allow_crash = false;
+
 bool IsMetrics() {
-  return false;
+  return s_allow_crash;
 }
 
 class ChromeCollectorTest : public ::testing::Test {
-  void SetUp() {
-    collector_.Initialize(CountCrash, IsMetrics);
-    pid_ = getpid();
-    chromeos::ClearLog();
-  }
-
  protected:
   void ExpectFileEquals(const char *golden,
-                        const char *file_path) {
+                        const FilePath &file_path) {
     std::string contents;
-    EXPECT_TRUE(base::ReadFileToString(FilePath(file_path), &contents));
+    EXPECT_TRUE(base::ReadFileToString(file_path, &contents));
     EXPECT_EQ(golden, contents);
   }
 
-  std::vector<std::string> SplitLines(const std::string &lines) const {
-    std::vector<std::string> result;
-    base::SplitString(lines, '\n', &result);
-    return result;
-  }
-
   ChromeCollector collector_;
-  pid_t pid_;
+
+ private:
+  void SetUp() OVERRIDE {
+    collector_.Initialize(CountCrash, IsMetrics);
+    chromeos::ClearLog();
+  }
 };
 
 TEST_F(ChromeCollectorTest, GoodValues) {
@@ -107,7 +100,9 @@
 }
 
 TEST_F(ChromeCollectorTest, File) {
-  FilePath dir(".");
+  base::ScopedTempDir scoped_temp_dir;
+  ASSERT_TRUE(scoped_temp_dir.CreateUniqueTempDir());
+  const FilePath& dir = scoped_temp_dir.path();
   EXPECT_TRUE(collector_.ParseCrashLog(kCrashFormatWithFile,
                                        dir, dir.Append("minidump.dmp"),
                                        "base"));
@@ -118,12 +113,34 @@
   EXPECT_TRUE(meta.find("value1=abcdefghij") != std::string::npos);
   EXPECT_TRUE(meta.find("value2=12345") != std::string::npos);
   EXPECT_TRUE(meta.find("value3=ok") != std::string::npos);
-  ExpectFileEquals("12345\n789\n12345",
-                   dir.Append("base-foo.txt.other").value().c_str());
-  base::DeleteFile(dir.Append("base-foo.txt.other"), false);
+  ExpectFileEquals("12345\n789\n12345", dir.Append("base-foo.txt.other"));
+}
+
+TEST_F(ChromeCollectorTest, HandleCrash) {
+  base::AutoReset<bool> auto_reset(&s_allow_crash, true);
+  base::ScopedTempDir scoped_temp_dir;
+  ASSERT_TRUE(scoped_temp_dir.CreateUniqueTempDir());
+  const FilePath& dir = scoped_temp_dir.path();
+  FilePath dump_file = dir.Append("test.dmp");
+  ASSERT_EQ(strlen(kCrashFormatWithFile),
+            file_util::WriteFile(dump_file, kCrashFormatWithFile,
+                                 strlen(kCrashFormatWithFile)));
+  collector_.ForceCrashDirectory(dir);
+
+  FilePath log_file;
+  {
+    file_util::ScopedFILE output(
+        base::CreateAndOpenTemporaryFileInDir(dir, &log_file));
+    ASSERT_TRUE(output.get());
+    base::AutoReset<FILE*> auto_reset_file_ptr(&collector_.output_file_ptr_,
+                                               output.get());
+    EXPECT_TRUE(collector_.HandleCrash(dump_file, "123", "456", "chrome_test"));
+  }
+  ExpectFileEquals(ChromeCollector::kSuccessMagic, log_file);
 }
 
 int main(int argc, char **argv) {
+  ::g_type_init();
   SetUpTests(&argc, argv, false);
   return RUN_ALL_TESTS();
 }
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index 0521a06..1a0898b 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -76,8 +76,7 @@
 using base::StringPrintf;
 
 CrashCollector::CrashCollector()
-    : forced_crash_directory_(NULL),
-      lsb_release_(kLsbRelease),
+    : lsb_release_(kLsbRelease),
       log_config_path_(kDefaultLogConfig) {
 }
 
@@ -262,8 +261,8 @@
   if (out_of_capacity != NULL) *out_of_capacity = false;
 
   // For testing.
-  if (forced_crash_directory_ != NULL) {
-    *crash_directory = FilePath(forced_crash_directory_);
+  if (!forced_crash_directory_.empty()) {
+    *crash_directory = forced_crash_directory_;
     return true;
   }
 
@@ -498,7 +497,7 @@
                                         const std::string &exec_name,
                                         const std::string &payload_path) {
   std::map<std::string, std::string> contents;
-  if (!ReadKeyValueFile(FilePath(std::string(lsb_release_)), '=', &contents)) {
+  if (!ReadKeyValueFile(FilePath(lsb_release_), '=', &contents)) {
     LOG(ERROR) << "Problem parsing " << lsb_release_;
     // Even though there was some failure, take as much as we could read.
   }
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
index f1cf32e..67ff870 100644
--- a/crash_reporter/crash_collector.h
+++ b/crash_reporter/crash_collector.h
@@ -32,6 +32,7 @@
 
  protected:
   friend class CrashCollectorTest;
+  FRIEND_TEST(ChromeCollectorTest, HandleCrash);
   FRIEND_TEST(CrashCollectorTest, CheckHasCapacityCorrectBasename);
   FRIEND_TEST(CrashCollectorTest, CheckHasCapacityStrangeNames);
   FRIEND_TEST(CrashCollectorTest, CheckHasCapacityUsual);
@@ -71,7 +72,7 @@
 
   // For testing, set the directory always returned by
   // GetCreatedCrashDirectoryByEuid.
-  void ForceCrashDirectory(const char *forced_directory) {
+  void ForceCrashDirectory(const base::FilePath &forced_directory) {
     forced_crash_directory_ = forced_directory;
   }
 
@@ -86,7 +87,8 @@
   bool GetUserInfoFromName(const std::string &name,
                            uid_t *uid,
                            gid_t *gid);
-  // Determines the crash directory for given eud, and creates the
+
+  // Determines the crash directory for given euid, and creates the
   // directory if necessary with appropriate permissions.  If
   // |out_of_capacity| is not NULL, it is set to indicate if the call
   // failed due to not having capacity in the crash directory. Returns
@@ -165,8 +167,8 @@
   CountCrashFunction count_crash_function_;
   IsFeedbackAllowedFunction is_feedback_allowed_function_;
   std::string extra_metadata_;
-  const char *forced_crash_directory_;
-  const char *lsb_release_;
+  base::FilePath forced_crash_directory_;
+  std::string lsb_release_;
   base::FilePath log_config_path_;
 };
 
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index 7d3c1fe..863fed3 100644
--- a/crash_reporter/crash_collector_test.cc
+++ b/crash_reporter/crash_collector_test.cc
@@ -34,8 +34,7 @@
 class CrashCollectorTest : public ::testing::Test {
  public:
   void SetUp() {
-    collector_.Initialize(CountCrash,
-                          IsMetrics);
+    collector_.Initialize(CountCrash, IsMetrics);
     test_dir_ = FilePath("test");
     base::CreateDirectory(test_dir_);
     chromeos::ClearLog();
@@ -281,7 +280,7 @@
   FilePath lsb_release = test_dir_.Append("lsb-release");
   FilePath payload_file = test_dir_.Append("payload-file");
   std::string contents;
-  collector_.lsb_release_ = lsb_release.value().c_str();
+  collector_.lsb_release_ = lsb_release.value();
   const char kLsbContents[] = "CHROMEOS_RELEASE_VERSION=version\n";
   ASSERT_TRUE(
       file_util::WriteFile(lsb_release,
diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc
index 1da1d87..828ec91 100644
--- a/crash_reporter/crash_reporter.cc
+++ b/crash_reporter/crash_reporter.cc
@@ -186,8 +186,8 @@
   CHECK(!FLAGS_exe.empty()) << "--exe= must be set";
 
   chromeos::LogToString(true);
-  bool handled = chrome_collector->HandleCrash(FLAGS_chrome, FLAGS_pid,
-                                               FLAGS_uid, FLAGS_exe);
+  bool handled = chrome_collector->HandleCrash(FilePath(FLAGS_chrome),
+                                               FLAGS_pid, FLAGS_uid, FLAGS_exe);
   chromeos::LogToString(false);
   if (!handled)
     return 1;
@@ -269,8 +269,7 @@
   ::g_type_init();
 
   KernelCollector kernel_collector;
-  kernel_collector.Initialize(CountKernelCrash,
-                              IsFeedbackAllowed);
+  kernel_collector.Initialize(CountKernelCrash, IsFeedbackAllowed);
   UserCollector user_collector;
   user_collector.Initialize(CountUserCrash,
                             my_path.value(),
diff --git a/crash_reporter/kernel_collector_test.cc b/crash_reporter/kernel_collector_test.cc
index dd6e5ba..14b8357 100644
--- a/crash_reporter/kernel_collector_test.cc
+++ b/crash_reporter/kernel_collector_test.cc
@@ -5,6 +5,7 @@
 #include <unistd.h>
 
 #include "base/file_util.h"
+#include "base/files/scoped_temp_dir.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chromeos/syslog_logging.h"
@@ -15,9 +16,6 @@
 static int s_crashes = 0;
 static bool s_metrics = false;
 
-static const char kTestKCrash[] = "test/kcrash";
-static const char kTestCrashDirectory[] = "test/crash_directory";
-
 using base::FilePath;
 using base::StringPrintf;
 using chromeos::FindLog;
@@ -32,24 +30,6 @@
 }
 
 class KernelCollectorTest : public ::testing::Test {
-  void SetUp() {
-    s_crashes = 0;
-    s_metrics = true;
-    collector_.Initialize(CountCrash,
-                          IsMetrics);
-    mkdir("test", 0777);
-    mkdir(kTestKCrash, 0777);
-    test_kcrash_ = FilePath(kTestKCrash);
-    collector_.OverridePreservedDumpPath(test_kcrash_);
-    test_kcrash_ = test_kcrash_.Append("dmesg-ramoops-0");
-    unlink(test_kcrash_.value().c_str());
-    if (mkdir(kTestCrashDirectory, 0777)) {
-      ASSERT_EQ(EEXIST, errno)
-          << "Error while creating directory '" << kTestCrashDirectory
-          << "': " << strerror(errno);
-    }
-    chromeos::ClearLog();
-  }
  protected:
   void WriteStringToFile(const FilePath &file_path,
                          const char *data) {
@@ -60,8 +40,32 @@
   void SetUpSuccessfulCollect();
   void ComputeKernelStackSignatureCommon();
 
+  const FilePath &kcrash_file() const { return test_kcrash_; }
+  const FilePath &test_crash_directory() const { return test_crash_directory_; }
+
   KernelCollector collector_;
+
+ private:
+  void SetUp() OVERRIDE {
+    s_crashes = 0;
+    s_metrics = true;
+    collector_.Initialize(CountCrash, IsMetrics);
+    ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
+    test_kcrash_ = scoped_temp_dir_.path().Append("kcrash");
+    ASSERT_TRUE(base::CreateDirectory(test_kcrash_));
+    collector_.OverridePreservedDumpPath(test_kcrash_);
+
+    test_kcrash_ = test_kcrash_.Append("dmesg-ramoops-0");
+    ASSERT_FALSE(base::PathExists(test_kcrash_));
+
+    test_crash_directory_ = scoped_temp_dir_.path().Append("crash_directory");
+    ASSERT_TRUE(base::CreateDirectory(test_crash_directory_));
+    chromeos::ClearLog();
+  }
+
   FilePath test_kcrash_;
+  FilePath test_crash_directory_;
+  base::ScopedTempDir scoped_temp_dir_;
 };
 
 TEST_F(KernelCollectorTest, ComputeKernelStackSignatureBase) {
@@ -70,16 +74,16 @@
 }
 
 TEST_F(KernelCollectorTest, LoadPreservedDump) {
-  ASSERT_FALSE(base::PathExists(test_kcrash_));
+  ASSERT_FALSE(base::PathExists(kcrash_file()));
   std::string dump;
   dump.clear();
 
-  WriteStringToFile(test_kcrash_, "emptydata");
+  WriteStringToFile(kcrash_file(), "emptydata");
   ASSERT_TRUE(collector_.LoadParameters());
   ASSERT_FALSE(collector_.LoadPreservedDump(&dump));
   ASSERT_EQ("", dump);
 
-  WriteStringToFile(test_kcrash_, "====1.1\nsomething");
+  WriteStringToFile(kcrash_file(), "====1.1\nsomething");
   ASSERT_TRUE(collector_.LoadParameters());
   ASSERT_TRUE(collector_.LoadPreservedDump(&dump));
   ASSERT_EQ("something", dump);
@@ -94,7 +98,7 @@
 }
 
 TEST_F(KernelCollectorTest, EnableOK) {
-  WriteStringToFile(test_kcrash_, "");
+  WriteStringToFile(kcrash_file(), "");
   ASSERT_TRUE(collector_.Enable());
   ASSERT_TRUE(collector_.IsEnabled());
   ASSERT_TRUE(FindLog("Enabling kernel crash handling"));
@@ -225,7 +229,7 @@
 }
 
 TEST_F(KernelCollectorTest, CollectNoCrash) {
-  WriteStringToFile(test_kcrash_, "");
+  WriteStringToFile(kcrash_file(), "");
   ASSERT_FALSE(collector_.Collect());
   ASSERT_TRUE(FindLog("No valid records found"));
   ASSERT_FALSE(FindLog("Stored kcrash to "));
@@ -233,7 +237,7 @@
 }
 
 TEST_F(KernelCollectorTest, CollectBadDirectory) {
-  WriteStringToFile(test_kcrash_, "====1.1\nsomething");
+  WriteStringToFile(kcrash_file(), "====1.1\nsomething");
   ASSERT_TRUE(collector_.Collect());
   ASSERT_TRUE(FindLog("Unable to create appropriate crash directory"))
       << "Did not find expected error string in log: {\n"
@@ -242,8 +246,8 @@
 }
 
 void KernelCollectorTest::SetUpSuccessfulCollect() {
-  collector_.ForceCrashDirectory(kTestCrashDirectory);
-  WriteStringToFile(test_kcrash_, "====1.1\nsomething");
+  collector_.ForceCrashDirectory(test_crash_directory());
+  WriteStringToFile(kcrash_file(), "====1.1\nsomething");
   ASSERT_EQ(0, s_crashes);
 }
 
@@ -272,7 +276,7 @@
   size_t end_pos = filename.find_first_of("\n");
   ASSERT_NE(std::string::npos, end_pos);
   filename = filename.substr(0, end_pos);
-  ASSERT_EQ(0, filename.find(kTestCrashDirectory));
+  ASSERT_EQ(0, filename.find(test_crash_directory().value()));
   ASSERT_TRUE(base::PathExists(FilePath(filename)));
   std::string contents;
   ASSERT_TRUE(base::ReadFileToString(FilePath(filename), &contents));
diff --git a/crash_reporter/udev_collector_test.cc b/crash_reporter/udev_collector_test.cc
index 6a5c856..db1550f 100644
--- a/crash_reporter/udev_collector_test.cc
+++ b/crash_reporter/udev_collector_test.cc
@@ -5,7 +5,6 @@
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/memory/scoped_ptr.h"
 #include "chromeos/test_helpers.h"
 #include "crash-reporter/udev_collector.h"
 #include "gtest/gtest.h"
@@ -47,21 +46,25 @@
 }  // namespace
 
 class UdevCollectorTest : public ::testing::Test {
-  void SetUp() {
+ protected:
+  base::ScopedTempDir temp_dir_generator_;
+
+  void HandleCrash(const std::string &udev_event) {
+    collector_.HandleCrash(udev_event);
+  }
+
+ private:
+  void SetUp() OVERRIDE {
     s_consent_given = true;
 
-    collector_.reset(new UdevCollector);
-    collector_->Initialize(CountCrash, IsMetrics);
+    collector_.Initialize(CountCrash, IsMetrics);
 
-    temp_dir_generator_.reset(new base::ScopedTempDir());
-    ASSERT_TRUE(temp_dir_generator_->CreateUniqueTempDir());
-    EXPECT_TRUE(temp_dir_generator_->IsValid());
+    ASSERT_TRUE(temp_dir_generator_.CreateUniqueTempDir());
 
     FilePath log_config_path =
-        temp_dir_generator_->path().Append(kLogConfigFileName);
-    collector_->log_config_path_ = log_config_path;
-    collector_->ForceCrashDirectory(
-        temp_dir_generator_->path().value().c_str());
+        temp_dir_generator_.path().Append(kLogConfigFileName);
+    collector_.log_config_path_ = log_config_path;
+    collector_.ForceCrashDirectory(temp_dir_generator_.path());
 
     // Write to a dummy log config file.
     ASSERT_EQ(strlen(kLogConfigFileContents),
@@ -72,30 +75,28 @@
     chromeos::ClearLog();
   }
 
- protected:
-  scoped_ptr<UdevCollector> collector_;
-  scoped_ptr<base::ScopedTempDir> temp_dir_generator_;
+  UdevCollector collector_;
 };
 
 TEST_F(UdevCollectorTest, TestNoConsent) {
   s_consent_given = false;
-  collector_->HandleCrash("ACTION=change:KERNEL=card0:SUBSYSTEM=drm");
-  EXPECT_EQ(0, GetNumLogFiles(temp_dir_generator_->path()));
+  HandleCrash("ACTION=change:KERNEL=card0:SUBSYSTEM=drm");
+  EXPECT_EQ(0, GetNumLogFiles(temp_dir_generator_.path()));
 }
 
 TEST_F(UdevCollectorTest, TestNoMatch) {
   // No rule should match this.
-  collector_->HandleCrash("ACTION=change:KERNEL=foo:SUBSYSTEM=bar");
-  EXPECT_EQ(0, GetNumLogFiles(temp_dir_generator_->path()));
+  HandleCrash("ACTION=change:KERNEL=foo:SUBSYSTEM=bar");
+  EXPECT_EQ(0, GetNumLogFiles(temp_dir_generator_.path()));
 }
 
 TEST_F(UdevCollectorTest, TestMatches) {
   // Try multiple udev events in sequence.  The number of log files generated
   // should increase.
-  collector_->HandleCrash("ACTION=change:KERNEL=card0:SUBSYSTEM=drm");
-  EXPECT_EQ(1, GetNumLogFiles(temp_dir_generator_->path()));
-  collector_->HandleCrash("ACTION=add:KERNEL=state0:SUBSYSTEM=cpu");
-  EXPECT_EQ(2, GetNumLogFiles(temp_dir_generator_->path()));
+  HandleCrash("ACTION=change:KERNEL=card0:SUBSYSTEM=drm");
+  EXPECT_EQ(1, GetNumLogFiles(temp_dir_generator_.path()));
+  HandleCrash("ACTION=add:KERNEL=state0:SUBSYSTEM=cpu");
+  EXPECT_EQ(2, GetNumLogFiles(temp_dir_generator_.path()));
 }
 
 // TODO(sque, crosbug.com/32238) - test wildcard cases, multiple identical udev
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index eed6daf..0dd35e0 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -52,10 +52,9 @@
 
  protected:
   void ExpectFileEquals(const char *golden,
-                        const char *file_path) {
+                        const FilePath &file_path) {
     std::string contents;
-    EXPECT_TRUE(base::ReadFileToString(FilePath(file_path),
-                                            &contents));
+    EXPECT_TRUE(base::ReadFileToString(file_path, &contents));
     EXPECT_EQ(golden, contents);
   }
 
@@ -71,8 +70,9 @@
 
 TEST_F(UserCollectorTest, EnableOK) {
   ASSERT_TRUE(collector_.Enable());
-  ExpectFileEquals("|/my/path --user=%P:%s:%u:%e", "test/core_pattern");
-  ExpectFileEquals("4", "test/core_pipe_limit");
+  ExpectFileEquals("|/my/path --user=%P:%s:%u:%e",
+                   FilePath("test/core_pattern"));
+  ExpectFileEquals("4", FilePath("test/core_pipe_limit"));
   ASSERT_EQ(s_crashes, 0);
   EXPECT_TRUE(FindLog("Enabling user crash handling"));
 }
@@ -98,7 +98,7 @@
 
 TEST_F(UserCollectorTest, DisableOK) {
   ASSERT_TRUE(collector_.Disable());
-  ExpectFileEquals("core", "test/core_pattern");
+  ExpectFileEquals("core", FilePath("test/core_pattern"));
   ASSERT_EQ(s_crashes, 0);
   EXPECT_TRUE(FindLog("Disabling user crash handling"));
 }