Chrome Cleaner UI: Add some UMA histograms

This adds UMA histograms for keeping track of how long it takes for
the Chrome Cleaner to scan and clean user's machines as well as the
time it takes for users to interact with the Chrome Cleaner prompt
dialog.

Bug: 746987
Change-Id: If4d70c196f68c462834cb0c1e8a82a8bfaa6ea05
Reviewed-on: https://chromium-review.googlesource.com/580087
Commit-Queue: Ali Tofigh <alito@chromium.org>
Reviewed-by: Robert Shield <robertshield@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488475}

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.cc

@@ -6,6 +6,7 @@
 
 #include <windows.h>
 
+#include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ -319,6 +320,9 @@ void ChromeCleanerController::ReplyWithUserResponse(
       ->PostTask(FROM_HERE,
                  base::BindOnce(std::move(prompt_user_callback_), acceptance));
 
+  if (new_state == State::kCleaning)
+    time_cleanup_started_ = base::Time::Now();
+
   // The transition to a new state should happen only after the response has
   // been posted on the UI thread so that if we transition to the kIdle state,
   // the response callback is not cleared before it has been posted.
@@ -419,6 +423,8 @@ void ChromeCleanerController::OnChromeCleanerFetchedAndVerified(
                  weak_factory_.GetWeakPtr()),
       // Our callbacks should be dispatched to the UI thread only.
       base::ThreadTaskRunnerHandle::Get());
+
+  time_scanning_started_ = base::Time::Now();
 }
 
 // static
@@ -448,6 +454,10 @@ void ChromeCleanerController::OnPromptUser(
   DCHECK_EQ(State::kScanning, state());
   DCHECK(!files_to_delete_);
   DCHECK(!prompt_user_callback_);
+  DCHECK(!time_scanning_started_.is_null());
+
+  UMA_HISTOGRAM_LONG_TIMES_100("SoftwareReporter.Cleaner.ScanningTime",
+                               base::Time::Now() - time_scanning_started_);
 
   if (files_to_delete->empty()) {
     BrowserThread::GetTaskRunnerForThread(BrowserThread::IO)
@@ -510,6 +520,15 @@ void ChromeCleanerController::OnCleanerProcessDone(
 
   if (process_status.launch_status ==
       ChromeCleanerRunner::LaunchStatus::kSuccess) {
+    if (process_status.exit_code == kRebootRequiredExitCode ||
+        process_status.exit_code == kRebootNotRequiredExitCode) {
+      DCHECK(!time_cleanup_started_.is_null());
+      UMA_HISTOGRAM_CUSTOM_TIMES("SoftwareReporter.Cleaner.CleaningTime",
+                                 base::Time::Now() - time_cleanup_started_,
+                                 base::TimeDelta::FromMilliseconds(1),
+                                 base::TimeDelta::FromHours(5), 100);
+    }
+
     if (process_status.exit_code == kRebootRequiredExitCode) {
       RecordCleanupResultHistogram(CLEANUP_RESULT_REBOOT_REQUIRED);
       SetStateAndNotifyObservers(State::kRebootRequired);
@@ -526,6 +545,8 @@ void ChromeCleanerController::OnCleanerProcessDone(
       RecordCleanupResultHistogram(CLEANUP_RESULT_SUCCEEDED);
       delegate_->ResetTaggedProfiles(
           g_browser_process->profile_manager()->GetLoadedProfiles(),
+          // OnSettingsResetCompleted() will take care of transitioning to the
+          // kIdle state with IdleReason kCleaningSucceeded.
           base::BindOnce(&ChromeCleanerController::OnSettingsResetCompleted,
                          base::Unretained(this)));
       ResetCleanerDataAndInvalidateWeakPtrs();

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h

@@ -15,6 +15,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/threading/thread_checker.h"
+#include "base/time/time.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_runner_win.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/reporter_runner_win.h"
 #include "components/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h"
@@ -255,6 +256,10 @@ class ChromeCleanerController {
   // Cleaner process. This must be posted to run on the IO thread.
   chrome_cleaner::mojom::ChromePrompt::PromptUserCallback prompt_user_callback_;
 
+  // For metrics reporting.
+  base::Time time_scanning_started_;
+  base::Time time_cleanup_started_;
+
   base::ObserverList<Observer> observer_list_;
 
   THREAD_CHECKER(thread_checker_);

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.cc

@@ -53,13 +53,18 @@ ChromeCleanerDialogControllerImpl::ChromeCleanerDialogControllerImpl(
 ChromeCleanerDialogControllerImpl::~ChromeCleanerDialogControllerImpl() =
     default;
 
-void ChromeCleanerDialogControllerImpl::DialogShown() {}
+void ChromeCleanerDialogControllerImpl::DialogShown() {
+  time_dialog_shown_ = base::Time::Now();
+}
 
 void ChromeCleanerDialogControllerImpl::Accept(bool logs_enabled) {
   DCHECK(browser_);
 
   RecordPromptDialogResponseHistogram(PROMPT_DIALOG_RESPONSE_ACCEPTED);
   RecordCleanupStartedHistogram(CLEANUP_STARTED_FROM_PROMPT_DIALOG);
+  UMA_HISTOGRAM_LONG_TIMES_100(
+      "SoftwareReporter.PromptDialog.TimeUntilDone_Accepted",
+      base::Time::Now() - time_dialog_shown_);
 
   cleaner_controller_->ReplyWithUserResponse(
       browser_->profile(),
@@ -74,6 +79,10 @@ void ChromeCleanerDialogControllerImpl::Cancel() {
   DCHECK(browser_);
 
   RecordPromptDialogResponseHistogram(PROMPT_DIALOG_RESPONSE_CANCELLED);
+  DCHECK(!time_dialog_shown_.is_null());
+  UMA_HISTOGRAM_LONG_TIMES_100(
+      "SoftwareReporter.PromptDialog.TimeUntilDone_Canceled",
+      base::Time::Now() - time_dialog_shown_);
 
   cleaner_controller_->ReplyWithUserResponse(
       browser_->profile(), ChromeCleanerController::UserResponse::kDenied);
@@ -84,6 +93,10 @@ void ChromeCleanerDialogControllerImpl::Close() {
   DCHECK(browser_);
 
   RecordPromptDialogResponseHistogram(PROMPT_DIALOG_RESPONSE_DISMISSED);
+  DCHECK(!time_dialog_shown_.is_null());
+  UMA_HISTOGRAM_LONG_TIMES_100(
+      "SoftwareReporter.PromptDialog.TimeUntilDone_Dismissed",
+      base::Time::Now() - time_dialog_shown_);
 
   cleaner_controller_->ReplyWithUserResponse(
       browser_->profile(), ChromeCleanerController::UserResponse::kDismissed);
@@ -93,6 +106,10 @@ void ChromeCleanerDialogControllerImpl::Close() {
 void ChromeCleanerDialogControllerImpl::DetailsButtonClicked(
     bool logs_enabled) {
   RecordPromptDialogResponseHistogram(PROMPT_DIALOG_RESPONSE_DETAILS);
+  DCHECK(!time_dialog_shown_.is_null());
+  UMA_HISTOGRAM_LONG_TIMES_100(
+      "SoftwareReporter.PromptDialog.TimeUntilDone_DetailsButtonClicked",
+      base::Time::Now() - time_dialog_shown_);
 
   cleaner_controller_->SetLogsEnabled(logs_enabled);
   OpenSettingsPage(browser_);

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_impl_win.h

@@ -9,6 +9,7 @@
 
 #include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/time/time.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_dialog_controller_win.h"
 
@@ -49,6 +50,7 @@ class ChromeCleanerDialogControllerImpl
 
   ChromeCleanerController* cleaner_controller_ = nullptr;
   bool dialog_shown_ = false;
+  base::Time time_dialog_shown_;  // Used for reporting metrics.
   Browser* browser_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeCleanerDialogControllerImpl);

tools/metrics/histograms/histograms.xml

@@ -74277,6 +74277,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="SoftwareReporter.Cleaner.CleaningTime" units="ms">
+  <owner>alito@chromium.org</owner>
+  <summary>
+    The time between sending the user's response to the Chrome Cleaner process
+    and the Cleaner process terminating. This histogram is logged only for
+    successfully completed runs of the cleaner.
+  </summary>
+</histogram>
+
 <histogram name="SoftwareReporter.Cleaner.CleanupResult"
     enum="SoftwareReporterCleanupResult">
   <owner>ftirelo@chromium.org</owner>
@@ -74328,6 +74337,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   <summary>How long it took to run the software reporter cleaner tool.</summary>
 </histogram>
 
+<histogram name="SoftwareReporter.Cleaner.ScanningTime" units="ms">
+  <owner>alito@chromium.org</owner>
+  <summary>
+    The time between launching the Chrome Cleaner process and the cleaner having
+    scanned the user's machine and Chrome receiving an IPC call with the
+    results.
+  </summary>
+</histogram>
+
 <histogram name="SoftwareReporter.Cleaner.Version">
   <owner>mad@chromium.org</owner>
   <summary>The build version of the software reporter cleaner tool.</summary>
@@ -74473,6 +74491,14 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="SoftwareReporter.PromptDialog.TimeUntilDone" units="ms">
+  <owner>alito@chromium.org</owner>
+  <summary>
+    The time between the Chrome Cleaner dialog being shown and the dialog being
+    closed.
+  </summary>
+</histogram>
+
 <histogram name="SoftwareReporter.PromptDialogResponse"
     enum="SoftwareReporterPromptDialogResponse">
   <owner>ftirelo@chromium.org</owner>
@@ -90228,6 +90254,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   <affected-histogram name="MobileStartup.ToolbarInflationTime"/>
 </histogram_suffixes>
 
+<histogram_suffixes name="ChromeCleanerDialogDoneReason" separator="_">
+  <suffix name="Accepted" label="User accepted the prompt."/>
+  <suffix name="Canceled" label="User clicked the cancel button."/>
+  <suffix name="Dismissed"
+      label="User dismissed the prompt, for example by pressing ESC."/>
+  <suffix name="DetailsButtonClicked" label="User clicked the details button."/>
+  <affected-histogram name="SoftwareReporter.PromptDialog.TimeUntilDone"/>
+</histogram_suffixes>
+
 <histogram_suffixes name="ChromeContentBrowserClientMetricSuffixes"
     separator=".">
   <suffix name="search"