Linux: Handle renderer and plugin crashes on a separate thread.

Also set retries / timeouts for wget to attempt to limit the duration of each crash handler. Move some code out of headers while we're at it.

BUG=54071
TEST=change /usr/bin/wget to sleep forever, visit about:crash in a Breakpad-enabled build, make sure other renderers continue to work.
Review URL: http://codereview.chromium.org/3308007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@58403 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 100 insertions, 39 deletions

diff --git a/chrome/app/breakpad_linux.cc b/chrome/app/breakpad_linux.cc
index 42d7b93..3619a71 100644
--- a/chrome/app/breakpad_linux.cc
+++ b/chrome/app/breakpad_linux.cc
@@ -558,6 +558,8 @@ pid_t HandleCrashDump(const BreakpadInfo& info) {
       header,
       post_file,
       kUploadURL,
+      "--timeout=10",  // Set a timeout so we don't hang forever.
+      "--tries=1",     // Don't retry if the upload fails.
       "-O",  // output reply to fd 3
       "/dev/fd/3",
       NULL,
diff --git a/chrome/browser/crash_handler_host_linux.cc b/chrome/browser/crash_handler_host_linux.cc
index e46c60d..06ceb96 100644
--- a/chrome/browser/crash_handler_host_linux.cc
+++ b/chrome/browser/crash_handler_host_linux.cc
@@ -20,6 +20,8 @@
 #include "base/path_service.h"
 #include "base/rand_util.h"
 #include "base/string_util.h"
+#include "base/task.h"
+#include "base/thread.h"
 #include "breakpad/src/client/linux/handler/exception_handler.h"
 #include "breakpad/src/client/linux/minidump_writer/linux_dumper.h"
 #include "breakpad/src/client/linux/minidump_writer/minidump_writer.h"
@@ -30,14 +32,27 @@
 
 using google_breakpad::ExceptionHandler;
 
+namespace {
+
+// Handles the crash dump and frees the allocated BreakpadInfo struct.
+void CrashDumpTask(BreakpadInfo* info) {
+  HandleCrashDump(*info);
+  delete[] info->filename;
+  delete[] info->process_type;
+  delete[] info->crash_url;
+  delete[] info->guid;
+  delete[] info->distro;
+  delete info;
+}
+
+}  // namespace
+
 // Since classes derived from CrashHandlerHostLinux are singletons, it's only
 // destroyed at the end of the processes lifetime, which is greater in span than
 // the lifetime of the IO message loop.
 DISABLE_RUNNABLE_METHOD_REFCOUNT(CrashHandlerHostLinux);
 
-CrashHandlerHostLinux::CrashHandlerHostLinux()
-    : process_socket_(-1),
-      browser_socket_(-1) {
+CrashHandlerHostLinux::CrashHandlerHostLinux() {
   int fds[2];
   // We use SOCK_SEQPACKET rather than SOCK_DGRAM to prevent the process from
   // sending datagrams to other sockets on the system. The sandbox may prevent
@@ -62,9 +77,17 @@ CrashHandlerHostLinux::CrashHandlerHostLinux()
 CrashHandlerHostLinux::~CrashHandlerHostLinux() {
   HANDLE_EINTR(close(process_socket_));
   HANDLE_EINTR(close(browser_socket_));
+
+  // If we are quitting and there are crash dumps in the queue, discard them.
+  uploader_thread_->message_loop()->QuitNow();
 }
 
 void CrashHandlerHostLinux::Init() {
+  SetProcessType();
+  uploader_thread_.reset(
+      new base::Thread(std::string(process_type_ + "_crash_uploader").c_str()));
+  uploader_thread_->Start();
+
   MessageLoopForIO* ml = MessageLoopForIO::current();
   CHECK(ml->WatchFileDescriptor(
       browser_socket_, true /* persistent */,
@@ -95,24 +118,26 @@ void CrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) {
   struct msghdr msg = {0};
   struct iovec iov[6];
   char crash_context[kCrashContextSize];
-  char guid[kGuidSize + 1];
-  char crash_url[kMaxActiveURLSize + 1];
-  char distro[kDistroSize + 1];
+  char* guid = new char[kGuidSize + 1];
+  char* crash_url = new char[kMaxActiveURLSize + 1];
+  char* distro = new char[kDistroSize + 1];
   char* tid_buf_addr = NULL;
   int tid_fd = -1;
   char control[kControlMsgSize];
-  const ssize_t expected_msg_size = sizeof(crash_context) + sizeof(guid) +
-      sizeof(crash_url) + sizeof(distro) +
+  const ssize_t expected_msg_size = sizeof(crash_context) +
+      kGuidSize + 1 +
+      kMaxActiveURLSize + 1 +
+      kDistroSize + 1 +
       sizeof(tid_buf_addr) + sizeof(tid_fd);
 
   iov[0].iov_base = crash_context;
   iov[0].iov_len = sizeof(crash_context);
   iov[1].iov_base = guid;
-  iov[1].iov_len = sizeof(guid);
+  iov[1].iov_len = kGuidSize + 1;
   iov[2].iov_base = crash_url;
-  iov[2].iov_len = sizeof(crash_url);
+  iov[2].iov_len = kMaxActiveURLSize + 1;
   iov[3].iov_base = distro;
-  iov[3].iov_len = sizeof(distro);
+  iov[3].iov_len = kDistroSize + 1;
   iov[4].iov_base = &tid_buf_addr;
   iov[4].iov_len = sizeof(tid_buf_addr);
   iov[5].iov_base = &tid_fd;
@@ -247,6 +272,7 @@ void CrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) {
 
   bool upload = true;
   FilePath dumps_path("/tmp");
+  PathService::Get(base::DIR_TEMP, &dumps_path);
   if (getenv(env_vars::kHeadless)) {
     upload = false;
     PathService::Get(chrome::DIR_CRASH_DUMPS, &dumps_path);
@@ -276,20 +302,55 @@ void CrashHandlerHostLinux::OnFileCanReadWithoutBlocking(int fd) {
   // Sanitize the string data a bit more
   guid[kGuidSize] = crash_url[kMaxActiveURLSize] = distro[kDistroSize] = 0;
 
-  BreakpadInfo info;
-  info.filename = minidump_filename.c_str();
-  info.process_type = process_type_.c_str();
-  info.process_type_length = process_type_.length();
-  info.crash_url = crash_url;
-  info.crash_url_length = strlen(crash_url);
-  info.guid = guid;
-  info.guid_length = strlen(guid);
-  info.distro = distro;
-  info.distro_length = strlen(distro);
-  info.upload = upload;
-  HandleCrashDump(info);
+  BreakpadInfo* info = new BreakpadInfo;
+
+  char* minidump_filename_str = new char[minidump_filename.length() + 1];
+  minidump_filename.copy(minidump_filename_str, minidump_filename.length());
+  minidump_filename_str[minidump_filename.length()] = '\0';
+  info->filename = minidump_filename_str;
+
+  info->process_type_length = process_type_.length();
+  char* process_type_str = new char[info->process_type_length + 1];
+  process_type_.copy(process_type_str, info->process_type_length);
+  process_type_str[info->process_type_length] = '\0';
+  info->process_type = process_type_str;
+
+  info->crash_url_length = strlen(crash_url);
+  info->crash_url = crash_url;
+
+  info->guid_length = strlen(guid);
+  info->guid = guid;
+
+  info->distro_length = strlen(distro);
+  info->distro = distro;
+
+  info->upload = upload;
+
+  uploader_thread_->message_loop()->PostTask(
+      FROM_HERE,
+      NewRunnableFunction(&CrashDumpTask, info));
 }
 
 void CrashHandlerHostLinux::WillDestroyCurrentMessageLoop() {
   file_descriptor_watcher_.StopWatchingFileDescriptor();
 }
+
+PluginCrashHandlerHostLinux::PluginCrashHandlerHostLinux() {
+}
+
+PluginCrashHandlerHostLinux::~PluginCrashHandlerHostLinux() {
+}
+
+void PluginCrashHandlerHostLinux::SetProcessType() {
+  process_type_ = "plugin";
+}
+
+RendererCrashHandlerHostLinux::RendererCrashHandlerHostLinux() {
+}
+
+RendererCrashHandlerHostLinux::~RendererCrashHandlerHostLinux() {
+}
+
+void RendererCrashHandlerHostLinux::SetProcessType() {
+  process_type_ = "renderer";
+}
diff --git a/chrome/browser/crash_handler_host_linux.h b/chrome/browser/crash_handler_host_linux.h
index 625ac8e..7aa8dbe 100644
--- a/chrome/browser/crash_handler_host_linux.h
+++ b/chrome/browser/crash_handler_host_linux.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -8,8 +8,13 @@
 
 #include <string>
 
-#include "base/singleton.h"
 #include "base/message_loop.h"
+#include "base/scoped_ptr.h"
+#include "base/singleton.h"
+
+namespace base {
+class Thread;
+}
 
 // This is the base class for singleton objects which crash dump renderers and
 // plugins on Linux. We perform the crash dump from the browser because it
@@ -51,6 +56,7 @@ class CrashHandlerHostLinux : public MessageLoopForIO::Watcher,
   int process_socket_;
   int browser_socket_;
   MessageLoopForIO::FileDescriptorWatcher file_descriptor_watcher_;
+  scoped_ptr<base::Thread> uploader_thread_;
 
   DISALLOW_COPY_AND_ASSIGN(CrashHandlerHostLinux);
 };
@@ -58,14 +64,10 @@ class CrashHandlerHostLinux : public MessageLoopForIO::Watcher,
 class PluginCrashHandlerHostLinux : public CrashHandlerHostLinux {
  private:
   friend struct DefaultSingletonTraits<PluginCrashHandlerHostLinux>;
-  PluginCrashHandlerHostLinux() {
-    SetProcessType();
-  }
-  virtual ~PluginCrashHandlerHostLinux() {}
+  PluginCrashHandlerHostLinux();
+  virtual ~PluginCrashHandlerHostLinux();
 
-  virtual void SetProcessType() {
-    process_type_ = "plugin";
-  }
+  virtual void SetProcessType();
 
   DISALLOW_COPY_AND_ASSIGN(PluginCrashHandlerHostLinux);
 };
@@ -73,14 +75,10 @@ class PluginCrashHandlerHostLinux : public CrashHandlerHostLinux {
 class RendererCrashHandlerHostLinux : public CrashHandlerHostLinux {
  private:
   friend struct DefaultSingletonTraits<RendererCrashHandlerHostLinux>;
-  RendererCrashHandlerHostLinux() {
-    SetProcessType();
-  }
-  virtual ~RendererCrashHandlerHostLinux() {}
+  RendererCrashHandlerHostLinux();
+  virtual ~RendererCrashHandlerHostLinux();
 
-  virtual void SetProcessType() {
-    process_type_ = "renderer";
-  }
+  virtual void SetProcessType();
 
   DISALLOW_COPY_AND_ASSIGN(RendererCrashHandlerHostLinux);
 };