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

chrome/common/crash_keys.cc

 // Copyright (c) 2013 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.
 
 #include "chrome/common/crash_keys.h"
 
 #include "base/command_line.h"
 #include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/strings/string_split.h"
 #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"
 #elif defined(OS_WIN)
 #include "breakpad/src/client/windows/common/ipc_protocol.h"
 #elif defined(OS_CHROMEOS)
 #include "chrome/common/chrome_switches.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
 #include "ui/gl/gl_switches.h"
 #endif
@@ skipping 193 matching lines @@
       DCHECK_GT(n, 0);
       base::debug::CrashKey crash_key = { formatted_keys[i], kSmallSize };
       keys.push_back(crash_key);
     }
   }
 
   return base::debug::InitCrashKeys(&keys.at(0), keys.size(),
                                     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) {
 #if defined(OS_WIN)
   return StartsWithASCII(flag, "--channel=", true) ||
 
          // No point to including this since we already have a ptype field.
          StartsWithASCII(flag, "--type=", true) ||
 
          // Not particularly interesting
@@ skipping 148 matching lines @@
 }
 
 ScopedPrinterInfo::~ScopedPrinterInfo() {
   for (size_t i = 0; i < kPrinterInfoCount; ++i) {
     std::string key = base::StringPrintf(kPrinterInfo, i + 1);
     base::debug::ClearCrashKey(key);
   }
 }
 
 }  // namespace crash_keys