Change id that identifies client in crash reports. Whenever metrics service recording is enabled, it sets the client id for crash reporting.

- On Windows this id gets stored in the registry so that we can read it pretty early regardless of the process type. If the id has not been generated (like in the case of first run) we initialize with empty string but the real id gets inserted once metrics service gets initialized.
- On Linux we were creating a hash and storing it in 'Consent to Send Stats'. This change replaces that hash with the metrics id. Unlike before calling SetConsentToSendStats doesn't generate a new id, if an id already exists.
- On Mac there was no id set. Now we use metrics id as guid for the browser process. For other process types a change is still required to pass that id as command line param to renderers/plugins (like Linux).

BUG=23658
TEST=Cause a deliberate crash in Chrome renderer/browser/plugin and make sure the clientID reported to the crash server is the right GUID.

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

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

15 files changed, 161 insertions, 43 deletions

diff --git a/chrome/app/breakpad_linux.cc b/chrome/app/breakpad_linux.cc
index c60b3e8..fe47108 100644
--- a/chrome/app/breakpad_linux.cc
+++ b/chrome/app/breakpad_linux.cc
@@ -522,11 +522,11 @@ pid_t HandleCrashDump(const BreakpadInfo& info) {
   return child;
 }
 
-// This is defined in chrome/browser/google_update_settings_linux.cc, it's the
+// 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 linux_guid;
+extern std::string posix_guid;
 }
 
 // This is defined in base/linux_util.cc, it's the static string containing the
@@ -562,8 +562,8 @@ 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::linux_guid.data();
-  info.guid_length = google_update::linux_guid.length();
+  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.upload = upload;
@@ -617,13 +617,13 @@ RendererCrashHandler(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::linux_guid.size(),
+  const size_t guid_len = std::min(google_update::posix_guid.size(),
                                    kGuidSize);
   const size_t crash_url_len =
       std::min(child_process_logging::active_url.size(), kMaxActiveURLSize);
   const size_t distro_len =
       std::min(base::linux_distro.size(), kDistroSize);
-  memcpy(guid, google_update::linux_guid.data(), guid_len);
+  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);
 
@@ -636,7 +636,7 @@ RendererCrashHandler(const void* crash_context, size_t crash_context_size,
   iov[0].iov_base = const_cast<void*>(crash_context);
   iov[0].iov_len = crash_context_size;
   iov[1].iov_base = guid;
-  iov[1].iov_len = kGuidSize + 1;
+  iov[1].iov_len = guid_len + 1;
   iov[2].iov_base = crash_url;
   iov[2].iov_len = kMaxActiveURLSize + 1;
   iov[3].iov_base = distro;
@@ -695,10 +695,10 @@ void InitCrashReporter() {
         parsed_command_line.GetSwitchValue(switches::kEnableCrashReporter));
     size_t separator = switch_value.find(",");
     if (separator != std::string::npos) {
-      google_update::linux_guid = switch_value.substr(0, separator);
+      google_update::posix_guid = switch_value.substr(0, separator);
       base::linux_distro = switch_value.substr(separator + 1);
     } else {
-      google_update::linux_guid = switch_value;
+      google_update::posix_guid = switch_value;
     }
     EnableRendererCrashDumping();
   }
diff --git a/chrome/app/breakpad_mac.mm b/chrome/app/breakpad_mac.mm
index 11b08d6..f66093b 100644
--- a/chrome/app/breakpad_mac.mm
+++ b/chrome/app/breakpad_mac.mm
@@ -131,6 +131,12 @@ void InitCrashReporter() {
   // TODO: Should this only be done for certain process types?
   child_process_logging::SetCrashKeyFunctions(SetCrashKeyValue,
                                               ClearCrashKeyValue);
+  if (!is_browser) {
+    // Get the guid from the command line switch.
+    std::string guid = WideToASCII(
+        parsed_command_line.GetSwitchValue(switches::kEnableCrashReporter));
+    child_process_logging::SetClientId(guid);
+   }
 }
 
 void InitCrashProcessInfo() {
diff --git a/chrome/app/breakpad_win.cc b/chrome/app/breakpad_win.cc
index 82122bb..fd635fc 100644
--- a/chrome/app/breakpad_win.cc
+++ b/chrome/app/breakpad_win.cc
@@ -35,6 +35,10 @@ google_breakpad::ExceptionHandler* g_breakpad = NULL;
 
 std::vector<wchar_t*>* g_url_chunks = NULL;
 
+// A string containing the user's unique metric services id. We send this
+// in the crash report.
+wchar_t* g_client_id = NULL;
+
 // Dumps the current process memory.
 extern "C" void __declspec(dllexport) __cdecl DumpProcess() {
   if (g_breakpad)
@@ -76,6 +80,13 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const std::wstring& dll_path,
   google_breakpad::CustomInfoEntry plat_entry(L"plat", L"Win32");
   google_breakpad::CustomInfoEntry type_entry(L"ptype", type.c_str());
 
+  // Read the id from registry. If reporting has never been enabled
+  // the result will be empty string. Its OK since when user enables reporting
+  // we will insert the new value at this location.
+  std::wstring guid;
+  GoogleUpdateSettings::GetMetricsId(&guid);
+  google_breakpad::CustomInfoEntry guid_entry(L"guid", guid.c_str());
+
   if (type == L"renderer" || type == L"plugin") {
     // Create entries for the URL. Currently we only allow each chunk to be 64
     // characters, which isn't enough for a URL. As a hack we create 8 entries
@@ -90,14 +101,16 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const std::wstring& dll_path,
     google_breakpad::CustomInfoEntry url8(L"url-chunk-8", L"");
 
     static google_breakpad::CustomInfoEntry entries[] =
-        { ver_entry, prod_entry, plat_entry, type_entry,
+        { ver_entry, prod_entry, plat_entry, type_entry, guid_entry,
           url1, url2, url3, url4, url5, url6, url7, url8 };
 
     std::vector<wchar_t*>* tmp_url_chunks = new std::vector<wchar_t*>(8);
     for (size_t i = 0; i < 8; ++i)
-      (*tmp_url_chunks)[i] = entries[4 + i].value;
+      (*tmp_url_chunks)[i] = entries[5 + i].value;
     g_url_chunks = tmp_url_chunks;
 
+    g_client_id = entries[4].value;
+
     static google_breakpad::CustomClientInfo custom_info_renderer
         = {entries, arraysize(entries)};
     return &custom_info_renderer;
@@ -119,7 +132,9 @@ google_breakpad::CustomClientInfo* GetCustomInfo(const std::wstring& dll_path,
   }
 
   static google_breakpad::CustomInfoEntry entries[] =
-      {ver_entry, prod_entry, plat_entry, type_entry, switch1, switch2};
+      {ver_entry, prod_entry, plat_entry, type_entry, guid_entry,
+       switch1, switch2};
+  g_client_id = entries[4].value;
   static google_breakpad::CustomClientInfo custom_info_browser =
       {entries, arraysize(entries)};
   return &custom_info_browser;
@@ -217,6 +232,16 @@ extern "C" void __declspec(dllexport) __cdecl SetActiveURL(
     (*g_url_chunks)[chunk_index][0] = L'\0';
 }
 
+extern "C" void __declspec(dllexport) __cdecl SetClientId(
+    const wchar_t* client_id) {
+  if (client_id == NULL)
+    return;
+
+  wcscpy_s(g_client_id,
+           google_breakpad::CustomInfoEntry::kValueMaxLength,
+           client_id);
+}
+
 }  // namespace
 
 // This function is executed by the child process that DumpDoneCallback()
diff --git a/chrome/browser/google_update_settings_posix.cc b/chrome/browser/google_update_settings_posix.cc
index defe492..ff5bb4c 100644..100755
--- a/chrome/browser/google_update_settings_posix.cc
+++ b/chrome/browser/google_update_settings_posix.cc
@@ -13,41 +13,29 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 
-#if !defined(OS_MACOSX)
 namespace google_update {
-std::string linux_guid;
+std::string posix_guid;
 }
-#endif  // !OS_MACOSX
 
 // File name used in the user data dir to indicate consent.
 static const char kConsentToSendStats[] = "Consent To Send Stats";
-static const int kGuidLen = sizeof(uint64) * 4;  // 128 bits -> 32 bytes hex.
 
 // static
 bool GoogleUpdateSettings::GetCollectStatsConsent() {
   bool forced_enable = CommandLine::ForCurrentProcess()->
       HasSwitch(switches::kEnableCrashReporter);
-#if defined(OS_MACOSX)
-  std::string linux_guid;
-#else
-  using google_update::linux_guid;
-#endif  // OS_MACOSX
   FilePath consent_file;
   PathService::Get(chrome::DIR_USER_DATA, &consent_file);
   consent_file = consent_file.Append(kConsentToSendStats);
-  bool consented = file_util::ReadFileToString(consent_file, &linux_guid);
-  linux_guid.resize(kGuidLen, '0');
+  std::string tmp_guid;
+  bool consented = file_util::ReadFileToString(consent_file, &tmp_guid);
+  if (forced_enable || consented)
+    google_update::posix_guid.assign(tmp_guid);
   return forced_enable || consented;
 }
 
 // static
 bool GoogleUpdateSettings::SetCollectStatsConsent(bool consented) {
-#if defined(OS_MACOSX)
-  std::string linux_guid;
-#else
-  using google_update::linux_guid;
-#endif  // OS_MACOSX
-
   FilePath consent_dir;
   PathService::Get(chrome::DIR_USER_DATA, &consent_dir);
   if (!file_util::DirectoryExists(consent_dir))
@@ -55,17 +43,29 @@ bool GoogleUpdateSettings::SetCollectStatsConsent(bool consented) {
 
   FilePath consent_file = consent_dir.AppendASCII(kConsentToSendStats);
   if (consented) {
-    uint64 random;
-    linux_guid.clear();
-    for (int i = 0; i < 2; i++) {
-      random = base::RandUint64();
-      linux_guid += HexEncode(&random, sizeof(uint64));
+    if ((!file_util::PathExists(consent_file)) ||
+        (file_util::PathExists(consent_file) &&
+         !google_update::posix_guid.empty())) {
+      const char* c_str = google_update::posix_guid.c_str();
+      int size = google_update::posix_guid.size();
+      return file_util::WriteFile(consent_file, c_str, size) == size;
     }
-    const char* c_str = linux_guid.c_str();
-    return file_util::WriteFile(consent_file, c_str, kGuidLen) == kGuidLen;
   } else {
-    linux_guid .clear();
-    linux_guid.resize(kGuidLen, '0');
+    google_update::posix_guid.clear();
     return file_util::Delete(consent_file, false);
   }
+  return true;
+}
+
+bool GoogleUpdateSettings::SetMetricsId(const std::wstring& client_id) {
+  // Make sure that user has consented to send crashes.
+  FilePath consent_dir;
+  PathService::Get(chrome::DIR_USER_DATA, &consent_dir);
+  if (!file_util::DirectoryExists(consent_dir) ||
+      !GoogleUpdateSettings::GetCollectStatsConsent())
+    return false;
+
+  // Since user has consented, write the metrics id to the file.
+  google_update::posix_guid = WideToASCII(client_id);
+  return GoogleUpdateSettings::SetCollectStatsConsent(true);
 }
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 20b7adc..dc052f9 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -177,6 +177,7 @@
 #include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/search_engines/template_url_model.h"
+#include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/histogram_synchronizer.h"
 #include "chrome/common/notification_service.h"
@@ -462,6 +463,7 @@ void MetricsService::SetRecording(bool enabled) {
                         Int64ToWString(Time::Now().ToTimeT()));
       }
     }
+    child_process_logging::SetClientId(client_id_);
     StartRecording();
 
     registrar_.Add(this, NotificationType::BROWSER_OPENED,
diff --git a/chrome/common/child_process_host.cc b/chrome/common/child_process_host.cc
index 69655b4..d4a9eda 100644
--- a/chrome/common/child_process_host.cc
+++ b/chrome/common/child_process_host.cc
@@ -26,14 +26,16 @@
 
 #if defined(OS_LINUX)
 #include "base/linux_util.h"
+#endif  // OS_LINUX
 
-// This is defined in chrome/browser/google_update_settings_linux.cc.  It's the
+#if defined(OS_POSIX)
+// 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 linux_guid;
+extern std::string posix_guid;
 }  // namespace google_update
-#endif  // OS_LINUX
+#endif  // OS_POSIX
 
 
 namespace {
@@ -111,13 +113,14 @@ void ChildProcessHost::SetCrashReporterCommandLine(CommandLine* command_line) {
   const bool unattended = (getenv("CHROME_HEADLESS") != NULL);
   if (unattended || GoogleUpdateSettings::GetCollectStatsConsent()) {
     command_line->AppendSwitchWithValue(switches::kEnableCrashReporter,
-                                        ASCIIToWide(google_update::linux_guid +
+                                        ASCIIToWide(google_update::posix_guid +
                                                     "," +
                                                     base::GetLinuxDistro()));
   }
 #elif defined(OS_MACOSX)
   if (GoogleUpdateSettings::GetCollectStatsConsent())
-    command_line->AppendSwitch(switches::kEnableCrashReporter);
+    command_line->AppendSwitchWithValue(switches::kEnableCrashReporter,
+                                        ASCIIToWide(google_update::posix_guid));
 #endif  // OS_MACOSX
 }
 
diff --git a/chrome/common/child_process_logging.h b/chrome/common/child_process_logging.h
index 7d1f16f..aa336af 100644
--- a/chrome/common/child_process_logging.h
+++ b/chrome/common/child_process_logging.h
@@ -14,6 +14,9 @@ namespace child_process_logging {
 // the URL.
 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);
+
 // Simple wrapper class that sets the active URL in it's constructor and clears
 // the active URL in the destructor.
 class ScopedActiveURLSetter {
diff --git a/chrome/common/child_process_logging_linux.cc b/chrome/common/child_process_logging_linux.cc
index 59b16f6..9b51303 100644
--- a/chrome/common/child_process_logging_linux.cc
+++ b/chrome/common/child_process_logging_linux.cc
@@ -7,6 +7,8 @@
 #include <string>
 
 #include "base/logging.h"
+#include "base/string_util.h"
+#include "chrome/installer/util/google_update_settings.h"
 #include "googleurl/src/gurl.h"
 
 namespace child_process_logging {
@@ -19,4 +21,14 @@ void SetActiveURL(const GURL& url) {
   active_url = url.possibly_invalid_spec();
 }
 
+void SetClientId(const std::string& client_id) {
+  std::string str(client_id);
+  ReplaceSubstringsAfterOffset(&str, 0, "-", "");
+
+  if (str.empty())
+    return;
+
+  std::wstring wstr = ASCIIToWide(str);
+  GoogleUpdateSettings::SetMetricsId(wstr);
+}
 }  // namespace child_process_logging
diff --git a/chrome/common/child_process_logging_mac.mm b/chrome/common/child_process_logging_mac.mm
index e863239..9e89c01 100644
--- a/chrome/common/child_process_logging_mac.mm
+++ b/chrome/common/child_process_logging_mac.mm
@@ -7,6 +7,7 @@
 #import <Foundation/Foundation.h>
 
 #include "base/string_util.h"
+#include "chrome/installer/util/google_update_settings.h"
 #include "googleurl/src/gurl.h"
 
 namespace child_process_logging {
@@ -14,6 +15,7 @@ namespace child_process_logging {
 const int kMaxNumCrashURLChunks = 8;
 const int kMaxNumURLChunkValueLength = 255;
 const char *kUrlChunkFormatStr = "url-chunk-%d";
+const char *kGuidParamName = "guid";
 
 static SetCrashKeyValueFuncPtr g_set_key_func;
 static ClearCrashKeyValueFuncPtr g_clear_key_func;
@@ -67,9 +69,26 @@ void SetActiveURLImpl(const GURL& url,
   }
 }
 
+void SetClientIdImpl(const std::string& client_id,
+                     SetCrashKeyValueFuncPtr set_key_func) {
+  NSString *key = [NSString stringWithUTF8String:kGuidParamName];
+  NSString *value = [NSString stringWithUTF8String:client_id.c_str()];
+  set_key_func(key, value);
+}
+
 void SetActiveURL(const GURL& url) {
   if (g_set_key_func && g_clear_key_func)
     SetActiveURLImpl(url, g_set_key_func, g_clear_key_func);
 }
 
+void SetClientId(const std::string& client_id) {
+  std::string str(client_id);
+  ReplaceSubstringsAfterOffset(&str, 0, "-", "");
+
+  if (g_set_key_func)
+    SetClientIdImpl(str, g_set_key_func);
+
+  std::wstring wstr = ASCIIToWide(str);
+  GoogleUpdateSettings::SetMetricsId(wstr);
+}
 }  // namespace child_process_logging
diff --git a/chrome/common/child_process_logging_win.cc b/chrome/common/child_process_logging_win.cc
index e8ddd02..94b77d8 100644
--- a/chrome/common/child_process_logging_win.cc
+++ b/chrome/common/child_process_logging_win.cc
@@ -8,12 +8,16 @@
 
 #include "base/string_util.h"
 #include "chrome/common/chrome_constants.h"
+#include "chrome/installer/util/google_update_settings.h"
 #include "googleurl/src/gurl.h"
 
 namespace child_process_logging {
 // exported in breakpad_win.cc: void __declspec(dllexport) __cdecl SetActiveURL.
 typedef void (__cdecl *MainSetActiveURL)(const wchar_t*);
 
+// exported in breakpad_win.cc: void __declspec(dllexport) __cdecl SetClientId.
+typedef void (__cdecl *MainSetClientId)(const wchar_t*);
+
 void SetActiveURL(const GURL& url) {
   static MainSetActiveURL set_active_url = NULL;
   // note: benign race condition on set_active_url.
@@ -30,4 +34,31 @@ void SetActiveURL(const GURL& url) {
   (set_active_url)(UTF8ToWide(url.possibly_invalid_spec()).c_str());
 }
 
+void SetClientId(const std::string& client_id) {
+  std::string str(client_id);
+  // Remove all instance of '-' char from the GUID. So BCD-WXY becomes BCDWXY.
+  ReplaceSubstringsAfterOffset(&str, 0, "-", "");
+
+  if (str.empty())
+    return;
+
+  std::wstring wstr = ASCIIToWide(str);
+  std::wstring old_wstr;
+  if (!GoogleUpdateSettings::GetMetricsId(&old_wstr) ||
+      wstr != old_wstr)
+    GoogleUpdateSettings::SetMetricsId(wstr);
+
+  static MainSetClientId set_client_id = NULL;
+  if (!set_client_id) {
+    HMODULE exe_module = GetModuleHandle(chrome::kBrowserProcessExecutableName);
+    if (!exe_module)
+      return;
+    set_client_id = reinterpret_cast<MainSetClientId>(
+        GetProcAddress(exe_module, "SetClientId"));
+    if (!set_client_id)
+      return;
+  }
+  (set_client_id)(wstr.c_str());
+}
+
 }  // namespace child_process_logging
diff --git a/chrome/common/temp_scaffolding_stubs.cc b/chrome/common/temp_scaffolding_stubs.cc
index a1dc297..a1dc297 100644..100755
--- a/chrome/common/temp_scaffolding_stubs.cc
+++ b/chrome/common/temp_scaffolding_stubs.cc
diff --git a/chrome/installer/util/google_update_constants.cc b/chrome/installer/util/google_update_constants.cc
index 7b9fc2a..1c22547 100644
--- a/chrome/installer/util/google_update_constants.cc
+++ b/chrome/installer/util/google_update_constants.cc
@@ -19,6 +19,7 @@ const wchar_t kRegClientField[] = L"client";
 const wchar_t kRegDidRunField[] = L"dr";
 const wchar_t kRegLangField[] = L"lang";
 const wchar_t kRegLastCheckedField[] = L"LastChecked";
+const wchar_t kRegMetricsId[] = L"metricsid";
 const wchar_t kRegNameField[] = L"name";
 const wchar_t kRegOldVersionField[] = L"opv";
 const wchar_t kRegRenameCmdField[] = L"cmd";
diff --git a/chrome/installer/util/google_update_constants.h b/chrome/installer/util/google_update_constants.h
index cfb7ab1..e1458ba 100644
--- a/chrome/installer/util/google_update_constants.h
+++ b/chrome/installer/util/google_update_constants.h
@@ -27,6 +27,7 @@ extern const wchar_t kRegClientField[];
 extern const wchar_t kRegDidRunField[];
 extern const wchar_t kRegLangField[];
 extern const wchar_t kRegLastCheckedField[];
+extern const wchar_t kRegMetricsId[];
 extern const wchar_t kRegNameField[];
 extern const wchar_t kRegOldVersionField[];
 extern const wchar_t kRegRenameCmdField[];
diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc
index 5d43ea48..3d3688b 100644
--- a/chrome/installer/util/google_update_settings.cc
+++ b/chrome/installer/util/google_update_settings.cc
@@ -67,6 +67,14 @@ bool GoogleUpdateSettings::SetCollectStatsConsent(bool consented) {
   return key_hkcu.WriteValue(google_update::kRegUsageStatsField, value);
 }
 
+bool GoogleUpdateSettings::GetMetricsId(std::wstring* metrics_id) {
+  return ReadGoogleUpdateStrKey(google_update::kRegMetricsId, metrics_id);
+}
+
+bool GoogleUpdateSettings::SetMetricsId(const std::wstring& metrics_id) {
+  return WriteGoogleUpdateStrKey(google_update::kRegMetricsId, metrics_id);
+}
+
 bool GoogleUpdateSettings::SetEULAConsent(bool consented) {
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateMediumKey();
diff --git a/chrome/installer/util/google_update_settings.h b/chrome/installer/util/google_update_settings.h
index a4814fe7..bdc1292 100644
--- a/chrome/installer/util/google_update_settings.h
+++ b/chrome/installer/util/google_update_settings.h
@@ -23,6 +23,13 @@ class GoogleUpdateSettings {
   // false if the setting could not be recorded.
   static bool SetCollectStatsConsent(bool consented);
 
+  // Returns the metrics id set in the registry (that can be used in crash
+  // reports). If none found, returns empty string.
+  static bool GetMetricsId(std::wstring* metrics_id);
+
+  // Sets the metrics id to be used in crash reports.
+  static bool SetMetricsId(const std::wstring& metrics_id);
+
   // Sets the machine-wide EULA consented flag required on OEM installs.
   // Returns false if the setting could not be recorded.
   static bool SetEULAConsent(bool consented);