Primary histograms for InBrowserCleanerUI experiment

chrome/browser/ui/webui/settings/chrome_cleanup_handler.cc

 // Copyright (c) 2017 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/ui/webui/settings/chrome_cleanup_handler.h"
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/synchronization/lock.h"
 #include "base/values.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 #include "ui/base/l10n/l10n_util.h"
 
 using safe_browsing::ChromeCleanerController;
 
@@ skipping 105 matching lines @@
 }
 
 void ChromeCleanupHandler::HandleRegisterChromeCleanerObserver(
     const base::ListValue* args) {
   DCHECK_EQ(0U, args->GetSize());
   // The Polymer element should never be attached if the feature is
   // disabled.
   DCHECK(
       base::FeatureList::IsEnabled(safe_browsing::kInBrowserCleanerUIFeature));
 
+  UMA_HISTOGRAM_BOOLEAN("SoftwareReporter.CleanupCardShown", true);

[csharp, 2017/07/07 17:32:51]

It seems odd to have a boolean histogram that is only every marked as true.

Looking at some uma comments, there is UMA_HISTOGRAM_COUNT_*, but the comment
there says if you just want to capture the number of some event, an enum should
be used, ideally with an appropriate baseline.

What about changing this enum to SoftwareReporter.CleanupCard and recording
SHOWN here, and also recording "SHOULD_SHOW" or something like that when users
click more detailed in the prompt.

[1]
https://cs.chromium.org/chromium/src/base/metrics/histogram_macros.h?type=cs&...

[ftirelo, 2017/07/07 20:27:53]

Used a single-value enum instead.

   AllowJavascript();
 }
 
 void ChromeCleanupHandler::HandleRestartComputer(const base::ListValue* args) {
   DCHECK_EQ(0U, args->GetSize());
 
   CallJavascriptFunction("cr.webUIListenerCallback",
                          base::Value("chrome-cleanup-on-dismiss"));
   // TODO(proberge): Show a prompt to reboot the system.
 }
 
 void ChromeCleanupHandler::HandleStartCleanup(const base::ListValue* args) {
   DCHECK_EQ(0U, args->GetSize());
 
+  safe_browsing::RecordCleanupStartedHistogram(

[csharp, 2017/07/07 17:32:51]

Maybe also worth recording this in the suggested SoftwareReporter.CleanupCard,
to make it easy to see what percentage of people that go to details actually run
the cleanup

[ftirelo, 2017/07/07 20:27:53]

As discussed offline, it will be tricky to do that unless we add a parameter to
the settings page, which is not ideal. We should be able to see the results in a
custom query.

+      safe_browsing::CLEANUP_STARTED_FROM_PROMPT_IN_SETTINGS);
   controller_->ReplyWithUserResponse(
       // TODO(proberge): Send kAcceptedWithLogs or kAcceptedWithoutLogs based on
       // the state of a logs upload permissions checkbox.
       profile_, ChromeCleanerController::UserResponse::kAcceptedWithoutLogs);
 }
 
 }  // namespace settings