Make crash reporting client_id accessible through child_process_logging.

On Mac and Linux, keep the client id in a global variable kept by the
child_process_logging implementations. This allows to read it in a
thread safe fashion when starting a child process.

Also replace std::string with statically allocated buffers for the
various items we add to the crash reports. This allows to properly
handle crashes upon shutdown (std::string would run its destructor,
invalidating the memory).

BUG=53231
TEST=Crash reporting should still work

Review URL: http://codereview.chromium.org/3186028

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

8 files changed, 87 insertions, 52 deletions

diff --git a/base/linux_util.cc b/base/linux_util.cc
index b8c78e4..246c95c 100644
--- a/base/linux_util.cc
+++ b/base/linux_util.cc
@@ -126,9 +126,12 @@ bool ProcPathGetInode(ino_t* inode_out, const char* path, bool log = false) {
 
 namespace base {
 
+// Account for the terminating null character.
+static const int kDistroSize = 128 + 1;
+
 // We use this static string to hold the Linux distro info. If we
 // crash, the crash handler code will send this in the crash dump.
-std::string linux_distro =
+char g_linux_distro[kDistroSize] =
 #if defined(OS_CHROMEOS)
     "CrOS";
 #else  // if defined(OS_LINUX)
@@ -137,7 +140,7 @@ std::string linux_distro =
 
 std::string GetLinuxDistro() {
 #if defined(OS_CHROMEOS)
-  return linux_distro;
+  return g_linux_distro;
 #elif defined(OS_LINUX)
   LinuxDistroHelper* distro_state_singleton = LinuxDistroHelper::Get();
   LinuxDistroState state = distro_state_singleton->State();
@@ -154,25 +157,30 @@ std::string GetLinuxDistro() {
       // lsb_release -d should return: Description:<tab>Distro Info
       static const std::string field = "Description:\t";
       if (output.compare(0, field.length(), field) == 0) {
-        linux_distro = output.substr(field.length());
-        TrimWhitespaceASCII(linux_distro, TRIM_ALL, &linux_distro);
+        SetLinuxDistro(output.substr(field.length()));
       }
     }
     distro_state_singleton->CheckFinished();
-    return linux_distro;
+    return g_linux_distro;
   } else if (STATE_CHECK_STARTED == state) {
     // If the distro check above is in progress in some other thread, we're
     // not going to wait for the results.
     return "Unknown";
   } else {
     // In STATE_CHECK_FINISHED, no more writing to |linux_distro|.
-    return linux_distro;
+    return g_linux_distro;
   }
 #else
   NOTIMPLEMENTED();
 #endif
 }
 
+void SetLinuxDistro(const std::string& distro) {
+  std::string trimmed_distro;
+  TrimWhitespaceASCII(distro, TRIM_ALL, &trimmed_distro);
+  base::strlcpy(g_linux_distro, trimmed_distro.c_str(), kDistroSize);
+}
+
 bool FileDescriptorGetInode(ino_t* inode_out, int fd) {
   DCHECK(inode_out);
 
diff --git a/base/linux_util.h b/base/linux_util.h
index 91cb113..0d2f20e 100644
--- a/base/linux_util.h
+++ b/base/linux_util.h
@@ -15,10 +15,17 @@ namespace base {
 
 static const char kFindInodeSwitch[] = "--find-inode";
 
+// This is declared here so the crash reporter can access the memory directly
+// in compromised context without going through the standard library.
+extern char g_linux_distro[];
+
 // Get the Linux Distro if we can, or return "Unknown", similar to
 // GetWinVersion() in base/win_util.h.
 std::string GetLinuxDistro();
 
+// Set the Linux Distro string.
+void SetLinuxDistro(const std::string& distro);
+
 // Return the inode number for the UNIX domain socket |fd|.
 bool FileDescriptorGetInode(ino_t* inode_out, int fd);
 
diff --git a/chrome/app/breakpad_linux.cc b/chrome/app/breakpad_linux.cc
index 795f3f0..4d32ac5 100644
--- a/chrome/app/breakpad_linux.cc
+++ b/chrome/app/breakpad_linux.cc
@@ -22,6 +22,7 @@
 #include "base/eintr_wrapper.h"
 #include "base/file_path.h"
 #include "base/global_descriptors_posix.h"
+#include "base/linux_util.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "breakpad/src/client/linux/handler/exception_handler.h"
@@ -29,12 +30,12 @@
 #include "breakpad/src/common/linux/linux_libc_support.h"
 #include "breakpad/src/common/linux/linux_syscall_support.h"
 #include "breakpad/src/common/linux/memory.h"
+#include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_descriptors.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info_posix.h"
 #include "chrome/common/env_vars.h"
-#include "chrome/installer/util/google_update_settings.h"
 
 // Some versions of gcc are prone to warn about unused return values. In cases
 // where we either a) know the call cannot fail, or b) there is nothing we
@@ -579,19 +580,6 @@ pid_t HandleCrashDump(const BreakpadInfo& info) {
   return child;
 }
 
-// This is defined in chrome/browser/google_update_settings_posix.cc, it's the
-// static string containing the user's unique GUID. We send this in the crash
-// report.
-namespace google_update {
-extern std::string posix_guid;
-}
-
-// This is defined in base/linux_util.cc, it's the static string containing the
-// user's distro info. We send this in the crash report.
-namespace base {
-extern std::string linux_distro;
-}
-
 static bool CrashDone(const char* dump_path,
                       const char* minidump_id,
                       const bool upload,
@@ -619,10 +607,10 @@ static bool CrashDone(const char* dump_path,
   info.process_type_length = 7;
   info.crash_url = NULL;
   info.crash_url_length = 0;
-  info.guid = google_update::posix_guid.data();
-  info.guid_length = google_update::posix_guid.length();
-  info.distro = base::linux_distro.data();
-  info.distro_length = base::linux_distro.length();
+  info.guid = child_process_logging::g_client_id;
+  info.guid_length = my_strlen(child_process_logging::g_client_id);
+  info.distro = base::g_linux_distro;
+  info.distro_length = my_strlen(base::g_linux_distro);
   info.upload = upload;
   HandleCrashDump(info);
 
@@ -659,13 +647,6 @@ void EnableCrashDumping(const bool unattended) {
   }
 }
 
-// This is defined in chrome/common/child_process_logging_linux.cc, it's the
-// static string containing the current active URL. We send this in the crash
-// report.
-namespace child_process_logging {
-extern std::string active_url;
-}
-
 // Currently Non-Browser = Renderer and Plugins
 static bool
 NonBrowserCrashHandler(const void* crash_context, size_t crash_context_size,
@@ -680,15 +661,16 @@ NonBrowserCrashHandler(const void* crash_context, size_t crash_context_size,
   char guid[kGuidSize + 1] = {0};
   char crash_url[kMaxActiveURLSize + 1] = {0};
   char distro[kDistroSize + 1] = {0};
-  const size_t guid_len = std::min(google_update::posix_guid.size(),
-                                   kGuidSize);
+  const size_t guid_len =
+      std::min(my_strlen(child_process_logging::g_client_id), kGuidSize);
   const size_t crash_url_len =
-      std::min(child_process_logging::active_url.size(), kMaxActiveURLSize);
+      std::min(my_strlen(child_process_logging::g_active_url),
+               kMaxActiveURLSize);
   const size_t distro_len =
-      std::min(base::linux_distro.size(), kDistroSize);
-  memcpy(guid, google_update::posix_guid.data(), guid_len);
-  memcpy(crash_url, child_process_logging::active_url.data(), crash_url_len);
-  memcpy(distro, base::linux_distro.data(), distro_len);
+      std::min(my_strlen(base::g_linux_distro), kDistroSize);
+  memcpy(guid, child_process_logging::g_client_id, guid_len);
+  memcpy(crash_url, child_process_logging::g_active_url, crash_url_len);
+  memcpy(distro, base::g_linux_distro, distro_len);
 
   char b;  // Dummy variable for sys_read below.
   const char* b_addr = &b;  // Get the address of |b| so we can create the
@@ -776,10 +758,10 @@ void InitCrashReporter() {
         parsed_command_line.GetSwitchValueASCII(switches::kEnableCrashReporter);
     size_t separator = switch_value.find(",");
     if (separator != std::string::npos) {
-      google_update::posix_guid = switch_value.substr(0, separator);
-      base::linux_distro = switch_value.substr(separator + 1);
+      child_process_logging::SetClientId(switch_value.substr(0, separator));
+      base::SetLinuxDistro(switch_value.substr(separator + 1));
     } else {
-      google_update::posix_guid = switch_value;
+      child_process_logging::SetClientId(switch_value);
     }
     EnableNonBrowserCrashDumping();
   }
diff --git a/chrome/browser/browser_child_process_host.cc b/chrome/browser/browser_child_process_host.cc
index cf2dc15..36f8411 100644
--- a/chrome/browser/browser_child_process_host.cc
+++ b/chrome/browser/browser_child_process_host.cc
@@ -14,9 +14,9 @@
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "chrome/app/breakpad_mac.h"
-#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
@@ -79,16 +79,13 @@ void BrowserChildProcessHost::SetCrashReporterCommandLine(
     CommandLine* command_line) {
 #if defined(USE_LINUX_BREAKPAD)
   if (IsCrashReporterEnabled()) {
-    std::string client_id =
-        g_browser_process->local_state()->GetString(prefs::kMetricsClientID);
     command_line->AppendSwitchASCII(switches::kEnableCrashReporter,
-                                    client_id + "," + base::GetLinuxDistro());
+        child_process_logging::GetClientId() + "," + base::GetLinuxDistro());
   }
 #elif defined(OS_MACOSX)
   if (IsCrashReporterEnabled()) {
-    std::string client_id =
-        g_browser_process->local_state()->GetString(prefs::kMetricsClientID);
-    command_line->AppendSwitchASCII(switches::kEnableCrashReporter, client_id);
+    command_line->AppendSwitchASCII(switches::kEnableCrashReporter,
+                                    child_process_logging::GetClientId());
   }
 #endif  // OS_MACOSX
 }
diff --git a/chrome/common/child_process_logging.h b/chrome/common/child_process_logging.h
index 6de7bfe..bdc5a44 100644
--- a/chrome/common/child_process_logging.h
+++ b/chrome/common/child_process_logging.h
@@ -23,6 +23,13 @@ static const int kMaxReportedActiveExtensions = 10;
 
 namespace child_process_logging {
 
+#if defined(OS_LINUX)
+// These are declared here so the crash reporter can access them directly in
+// compromised context without going through the standard library.
+extern char g_active_url[];
+extern char g_client_id[];
+#endif
+
 // Sets the URL that is logged if the child process crashes. Use GURL() to clear
 // the URL.
 void SetActiveURL(const GURL& url);
@@ -30,6 +37,10 @@ void SetActiveURL(const GURL& url);
 // Sets the Client ID that is used as GUID if a Chrome process crashes.
 void SetClientId(const std::string& client_id);
 
+// Gets the Client ID to be used as GUID for crash reporting. Returns the client
+// id in |client_id| if it's known, an empty string otherwise.
+std::string GetClientId();
+
 // Sets the list of "active" extensions in this process. We overload "active" to
 // mean different things depending on the process type:
 // - browser: all enabled extensions
diff --git a/chrome/common/child_process_logging_linux.cc b/chrome/common/child_process_logging_linux.cc
index 40ad90c..d8f216e 100644
--- a/chrome/common/child_process_logging_linux.cc
+++ b/chrome/common/child_process_logging_linux.cc
@@ -13,12 +13,20 @@
 
 namespace child_process_logging {
 
-// We use a static string to hold the most recent active url. If we crash, the
-// crash handler code will send the contents of this string to the browser.
-std::string active_url;
+// Account for the terminating null character.
+static const size_t kMaxActiveURLSize = 1024 + 1;
+static const size_t kClientIdSize = 32 + 1;
+
+// We use static strings to hold the most recent active url and the client
+// identifier. If we crash, the crash handler code will send the contents of
+// these strings to the browser.
+char g_active_url[kMaxActiveURLSize];
+char g_client_id[kClientIdSize];
 
 void SetActiveURL(const GURL& url) {
-  active_url = url.possibly_invalid_spec();
+  base::strlcpy(g_active_url,
+                url.possibly_invalid_spec().c_str(),
+                kMaxActiveURLSize);
 }
 
 void SetClientId(const std::string& client_id) {
@@ -28,10 +36,15 @@ void SetClientId(const std::string& client_id) {
   if (str.empty())
     return;
 
+  base::strlcpy(g_client_id, str.c_str(), kClientIdSize);
   std::wstring wstr = ASCIIToWide(str);
   GoogleUpdateSettings::SetMetricsId(wstr);
 }
 
+std::string GetClientId() {
+  return std::string(g_client_id);
+}
+
 void SetActiveExtensions(const std::set<std::string>& extension_ids) {
   // TODO(port)
 }
diff --git a/chrome/common/child_process_logging_mac.mm b/chrome/common/child_process_logging_mac.mm
index a53b4ed..04f4ca4 100644
--- a/chrome/common/child_process_logging_mac.mm
+++ b/chrome/common/child_process_logging_mac.mm
@@ -28,6 +28,10 @@ const char *kGPUVertexShaderVersionParamName = "vsver";
 static SetCrashKeyValueFuncPtr g_set_key_func;
 static ClearCrashKeyValueFuncPtr g_clear_key_func;
 
+// Account for the terminating null character.
+static const size_t kClientIdSize = 32 + 1;
+static char g_client_id[kClientIdSize];
+
 void SetCrashKeyFunctions(SetCrashKeyValueFuncPtr set_key_func,
                           ClearCrashKeyValueFuncPtr clear_key_func) {
   g_set_key_func = set_key_func;
@@ -93,6 +97,7 @@ void SetClientId(const std::string& client_id) {
   std::string str(client_id);
   ReplaceSubstringsAfterOffset(&str, 0, "-", "");
 
+  base::strlcpy(g_client_id, str.c_str(), kClientIdSize);
   if (g_set_key_func)
     SetClientIdImpl(str, g_set_key_func);
 
@@ -100,6 +105,10 @@ void SetClientId(const std::string& client_id) {
   GoogleUpdateSettings::SetMetricsId(wstr);
 }
 
+std::string GetClientId() {
+  return std::string(g_client_id);
+}
+
 void SetActiveExtensions(const std::set<std::string>& extension_ids) {
   // TODO(port)
 }
diff --git a/chrome/common/child_process_logging_win.cc b/chrome/common/child_process_logging_win.cc
index 046244f..d587f2e 100644
--- a/chrome/common/child_process_logging_win.cc
+++ b/chrome/common/child_process_logging_win.cc
@@ -73,6 +73,14 @@ void SetClientId(const std::string& client_id) {
   (set_client_id)(wstr.c_str());
 }
 
+std::string GetClientId() {
+  std::wstring wstr_client_id;
+  if (GoogleUpdateSettings::GetMetricsId(&wstr_client_id))
+    return WideToASCII(wstr_client_id);
+  else
+    return std::string();
+}
+
 void SetActiveExtensions(const std::set<std::string>& extension_ids) {
   static MainSetExtensionID set_extension_id = NULL;
   if (!set_extension_id) {