Collect and send kernel crash diagnostics

BUG=1512,1914

Review URL: http://codereview.chromium.org/3179006
diff --git a/crash_reporter/Makefile b/crash_reporter/Makefile
index b51743d..fc2d680 100644
--- a/crash_reporter/Makefile
+++ b/crash_reporter/Makefile
@@ -4,9 +4,18 @@
 
 CRASH_REPORTER = crash_reporter
 REPORTER_BINS = $(CRASH_REPORTER)
-CRASH_OBJS = system_logging.o user_collector.o
+CRASH_OBJS = \
+	crash_collector.o \
+	kernel_collector.o \
+	system_logging.o \
+	unclean_shutdown_collector.o \
+	user_collector.o
 TEST_OBJS = $(CRASH_OBJS) system_logging_mock.o
-TEST_BINS = user_collector_test
+TEST_BINS = \
+	crash_collector_test \
+	kernel_collector_test \
+	unclean_shutdown_collector_test \
+	user_collector_test
 
 # -lglib-2.0 is needed by libbase.a now.
 COMMON_LIBS = -lbase -lpthread -lglib-2.0 -lgflags -lrt
@@ -24,7 +33,7 @@
 
 tests: $(TEST_BINS)
 
-user_collector_test: user_collector_test.o $(TEST_OBJS)
+%_test: %_test.o $(TEST_OBJS)
 	$(CXX) $(CXXFLAGS) $(LIB_DIRS) $^ $(TEST_LIBS) -o $@
 
 .cc.o:
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
new file mode 100644
index 0000000..492f2e8
--- /dev/null
+++ b/crash_reporter/crash_collector.cc
@@ -0,0 +1,150 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "crash-reporter/crash_collector.h"
+
+#include <pwd.h>  // For struct passwd.
+#include <sys/types.h>  // for mode_t.
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/string_util.h"
+#include "crash-reporter/system_logging.h"
+
+static const char kDefaultUserName[] = "chronos";
+static const char kSystemCrashPath[] = "/var/spool/crash";
+static const char kUserCrashPath[] = "/home/chronos/user/crash";
+
+// Directory mode of the user crash spool directory.
+static const mode_t kUserCrashPathMode = 0755;
+
+// Directory mode of the system crash spool directory.
+static const mode_t kSystemCrashPathMode = 01755;
+
+static const uid_t kRootOwner = 0;
+static const uid_t kRootGroup = 0;
+
+CrashCollector::CrashCollector() : forced_crash_directory_(NULL) {
+}
+
+CrashCollector::~CrashCollector() {
+}
+
+void CrashCollector::Initialize(
+    CrashCollector::CountCrashFunction count_crash_function,
+    CrashCollector::IsFeedbackAllowedFunction is_feedback_allowed_function,
+    SystemLogging *logger) {
+  CHECK(count_crash_function != NULL);
+  CHECK(is_feedback_allowed_function != NULL);
+  CHECK(logger != NULL);
+
+  count_crash_function_ = count_crash_function;
+  is_feedback_allowed_function_ = is_feedback_allowed_function;
+  logger_ = logger;
+}
+
+std::string CrashCollector::FormatDumpBasename(const std::string &exec_name,
+                                               time_t timestamp,
+                                               pid_t pid) {
+  struct tm tm;
+  localtime_r(&timestamp, &tm);
+  return StringPrintf("%s.%04d%02d%02d.%02d%02d%02d.%d",
+                      exec_name.c_str(),
+                      tm.tm_year + 1900,
+                      tm.tm_mon + 1,
+                      tm.tm_mday,
+                      tm.tm_hour,
+                      tm.tm_min,
+                      tm.tm_sec,
+                      pid);
+}
+
+FilePath CrashCollector::GetCrashDirectoryInfo(
+    uid_t process_euid,
+    uid_t default_user_id,
+    gid_t default_user_group,
+    mode_t *mode,
+    uid_t *directory_owner,
+    gid_t *directory_group) {
+  if (process_euid == default_user_id) {
+    *mode = kUserCrashPathMode;
+    *directory_owner = default_user_id;
+    *directory_group = default_user_group;
+    return FilePath(kUserCrashPath);
+  } else {
+    *mode = kSystemCrashPathMode;
+    *directory_owner = kRootOwner;
+    *directory_group = kRootGroup;
+    return FilePath(kSystemCrashPath);
+  }
+}
+
+bool CrashCollector::GetUserInfoFromName(const std::string &name,
+                                         uid_t *uid,
+                                         gid_t *gid) {
+  char storage[256];
+  struct passwd passwd_storage;
+  struct passwd *passwd_result = NULL;
+
+  if (getpwnam_r(name.c_str(), &passwd_storage, storage, sizeof(storage),
+                 &passwd_result) != 0 || passwd_result == NULL) {
+    logger_->LogError("Cannot find user named %s", name.c_str());
+    return false;
+  }
+
+  *uid = passwd_result->pw_uid;
+  *gid = passwd_result->pw_gid;
+  return true;
+}
+
+bool CrashCollector::GetCreatedCrashDirectoryByEuid(uid_t euid,
+                                                    FilePath *crash_directory) {
+  uid_t default_user_id;
+  gid_t default_user_group;
+
+  // For testing.
+  if (forced_crash_directory_ != NULL) {
+    *crash_directory = FilePath(forced_crash_directory_);
+    return true;
+  }
+
+  if (!GetUserInfoFromName(kDefaultUserName,
+                           &default_user_id,
+                           &default_user_group)) {
+    logger_->LogError("Could not find default user info");
+    return false;
+  }
+  mode_t directory_mode;
+  uid_t directory_owner;
+  gid_t directory_group;
+  *crash_directory =
+      GetCrashDirectoryInfo(euid,
+                            default_user_id,
+                            default_user_group,
+                            &directory_mode,
+                            &directory_owner,
+                            &directory_group);
+
+  if (!file_util::PathExists(*crash_directory)) {
+    // Create the spool directory with the appropriate mode (regardless of
+    // umask) and ownership.
+    mode_t old_mask = umask(0);
+    if (mkdir(crash_directory->value().c_str(), directory_mode) < 0 ||
+        chown(crash_directory->value().c_str(),
+              directory_owner,
+              directory_group) < 0) {
+      logger_->LogError("Unable to create appropriate crash directory");
+      return false;
+    }
+    umask(old_mask);
+  }
+
+  if (!file_util::PathExists(*crash_directory)) {
+    logger_->LogError("Unable to create crash directory %s",
+                      crash_directory->value().c_str());
+    return false;
+  }
+
+  return true;
+}
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
new file mode 100644
index 0000000..ef0d2a7
--- /dev/null
+++ b/crash_reporter/crash_collector.h
@@ -0,0 +1,70 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef _CRASH_REPORTER_CRASH_COLLECTOR_H_
+#define _CRASH_REPORTER_CRASH_COLLECTOR_H_
+
+#include <string>
+#include <sys/stat.h>
+
+#include "gtest/gtest_prod.h"  // for FRIEND_TEST
+
+class FilePath;
+class SystemLogging;
+
+// User crash collector.
+class CrashCollector {
+ public:
+  typedef void (*CountCrashFunction)();
+  typedef bool (*IsFeedbackAllowedFunction)();
+
+  CrashCollector();
+
+  virtual ~CrashCollector();
+
+  // Initialize the crash collector for detection of crashes, given a
+  // crash counting function, metrics collection enabled oracle, and
+  // system logger facility.
+  void Initialize(CountCrashFunction count_crash,
+                  IsFeedbackAllowedFunction is_metrics_allowed,
+                  SystemLogging *logger);
+
+ protected:
+  friend class CrashCollectorTest;
+  FRIEND_TEST(CrashCollectorTest, GetCrashDirectoryInfo);
+  FRIEND_TEST(CrashCollectorTest, FormatDumpBasename);
+  FRIEND_TEST(CrashCollectorTest, Initialize);
+
+  // For testing, set the directory always returned by
+  // GetCreatedCrashDirectoryByEuid.
+  void ForceCrashDirectory(const char *forced_directory) {
+    forced_crash_directory_ = forced_directory;
+  }
+
+  FilePath GetCrashDirectoryInfo(uid_t process_euid,
+                                 uid_t default_user_id,
+                                 gid_t default_user_group,
+                                 mode_t *mode,
+                                 uid_t *directory_owner,
+                                 gid_t *directory_group);
+  bool GetUserInfoFromName(const std::string &name,
+                           uid_t *uid,
+                           gid_t *gid);
+  // Determines the crash directory for given eud, and creates the
+  // directory if necessary with appropriate permissions.  Returns
+  // true whether or not directory needed to be created, false on any
+  // failure.
+  bool GetCreatedCrashDirectoryByEuid(uid_t euid,
+                                      FilePath *crash_file_path);
+  std::string FormatDumpBasename(const std::string &exec_name,
+                                 time_t timestamp,
+                                 pid_t pid);
+
+  CountCrashFunction count_crash_function_;
+  IsFeedbackAllowedFunction is_feedback_allowed_function_;
+  SystemLogging *logger_;
+  const char *forced_crash_directory_;
+};
+
+#endif  // _CRASH_REPORTER_CRASH_COLLECTOR_H_
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
new file mode 100644
index 0000000..48b7fa0
--- /dev/null
+++ b/crash_reporter/crash_collector_test.cc
@@ -0,0 +1,105 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <unistd.h>
+
+#include "base/file_util.h"
+#include "crash-reporter/crash_collector.h"
+#include "crash-reporter/system_logging_mock.h"
+#include "gflags/gflags.h"
+#include "gtest/gtest.h"
+
+void CountCrash() {
+  ADD_FAILURE();
+}
+
+bool IsMetrics() {
+  ADD_FAILURE();
+  return false;
+}
+
+class CrashCollectorTest : public ::testing::Test {
+  void SetUp() {
+    collector_.Initialize(CountCrash,
+                          IsMetrics,
+                          &logging_);
+  }
+ protected:
+  SystemLoggingMock logging_;
+  CrashCollector collector_;
+  pid_t pid_;
+};
+
+TEST_F(CrashCollectorTest, Initialize) {
+  ASSERT_TRUE(CountCrash == collector_.count_crash_function_);
+  ASSERT_TRUE(IsMetrics == collector_.is_feedback_allowed_function_);
+  ASSERT_TRUE(&logging_ == collector_.logger_);
+}
+
+TEST_F(CrashCollectorTest, GetCrashDirectoryInfo) {
+  FilePath path;
+  const int kRootUid = 0;
+  const int kRootGid = 0;
+  const int kNtpUid = 5;
+  const int kChronosUid = 1000;
+  const int kChronosGid = 1001;
+  const mode_t kExpectedSystemMode = 01755;
+  const mode_t kExpectedUserMode = 0755;
+
+  mode_t directory_mode;
+  uid_t directory_owner;
+  gid_t directory_group;
+
+  path = collector_.GetCrashDirectoryInfo(kRootUid,
+                                          kChronosUid,
+                                          kChronosGid,
+                                          &directory_mode,
+                                          &directory_owner,
+                                          &directory_group);
+  EXPECT_EQ("/var/spool/crash", path.value());
+  EXPECT_EQ(kExpectedSystemMode, directory_mode);
+  EXPECT_EQ(kRootUid, directory_owner);
+  EXPECT_EQ(kRootGid, directory_group);
+
+  path = collector_.GetCrashDirectoryInfo(kNtpUid,
+                                          kChronosUid,
+                                          kChronosGid,
+                                          &directory_mode,
+                                          &directory_owner,
+                                          &directory_group);
+  EXPECT_EQ("/var/spool/crash", path.value());
+  EXPECT_EQ(kExpectedSystemMode, directory_mode);
+  EXPECT_EQ(kRootUid, directory_owner);
+  EXPECT_EQ(kRootGid, directory_group);
+
+  path = collector_.GetCrashDirectoryInfo(kChronosUid,
+                                          kChronosUid,
+                                          kChronosGid,
+                                          &directory_mode,
+                                          &directory_owner,
+                                          &directory_group);
+  EXPECT_EQ("/home/chronos/user/crash", path.value());
+  EXPECT_EQ(kExpectedUserMode, directory_mode);
+  EXPECT_EQ(kChronosUid, directory_owner);
+  EXPECT_EQ(kChronosGid, directory_group);
+}
+
+TEST_F(CrashCollectorTest, FormatDumpBasename) {
+  struct tm tm = {0};
+  tm.tm_sec = 15;
+  tm.tm_min = 50;
+  tm.tm_hour = 13;
+  tm.tm_mday = 23;
+  tm.tm_mon = 4;
+  tm.tm_year = 110;
+  tm.tm_isdst = -1;
+  std::string basename =
+      collector_.FormatDumpBasename("foo", mktime(&tm), 100);
+  ASSERT_EQ("foo.20100523.135015.100", basename);
+}
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc
index 0930cea..e850e30 100644
--- a/crash_reporter/crash_reporter.cc
+++ b/crash_reporter/crash_reporter.cc
@@ -7,7 +7,9 @@
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
+#include "crash-reporter/kernel_collector.h"
 #include "crash-reporter/system_logging.h"
+#include "crash-reporter/unclean_shutdown_collector.h"
 #include "crash-reporter/user_collector.h"
 #include "gflags/gflags.h"
 #include "metrics/metrics_library.h"
@@ -22,69 +24,52 @@
 #pragma GCC diagnostic error "-Wstrict-aliasing"
 
 static const char kCrashCounterHistogram[] = "Logging.CrashCounter";
-static const char kEmpty[] = "";
+static const char kUserCrashSignal[] =
+    "org.chromium.CrashReporter.UserCrash";
 static const char kUncleanShutdownFile[] =
     "/var/lib/crash_reporter/pending_clean_shutdown";
 
 
 // Enumeration of kinds of crashes to be used in the CrashCounter histogram.
 enum CrashKinds {
-  kCrashKindKernel = 1,
-  kCrashKindUser   = 2,
+  kCrashKindUncleanShutdown = 1,
+  kCrashKindUser = 2,
+  kCrashKindKernel = 3,
   kCrashKindMax
 };
 
 static MetricsLibrary s_metrics_lib;
 static SystemLoggingImpl s_system_log;
 
-static bool IsMetricsCollectionAllowed() {
-  // TODO(kmixter): Eventually check system tainted state and
-  // move this down in metrics library where it would be explicitly
-  // checked when asked to send stats.
+static bool IsFeedbackAllowed() {
+  // Once crosbug.com/5814 is fixed, call the is opted in function
+  // here.
   return true;
 }
 
-static void CheckUncleanShutdown() {
-  FilePath unclean_file_path(kUncleanShutdownFile);
-  if (!file_util::PathExists(unclean_file_path)) {
-    return;
-  }
-  s_system_log.LogWarning("Last shutdown was not clean");
-  if (IsMetricsCollectionAllowed()) {
-    s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram),
-                                kCrashKindKernel,
-                                kCrashKindMax);
-  }
-  if (!file_util::Delete(unclean_file_path, false)) {
-    s_system_log.LogError("Failed to delete unclean shutdown file %s",
-                          kUncleanShutdownFile);
-  }
-
-  // Touch a file to notify the metrics daemon that a kernel crash has
-  // been detected so that it can log the time since the last kernel
-  // crash.
-  static const char kKernelCrashDetectedFile[] = "/tmp/kernel-crash-detected";
-  FilePath crash_detected(kKernelCrashDetectedFile);
-  file_util::WriteFile(crash_detected, kEmpty, 0);
+static bool TouchFile(const FilePath &file_path) {
+  return file_util::WriteFile(file_path, "", 0) == 0;
 }
 
-static bool PrepareUncleanShutdownCheck() {
-  FilePath file_path(kUncleanShutdownFile);
-  file_util::CreateDirectory(file_path.DirName());
-  return file_util::WriteFile(file_path, kEmpty, 0) == 0;
+static void CountKernelCrash() {
+  s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram),
+                              kCrashKindKernel,
+                              kCrashKindMax);
 }
 
-static void SignalCleanShutdown() {
-  s_system_log.LogInfo("Clean shutdown signalled");
-  file_util::Delete(FilePath(kUncleanShutdownFile), false);
+static void CountUncleanShutdown() {
+  s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram),
+                              kCrashKindUncleanShutdown,
+                              kCrashKindMax);
 }
 
 static void CountUserCrash() {
-  CHECK(IsMetricsCollectionAllowed());
   s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram),
                               kCrashKindUser,
                               kCrashKindMax);
-
+  std::string command = StringPrintf(
+      "/usr/bin/dbus-send --type=signal --system / \"%s\"",
+      kUserCrashSignal);
   // Announce through D-Bus whenever a user crash happens. This is
   // used by the metrics daemon to log active use time between
   // crashes.
@@ -93,42 +78,47 @@
   // using a dbus library directly. However, this should run
   // relatively rarely and longer term we may need to implement a
   // better way to do this that doesn't rely on D-Bus.
-  int status __attribute__((unused)) =
-      system("/usr/bin/dbus-send --type=signal --system / "
-             "org.chromium.CrashReporter.UserCrash");
+
+  int status __attribute__((unused)) = system(command.c_str());
 }
 
-int main(int argc, char *argv[]) {
-  google::ParseCommandLineFlags(&argc, &argv, true);
-  FilePath my_path(argv[0]);
-  file_util::AbsolutePath(&my_path);
-  s_metrics_lib.Init();
-  s_system_log.Initialize(my_path.BaseName().value().c_str());
-  UserCollector user_collector;
-  user_collector.Initialize(CountUserCrash,
-                            my_path.value(),
-                            IsMetricsCollectionAllowed,
-                            &s_system_log,
-                            true);  // generate_diagnostics
+static int Initialize(KernelCollector *kernel_collector,
+                      UserCollector *user_collector,
+                      UncleanShutdownCollector *unclean_shutdown_collector) {
+  CHECK(!FLAGS_clean_shutdown) << "Incompatible options";
 
-  if (FLAGS_init) {
-    CHECK(!FLAGS_clean_shutdown) << "Incompatible options";
-    user_collector.Enable();
-    if (FLAGS_unclean_check) {
-      CheckUncleanShutdown();
-      if (!PrepareUncleanShutdownCheck()) {
-        s_system_log.LogError("Unable to create shutdown check file");
-      }
+  bool was_kernel_crash = false;
+  bool was_unclean_shutdown = false;
+  if (kernel_collector->IsEnabled()) {
+    was_kernel_crash = kernel_collector->Collect();
+  }
+
+  if (FLAGS_unclean_check) {
+    was_unclean_shutdown = unclean_shutdown_collector->Collect();
+  }
+
+  // Touch a file to notify the metrics daemon that a kernel
+  // crash has been detected so that it can log the time since
+  // the last kernel crash.
+  if (IsFeedbackAllowed()) {
+    if (was_kernel_crash) {
+      TouchFile(FilePath("/tmp/kernel-crash-detected"));
+    } else if (was_unclean_shutdown) {
+      // We only count an unclean shutdown if it did not come with
+      // an associated kernel crash.
+      TouchFile(FilePath("/tmp/unclean-shutdown-detected"));
     }
-    return 0;
   }
 
-  if (FLAGS_clean_shutdown) {
-    SignalCleanShutdown();
-    user_collector.Disable();
-    return 0;
-  }
+  // Must enable the unclean shutdown collector *after* collecting.
+  kernel_collector->Enable();
+  unclean_shutdown_collector->Enable();
+  user_collector->Enable();
 
+  return 0;
+}
+
+static int HandleUserCrash(UserCollector *user_collector) {
   // Handle a specific user space crash.
   CHECK(FLAGS_signal != -1) << "Signal must be set";
   CHECK(FLAGS_pid != -1) << "PID must be set";
@@ -141,9 +131,45 @@
   }
 
   // Handle the crash, get the name of the process from procfs.
-  if (!user_collector.HandleCrash(FLAGS_signal, FLAGS_pid, NULL)) {
+  if (!user_collector->HandleCrash(FLAGS_signal, FLAGS_pid, NULL)) {
     return 1;
   }
-
   return 0;
 }
+
+
+int main(int argc, char *argv[]) {
+  google::ParseCommandLineFlags(&argc, &argv, true);
+  FilePath my_path(argv[0]);
+  file_util::AbsolutePath(&my_path);
+  s_metrics_lib.Init();
+  s_system_log.Initialize(my_path.BaseName().value().c_str());
+  KernelCollector kernel_collector;
+  kernel_collector.Initialize(CountKernelCrash,
+                              IsFeedbackAllowed,
+                              &s_system_log);
+  UserCollector user_collector;
+  user_collector.Initialize(CountUserCrash,
+                            my_path.value(),
+                            IsFeedbackAllowed,
+                            &s_system_log,
+                            true);  // generate_diagnostics
+  UncleanShutdownCollector unclean_shutdown_collector;
+  unclean_shutdown_collector.Initialize(CountUncleanShutdown,
+                                        IsFeedbackAllowed,
+                                        &s_system_log);
+
+  if (FLAGS_init) {
+    return Initialize(&kernel_collector,
+                      &user_collector,
+                      &unclean_shutdown_collector);
+  }
+
+  if (FLAGS_clean_shutdown) {
+    unclean_shutdown_collector.Disable();
+    user_collector.Disable();
+    return 0;
+  }
+
+  return HandleUserCrash(&user_collector);
+}
diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender
index 99cc0de..5ada4ef 100644
--- a/crash_reporter/crash_sender
+++ b/crash_reporter/crash_sender
@@ -19,18 +19,19 @@
 # Send up to 8 crashes per day.
 MAX_CRASH_RATE=8
 
-# URL to send non-official build crashes to.
-MINIDUMP_UPLOAD_STAGING_URL="http://clients2.google.com/cr/staging_report"
-
-# URL to send official build crashes to.
-MINIDUMP_UPLOAD_PROD_URL="http://clients2.google.com/cr/report"
-
 # File whose existence mocks crash sending.  If empty we pretend the
 # crash sending was successful, otherwise unsuccessful.
 MOCK_CRASH_SENDING="/tmp/mock-crash-sending"
 
 # File whose existence causes crash sending to be delayed (for testing).
-PAUSE_CRASH_SENDING="/tmp/pause-crash-sending"
+# Must be stateful to enable testing kernel crashes.
+PAUSE_CRASH_SENDING="/var/lib/crash_sender_paused"
+
+# URL to send non-official build crash reports to.
+REPORT_UPLOAD_STAGING_URL="http://clients2.google.com/cr/staging_report"
+
+# URL to send official build crash reports to.
+REPORT_UPLOAD_PROD_URL="http://clients2.google.com/cr/report"
 
 # File whose existence implies we're running and not to start again.
 RUN_FILE="/var/run/crash_sender.pid"
@@ -118,31 +119,35 @@
 }
 
 # Return if $1 is a .core file
-is_core_file() {
-  local filename=$1
-  [ "${filename##*.}" = "core" ]
+get_kind() {
+  local kind="${1##*.}"
+  if [ "${kind}" = "dmp" ]; then
+    kind="minidump"
+  fi
+  echo ${kind}
+}
+
+get_exec_name() {
+  local filename=$(basename "$1")
+  echo "${filename%%.*}"
 }
 
 send_crash() {
+  local report_path="$1"
+  local kind=$(get_kind "${report_path}")
+  local exec_name=$(get_exec_name "${report_path}")
   local sleep_time=$(generate_uniform_random $SECONDS_SEND_SPREAD)
-  local url="${MINIDUMP_UPLOAD_STAGING_URL}"
+  local url="${REPORT_UPLOAD_STAGING_URL}"
   if is_official; then
-    url="${MINIDUMP_UPLOAD_PROD_URL}"
+    url="${REPORT_UPLOAD_PROD_URL}"
   fi
   lecho "Sending crash:"
   lecho "  Scheduled to send in ${sleep_time}s"
-  lecho "  Minidump: ${minidump_path}"
+  lecho "  Report: ${report_path} (${kind})"
   lecho "  URL: ${url}"
   lecho "  Product: ${CHROMEOS_PRODUCT}"
   lecho "  Version: ${chromeos_version}"
-  if [ -s "${minidump_path}" ]; then
-    # We cannot tell much from the minidump without symbols, but we can tell
-    # at least what modules were loaded at the time of crash
-    local modules="$(/usr/bin/minidump_dump "${minidump_path}" 2>&- | \
-                     grep 'code_file' | sed -e 's/^.* = "//g;s/"//g' | \
-                     tr '\n' ' ')"
-    lecho "  Mapped: ${modules}"
-  fi
+  lecho "  Exec name: ${exec_name}"
   if [ -f "${MOCK_CRASH_SENDING}" ]; then
     local mock_in=$(cat "${MOCK_CRASH_SENDING}")
     if [ "${mock_in}" = "" ]; then
@@ -166,9 +171,10 @@
   curl "${url}" \
     -F "prod=${CHROMEOS_PRODUCT}" \
     -F "ver=${chromeos_version}" \
-    -F "upload_file_minidump=@${minidump_path}" \
+    -F "upload_file_${kind}=@${report_path}" \
+    -F "exec_name=${exec_name}" \
     -F "guid=<${CONSENT_ID}" -o "${report_id}" 2>"${curl_stderr}"
-  local curl_result=$?
+  curl_result=$?
   set -e
 
   if [ ${curl_result} -eq 0 ]; then
@@ -194,28 +200,34 @@
     return
   fi
   for file in $(ls -1t "${dir}"); do
-    local minidump_path="${dir}/${file}"
-    lecho "Considering file ${minidump_path}"
-    if is_core_file "${minidump_path}"; then
+    local report_path="${dir}/${file}"
+    lecho "Considering file ${report_path}"
+    local kind=$(get_kind "${report_path}")
+
+    if [ "${kind}" = "core" ]; then
       lecho "Ignoring core file."
       continue
+    elif [ "${kind}" != "minidump" ] && [ "${kind}" != "kcrash" ]; then
+      lecho "Unknown report kind: ${kind}.  Removing report."
+      rm -f "${report_path}"
+      continue
     fi
     if ! check_rate; then
-      lecho "Sending ${minidump_path} would exceed rate.  Leaving for later."
+      lecho "Sending ${report_path} would exceed rate.  Leaving for later."
       return 0
     fi
     local chromeos_version=$(get_version)
     if is_feedback_disabled; then
       lecho "Uploading is disabled.  Removing crash."
-      rm "${minidump_path}"
+      rm "${report_path}"
     elif is_on_3g; then
       lecho "Not sending crash report while on 3G, saving for later."
-    elif send_crash ${minidump_path}; then
+    elif send_crash "${report_path}"; then
       # Send was successful, now remove
-      lecho "Successfully sent crash ${minidump_path} and removing"
-      rm "${minidump_path}"
+      lecho "Successfully sent crash ${report_path} and removing"
+      rm "${report_path}"
     else
-      lecho "Problem sending ${minidump_path}, not removing"
+      lecho "Problem sending ${report_path}, not removing"
     fi
   done
 }
diff --git a/crash_reporter/kernel_collector.cc b/crash_reporter/kernel_collector.cc
new file mode 100644
index 0000000..1a79c9d
--- /dev/null
+++ b/crash_reporter/kernel_collector.cc
@@ -0,0 +1,118 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "crash-reporter/kernel_collector.h"
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/string_util.h"
+#include "crash-reporter/system_logging.h"
+
+static const char kKernelExecName[] = "kernel";
+static const char kPreservedDumpPath[] = "/sys/kernel/debug/preserved/kcrash";
+const pid_t kKernelPid = 0;
+const uid_t kRootUid = 0;
+
+const char KernelCollector::kClearingSequence[] = " ";
+
+KernelCollector::KernelCollector()
+    : is_enabled_(false),
+      preserved_dump_path_(kPreservedDumpPath) {
+}
+
+KernelCollector::~KernelCollector() {
+}
+
+void KernelCollector::OverridePreservedDumpPath(const FilePath &file_path) {
+  preserved_dump_path_ = file_path;
+}
+
+bool KernelCollector::LoadPreservedDump(std::string *contents) {
+  // clear contents since ReadFileToString actually appends to the string.
+  contents->clear();
+  if (!file_util::ReadFileToString(preserved_dump_path_, contents)) {
+    logger_->LogError("Unable to read %s",
+                      preserved_dump_path_.value().c_str());
+    return false;
+  }
+  return true;
+}
+
+bool KernelCollector::Enable() {
+  if (!file_util::PathExists(preserved_dump_path_)) {
+    logger_->LogWarning("Kernel does not support crash dumping");
+    return false;
+  }
+
+  // To enable crashes, we will eventually need to set
+  // the chnv bit in BIOS, but it does not yet work.
+  logger_->LogInfo("Enabling kernel crash handling");
+  is_enabled_ = true;
+  return true;
+}
+
+bool KernelCollector::ClearPreservedDump() {
+  // It is necessary to write at least one byte to the kcrash file for
+  // the log to actually be cleared.
+  if (file_util::WriteFile(
+          preserved_dump_path_,
+          kClearingSequence,
+          strlen(kClearingSequence)) != strlen(kClearingSequence)) {
+    logger_->LogError("Failed to clear kernel crash dump");
+    return false;
+  }
+  logger_->LogInfo("Cleared kernel crash diagnostics");
+  return true;
+}
+
+FilePath KernelCollector::GetKernelCrashPath(
+    const FilePath &root_crash_path,
+    time_t timestamp) {
+  std::string dump_basename =
+      FormatDumpBasename(kKernelExecName,
+                         timestamp,
+                         kKernelPid);
+  return root_crash_path.Append(
+      StringPrintf("%s.kcrash", dump_basename.c_str()));
+}
+
+bool KernelCollector::Collect() {
+  std::string kernel_dump;
+  FilePath root_crash_directory;
+  if (!LoadPreservedDump(&kernel_dump)) {
+    return false;
+  }
+  if (kernel_dump.empty()) {
+    return false;
+  }
+  if (is_feedback_allowed_function_()) {
+    count_crash_function_();
+
+    if (!GetCreatedCrashDirectoryByEuid(kRootUid,
+                                        &root_crash_directory)) {
+      return true;
+    }
+
+    FilePath kernel_crash_path = GetKernelCrashPath(root_crash_directory,
+                                                    time(NULL));
+    if (file_util::WriteFile(kernel_crash_path,
+                             kernel_dump.data(),
+                             kernel_dump.length()) !=
+        static_cast<int>(kernel_dump.length())) {
+      logger_->LogInfo("Failed to write kernel dump to %s",
+                       kernel_crash_path.value().c_str());
+      return true;
+    }
+
+    logger_->LogInfo("Collected kernel crash diagnostics into %s",
+                     kernel_crash_path.value().c_str());
+  } else {
+    logger_->LogInfo("Crash not saved since metrics disabled");
+  }
+  if (!ClearPreservedDump()) {
+    return false;
+  }
+
+  return true;
+}
diff --git a/crash_reporter/kernel_collector.h b/crash_reporter/kernel_collector.h
new file mode 100644
index 0000000..3e29442
--- /dev/null
+++ b/crash_reporter/kernel_collector.h
@@ -0,0 +1,54 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef _CRASH_REPORTER_KERNEL_COLLECTOR_H_
+#define _CRASH_REPORTER_KERNEL_COLLECTOR_H_
+
+#include <string>
+
+#include "base/file_path.h"
+#include "crash-reporter/crash_collector.h"
+#include "gtest/gtest_prod.h"  // for FRIEND_TEST
+
+class FilePath;
+
+// Kernel crash collector.
+class KernelCollector : public CrashCollector {
+ public:
+  KernelCollector();
+
+  virtual ~KernelCollector();
+
+  void OverridePreservedDumpPath(const FilePath &file_path);
+
+  // Enable collection.
+  bool Enable();
+
+  // Returns true if the kernel collection currently enabled.
+  bool IsEnabled() {
+    return is_enabled_;
+  }
+
+  // Collect any preserved kernel crash dump. Returns true if there was
+  // a dump (even if there were problems storing the dump), false otherwise.
+  bool Collect();
+
+ private:
+  friend class KernelCollectorTest;
+  FRIEND_TEST(KernelCollectorTest, ClearPreservedDump);
+  FRIEND_TEST(KernelCollectorTest, GetKernelCrashPath);
+  FRIEND_TEST(KernelCollectorTest, LoadPreservedDump);
+  FRIEND_TEST(KernelCollectorTest, CollectOK);
+
+  bool LoadPreservedDump(std::string *contents);
+  bool ClearPreservedDump();
+  FilePath GetKernelCrashPath(const FilePath &root_crash_path,
+                              time_t timestamp);
+
+  bool is_enabled_;
+  FilePath preserved_dump_path_;
+  static const char kClearingSequence[];
+};
+
+#endif  // _CRASH_REPORTER_KERNEL_COLLECTOR_H_
diff --git a/crash_reporter/kernel_collector_test.cc b/crash_reporter/kernel_collector_test.cc
new file mode 100644
index 0000000..7188e2f
--- /dev/null
+++ b/crash_reporter/kernel_collector_test.cc
@@ -0,0 +1,186 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <unistd.h>
+
+#include "base/file_util.h"
+#include "base/string_util.h"
+#include "crash-reporter/kernel_collector.h"
+#include "crash-reporter/system_logging_mock.h"
+#include "gflags/gflags.h"
+#include "gtest/gtest.h"
+
+static int s_crashes = 0;
+static bool s_metrics = false;
+
+static const char kTestKCrash[] = "test/kcrash";
+static const char kTestCrashDirectory[] = "test/crash_directory";
+
+void CountCrash() {
+  ++s_crashes;
+}
+
+bool IsMetrics() {
+  return s_metrics;
+}
+
+class KernelCollectorTest : public ::testing::Test {
+  void SetUp() {
+    s_crashes = 0;
+    s_metrics = true;
+    collector_.Initialize(CountCrash,
+                          IsMetrics,
+                          &logging_);
+    mkdir("test", 0777);
+    test_kcrash_ = FilePath(kTestKCrash);
+    collector_.OverridePreservedDumpPath(test_kcrash_);
+    unlink(kTestKCrash);
+    mkdir(kTestCrashDirectory, 0777);
+  }
+ protected:
+  void WriteStringToFile(const FilePath &file_path,
+                         const char *data) {
+    ASSERT_EQ(strlen(data),
+              file_util::WriteFile(file_path, data, strlen(data)));
+  }
+
+  void SetUpSuccessfulCollect();
+  void CheckPreservedDumpClear();
+
+  SystemLoggingMock logging_;
+  KernelCollector collector_;
+  FilePath test_kcrash_;
+};
+
+TEST_F(KernelCollectorTest, LoadPreservedDump) {
+  ASSERT_FALSE(file_util::PathExists(test_kcrash_));
+  std::string dump;
+  ASSERT_FALSE(collector_.LoadPreservedDump(&dump));
+  WriteStringToFile(test_kcrash_, "");
+  ASSERT_TRUE(collector_.LoadPreservedDump(&dump));
+  ASSERT_EQ("", dump);
+  WriteStringToFile(test_kcrash_, "something");
+  ASSERT_TRUE(collector_.LoadPreservedDump(&dump));
+  ASSERT_EQ("something", dump);
+}
+
+TEST_F(KernelCollectorTest, EnableMissingKernel) {
+  ASSERT_FALSE(collector_.Enable());
+  ASSERT_FALSE(collector_.IsEnabled());
+  ASSERT_EQ(std::string::npos,
+            logging_.log().find("Enabling kernel crash handling"));
+  ASSERT_NE(std::string::npos,
+            logging_.log().find("Kernel does not support crash dumping"));
+  ASSERT_EQ(s_crashes, 0);
+}
+
+TEST_F(KernelCollectorTest, EnableOK) {
+  WriteStringToFile(test_kcrash_, "");
+  ASSERT_TRUE(collector_.Enable());
+  ASSERT_TRUE(collector_.IsEnabled());
+  ASSERT_NE(std::string::npos,
+            logging_.log().find("Enabling kernel crash handling"));
+  ASSERT_EQ(s_crashes, 0);
+}
+
+TEST_F(KernelCollectorTest, ClearPreservedDump) {
+  std::string dump;
+  ASSERT_FALSE(file_util::PathExists(test_kcrash_));
+  WriteStringToFile(test_kcrash_, "something");
+  ASSERT_TRUE(collector_.LoadPreservedDump(&dump));
+  ASSERT_EQ("something", dump);
+  ASSERT_TRUE(collector_.ClearPreservedDump());
+  ASSERT_TRUE(collector_.LoadPreservedDump(&dump));
+  ASSERT_EQ(KernelCollector::kClearingSequence, dump);
+}
+
+TEST_F(KernelCollectorTest, GetKernelCrashPath) {
+  FilePath root("/var/spool/crash");
+  struct tm tm = {0};
+  tm.tm_sec = 15;
+  tm.tm_min = 50;
+  tm.tm_hour = 13;
+  tm.tm_mday = 23;
+  tm.tm_mon = 4;
+  tm.tm_year = 110;
+  tm.tm_isdst = -1;
+  FilePath path = collector_.GetKernelCrashPath(root, mktime(&tm));
+  ASSERT_EQ("/var/spool/crash/kernel.20100523.135015.0.kcrash",
+            path.value());
+}
+
+TEST_F(KernelCollectorTest, CollectPreservedFileMissing) {
+  ASSERT_FALSE(collector_.Collect());
+  ASSERT_NE(logging_.log().find("Unable to read test/kcrash"),
+            std::string::npos);
+  ASSERT_EQ(0, s_crashes);
+}
+
+TEST_F(KernelCollectorTest, CollectNoCrash) {
+  WriteStringToFile(test_kcrash_, "");
+  ASSERT_FALSE(collector_.Collect());
+  ASSERT_EQ(logging_.log().find("Collected kernel crash"),
+            std::string::npos);
+  ASSERT_EQ(0, s_crashes);
+}
+
+TEST_F(KernelCollectorTest, CollectBadDirectory) {
+  WriteStringToFile(test_kcrash_, "something");
+  ASSERT_TRUE(collector_.Collect());
+  ASSERT_NE(logging_.log().find(
+      "Unable to create appropriate crash directory"), std::string::npos);
+  ASSERT_EQ(1, s_crashes);
+}
+
+void KernelCollectorTest::SetUpSuccessfulCollect() {
+  collector_.ForceCrashDirectory(kTestCrashDirectory);
+  WriteStringToFile(test_kcrash_, "something");
+  ASSERT_EQ(0, s_crashes);
+}
+
+void KernelCollectorTest::CheckPreservedDumpClear() {
+  // Make sure the preserved dump is now clear.
+  std::string dump;
+  ASSERT_TRUE(collector_.LoadPreservedDump(&dump));
+  ASSERT_EQ(KernelCollector::kClearingSequence, dump);
+}
+
+TEST_F(KernelCollectorTest, CollectOptedOut) {
+  SetUpSuccessfulCollect();
+  s_metrics = false;
+  ASSERT_TRUE(collector_.Collect());
+  ASSERT_NE(std::string::npos,
+            logging_.log().find("Crash not saved since metrics disabled"));
+  ASSERT_EQ(0, s_crashes);
+
+  CheckPreservedDumpClear();
+}
+
+
+TEST_F(KernelCollectorTest, CollectOK) {
+  SetUpSuccessfulCollect();
+  ASSERT_TRUE(collector_.Collect());
+  ASSERT_EQ(1, s_crashes);
+  static const char kNamePrefix[] = "Collected kernel crash diagnostics into ";
+  size_t pos = logging_.log().find(kNamePrefix);
+  ASSERT_NE(std::string::npos, pos);
+  pos += strlen(kNamePrefix);
+  std::string filename = logging_.log().substr(pos, std::string::npos);
+  // Take the name up until \n
+  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_TRUE(file_util::PathExists(FilePath(filename)));
+  std::string contents;
+  ASSERT_TRUE(file_util::ReadFileToString(FilePath(filename), &contents));
+  ASSERT_EQ("something", contents);
+
+  CheckPreservedDumpClear();
+}
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
diff --git a/crash_reporter/unclean_shutdown_collector.cc b/crash_reporter/unclean_shutdown_collector.cc
new file mode 100644
index 0000000..bb2dd7a
--- /dev/null
+++ b/crash_reporter/unclean_shutdown_collector.cc
@@ -0,0 +1,57 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "crash-reporter/unclean_shutdown_collector.h"
+
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "crash-reporter/system_logging.h"
+
+static const char kUncleanShutdownFile[] =
+    "/var/lib/crash_reporter/pending_clean_shutdown";
+
+UncleanShutdownCollector::UncleanShutdownCollector()
+    : unclean_shutdown_file_(kUncleanShutdownFile) {
+}
+
+UncleanShutdownCollector::~UncleanShutdownCollector() {
+}
+
+bool UncleanShutdownCollector::Enable() {
+  FilePath file_path(unclean_shutdown_file_);
+  file_util::CreateDirectory(file_path.DirName());
+  if (file_util::WriteFile(file_path, "", 0) != 0) {
+    logger_->LogError("Unable to create shutdown check file");
+    return false;
+  }
+  return true;
+}
+
+bool UncleanShutdownCollector::DeleteUncleanShutdownFile() {
+  if (!file_util::Delete(FilePath(unclean_shutdown_file_), false)) {
+    logger_->LogError("Failed to delete unclean shutdown file %s",
+                      unclean_shutdown_file_);
+    return false;
+  }
+  return true;
+}
+
+bool UncleanShutdownCollector::Collect() {
+  FilePath unclean_file_path(unclean_shutdown_file_);
+  if (!file_util::PathExists(unclean_file_path)) {
+    return false;
+  }
+  logger_->LogWarning("Last shutdown was not clean");
+  DeleteUncleanShutdownFile();
+
+  if (is_feedback_allowed_function_()) {
+    count_crash_function_();
+  }
+  return true;
+}
+
+bool UncleanShutdownCollector::Disable() {
+  logger_->LogInfo("Clean shutdown signalled");
+  return DeleteUncleanShutdownFile();
+}
diff --git a/crash_reporter/unclean_shutdown_collector.h b/crash_reporter/unclean_shutdown_collector.h
new file mode 100644
index 0000000..e7d8cef
--- /dev/null
+++ b/crash_reporter/unclean_shutdown_collector.h
@@ -0,0 +1,39 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef _CRASH_REPORTER_UNCLEAN_SHUTDOWN_COLLECTOR_H_
+#define _CRASH_REPORTER_UNCLEAN_SHUTDOWN_COLLECTOR_H_
+
+#include <string>
+
+#include "base/file_path.h"
+#include "crash-reporter/crash_collector.h"
+#include "gtest/gtest_prod.h"  // for FRIEND_TEST
+
+// Unclean shutdown collector.
+class UncleanShutdownCollector : public CrashCollector {
+ public:
+  UncleanShutdownCollector();
+  virtual ~UncleanShutdownCollector();
+
+  // Enable collection - signal that a boot has started.
+  bool Enable();
+
+  // Collect if there is was an unclean shutdown. Returns true if
+  // there was, false otherwise.
+  bool Collect();
+
+  // Disable collection - signal that the system has been shutdown cleanly.
+  bool Disable();
+
+ private:
+  friend class UncleanShutdownCollectorTest;
+  FRIEND_TEST(UncleanShutdownCollectorTest, EnableCannotWrite);
+
+  bool DeleteUncleanShutdownFile();
+
+  const char *unclean_shutdown_file_;
+};
+
+#endif  // _CRASH_REPORTER_UNCLEAN_SHUTDOWN_COLLECTOR_H_
diff --git a/crash_reporter/unclean_shutdown_collector_test.cc b/crash_reporter/unclean_shutdown_collector_test.cc
new file mode 100644
index 0000000..7be52af
--- /dev/null
+++ b/crash_reporter/unclean_shutdown_collector_test.cc
@@ -0,0 +1,103 @@
+// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <unistd.h>
+
+#include "base/file_util.h"
+#include "base/string_util.h"
+#include "crash-reporter/unclean_shutdown_collector.h"
+#include "crash-reporter/system_logging_mock.h"
+#include "gflags/gflags.h"
+#include "gtest/gtest.h"
+
+static int s_crashes = 0;
+static bool s_metrics = false;
+
+static const char kTestUnclean[] = "test/unclean";
+
+void CountCrash() {
+  ++s_crashes;
+}
+
+bool IsMetrics() {
+  return s_metrics;
+}
+
+class UncleanShutdownCollectorTest : public ::testing::Test {
+  void SetUp() {
+    s_crashes = 0;
+    collector_.Initialize(CountCrash,
+                          IsMetrics,
+                          &logging_);
+    rmdir("test");
+    test_unclean_ = FilePath(kTestUnclean);
+    collector_.unclean_shutdown_file_ = kTestUnclean;
+    file_util::Delete(test_unclean_, true);
+  }
+ protected:
+  void WriteStringToFile(const FilePath &file_path,
+                         const char *data) {
+    ASSERT_EQ(strlen(data),
+              file_util::WriteFile(file_path, data, strlen(data)));
+  }
+
+  SystemLoggingMock logging_;
+  UncleanShutdownCollector collector_;
+  FilePath test_unclean_;
+};
+
+TEST_F(UncleanShutdownCollectorTest, EnableWithoutParent) {
+  ASSERT_TRUE(collector_.Enable());
+  ASSERT_TRUE(file_util::PathExists(test_unclean_));
+}
+
+TEST_F(UncleanShutdownCollectorTest, EnableWithParent) {
+  mkdir("test", 0777);
+  ASSERT_TRUE(collector_.Enable());
+  ASSERT_TRUE(file_util::PathExists(test_unclean_));
+}
+
+TEST_F(UncleanShutdownCollectorTest, EnableCannotWrite) {
+  collector_.unclean_shutdown_file_ = "/bad/path";
+  ASSERT_FALSE(collector_.Enable());
+  ASSERT_NE(std::string::npos,
+            logging_.log().find("Unable to create shutdown check file"));
+}
+
+TEST_F(UncleanShutdownCollectorTest, CollectTrue) {
+  ASSERT_TRUE(collector_.Enable());
+  ASSERT_TRUE(file_util::PathExists(test_unclean_));
+  ASSERT_TRUE(collector_.Collect());
+  ASSERT_FALSE(file_util::PathExists(test_unclean_));
+  ASSERT_NE(std::string::npos,
+            logging_.log().find("Last shutdown was not clean"));
+}
+
+TEST_F(UncleanShutdownCollectorTest, CollectFalse) {
+  ASSERT_FALSE(collector_.Collect());
+}
+
+TEST_F(UncleanShutdownCollectorTest, Disable) {
+  ASSERT_TRUE(collector_.Enable());
+  ASSERT_TRUE(file_util::PathExists(test_unclean_));
+  ASSERT_TRUE(collector_.Disable());
+  ASSERT_FALSE(file_util::PathExists(test_unclean_));
+  ASSERT_FALSE(collector_.Collect());
+}
+
+TEST_F(UncleanShutdownCollectorTest, DisableWhenNotEnabled) {
+  ASSERT_TRUE(collector_.Disable());
+}
+
+TEST_F(UncleanShutdownCollectorTest, CantDisable) {
+  mkdir(kTestUnclean, 0700);
+  file_util::WriteFile(test_unclean_.Append("foo"), "", 0);
+  ASSERT_FALSE(collector_.Disable());
+  rmdir(kTestUnclean);
+}
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index 18ff39a..e051a4a 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "crash-reporter/user_collector.h"
+
 #include <grp.h>  // For struct group.
 #include <pwd.h>  // For struct passwd.
 #include <sys/types.h>  // For getpwuid_r and getgrnam_r.
@@ -11,27 +13,14 @@
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
-#include "crash-reporter/user_collector.h"
-#include "metrics/metrics_library.h"
+#include "crash-reporter/system_logging.h"
 
 // This procfs file is used to cause kernel core file writing to
 // instead pipe the core file into a user space process.  See
 // core(5) man page.
 static const char kCorePatternFile[] = "/proc/sys/kernel/core_pattern";
 static const char kCoreToMinidumpConverterPath[] = "/usr/bin/core2md";
-static const char kDefaultUserName[] = "chronos";
 static const char kLeaveCoreFile[] = "/root/.leave_core";
-static const char kSystemCrashPath[] = "/var/spool/crash";
-static const char kUserCrashPath[] = "/home/chronos/user/crash";
-
-// Directory mode of the user crash spool directory.
-static const mode_t kUserCrashPathMode = 0755;
-
-// Directory mode of the system crash spool directory.
-static const mode_t kSystemCrashPathMode = 01755;
-
-static const uid_t kRootOwner = 0;
-static const uid_t kRootGroup = 0;
 
 const char *UserCollector::kUserId = "Uid:\t";
 const char *UserCollector::kGroupId = "Gid:\t";
@@ -39,10 +28,7 @@
 UserCollector::UserCollector()
     : generate_diagnostics_(false),
       core_pattern_file_(kCorePatternFile),
-      count_crash_function_(NULL),
-      initialized_(false),
-      is_feedback_allowed_function_(NULL),
-      logger_(NULL) {
+      initialized_(false) {
 }
 
 void UserCollector::Initialize(
@@ -51,14 +37,10 @@
     UserCollector::IsFeedbackAllowedFunction is_feedback_allowed_function,
     SystemLogging *logger,
     bool generate_diagnostics) {
-  CHECK(count_crash_function != NULL);
-  CHECK(is_feedback_allowed_function != NULL);
-  CHECK(logger != NULL);
-
-  count_crash_function_ = count_crash_function;
+  CrashCollector::Initialize(count_crash_function,
+                             is_feedback_allowed_function,
+                             logger);
   our_path_ = our_path;
-  is_feedback_allowed_function_ = is_feedback_allowed_function;
-  logger_ = logger;
   initialized_ = true;
   generate_diagnostics_ = generate_diagnostics;
 }
@@ -76,7 +58,8 @@
 
 bool UserCollector::SetUpInternal(bool enabled) {
   CHECK(initialized_);
-  logger_->LogInfo("%s crash handling", enabled ? "Enabling" : "Disabling");
+  logger_->LogInfo("%s user crash handling",
+                   enabled ? "Enabling" : "Disabling");
   std::string pattern = GetPattern(enabled);
   if (file_util::WriteFile(FilePath(core_pattern_file_),
                            pattern.c_str(),
@@ -161,24 +144,6 @@
   return true;
 }
 
-bool UserCollector::GetUserInfoFromName(const std::string &name,
-                                        uid_t *uid,
-                                        gid_t *gid) {
-  char storage[256];
-  struct passwd passwd_storage;
-  struct passwd *passwd_result = NULL;
-
-  if (getpwnam_r(name.c_str(), &passwd_storage, storage, sizeof(storage),
-                 &passwd_result) != 0 || passwd_result == NULL) {
-    logger_->LogError("Cannot find user named %s", name.c_str());
-    return false;
-  }
-
-  *uid = passwd_result->pw_uid;
-  *gid = passwd_result->pw_gid;
-  return true;
-}
-
 bool UserCollector::CopyOffProcFiles(pid_t pid,
                                      const FilePath &container_dir) {
   if (!file_util::CreateDirectory(container_dir)) {
@@ -208,26 +173,6 @@
   return true;
 }
 
-FilePath UserCollector::GetCrashDirectoryInfo(
-    uid_t process_euid,
-    uid_t default_user_id,
-    gid_t default_user_group,
-    mode_t *mode,
-    uid_t *directory_owner,
-    gid_t *directory_group) {
-  if (process_euid == default_user_id) {
-    *mode = kUserCrashPathMode;
-    *directory_owner = default_user_id;
-    *directory_group = default_user_group;
-    return FilePath(kUserCrashPath);
-  } else {
-    *mode = kSystemCrashPathMode;
-    *directory_owner = kRootOwner;
-    *directory_group = kRootGroup;
-    return FilePath(kSystemCrashPath);
-  }
-}
-
 bool UserCollector::GetCreatedCrashDirectory(pid_t pid,
                                              FilePath *crash_file_path) {
   FilePath process_path = GetProcessPath(pid);
@@ -242,64 +187,8 @@
     logger_->LogError("Could not find euid in status file");
     return false;
   }
-  uid_t default_user_id;
-  gid_t default_user_group;
-  if (!GetUserInfoFromName(kDefaultUserName,
-                           &default_user_id,
-                           &default_user_group)) {
-    logger_->LogError("Could not find default user info");
-    return false;
-  }
-  mode_t directory_mode;
-  uid_t directory_owner;
-  gid_t directory_group;
-  *crash_file_path =
-      GetCrashDirectoryInfo(process_euid,
-                            default_user_id,
-                            default_user_group,
-                            &directory_mode,
-                            &directory_owner,
-                            &directory_group);
-
-
-  if (!file_util::PathExists(*crash_file_path)) {
-    // Create the spool directory with the appropriate mode (regardless of
-    // umask) and ownership.
-    mode_t old_mask = umask(0);
-    if (mkdir(crash_file_path->value().c_str(), directory_mode) < 0 ||
-        chown(crash_file_path->value().c_str(),
-              directory_owner,
-              directory_group) < 0) {
-      logger_->LogError("Unable to create appropriate crash directory");
-      return false;
-    }
-    umask(old_mask);
-  }
-
-  if (!file_util::PathExists(*crash_file_path)) {
-    logger_->LogError("Unable to create crash directory %s",
-                      crash_file_path->value().c_str());
-    return false;
-  }
-
-
-  return true;
-}
-
-std::string UserCollector::FormatDumpBasename(const std::string &exec_name,
-                                              time_t timestamp,
-                                              pid_t pid) {
-  struct tm tm;
-  localtime_r(&timestamp, &tm);
-  return StringPrintf("%s.%04d%02d%02d.%02d%02d%02d.%d",
-                      exec_name.c_str(),
-                      tm.tm_year + 1900,
-                      tm.tm_mon + 1,
-                      tm.tm_mday,
-                      tm.tm_hour,
-                      tm.tm_min,
-                      tm.tm_sec,
-                      pid);
+  return GetCreatedCrashDirectoryByEuid(process_euid,
+                                        crash_file_path);
 }
 
 bool UserCollector::CopyStdinToCoreFile(const FilePath &core_path) {
@@ -360,13 +249,13 @@
     return false;
   }
 
-  FilePath spool_path;
-  if (!GetCreatedCrashDirectory(pid, &spool_path)) {
+  FilePath crash_path;
+  if (!GetCreatedCrashDirectory(pid, &crash_path)) {
     file_util::Delete(container_dir, true);
     return false;
   }
   std::string dump_basename = FormatDumpBasename(exec_name, time(NULL), pid);
-  FilePath core_path = spool_path.Append(
+  FilePath core_path = crash_path.Append(
       StringPrintf("%s.core", dump_basename.c_str()));
 
   if (!CopyStdinToCoreFile(core_path)) {
@@ -374,7 +263,7 @@
     return false;
   }
 
-  FilePath minidump_path = spool_path.Append(
+  FilePath minidump_path = crash_path.Append(
       StringPrintf("%s.dmp", dump_basename.c_str()));
 
   bool conversion_result = true;
@@ -414,10 +303,10 @@
 
   if (is_feedback_allowed_function_()) {
     count_crash_function_();
-  }
 
-  if (generate_diagnostics_) {
-    return GenerateDiagnostics(pid, exec);
+    if (generate_diagnostics_) {
+      return GenerateDiagnostics(pid, exec);
+    }
   }
   return true;
 }
diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h
index dc633bb..484445f 100644
--- a/crash_reporter/user_collector.h
+++ b/crash_reporter/user_collector.h
@@ -7,17 +7,15 @@
 
 #include <string>
 
-#include "crash-reporter/system_logging.h"
+#include "crash-reporter/crash_collector.h"
 #include "gtest/gtest_prod.h"  // for FRIEND_TEST
 
 class FilePath;
+class SystemLogging;
 
 // User crash collector.
-class UserCollector {
+class UserCollector : public CrashCollector {
  public:
-  typedef void (*CountCrashFunction)();
-  typedef bool (*IsFeedbackAllowedFunction)();
-
   UserCollector();
 
   // Initialize the user crash collector for detection of crashes,
@@ -53,8 +51,6 @@
   FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPath);
   FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPid);
   FRIEND_TEST(UserCollectorTest, CopyOffProcFilesOK);
-  FRIEND_TEST(UserCollectorTest, FormatDumpBasename);
-  FRIEND_TEST(UserCollectorTest, GetCrashDirectoryInfo);
   FRIEND_TEST(UserCollectorTest, GetIdFromStatus);
   FRIEND_TEST(UserCollectorTest, GetProcessPath);
   FRIEND_TEST(UserCollectorTest, GetSymlinkTarget);
@@ -82,25 +78,13 @@
                        IdKind kind,
                        const std::string &status_contents,
                        int *id);
-  bool GetUserInfoFromName(const std::string &name,
-                           uid_t *uid,
-                           gid_t *gid);
   bool CopyOffProcFiles(pid_t pid, const FilePath &process_map);
-  FilePath GetCrashDirectoryInfo(uid_t process_euid,
-                                 uid_t default_user_id,
-                                 gid_t default_user_group,
-                                 mode_t *mode,
-                                 uid_t *directory_owner,
-                                 gid_t *directory_group);
   // Determines the crash directory for given pid based on pid's owner,
   // and creates the directory if necessary with appropriate permissions.
   // Returns true whether or not directory needed to be created, false on
   // any failure.
   bool GetCreatedCrashDirectory(pid_t pid,
                                 FilePath *crash_file_path);
-  std::string FormatDumpBasename(const std::string &exec_name,
-                                 time_t timestamp,
-                                 pid_t pid);
   bool CopyStdinToCoreFile(const FilePath &core_path);
   bool ConvertCoreToMinidump(const FilePath &core_path,
                              const FilePath &procfs_directory,
@@ -110,11 +94,8 @@
 
   bool generate_diagnostics_;
   std::string core_pattern_file_;
-  CountCrashFunction count_crash_function_;
   std::string our_path_;
   bool initialized_;
-  IsFeedbackAllowedFunction is_feedback_allowed_function_;
-  SystemLogging *logger_;
 
   static const char *kUserId;
   static const char *kGroupId;
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index 335821c..3390caa 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -10,8 +10,8 @@
 #include "gflags/gflags.h"
 #include "gtest/gtest.h"
 
-int s_crashes = 0;
-bool s_metrics = false;
+static int s_crashes = 0;
+static bool s_metrics = false;
 
 static const char kFilePath[] = "/my/path";
 
@@ -50,14 +50,16 @@
                                                    &contents));
   ASSERT_EQ("|/my/path --signal=%s --pid=%p", contents);
   ASSERT_EQ(s_crashes, 0);
-  ASSERT_NE(logging_.log().find("Enabling crash handling"), std::string::npos);
+  ASSERT_NE(logging_.log().find("Enabling user crash handling"),
+            std::string::npos);
 }
 
 TEST_F(UserCollectorTest, EnableNoFileAccess) {
   collector_.set_core_pattern_file("/does_not_exist");
   ASSERT_FALSE(collector_.Enable());
   ASSERT_EQ(s_crashes, 0);
-  ASSERT_NE(logging_.log().find("Enabling crash handling"), std::string::npos);
+  ASSERT_NE(logging_.log().find("Enabling user crash handling"),
+            std::string::npos);
   ASSERT_NE(logging_.log().find("Unable to write /does_not_exist"),
             std::string::npos);
 }
@@ -69,7 +71,7 @@
                                           &contents));
   ASSERT_EQ("core", contents);
   ASSERT_EQ(s_crashes, 0);
-  ASSERT_NE(logging_.log().find("Disabling crash handling"),
+  ASSERT_NE(logging_.log().find("Disabling user crash handling"),
             std::string::npos);
 }
 
@@ -77,7 +79,8 @@
   collector_.set_core_pattern_file("/does_not_exist");
   ASSERT_FALSE(collector_.Disable());
   ASSERT_EQ(s_crashes, 0);
-  ASSERT_NE(logging_.log().find("Disabling crash handling"), std::string::npos);
+  ASSERT_NE(logging_.log().find("Disabling user crash handling"),
+            std::string::npos);
   ASSERT_NE(logging_.log().find("Unable to write /does_not_exist"),
             std::string::npos);
 }
@@ -206,54 +209,6 @@
   EXPECT_EQ(0, gid);
 }
 
-TEST_F(UserCollectorTest, GetCrashDirectoryInfo) {
-  FilePath path;
-  const int kRootUid = 0;
-  const int kRootGid = 0;
-  const int kNtpUid = 5;
-  const int kChronosUid = 1000;
-  const int kChronosGid = 1001;
-  const mode_t kExpectedSystemMode = 01755;
-  const mode_t kExpectedUserMode = 0755;
-
-  mode_t directory_mode;
-  uid_t directory_owner;
-  gid_t directory_group;
-
-  path = collector_.GetCrashDirectoryInfo(kRootUid,
-                                          kChronosUid,
-                                          kChronosGid,
-                                          &directory_mode,
-                                          &directory_owner,
-                                          &directory_group);
-  EXPECT_EQ("/var/spool/crash", path.value());
-  EXPECT_EQ(kExpectedSystemMode, directory_mode);
-  EXPECT_EQ(kRootUid, directory_owner);
-  EXPECT_EQ(kRootGid, directory_group);
-
-  path = collector_.GetCrashDirectoryInfo(kNtpUid,
-                                          kChronosUid,
-                                          kChronosGid,
-                                          &directory_mode,
-                                          &directory_owner,
-                                          &directory_group);
-  EXPECT_EQ("/var/spool/crash", path.value());
-  EXPECT_EQ(kExpectedSystemMode, directory_mode);
-  EXPECT_EQ(kRootUid, directory_owner);
-  EXPECT_EQ(kRootGid, directory_group);
-
-  path = collector_.GetCrashDirectoryInfo(kChronosUid,
-                                          kChronosUid,
-                                          kChronosGid,
-                                          &directory_mode,
-                                          &directory_owner,
-                                          &directory_group);
-  EXPECT_EQ("/home/chronos/user/crash", path.value());
-  EXPECT_EQ(kExpectedUserMode, directory_mode);
-  EXPECT_EQ(kChronosUid, directory_owner);
-  EXPECT_EQ(kChronosGid, directory_group);
-}
-
 TEST_F(UserCollectorTest, CopyOffProcFilesBadPath) {
   // Try a path that is not writable.
   ASSERT_FALSE(collector_.CopyOffProcFiles(pid_, FilePath("/bad/path")));
@@ -295,20 +250,6 @@
   }
 }
 
-TEST_F(UserCollectorTest, FormatDumpBasename) {
-  struct tm tm = {0};
-  tm.tm_sec = 15;
-  tm.tm_min = 50;
-  tm.tm_hour = 13;
-  tm.tm_mday = 23;
-  tm.tm_mon = 4;
-  tm.tm_year = 110;
-  tm.tm_isdst = -1;
-  std::string basename =
-      collector_.FormatDumpBasename("foo", mktime(&tm), 100);
-  ASSERT_EQ("foo.20100523.135015.100", basename);
-}
-
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();