Refactor CrashDump*Manager to use a shared CrashDumpObserver.

components/crash/content/browser/crash_dump_manager_android.cc

Index: components/crash/content/browser/crash_dump_manager_android.cc
diff --git a/components/crash/content/browser/crash_dump_manager_android.cc b/components/crash/content/browser/crash_dump_manager_android.cc
index 1a618ff205f0443a36850a0a2c56282c625bd0a0..05b04f90e5db4e356789221c8a39f3e5fdd0d406 100644
--- a/components/crash/content/browser/crash_dump_manager_android.cc
+++ b/components/crash/content/browser/crash_dump_manager_android.cc
@@ -16,6 +16,7 @@
 #include "base/rand_util.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
+#include "components/crash/content/app/breakpad_linux.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/file_descriptor_info.h"
@@ -27,43 +28,28 @@ using content::BrowserThread;
 
 namespace breakpad {
 
-// static
-CrashDumpManager* CrashDumpManager::instance_ = NULL;
-
-// static
-CrashDumpManager* CrashDumpManager::GetInstance() {
-  CHECK(instance_);
-  return instance_;
-}
-
-CrashDumpManager::CrashDumpManager(const base::FilePath& crash_dump_dir)
-    : crash_dump_dir_(crash_dump_dir) {
+CrashDumpManager::CrashDumpManager(const base::FilePath& crash_dump_dir,
+                                   int mapping_index)
+    : crash_dump_dir_(crash_dump_dir), mapping_index_(mapping_index) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(!instance_);
-
-  instance_ = this;
-
-  notification_registrar_.Add(this,
-                              content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                              content::NotificationService::AllSources());
-  notification_registrar_.Add(this,
-                              content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
-                              content::NotificationService::AllSources());
-
-  BrowserChildProcessObserver::Add(this);
 }
 
 CrashDumpManager::~CrashDumpManager() {
-  instance_ = NULL;
-
-  BrowserChildProcessObserver::Remove(this);
 }
 
-base::File CrashDumpManager::CreateMinidumpFile(int child_process_id) {
+void CrashDumpManager::OnChildStart(int child_process_id,
+                                    content::FileDescriptorInfo* mappings) {
   DCHECK_CURRENTLY_ON(BrowserThread::PROCESS_LAUNCHER);
+
+  if (!breakpad::IsCrashReporterEnabled()) {

[Bernhard Bauer, 2016/08/04 16:01:32]

Nit: no braces for oneline bodies.

[Tobias Sargeant, 2016/08/05 14:59:21]

Done.

+    return;
+  }
+
   base::FilePath minidump_path;
-  if (!base::CreateTemporaryFile(&minidump_path))
-    return base::File();
+  if (!base::CreateTemporaryFile(&minidump_path)) {
+    LOG(ERROR) << "Failed to create temporary file, crash won't be reported.";
+    return;
+  }
 
   // We need read permission as the minidump is generated in several phases
   // and needs to be read at some point.
@@ -71,8 +57,8 @@ base::File CrashDumpManager::CreateMinidumpFile(int child_process_id) {
               base::File::FLAG_WRITE;
   base::File minidump_file(minidump_path, flags);
   if (!minidump_file.IsValid()) {
-    LOG(ERROR) << "Failed to create temporary file, crash won't be reported.";
-    return base::File();
+    LOG(ERROR) << "Failed to open temporary file, crash won't be reported.";
+    return;
   }
 
   {
@@ -80,10 +66,10 @@ base::File CrashDumpManager::CreateMinidumpFile(int child_process_id) {
     DCHECK(!ContainsKey(child_process_id_to_minidump_path_, child_process_id));
     child_process_id_to_minidump_path_[child_process_id] = minidump_path;
   }
-  return minidump_file;
+  mappings->Transfer(mapping_index_,
+                     base::ScopedFD(minidump_file.TakePlatformFile()));
 }
 
-// static
 void CrashDumpManager::ProcessMinidump(
     const base::FilePath& minidump_path,
     base::ProcessHandle pid,
@@ -91,7 +77,7 @@ void CrashDumpManager::ProcessMinidump(
     base::TerminationStatus termination_status,
     base::android::ApplicationState app_state) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  CHECK(instance_);
+
   int64_t file_size = 0;
   int r = base::GetFileSize(minidump_path, &file_size);
   DCHECK(r) << "Failed to retrieve size for minidump "
@@ -145,7 +131,7 @@ void CrashDumpManager::ProcessMinidump(
 
   // We are dealing with a valid minidump. Copy it to the crash report
   // directory from where Java code will upload it later on.
-  if (instance_->crash_dump_dir_.empty()) {
+  if (crash_dump_dir_.empty()) {
     NOTREACHED() << "Failed to retrieve the crash dump directory.";
     return;
   }
@@ -153,7 +139,7 @@ void CrashDumpManager::ProcessMinidump(
   const std::string filename =
       base::StringPrintf("chromium-renderer-minidump-%016" PRIx64 ".dmp%d",
                          rand, pid);
-  base::FilePath dest_path = instance_->crash_dump_dir_.Append(filename);
+  base::FilePath dest_path = crash_dump_dir_.Append(filename);
   r = base::Move(minidump_path, dest_path);
   if (!r) {
     LOG(ERROR) << "Failed to move crash dump from " << minidump_path.value()
@@ -161,67 +147,8 @@ void CrashDumpManager::ProcessMinidump(
     base::DeleteFile(minidump_path, false);
     return;
   }
-  VLOG(1) << "Crash minidump successfully generated: " <<
-      instance_->crash_dump_dir_.Append(filename).value();
-}
-
-void CrashDumpManager::BrowserChildProcessHostDisconnected(
-    const content::ChildProcessData& data) {
-  OnChildExit(data.id,
-              data.handle,
-              static_cast<content::ProcessType>(data.process_type),
-              base::TERMINATION_STATUS_MAX_ENUM,
-              base::android::APPLICATION_STATE_UNKNOWN);
-}
-
-void CrashDumpManager::BrowserChildProcessCrashed(
-    const content::ChildProcessData& data,
-    int exit_code) {
-  OnChildExit(data.id,
-              data.handle,
-              static_cast<content::ProcessType>(data.process_type),
-              base::TERMINATION_STATUS_ABNORMAL_TERMINATION,
-              base::android::APPLICATION_STATE_UNKNOWN);
-}
-
-void CrashDumpManager::Observe(int type,
-                               const content::NotificationSource& source,
-                               const content::NotificationDetails& details) {
-  content::RenderProcessHost* rph =
-      content::Source<content::RenderProcessHost>(source).ptr();
-  base::TerminationStatus term_status = base::TERMINATION_STATUS_MAX_ENUM;
-  base::android::ApplicationState app_state =
-      base::android::APPLICATION_STATE_UNKNOWN;
-  switch (type) {
-    case content::NOTIFICATION_RENDERER_PROCESS_TERMINATED: {
-      // NOTIFICATION_RENDERER_PROCESS_TERMINATED is sent when the renderer
-      // process is cleanly shutdown. However, we still need to close the
-      // minidump_fd we kept open.
-      term_status = base::TERMINATION_STATUS_NORMAL_TERMINATION;
-      break;
-    }
-    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED: {
-      // We do not care about android fast shutdowns as it is a known case where
-      // the renderer is intentionally killed when we are done with it.
-      if (rph->FastShutdownStarted()) {
-        break;
-      }
-      content::RenderProcessHost::RendererClosedDetails* process_details =
-          content::Details<content::RenderProcessHost::RendererClosedDetails>(
-              details).ptr();
-      term_status = process_details->status;
-      app_state = base::android::ApplicationStatusListener::GetState();
-      break;
-    }
-    default:
-      NOTREACHED();
-      return;
-  }
-  OnChildExit(rph->GetID(),
-              rph->GetHandle(),
-              content::PROCESS_TYPE_RENDERER,
-              term_status,
-              app_state);
+  VLOG(1) << "Crash minidump successfully generated: "
+          << crash_dump_dir_.Append(filename).value();
 }
 
 void CrashDumpManager::OnChildExit(int child_process_id,
@@ -229,6 +156,8 @@ void CrashDumpManager::OnChildExit(int child_process_id,
                                    content::ProcessType process_type,
                                    base::TerminationStatus termination_status,
                                    base::android::ApplicationState app_state) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::FILE);
+
   base::FilePath minidump_path;
   {
     base::AutoLock auto_lock(child_process_id_to_minidump_path_lock_);
@@ -242,14 +171,8 @@ void CrashDumpManager::OnChildExit(int child_process_id,
     minidump_path = iter->second;
     child_process_id_to_minidump_path_.erase(iter);
   }
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&CrashDumpManager::ProcessMinidump,
-                 minidump_path,
-                 pid,
-                 process_type,
-                 termination_status,
-                 app_state));
+  CrashDumpManager::ProcessMinidump(minidump_path, pid, process_type,

[Robert Sesek, 2016/08/04 16:59:01]

I think you can remove the "CrashDumpManager::" since ProcessMindiump is no
longer static

[Tobias Sargeant, 2016/08/05 14:59:21]

Done.

+                                    termination_status, app_state);
 }
 
 }  // namespace breakpad