Refactor SetClientID such that metrics rather than crash backs up the client id in Google Update set

chrome/common/crash_keys.cc

Index: chrome/common/crash_keys.cc
diff --git a/chrome/common/crash_keys.cc b/chrome/common/crash_keys.cc
index edb6cac4edd35110539c9d57dea893c34620498a..4914f366325d2f763c8f2840b5e21ef18bb619af 100644
--- a/chrome/common/crash_keys.cc
+++ b/chrome/common/crash_keys.cc
@@ -11,7 +11,6 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/installer/util/google_update_settings.h"
 
 #if defined(OS_MACOSX)
 #include "breakpad/src/common/simple_string_dictionary.h"
@@ -225,15 +224,14 @@ size_t RegisterChromeCrashKeys() {
                                     kSingleChunkLength);
 }
 
-void SetClientID(const std::string& client_id) {
-  std::string guid(client_id);
+void SetClientIDFromGUID(const std::string& client_guid) {
+  std::string stripped_guid(client_guid);
   // Remove all instance of '-' char from the GUID. So BCD-WXY becomes BCDWXY.

[Ilya Sherman, 2014/07/08 03:21:38]

nit: "instance" -> "instances"

[gab, 2014/07/08 18:42:23]

Done.

-  ReplaceSubstringsAfterOffset(&guid, 0, "-", "");
-  if (guid.empty())
+  ReplaceSubstringsAfterOffset(&stripped_guid, 0, "-", "");
+  if (stripped_guid.empty())
     return;
 
-  base::debug::SetCrashKeyValue(kClientID, guid);
-  GoogleUpdateSettings::SetMetricsId(guid);

[Ilya Sherman, 2014/07/08 03:21:38]

Why is this line safe to remove?  Are you sure that it's not needed via the call
from chrome_breakpad_client.cc?

(I realize that this is the main point of the CL, but I'm not super familiar
with the code, so I don't know the exact reasons why this was here.)

[gab, 2014/07/08 18:42:23]

No it's not needed from the other call and yes this code is confusing (hence why
I'm trying to clean it up a bit first before putting another CL on top of it!).

The other call is only invoked from breakpad_linux.cc [1] which takes the value
from the command-line which was previously set using the value in
GoogleUpdateSettings [2] (so as long as that's set it doesn't matter where it
was set from...).

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/components/breakpa...
(breakpad_mac.mm does the same thing)
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

+  base::debug::SetCrashKeyValue(kClientID, stripped_guid);
 }
 
 static bool IsBoringSwitch(const std::string& flag) {