Simplify the default browser infobar.

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 59e043d6ee2ccb931d24fd4e0059d947e6917e9e..c9b19ae2c1363a35373c076bee00a44384576a5e 100644
--- a/chrome/browser/ui/startup/default_browser_prompt.cc
+++ b/chrome/browser/ui/startup/default_browser_prompt.cc
@@ -12,7 +12,9 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/thread_task_runner_handle.h"
+#include "base/time/time.h"
 #include "base/version.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
@@ -29,17 +31,22 @@
 #include "components/infobars/core/infobar.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
+#include "components/variations/variations_associated_data.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"
 #include "ui/gfx/vector_icons_public.h"
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 namespace {
 
 // The delegate for the infobar shown when Chrome is not the default browser.
+// Ownership of the delegate is given to the infobar itself, the lifetime of
+// which is bound to the containing WebContents.
 class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
   // Creates a default browser infobar and delegate and adds the infobar to
@@ -50,12 +57,12 @@ class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate {
   // Possible user interactions with the default browser info bar.
   // Do not modify the ordering as it is important for UMA.
   enum InfoBarUserInteraction {
-    // The user clicked the "Set as default" button.
-    START_SET_AS_DEFAULT,
-    // The user doesn't want to be reminded again.
-    DONT_ASK_AGAIN,
+    // The user clicked the "OK" (i.e., "Set as default") button.
+    ACCEPT_INFO_BAR = 0,

[rkaplow, 2016/04/11 22:16:29]

since you're renaming here, I would suggest rename the histogram enum values as
well

[grt (UTC plus 2), 2016/04/12 13:14:45]

Do you mean the values for the DefaultBrowserInfoBarUserInteraction enum in
histograms.xml? I'm doing that:
https://codereview.chromium.org/1748773002/diff/200001/tools/metrics/histogra....
Did I miss a spot?

+    // The user clicked the "Cancel" (i.e., "Don't ask again") button.
+    CANCEL_INFO_BAR = 1,
     // The user did not interact with the info bar.
-    IGNORE_INFO_BAR,
+    IGNORE_INFO_BAR = 2,
     NUM_INFO_BAR_USER_INTERACTION_TYPES
   };
 
@@ -64,12 +71,15 @@ class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate {
 
   void AllowExpiry() { should_expire_ = true; }
 
-  // ConfirmInfoBarDelegate:
+  // InfoBarDelegate:
   Type GetInfoBarType() const override;
   infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override;
   int GetIconId() const override;
   gfx::VectorIconId GetVectorIconId() const override;
   bool ShouldExpire(const NavigationDetails& details) const override;
+  void InfoBarDismissed() override;
+
+  // ConfirmInfoBarDelegate:
   base::string16 GetMessageText() const override;
   base::string16 GetButtonLabel(InfoBarButton button) const override;
   bool OKButtonTriggersUACPrompt() const override;
@@ -79,11 +89,8 @@ class DefaultBrowserInfoBarDelegate : public ConfirmInfoBarDelegate {
   // The WebContents's corresponding profile.
   Profile* profile_;
 
-  // Whether the user clicked one of the buttons.
-  bool action_taken_;
-
   // Whether the info-bar should be dismissed on the next navigation.
-  bool should_expire_;
+  bool should_expire_ = false;
 
   // Used to delay the expiration of the info-bar.
   base::WeakPtrFactory<DefaultBrowserInfoBarDelegate> weak_factory_;
@@ -100,11 +107,7 @@ void DefaultBrowserInfoBarDelegate::Create(InfoBarService* infobar_service,
 }
 
 DefaultBrowserInfoBarDelegate::DefaultBrowserInfoBarDelegate(Profile* profile)
-    : ConfirmInfoBarDelegate(),
-      profile_(profile),
-      action_taken_(false),
-      should_expire_(false),
-      weak_factory_(this) {
+    : ConfirmInfoBarDelegate(), profile_(profile), weak_factory_(this) {
   // We want the info-bar to stick-around for few seconds and then be hidden
   // on the next navigation after that.
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
@@ -113,12 +116,7 @@ DefaultBrowserInfoBarDelegate::DefaultBrowserInfoBarDelegate(Profile* profile)
       base::TimeDelta::FromSeconds(8));
 }
 
-DefaultBrowserInfoBarDelegate::~DefaultBrowserInfoBarDelegate() {
-  if (!action_taken_)
-    UMA_HISTOGRAM_ENUMERATION("DefaultBrowser.InfoBar.UserInteraction",
-                              InfoBarUserInteraction::IGNORE_INFO_BAR,
-                              NUM_INFO_BAR_USER_INTERACTION_TYPES);
-}
+DefaultBrowserInfoBarDelegate::~DefaultBrowserInfoBarDelegate() = default;
 
 infobars::InfoBarDelegate::Type DefaultBrowserInfoBarDelegate::GetInfoBarType()
     const {
@@ -151,15 +149,34 @@ bool DefaultBrowserInfoBarDelegate::ShouldExpire(
   return should_expire_ && ConfirmInfoBarDelegate::ShouldExpire(details);
 }
 
+void DefaultBrowserInfoBarDelegate::InfoBarDismissed() {

[msw, 2016/04/11 21:54:27]

We don't update kDefaultBrowserLastDeclined (on clicking (x) to close?), so this
will now repeatedly show the infobar (on startup?) until someone actually
declines; is that consistent with the existing behavior? Is it what we want?

[grt (UTC plus 2), 2016/04/12 13:14:45]

Yes, this is identical to the current behavior. 'x' dismisses it for the current
session, but does not do so for future sessions.
Yup. :-)

+  content::RecordAction(
+      base::UserMetricsAction("DefaultBrowserInfoBar_Dismiss"));
+  UMA_HISTOGRAM_ENUMERATION("DefaultBrowser.InfoBar.UserInteraction",
+                            IGNORE_INFO_BAR,
+                            NUM_INFO_BAR_USER_INTERACTION_TYPES);
+}
+
 base::string16 DefaultBrowserInfoBarDelegate::GetMessageText() const {
   return l10n_util::GetStringUTF16(IDS_DEFAULT_BROWSER_INFOBAR_SHORT_TEXT);
 }
 
 base::string16 DefaultBrowserInfoBarDelegate::GetButtonLabel(
     InfoBarButton button) const {
-  return l10n_util::GetStringUTF16((button == BUTTON_OK) ?
-      IDS_SET_AS_DEFAULT_INFOBAR_BUTTON_LABEL :
-      IDS_DONT_ASK_AGAIN_INFOBAR_BUTTON_LABEL);
+#if defined(OS_WIN)
+  // On Windows 10, the "OK" button opens the Windows Settings application,
+  // through which the user must make their default browser choice.
+  const int kSetAsDefaultButtonMessageId =
+      base::win::GetVersion() >= base::win::VERSION_WIN10
+          ? IDS_DEFAULT_BROWSER_INFOBAR_OK_BUTTON_LABEL_WIN_10
+          : IDS_DEFAULT_BROWSER_INFOBAR_OK_BUTTON_LABEL;
+#else
+  const int kSetAsDefaultButtonMessageId =
+      IDS_DEFAULT_BROWSER_INFOBAR_OK_BUTTON_LABEL;
+#endif
+  return l10n_util::GetStringUTF16(
+      button == BUTTON_OK ? kSetAsDefaultButtonMessageId
+                          : IDS_DEFAULT_BROWSER_INFOBAR_CANCEL_BUTTON_LABEL);
 }
 
 // Setting an app as the default browser doesn't require elevation directly, but
@@ -170,11 +187,10 @@ bool DefaultBrowserInfoBarDelegate::OKButtonTriggersUACPrompt() const {
 }
 
 bool DefaultBrowserInfoBarDelegate::Accept() {
-  action_taken_ = true;
   content::RecordAction(
       base::UserMetricsAction("DefaultBrowserInfoBar_Accept"));
   UMA_HISTOGRAM_ENUMERATION("DefaultBrowser.InfoBar.UserInteraction",
-                            InfoBarUserInteraction::START_SET_AS_DEFAULT,
+                            ACCEPT_INFO_BAR,
                             NUM_INFO_BAR_USER_INTERACTION_TYPES);
   // The worker pointer is reference counted. While it is running, the
   // message loops of the FILE and UI thread will hold references to it
@@ -187,14 +203,12 @@ bool DefaultBrowserInfoBarDelegate::Accept() {
 }
 
 bool DefaultBrowserInfoBarDelegate::Cancel() {
-  action_taken_ = true;
+  chrome::DefaultBrowserPromptDeclined(profile_);
   content::RecordAction(
-      base::UserMetricsAction("DefaultBrowserInfoBar_DontAskAgain"));
+      base::UserMetricsAction("DefaultBrowserInfoBar_Cancel"));
   UMA_HISTOGRAM_ENUMERATION("DefaultBrowser.InfoBar.UserInteraction",
-                            InfoBarUserInteraction::DONT_ASK_AGAIN,
+                            CANCEL_INFO_BAR,
                             NUM_INFO_BAR_USER_INTERACTION_TYPES);
-  // User clicked "Don't ask me again", remember that.
-  chrome::DefaultBrowserPromptDeclined(profile_);
   return true;
 }
 
@@ -221,6 +235,42 @@ void ShowPrompt() {
       InfoBarService::FromWebContents(web_contents), browser->profile());
 }
 
+// Returns true if the default browser prompt should be shown if Chrome is not
+// the user's default browser.
+bool ShouldShowDefaultBrowserPrompt(Profile* profile) {
+  // Do not show the prompt if the "suppress_default_browser_prompt_for_version"
+  // master preference is set to the current version.
+  const std::string disable_version_string =
+      g_browser_process->local_state()->GetString(
+          prefs::kBrowserSuppressDefaultBrowserPrompt);
+  const Version disable_version(disable_version_string);
+  DCHECK(disable_version_string.empty() || disable_version.IsValid());
+  if (disable_version.IsValid() &&
+      disable_version == Version(version_info::GetVersionNumber())) {
+    return false;
+  }
+
+  // Do not show if the prompt period has yet to pass since the user previously
+  // dismissed the infobar.
+  int64_t last_dismissed_value =
+      profile->GetPrefs()->GetInt64(prefs::kDefaultBrowserLastDeclined);
+  if (last_dismissed_value) {
+    int period_days = 0;
+    base::StringToInt(variations::GetVariationParamValue(
+                          "DefaultBrowserInfobar", "RefreshPeriodDays"),
+                      &period_days);
+    if (period_days <= 0 || period_days == std::numeric_limits<int>::max())
+      return false;  // Failed to parse a reasonable period.
+    base::Time show_on_or_after =
+        base::Time::FromInternalValue(last_dismissed_value) +
+        base::TimeDelta::FromDays(period_days);
+    if (base::Time::Now() < show_on_or_after)
+      return false;
+  }
+
+  return true;
+}
+
 void OnCheckIsDefaultBrowserFinished(
     const base::FilePath& profile_path,
     bool show_prompt,
@@ -262,35 +312,20 @@ void ShowDefaultBrowserPrompt(Profile* profile) {
     ResetDefaultBrowserPrompt(profile);
   }
 
-  // Check if Chrome is the default browser but do not prompt if:
-  // - The user said "don't ask me again" on the infobar earlier.
-  // - The "suppress_default_browser_prompt_for_version" master preference is
-  //     set to the current version.
-  bool show_prompt = prefs->GetBoolean(prefs::kCheckDefaultBrowser);
-  if (show_prompt) {
-    const std::string disable_version_string =
-        g_browser_process->local_state()->GetString(
-            prefs::kBrowserSuppressDefaultBrowserPrompt);
-    const Version disable_version(disable_version_string);
-    DCHECK(disable_version_string.empty() || disable_version.IsValid());
-    if (disable_version.IsValid() &&
-        disable_version == Version(version_info::GetVersionNumber())) {
-      show_prompt = false;
-    }
-  }
-
   scoped_refptr<shell_integration::DefaultBrowserWorker>(
-      new shell_integration::DefaultBrowserWorker(base::Bind(
-          &OnCheckIsDefaultBrowserFinished, profile->GetPath(), show_prompt)))
+      new shell_integration::DefaultBrowserWorker(
+          base::Bind(&OnCheckIsDefaultBrowserFinished, profile->GetPath(),
+                     ShouldShowDefaultBrowserPrompt(profile))))
       ->StartCheckIsDefault();
 }
 
 void DefaultBrowserPromptDeclined(Profile* profile) {
-  profile->GetPrefs()->SetBoolean(prefs::kCheckDefaultBrowser, false);
+  profile->GetPrefs()->SetInt64(prefs::kDefaultBrowserLastDeclined,
+                                base::Time::Now().ToInternalValue());
 }
 
 void ResetDefaultBrowserPrompt(Profile* profile) {
-  profile->GetPrefs()->ClearPref(prefs::kCheckDefaultBrowser);
+  profile->GetPrefs()->ClearPref(prefs::kDefaultBrowserLastDeclined);
 }
 
 #if !defined(OS_WIN)