Added UMA statistics for Chrome Cleaner.

chrome/browser/component_updater/sw_reporter_installer_win.cc

 // Copyright (c) 2014 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/component_updater/sw_reporter_installer_win.h"
 
 #include <stdint.h>
 #include <string>
 #include <vector>
 
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/process/kill.h"
 #include "base/process/launch.h"
+#include "base/strings/stringprintf.h"
 #include "base/task_runner_util.h"
 #include "base/threading/worker_pool.h"
 #include "base/time/time.h"
 #include "base/win/registry.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/srt_global_error_win.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/global_error/global_error_service.h"
@@ skipping 39 matching lines @@
                                0xf4, 0xc9, 0x78, 0x6c, 0x0c, 0x24, 0x73, 0x3e,
                                0x05, 0xa5, 0x62, 0x4b, 0x2e, 0xc7, 0xb7, 0x1c,
                                0x5f, 0xea, 0xf0, 0x88, 0xf6, 0x97, 0x9b, 0xc7};
 
 const base::FilePath::CharType kSwReporterExeName[] =
     FILE_PATH_LITERAL("software_reporter_tool.exe");
 
 // Where to fetch the reporter exit code in the registry.
 const wchar_t kSoftwareRemovalToolRegistryKey[] =
     L"Software\\Google\\Software Removal Tool";
+const wchar_t kCleanerSuffixRegistryKey[] = L"%ls\\Cleaner";

[MAD, 2014/10/20 01:38:33]

Since the other string is simply at the beginning, you should use simpler
concatenation instead.

[Georges Khalil, 2014/10/24 12:37:58]

Done.

 const wchar_t kExitCodeRegistryValueName[] = L"ExitCode";
+const wchar_t kVersionRegistryValueName[] = L"Version";
+const wchar_t kStartTimeRegistryValueName[] = L"StartTime";
+const wchar_t kEndTimeRegistryValueName[] = L"EndTime";
 
 // Field trial strings.
 const char kSRTPromptTrialName[] = "SRTPromptFieldTrial";
 const char kSRTPromptOnGroup[] = "On";
 
 // Exit codes that identify that a cleanup is needed.
 const int kCleanupNeeded = 0;
 const int kPostRebootCleanupNeeded = 4;
 
 void ReportUmaStep(SwReporterUmaValue value) {
@@ skipping 227 matching lines @@
 void RegisterSwReporterComponent(ComponentUpdateService* cus,
                                  PrefService* prefs) {
   // The Sw reporter doesn't need to run if the user isn't reporting metrics and
   // isn't in the SRTPrompt field trial "On" group.
   if (!ChromeMetricsServiceAccessor::IsMetricsReportingEnabled() &&
       base::FieldTrialList::FindFullName(kSRTPromptTrialName) !=
           kSRTPromptOnGroup) {
     return;
   }
 
+  // Check if we have information from Cleaner and record UMA statistics.
+  base::string16 cleaner_key_name = base::StringPrintf(
+      kCleanerSuffixRegistryKey, kSoftwareRemovalToolRegistryKey);
+  base::win::RegKey cleaner_key(
+      HKEY_CURRENT_USER, cleaner_key_name.c_str(), KEY_ALL_ACCESS);
+  // Cleaner is assumed to have run if we have a start time.
+  if (cleaner_key.Valid() &&
+      cleaner_key.HasValue(kStartTimeRegistryValueName)) {
+    int64 start_time_value;
+    cleaner_key.ReadInt64(kStartTimeRegistryValueName, &start_time_value);

[MAD, 2014/10/20 01:38:33]

Since we only use the start_time when we have a value for end time, we should
only read and convert it below.

[Georges Khalil, 2014/10/24 12:37:58]

Done.

+    base::Time start_time = base::Time::FromInternalValue(start_time_value);

[MAD, 2014/10/20 01:38:33]

We usually prefer using the constructor instead of the = operator. But since we
use the variable only once, we could not have a local variable for this and
simply put the FromInternalValue call within the operation to compute run_time.

[Georges Khalil, 2014/10/24 12:37:58]

Done.

+    cleaner_key.DeleteValue(kStartTimeRegistryValueName);

[MAD, 2014/10/20 01:38:33]

Then this would need to move lower below of course...

[Georges Khalil, 2014/10/24 12:37:58]

Done.

+    // Get version number.
+    if (cleaner_key.HasValue(kVersionRegistryValueName)) {
+      DWORD version;
+      cleaner_key.ReadValueDW(kVersionRegistryValueName, &version);
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Cleaner.Version", version);
+      cleaner_key.DeleteValue(kVersionRegistryValueName);
+    }
+    // Get end time.
+    if (cleaner_key.HasValue(kEndTimeRegistryValueName)) {
+      int64 end_time_value;
+      cleaner_key.ReadInt64(kEndTimeRegistryValueName, &end_time_value);
+      base::Time end_time = base::Time::FromInternalValue(end_time_value);

[MAD, 2014/10/20 01:38:33]

Use constructor instead, or use the return value directly in the computation of
run_time... which should also be in a constructor.

[Georges Khalil, 2014/10/24 12:37:58]

Done.

+      cleaner_key.DeleteValue(kEndTimeRegistryValueName);
+      base::TimeDelta run_time = end_time - start_time;
+      UMA_HISTOGRAM_LONG_TIMES("Cleaner.RunTime", run_time);
+    } else {
+      // If we don't have an end time, we can assume the cleaner crashed and we
+      // represent this using an infinite delta.
+      UMA_HISTOGRAM_LONG_TIMES("Cleaner.RunTime", base::TimeDelta::Max());
+    }
+    // Get exit code.
+    if (cleaner_key.HasValue(kExitCodeRegistryValueName)) {
+      DWORD exit_code;
+      cleaner_key.ReadValueDW(kExitCodeRegistryValueName, &exit_code);
+      UMA_HISTOGRAM_SPARSE_SLOWLY("Cleaner.ExitCode", exit_code);
+      cleaner_key.DeleteValue(kExitCodeRegistryValueName);
+    }
+  }
+
   // Install the component.
   scoped_ptr<ComponentInstallerTraits> traits(
       new SwReporterInstallerTraits(prefs));
   // |cus| will take ownership of |installer| during installer->Register(cus).
   DefaultComponentInstaller* installer =
       new DefaultComponentInstaller(traits.Pass());
   installer->Register(cus);
 }
 
 void RegisterPrefsForSwReporter(PrefRegistrySimple* registry) {
   registry->RegisterInt64Pref(prefs::kSwReporterLastTimeTriggered, 0);
   registry->RegisterIntegerPref(prefs::kSwReporterLastExitCode, -1);
 }
 
 void RegisterProfilePrefsForSwReporter(
     user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterIntegerPref(
       prefs::kSwReporterPromptReason,
       -1,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 
   registry->RegisterStringPref(
       prefs::kSwReporterPromptVersion,
       "",
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 }
 
 }  // namespace component_updater