Improve the code added in r194423 by using the Version class.

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc

Index: chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
===================================================================
--- chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc	(revision 194467)
+++ chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc	(working copy)
@@ -5,7 +5,6 @@
 #include "chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.h"
 
 #include <string>
-#include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ -24,8 +23,7 @@
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 #include <gnu/libc-version.h>
 
-#include "base/strings/string_number_conversions.h"
-#include "base/strings/string_split.h"
+#include "base/version.h"
 #endif
 
 namespace {
@@ -69,23 +67,19 @@
 
 void RecordLinuxGlibcVersion() {
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
-  std::string glibc_version_string(gnu_get_libc_version());
-  std::vector<std::string> split_glibc_version;
-  base::SplitString(glibc_version_string, '.', &split_glibc_version);
+  Version version(gnu_get_libc_version());
 
   UMALinuxGlibcVersion glibc_version_result = UMA_LINUX_GLIBC_NOT_PARSEABLE;
-  unsigned glibc_major_version = 0;
-  unsigned glibc_minor_version = 0;
-  if (split_glibc_version.size() == 2 &&
-      base::StringToUint(split_glibc_version[0], &glibc_major_version) &&
-      base::StringToUint(split_glibc_version[1], &glibc_minor_version)) {
+  if (version.IsValid() && version.components().size() == 2) {
     glibc_version_result = UMA_LINUX_GLIBC_UNKNOWN;
+    uint16 glibc_major_version = version.components()[0];

[xhwang, 2013/04/16 22:51:38]

here and below, just use int? 

[Lei Zhang, 2013/04/16 23:20:23]

|version.components()| returns a vector of uint16s.

+    uint16 glibc_minor_version = version.components()[1];
     if (glibc_major_version == 2) {
       // A constant to translate glibc 2.x minor versions to their
       // equivalent UMALinuxGlibcVersion values.
-      const unsigned kGlibcMinorVersionTranslationOffset =
+      const uint16 kGlibcMinorVersionTranslationOffset =

[jar (doing other things), 2013/04/16 23:42:52]

nit: not a big deal... but why mess with uint16?  The natural int is generally
nice and simple and fast. 

           11 - UMA_LINUX_GLIBC_2_11;

[xhwang, 2013/04/16 22:51:38]

wondering if it'll make our life easier if we just letting UMA_LINUX_GLIBC_2_11
== 11. Then we don't need translation.

[Lei Zhang, 2013/04/16 23:20:23]

jar@ preferred if the enum did not have any gaps. See previous CL.

[jar (doing other things), 2013/04/16 23:42:52]

... and also... we'll eventually support major version 3 (I'm guessing) and it
is nice to map each one into its own range.  

We do waste space with an enumerated histogram when we don't use the slots, so
it is just a good practice to not leave gaps.  ...in this case.. not the end of
the world... but a good practice.


On 2013/04/16 23:20:23, Lei Zhang wrote:

[xhwang, 2013/04/17 00:05:02]

sg, I just hate the number "11" here :) But not a big deal.

-      unsigned translated_glibc_minor_version =
+      uint16 translated_glibc_minor_version =
           glibc_minor_version - kGlibcMinorVersionTranslationOffset;
       if (translated_glibc_minor_version >= UMA_LINUX_GLIBC_2_11 &&
           translated_glibc_minor_version <= UMA_LINUX_GLIBC_2_19) {