Make the histograms for setting the default browser consistent

chrome/browser/ui/startup/default_browser_prompt.cc

Index: chrome/browser/ui/startup/default_browser_prompt.cc
diff --git a/chrome/browser/ui/startup/default_browser_prompt.cc b/chrome/browser/ui/startup/default_browser_prompt.cc
index d1c9143591d6beab85bf26a5b7c0e8adee3a1323..f2af878a91c5b1ca3fa56b3e20a26ce61e254df2 100644
--- a/chrome/browser/ui/startup/default_browser_prompt.cc
+++ b/chrome/browser/ui/startup/default_browser_prompt.cc
@@ -9,6 +9,7 @@
 #include "base/location.h"
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/metrics/user_metrics_action.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/single_thread_task_runner.h"
@@ -18,7 +19,6 @@
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/shell_integration.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -29,6 +29,7 @@
 #include "components/infobars/core/infobar.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/navigation_details.h"
+#include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -36,71 +37,6 @@
 
 namespace {
 
-// A ShellIntegration::DefaultWebClientObserver that records user metrics for
-// the result of making Chrome the default browser.
-class SetDefaultBrowserObserver
-    : public ShellIntegration::DefaultWebClientObserver {
- public:
-  SetDefaultBrowserObserver();
-  ~SetDefaultBrowserObserver() override;
-
- private:
-  void SetDefaultWebClientUIState(
-      ShellIntegration::DefaultWebClientUIState state) override;
-  void OnSetAsDefaultConcluded(bool succeeded) override;
-  bool IsOwnedByWorker() override;
-  bool IsInteractiveSetDefaultPermitted() override;
-
-  // True if an interactive flow will be used (i.e., Windows 8+).
-  bool interactive_;
-
-  // The result of the call to ShellIntegration::SetAsDefaultBrowser() or
-  // ShellIntegration::SetAsDefaultBrowserInteractive().
-  bool interaction_succeeded_ = false;
-
-  DISALLOW_COPY_AND_ASSIGN(SetDefaultBrowserObserver);
-};
-
-SetDefaultBrowserObserver::SetDefaultBrowserObserver()
-    : interactive_(ShellIntegration::CanSetAsDefaultBrowser() ==
-                   ShellIntegration::SET_DEFAULT_INTERACTIVE) {
-  // Log that an attempt is about to be made to set one way or the other.
-  if (interactive_)
-    UMA_HISTOGRAM_BOOLEAN("DefaultBrowserWarning.SetAsDefaultUI", true);
-  else
-    UMA_HISTOGRAM_BOOLEAN("DefaultBrowserWarning.SetAsDefault", true);
-}
-
-SetDefaultBrowserObserver::~SetDefaultBrowserObserver() {}
-
-void SetDefaultBrowserObserver::SetDefaultWebClientUIState(
-    ShellIntegration::DefaultWebClientUIState state) {
-  if (interactive_ && interaction_succeeded_ &&
-      state == ShellIntegration::STATE_NOT_DEFAULT) {
-    // The interactive flow succeeded, yet Chrome is not the default browser.
-    // This likely means that the user selected another browser from the panel.
-    // Consider this the same as canceling the infobar.
-    UMA_HISTOGRAM_BOOLEAN("DefaultBrowserWarning.DontSetAsDefault", true);
-  }
-}
-
-void SetDefaultBrowserObserver::OnSetAsDefaultConcluded(bool succeeded) {
-  interaction_succeeded_ = succeeded;
-  if (interactive_ && !succeeded) {
-    // Log that the interactive flow failed.
-    UMA_HISTOGRAM_BOOLEAN("DefaultBrowserWarning.SetAsDefaultUIFailed", true);
-  }
-}
-
-bool SetDefaultBrowserObserver::IsOwnedByWorker() {
-  // Instruct the DefaultBrowserWorker to delete this instance when it is done.
-  return true;
-}
-
-bool SetDefaultBrowserObserver::IsInteractiveSetDefaultPermitted() {
-  return true;
-}
-
 // The delegate for the infobar shown when Chrome is not the default browser.
 class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
@@ -164,7 +100,7 @@ DefaultBrowserInfoBarDelegate::DefaultBrowserInfoBarDelegate(PrefService* prefs)
 
 DefaultBrowserInfoBarDelegate::~DefaultBrowserInfoBarDelegate() {
   if (!action_taken_)
-    UMA_HISTOGRAM_BOOLEAN("DefaultBrowserWarning.Ignored", true);
+    UMA_HISTOGRAM_BOOLEAN("DefaultBrowser.InfoBar.Ignored", true);
 }
 
 infobars::InfoBarDelegate::Type DefaultBrowserInfoBarDelegate::GetInfoBarType()
@@ -213,15 +149,23 @@ bool DefaultBrowserInfoBarDelegate::OKButtonTriggersUACPrompt() const {
 
 bool DefaultBrowserInfoBarDelegate::Accept() {
   action_taken_ = true;
+  content::RecordAction(
+      base::UserMetricsAction("DefaultBrowserInfoBar_Accept"));
+  UMA_HISTOGRAM_ENUMERATION(

[grt (UTC plus 2), 2015/11/03 21:12:22]

I'm not a fan of having an enum in ShellIntegration used by consumers to UMA why
they call StartSetAsDefault. I think either one of the following is preferable
(each has drawbacks):

- Change StartSetAsDefault() so that it takes a value indicating why it is being
invoked. This value could be the enum used to UMA within StartSetAsDefault. This
has the drawback that a new value must be added to the enum each time a new
callsite is introduced.

- Change this so that it UMAs something like
DefaultBrowser.InfoBar.StartSetAsDefault. Other callers would do similarly in
their own namespace.

I think the latter is good, since it is easy to view all
DefaultBrowser.InfoBar.* histograms at once.

[Patrick Monette, 2015/11/10 19:22:47]

Done.

+      "SetDefaultBrowser.Location",
+      ShellIntegration::SetDefaultBrowserLocation::INFO_BAR,
+      ShellIntegration::SetDefaultBrowserLocation::
+          NUM_SET_DEFAULT_BROWSER_LOCATION_TYPES);
   scoped_refptr<ShellIntegration::DefaultBrowserWorker>(
-      new ShellIntegration::DefaultBrowserWorker(new SetDefaultBrowserObserver))
+      new ShellIntegration::DefaultBrowserWorker(nullptr))
       ->StartSetAsDefault();
   return true;
 }
 
 bool DefaultBrowserInfoBarDelegate::Cancel() {
   action_taken_ = true;
-  UMA_HISTOGRAM_BOOLEAN("DefaultBrowserWarning.DontSetAsDefault", true);
+  content::RecordAction(
+      base::UserMetricsAction("DefaultBrowserInfoBar_Cancel"));
   // User clicked "Don't ask me again", remember that.
   prefs_->SetBoolean(prefs::kCheckDefaultBrowser, false);
   return true;