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

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.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/browser/metrics/chrome_browser_main_extra_parts_metrics.h"
 
 #include <string>
-#include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/cpu.h"
 #include "base/metrics/histogram.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/time.h"
 #include "chrome/browser/about_flags.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_browser_main.h"
 #include "chrome/browser/shell_integration.h"
 #include "content/public/browser/browser_thread.h"
 #include "ui/base/touch/touch_device.h"
 #include "ui/base/ui_base_switches.h"
 
 #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 {
 
 enum UMALinuxGlibcVersion {
   UMA_LINUX_GLIBC_NOT_PARSEABLE,
   UMA_LINUX_GLIBC_UNKNOWN,
   UMA_LINUX_GLIBC_2_11,
   UMA_LINUX_GLIBC_2_19 = UMA_LINUX_GLIBC_2_11 + 8,
   // NOTE: Add new version above this line and update the enum list in
@@ skipping 23 matching lines @@
 void RecordDefaultBrowserUMAStat() {
   // Record whether Chrome is the default browser or not.
   ShellIntegration::DefaultWebClientState default_state =
       ShellIntegration::GetDefaultBrowser();
   UMA_HISTOGRAM_ENUMERATION("DefaultBrowser.State", default_state,
                             ShellIntegration::NUM_DEFAULT_STATES);
 }
 
 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) {
         glibc_version_result =
             static_cast<UMALinuxGlibcVersion>(translated_glibc_minor_version);
       }
     }
   }
   UMA_HISTOGRAM_ENUMERATION("Linux.GlibcVersion", glibc_version_result,
                             UMA_LINUX_GLIBC_VERSION_COUNT);
@@ skipping 52 matching lines @@
   RecordTouchEventState();
 }
 
 namespace chrome {
 
 void AddMetricsExtraParts(ChromeBrowserMainParts* main_parts) {
   main_parts->AddParts(new ChromeBrowserMainExtraPartsMetrics());
 }
 
 }  // namespace chrome