Chrome Cleaner: record how the user was prompted to reboot

Bug: 770749 Change-Id: Ia34fd204174a9ec61e232a7e967c46cd0e0c57ef Reviewed-on: https://chromium-review.googlesource.com/966894 Commit-Queue: Veranika Liaukevich <veranika@chromium.org> Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Joe Mason <joenotcharles@chromium.org> Cr-Commit-Position: refs/heads/master@{#547016}
Denger-Network · Mar 29, 2018 · 2f48316 · 2f48316
1 parent 3f225c8
commit 2f48316
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 14 deletions.
diff --git a/.../browser/safe_browsing/chrome_cleaner/chrome_cleaner_reboot_dialog_controller_impl_win.cc b/.../browser/safe_browsing/chrome_cleaner/chrome_cleaner_reboot_dialog_controller_impl_win.cc
@@ -30,6 +30,16 @@ enum SettingsPageActiveOnRebootRequiredHistogramValue {
   SETTINGS_PAGE_ON_REBOOT_REQUIRED_MAX,
 };
 
+// These values are used to record how the user was prompted to reboot the
+// machine. Must be in sync with the SoftwareReporterRebootPromptType enum from
+// enums.xml.
+enum SoftwareReporterRebootPromptTypeHistogramValue {
+  REBOOT_PROMPT_TYPE_SETTINGS_PAGE_OPENED = 1,
+  REBOOT_PROMPT_TYPE_MODAL_DIALOG_SHOWN = 2,
+  REBOOT_PROMPT_TYPE_NON_MODAL_DIALOG_SHOWN = 3,
+  REBOOT_PROMPT_TYPE_MAX,
+};
+
 class PromptDelegateImpl
     : public ChromeCleanerRebootDialogControllerImpl::PromptDelegate {
  public:
@@ -89,14 +99,6 @@ ChromeCleanerRebootDialogControllerImpl::Create(
   return controller;
 }
 
-void ChromeCleanerRebootDialogControllerImpl::DialogShown() {
-  DCHECK(base::FeatureList::IsEnabled(kRebootPromptDialogFeature));
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  // TODO(crbug.com/770749) Collect metrics on how many times this dialog is
-  //                        shown.
-}
-
 void ChromeCleanerRebootDialogControllerImpl::Accept() {
   DCHECK(base::FeatureList::IsEnabled(kRebootPromptDialogFeature));
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -181,8 +183,16 @@ void ChromeCleanerRebootDialogControllerImpl::StartRebootPromptForBrowser(
 
   if (base::FeatureList::IsEnabled(kRebootPromptDialogFeature)) {
     prompt_delegate_->ShowChromeCleanerRebootPrompt(browser, this);
+    SoftwareReporterRebootPromptTypeHistogramValue prompt_type =
+        IsRebootPromptModal() ? REBOOT_PROMPT_TYPE_MODAL_DIALOG_SHOWN
+                              : REBOOT_PROMPT_TYPE_NON_MODAL_DIALOG_SHOWN;
+    UMA_HISTOGRAM_ENUMERATION("SoftwareReporter.Cleaner.RebootPromptShown",
+                              prompt_type, REBOOT_PROMPT_TYPE_MAX);
   } else {
     prompt_delegate_->OpenSettingsPage(browser);
+    UMA_HISTOGRAM_ENUMERATION("SoftwareReporter.Cleaner.RebootPromptShown",
+                              REBOOT_PROMPT_TYPE_SETTINGS_PAGE_OPENED,
+                              REBOOT_PROMPT_TYPE_MAX);
     OnInteractionDone();
   }
 }

diff --git a/...e/browser/safe_browsing/chrome_cleaner/chrome_cleaner_reboot_dialog_controller_impl_win.h b/...e/browser/safe_browsing/chrome_cleaner/chrome_cleaner_reboot_dialog_controller_impl_win.h
@@ -40,7 +40,6 @@ class ChromeCleanerRebootDialogControllerImpl
       std::unique_ptr<PromptDelegate> delegate);
 
   // ChromeCleanerRebootDialogController overrides.
-  void DialogShown() override;
   void Accept() override;
   void Cancel() override;
   void Close() override;

diff --git a/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_reboot_dialog_controller_win.h b/chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_reboot_dialog_controller_win.h
@@ -16,9 +16,6 @@ namespace safe_browsing {
 // the user interacts with it.
 class ChromeCleanerRebootDialogController {
  public:
-  // Called by the reboot dialog when the dialog has been shown. Used for
-  // reporting metrics.
-  virtual void DialogShown() = 0;
   // Called by the reboot dialog when user accepts the reboot prompt. Once
   // |Accept()| has been called, the controller will eventually delete itself
   // and no member functions should be called after that.

diff --git a/chrome/browser/ui/views/chrome_cleaner_reboot_dialog_win.cc b/chrome/browser/ui/views/chrome_cleaner_reboot_dialog_win.cc
@@ -70,8 +70,6 @@ void ChromeCleanerRebootDialog::Show(Browser* browser) {
       this, nullptr, browser->window()->GetNativeWindow());
   widget->SetBounds(GetDialogBounds(browser));
   widget->Show();
-
-  dialog_controller_->DialogShown();
 }
 
 // WidgetDelegate overrides.

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -41018,6 +41018,12 @@ Called by update_net_trust_anchors.py.-->
   <int value="2" label="On browser window available"/>
 </enum>
 
+<enum name="SoftwareReporterRebootPromptType">
+  <int value="1" label="SettingsPageOpened"/>
+  <int value="2" label="ModalDialogShown"/>
+  <int value="3" label="NonModalDialogShown"/>
+</enum>
+
 <enum name="SoftwareReporterSequenceResult">
   <int value="1" label="Not scheduled"/>
   <int value="2" label="Timed-out"/>

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -86967,6 +86967,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="SoftwareReporter.Cleaner.RebootPromptShown"
+    enum="SoftwareReporterRebootPromptType">
+  <owner>veranika@chromium.org</owner>
+  <summary>
+    Indicates how the user was prompted to reboot the machine to complete a run
+    of the Chrome Cleanup Tool.
+  </summary>
+</histogram>
+
 <histogram name="SoftwareReporter.Cleaner.RebootResponse" enum="Boolean">
   <owner>ftirelo@chromium.org</owner>
   <summary>