Changing minidump process generation to be in-process on Android.

chrome/browser/android/crash_dump_manager.cc

Index: chrome/browser/android/crash_dump_manager.cc
diff --git a/chrome/browser/android/crash_dump_manager.cc b/chrome/browser/android/crash_dump_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..04604c7c218143387b44200315bfba407f107615
--- /dev/null
+++ b/chrome/browser/android/crash_dump_manager.cc
@@ -0,0 +1,147 @@
+// Copyright (c) 2012 The Chromium 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 "chrome/browser/android/crash_dump_manager.h"
+
+#include <inttypes.h>

[Lei Zhang, 2012/10/23 02:57:34]

why do we need this?

[Jay Civelli, 2012/10/23 20:39:55]

We need it for the PRIx64.

[Lei Zhang, 2012/10/23 21:34:57]

Can we use base/format_macros.h instead?

[Jay Civelli, 2012/10/24 00:12:53]

Done.

+
+#include "base/bind.h"
+#include "base/file_util.h"
+#include "base/global_descriptors_posix.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "base/process.h"
+#include "base/rand_util.h"
+#include "base/stringprintf.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/descriptors_android.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/child_process_data.h"
+#include "content/public/browser/file_descriptor_info.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host.h"
+
+using content::BrowserThread;
+
+CrashDumpManager::CrashDumpManager() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  notification_registrar_.Add(this,
+                              content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
+                              content::NotificationService::AllSources());
+  notification_registrar_.Add(this,
+      content::NOTIFICATION_CHILD_PROCESS_HOST_DISCONNECTED,
+      content::NotificationService::AllSources());
+

[jam, 2012/10/23 01:20:47]

nit: extra line

[Jay Civelli, 2012/10/23 20:39:55]

Done.

+}
+
+CrashDumpManager::~CrashDumpManager() {
+}
+
+int CrashDumpManager::CreateMinidumpFile(int process_host_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::PROCESS_LAUNCHER));
+  FilePath minidump_path;
+  if (!file_util::CreateTemporaryFile(&minidump_path))
+    return base::kInvalidPlatformFileValue;
+
+  base::PlatformFileError error;
+  // We need read permission as the minidump is generated in several phases
+  // and needs to be read at some point.
+  int flags = base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ |
+      base::PLATFORM_FILE_WRITE;
+  base::PlatformFile f =

[Lei Zhang, 2012/10/23 02:57:34]

nit: can we name the variable to something that's easier to search for? pf,
even.

[Jay Civelli, 2012/10/23 20:39:55]

Done.

[Jay Civelli, 2012/10/23 20:39:55]

Done.

+      base::CreatePlatformFile(minidump_path, flags, NULL, &error);
+  if (f == base::kInvalidPlatformFileValue) {
+    LOG(ERROR) << "Failed to create temporary file, crash won't be reported.";
+    return base::kInvalidPlatformFileValue;
+  }
+
+  MinidumpInfo minidump_info;
+  minidump_info.file = f;
+  minidump_info.path = minidump_path;
+  {
+    base::AutoLock auto_lock(process_host_id_to_minidump_info_lock_);
+    DCHECK(process_host_id_to_minidump_info_.find(process_host_id) ==

[Lei Zhang, 2012/10/23 02:57:34]

nit: more readable with ContainsKey().

[Jay Civelli, 2012/10/23 20:39:55]

Done.

+        process_host_id_to_minidump_info_.end());
+    process_host_id_to_minidump_info_[process_host_id] = minidump_info;
+  }
+  return f;
+}
+
+void CrashDumpManager::ProcessMinidump(const MinidumpInfo& minidump) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // Close the file descriptor, it is still open.
+  bool r = base::ClosePlatformFile(minidump.file);
+  DCHECK(r) << "Failed to close minidump file descriptor.";
+
+  int64 file_size = 0;
+  r = file_util::GetFileSize(minidump.path, &file_size);
+  DCHECK(r) << "Failed to retrieve size for minidump " <<

[Lei Zhang, 2012/10/23 02:57:34]

nit: put the trailing << on the next line and line it up. Ditto below.

[Jay Civelli, 2012/10/23 20:39:55]

Done.

+      minidump.path.value();
+
+  if (file_size == 0) {
+    // Empty minidump, this process did not crash. Just remove the file.
+    r = file_util::Delete(minidump.path, false);
+    DCHECK(r) << "Failed to delete temporary minidump file " <<
+        minidump.path.value();
+    return;
+  }
+
+  // We are dealing with a valid minidump. Copy it to the crash report
+  // directory from where Java code will upload it later on.
+  FilePath crash_dump_dir;
+  r = PathService::Get(chrome::DIR_CRASH_DUMPS, &crash_dump_dir);
+  if (!r) {
+    NOTREACHED() << "Failed to retrieve the crash dump directory.";
+    return;
+  }
+
+  const uint64 rand = base::RandUint64();
+  const std::string filename =
+      base::StringPrintf("chromium-renderer-minidump-%016" PRIx64 ".dmp%d",
+                         rand, minidump.pid);
+  FilePath dest_path = crash_dump_dir.Append(filename);
+  r = file_util::Move(minidump.path, dest_path);
+  if (!r) {
+    LOG(ERROR) << "Failed to move crash dump from " << minidump.path.value() <<
+      " to " << dest_path.value();
+    file_util::Delete(minidump.path, false);
+    return;
+  }
+  LOG(INFO) << "Crash minidump successfully generated: " <<
+      crash_dump_dir.Append(filename).value();
+}
+
+void CrashDumpManager::Observe(int type,
+                               const content::NotificationSource& source,
+                               const content::NotificationDetails& details) {
+  int process_host_id;
+  switch (type) {
+    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED: {
+      content::RenderProcessHost* rph =
+          content::Source<content::RenderProcessHost>(source).ptr();
+      process_host_id = rph->GetID();
+      break;
+    }
+    case content::NOTIFICATION_CHILD_PROCESS_HOST_DISCONNECTED: {
+      content::ChildProcessData* chil_process_data =

[Lei Zhang, 2012/10/23 02:57:34]

nit: chil_ -> child_ ?

[Jay Civelli, 2012/10/23 20:39:55]

Done.

+          content::Details<content::ChildProcessData>(details).ptr();
+      process_host_id = chil_process_data->id;
+      break;
+    }
+    default:
+      NOTREACHED();
+      return;
+  }
+  MinidumpInfo minidump_info;
+  {
+    base::AutoLock auto_lock(process_host_id_to_minidump_info_lock_);
+    ProcessHostIDToMinidumpInfo::iterator iter =
+        process_host_id_to_minidump_info_.find(process_host_id);
+    DCHECK(iter != process_host_id_to_minidump_info_.end());
+    minidump_info = iter->second;
+    process_host_id_to_minidump_info_.erase(iter);
+  }
+  ProcessMinidump(minidump_info);
+}