Record histograms for on-demand update check results.

And stop mis-using user metrics action.

BUG=424689

Review URL: https://codereview.chromium.org/659333003

Cr-Commit-Position: refs/heads/master@{#300217}

Author: GregTho

chrome/browser/google/google_update_win.cc

@@ -10,6 +10,8 @@
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/message_loop/message_loop.h"
+#include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -391,6 +393,12 @@ void GoogleUpdate::ReportResults(GoogleUpdateUpgradeResult results,
   // If there is an error, then error code must not be blank, and vice versa.
   DCHECK(results == UPGRADE_ERROR ? error_code != GOOGLE_UPDATE_NO_ERROR :
                                     error_code == GOOGLE_UPDATE_NO_ERROR);
+  UMA_HISTOGRAM_ENUMERATION(
+      "GoogleUpdate.UpgradeResult", results, NUM_UPGRADE_RESULTS);
+  if (results == UPGRADE_ERROR) {
+    UMA_HISTOGRAM_ENUMERATION(
+        "GoogleUpdate.UpdateErrorCode", error_code, NUM_ERROR_CODES);
+  }
   if (listener_) {
     listener_->OnReportResults(
         results, error_code, error_message, version_available_);
@@ -404,6 +412,7 @@ bool GoogleUpdate::ReportFailure(HRESULT hr,
   DLOG(ERROR) << "Communication with Google Update failed: " << hr
               << " error: " << error_code
               << ", message: " << error_message.c_str();
+  UMA_HISTOGRAM_SPARSE_SLOWLY("GoogleUpdate.ErrorHresult", hr);
   main_loop->PostTask(
       FROM_HERE,
       base::Bind(&GoogleUpdate::ReportResults, this,

chrome/browser/google/google_update_win.h

@@ -20,48 +20,52 @@ class Widget;
 
 // The status of the upgrade. UPGRADE_STARTED and UPGRADE_CHECK_STARTED are
 // internal states and will not be reported as results to the listener.
+// These values are used for a histogram. Do not reorder.
 enum GoogleUpdateUpgradeResult {
   // The upgrade has started.
   UPGRADE_STARTED = 0,
   // A check for upgrade has been initiated.
-  UPGRADE_CHECK_STARTED,
+  UPGRADE_CHECK_STARTED = 1,
   // An update is available.
-  UPGRADE_IS_AVAILABLE,
+  UPGRADE_IS_AVAILABLE = 2,
   // The upgrade happened successfully.
-  UPGRADE_SUCCESSFUL,
+  UPGRADE_SUCCESSFUL = 3,
   // No need to upgrade, Chrome is up to date.
-  UPGRADE_ALREADY_UP_TO_DATE,
+  UPGRADE_ALREADY_UP_TO_DATE = 4,
   // An error occurred.
-  UPGRADE_ERROR,
+  UPGRADE_ERROR = 5,
+  NUM_UPGRADE_RESULTS
 };
 
+// These values are used for a histogram. Do not reorder.
 enum GoogleUpdateErrorCode {
   // The upgrade completed successfully (or hasn't been started yet).
   GOOGLE_UPDATE_NO_ERROR = 0,
   // Google Update only supports upgrading if Chrome is installed in the default
   // location. This error will appear for developer builds and with
   // installations unzipped to random locations.
-  CANNOT_UPGRADE_CHROME_IN_THIS_DIRECTORY,
+  CANNOT_UPGRADE_CHROME_IN_THIS_DIRECTORY = 1,
   // Failed to create Google Update JobServer COM class.
-  GOOGLE_UPDATE_JOB_SERVER_CREATION_FAILED,
+  GOOGLE_UPDATE_JOB_SERVER_CREATION_FAILED = 2,
   // Failed to create Google Update OnDemand COM class.
-  GOOGLE_UPDATE_ONDEMAND_CLASS_NOT_FOUND,
+  GOOGLE_UPDATE_ONDEMAND_CLASS_NOT_FOUND = 3,
   // Google Update OnDemand COM class reported an error during a check for
   // update (or while upgrading).
-  GOOGLE_UPDATE_ONDEMAND_CLASS_REPORTED_ERROR,
+  GOOGLE_UPDATE_ONDEMAND_CLASS_REPORTED_ERROR = 4,
   // A call to GetResults failed.
-  GOOGLE_UPDATE_GET_RESULT_CALL_FAILED,
+  GOOGLE_UPDATE_GET_RESULT_CALL_FAILED = 5,
   // A call to GetVersionInfo failed.
-  GOOGLE_UPDATE_GET_VERSION_INFO_FAILED,
+  GOOGLE_UPDATE_GET_VERSION_INFO_FAILED = 6,
   // An error occurred while upgrading (or while checking for update).
   // Check the Google Update log in %TEMP% for more details.
-  GOOGLE_UPDATE_ERROR_UPDATING,
+  GOOGLE_UPDATE_ERROR_UPDATING = 7,
   // Updates can not be downloaded because the administrator has disabled all
   // types of updating.
-  GOOGLE_UPDATE_DISABLED_BY_POLICY,
+  GOOGLE_UPDATE_DISABLED_BY_POLICY = 8,
   // Updates can not be downloaded because the administrator has disabled
   // manual (on-demand) updates.  Automatic background updates are allowed.
-  GOOGLE_UPDATE_DISABLED_BY_POLICY_AUTO_ONLY,
+  GOOGLE_UPDATE_DISABLED_BY_POLICY_AUTO_ONLY = 9,
+  NUM_ERROR_CODES
 };
 
 // The GoogleUpdateStatusListener interface is used by components to receive

chrome/browser/ui/webui/help/version_updater_win.cc

@@ -19,11 +19,9 @@
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/install_util.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/user_metrics.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/views/widget/widget.h"
 
-using base::UserMetricsAction;
 using content::BrowserThread;
 
 namespace {
@@ -178,18 +176,14 @@ void VersionUpdaterWin::UpdateStatus(GoogleUpdateUpgradeResult result,
 
   switch (result) {
     case UPGRADE_CHECK_STARTED: {
-      content::RecordAction(UserMetricsAction("UpgradeCheck_Started"));
       status = CHECKING;
       break;
     }
     case UPGRADE_STARTED: {
-      content::RecordAction(UserMetricsAction("Upgrade_Started"));
       status = UPDATING;
       break;
     }
     case UPGRADE_IS_AVAILABLE: {
-      content::RecordAction(
-          UserMetricsAction("UpgradeCheck_UpgradeIsAvailable"));
       DCHECK(!google_updater_);  // Should have been nulled out already.
       CreateGoogleUpdater();
       UpdateStatus(UPGRADE_STARTED, GOOGLE_UPDATE_NO_ERROR, base::string16());
@@ -208,12 +202,10 @@ void VersionUpdaterWin::UpdateStatus(GoogleUpdateUpgradeResult result,
       return;
     }
     case UPGRADE_SUCCESSFUL: {
-      content::RecordAction(UserMetricsAction("UpgradeCheck_Upgraded"));
       status = NEARLY_UPDATED;
       break;
     }
     case UPGRADE_ERROR: {
-      content::RecordAction(UserMetricsAction("UpgradeCheck_Error"));
       status = FAILED;
       if (error_code == GOOGLE_UPDATE_DISABLED_BY_POLICY) {
         message =
@@ -253,14 +245,11 @@ void VersionUpdaterWin::GotInstalledVersion(const Version& version) {
   // out of date.
   chrome::VersionInfo version_info;
   Version running_version(version_info.Version());
-  if (!version.IsValid() || version.CompareTo(running_version) <= 0) {
-    content::RecordAction(
-        UserMetricsAction("UpgradeCheck_AlreadyUpToDate"));
-    callback_.Run(UPDATED, 0, base::string16());
-  } else {
-    content::RecordAction(UserMetricsAction("UpgradeCheck_AlreadyUpgraded"));
-    callback_.Run(NEARLY_UPDATED, 0, base::string16());
-  }
+  callback_.Run((version.IsValid() && version.CompareTo(running_version) > 0)
+                    ? NEARLY_UPDATED
+                    : UPDATED,
+                0,
+                base::string16());
 }
 
 void VersionUpdaterWin::CreateGoogleUpdater() {

tools/metrics/actions/actions.xml

@@ -11162,46 +11162,58 @@ should be able to be added at any place in this file.
 <action name="UpgradeCheck_AlreadyUpToDate">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Removed from codebase; see GoogleUpdate.UpgradeResult.</obsolete>
 </action>
 
 <action name="UpgradeCheck_AlreadyUpgraded">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Removed from codebase; see GoogleUpdate.UpgradeResult.</obsolete>
 </action>
 
 <action name="UpgradeCheck_Done">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Not present in codebase.</obsolete>
 </action>
 
 <action name="UpgradeCheck_Error">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>
+    Removed from codebase; see GoogleUpdate.UpgradeResult,
+    GoogleUpdate.UpdateErrorCode, and GoogleUpdate.ErrorHresult.
+  </obsolete>
 </action>
 
 <action name="UpgradeCheck_Started">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Removed from codebase; see GoogleUpdate.UpgradeResult.</obsolete>
 </action>
 
 <action name="UpgradeCheck_TimedOut">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Not present in codebase.</obsolete>
 </action>
 
 <action name="UpgradeCheck_UpgradeIsAvailable">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Removed from codebase; see GoogleUpdate.UpgradeResult.</obsolete>
 </action>
 
 <action name="UpgradeCheck_Upgraded">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Removed from codebase; see GoogleUpdate.UpgradeResult.</obsolete>
 </action>
 
 <action name="Upgrade_Started">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <description>Please enter the description of this user action.</description>
+  <obsolete>Removed from codebase; see GoogleUpdate.UpgradeResult.</obsolete>
 </action>
 
 <action name="ViewAboutConflicts">

tools/metrics/histograms/histograms.xml

@@ -9638,6 +9638,16 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="GoogleUpdate.ErrorHresult">
+  <owner>[email protected]</owner>
+  <summary>The HRESULT for a failed on-demand update check.</summary>
+</histogram>
+
+<histogram name="GoogleUpdate.UpdateErrorCode" enum="GoogleUpdateErrorCode">
+  <owner>[email protected]</owner>
+  <summary>The error code for a failed on-demand update check.</summary>
+</histogram>
+
 <histogram name="GoogleUpdate.UpdatePolicyIsOverridden" enum="Boolean">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <summary>
@@ -9647,6 +9657,11 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="GoogleUpdate.UpgradeResult" enum="GoogleUpdateUpgradeResult">
+  <owner>[email protected]</owner>
+  <summary>The result of an on-demand update check.</summary>
+</histogram>
+
 <histogram name="GPU.AcceleratedSurfaceRefreshRate" units="hz">
   <owner>[email protected]</owner>
   <summary>
@@ -44623,6 +44638,28 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <int value="13" label="WEB_LOGIN_REQUIRED"/>
 </enum>
 
+<enum name="GoogleUpdateErrorCode" type="int">
+  <int value="0" label="GOOGLE_UPDATE_NO_ERROR"/>
+  <int value="1" label="CANNOT_UPGRADE_CHROME_IN_THIS_DIRECTORY"/>
+  <int value="2" label="GOOGLE_UPDATE_JOB_SERVER_CREATION_FAILED"/>
+  <int value="3" label="GOOGLE_UPDATE_ONDEMAND_CLASS_NOT_FOUND"/>
+  <int value="4" label="GOOGLE_UPDATE_ONDEMAND_CLASS_REPORTED_ERROR"/>
+  <int value="5" label="GOOGLE_UPDATE_GET_RESULT_CALL_FAILED"/>
+  <int value="6" label="GOOGLE_UPDATE_GET_VERSION_INFO_FAILED"/>
+  <int value="7" label="GOOGLE_UPDATE_ERROR_UPDATING"/>
+  <int value="8" label="GOOGLE_UPDATE_DISABLED_BY_POLICY"/>
+  <int value="9" label="GOOGLE_UPDATE_DISABLED_BY_POLICY_AUTO_ONLY"/>
+</enum>
+
+<enum name="GoogleUpdateUpgradeResult" type="int">
+  <int value="0" label="UPGRADE_STARTED"/>
+  <int value="1" label="UPGRADE_CHECK_STARTED"/>
+  <int value="2" label="UPGRADE_IS_AVAILABLE"/>
+  <int value="3" label="UPGRADE_SUCCESSFUL"/>
+  <int value="4" label="UPGRADE_ALREADY_UP_TO_DATE"/>
+  <int value="5" label="UPGRADE_ERROR"/>
+</enum>
+
 <enum name="HIDContinueScenarioType" type="int">
   <summary>Possible detected devices combination on leaving dialog</summary>
   <int value="0" label="Pointing device only detected."/>