Add heuristics to limit showing of new backspace UI bubble.

chrome/browser/ui/views/frame/browser_view.cc

Index: chrome/browser/ui/views/frame/browser_view.cc
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 8291251d20d8e8c3feacdacd422ba266d15eaacb..536a3f8e286ba129fcb5bf6c52284de1635b767f 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -958,15 +958,16 @@ void BrowserView::UpdateExclusiveAccessExitBubbleContent(
     return;
   }
 
-  // Hide the backspace shortcut bubble, to avoid overlapping.
-  new_back_shortcut_bubble_.reset();
-
   if (exclusive_access_bubble_) {
     exclusive_access_bubble_->UpdateContent(url, bubble_type);
-  } else {
-    exclusive_access_bubble_.reset(
-        new ExclusiveAccessBubbleViews(this, url, bubble_type));
+    return;
   }
+
+  // Hide the backspace shortcut bubble, to avoid overlapping.
+  new_back_shortcut_bubble_.reset();
+
+  exclusive_access_bubble_.reset(
+      new ExclusiveAccessBubbleViews(this, url, bubble_type));
 }
 
 void BrowserView::OnExclusiveAccessUserInput() {
@@ -990,14 +991,36 @@ bool BrowserView::IsFullscreenBubbleVisible() const {
   return exclusive_access_bubble_ != nullptr;
 }
 
-void BrowserView::ShowNewBackShortcutBubble(bool forward) {
-  // Hide the exclusive access bubble, to avoid overlapping.
-  exclusive_access_bubble_.reset();
+void BrowserView::MaybeShowNewBackShortcutBubble(bool forward) {
+  if (!new_back_shortcut_bubble_ || !new_back_shortcut_bubble_->IsVisible()) {

[Matt Giuca, 2016/06/08 01:29:03]

I think my (weak) preference would be to keep the constructor of NBSB calling
UpdateContent so you don't have to unconditionally call it here. Then you could
do an early exit and not have the bulk of this function indented:

if (new_back_shortcut_bubble_ && new_back_shortcut_bubble_->IsVisible()) {
  new_back_shortcut_bubble_->UpdateContent(forward);
  return;
}
...

Up to you.

[Peter Kasting, 2016/06/09 19:49:31]

I originally wrote the code that way.  What I didn't like about it was that then
I had multiple function exit points all having to call UpdateContent().  This
seemed error-prone, so I eliminated the early-return so that UpdateContent()
would get called exactly once.  But once I'd done that it made the code simpler
to just do it in every case, at which point I didn't need this call.

It can be done either way, but there was a method to my madness :)

[Matt Giuca, 2016/06/10 01:07:05]

Acknowledged.

+    PrefService* prefs = browser_->profile()->GetPrefs();
+    int shown_count = prefs->GetInteger(prefs::kBackShortcutBubbleShownCount);
+    constexpr int kMaxShownCount = 5;
+    if (shown_count >= kMaxShownCount)
+      return;
+
+    const base::TimeTicks now = base::TimeTicks::Now();
+    constexpr base::TimeDelta kRepeatWindow = base::TimeDelta::FromSeconds(3);

[Matt Giuca, 2016/06/08 01:29:03]

Can this just be const?

[Peter Kasting, 2016/06/09 19:49:31]

Why?  We should use constexpr for true compile-time constants.

[Matt Giuca, 2016/06/10 01:07:05]

Acknowledged.

+    if (last_back_shortcut_press_time_.is_null() ||

[Matt Giuca, 2016/06/08 01:29:03]

// Only show the bubble if there have been two successive presses within the
last 3 seconds.

[Peter Kasting, 2016/06/09 19:49:31]

Done.

+        ((now - last_back_shortcut_press_time_) > kRepeatWindow)) {
+      last_back_shortcut_press_time_ = now;
+      return;
+    }
+
+    // Hide the exclusive access bubble, to avoid overlapping.
+    exclusive_access_bubble_.reset();
+
+    new_back_shortcut_bubble_.reset(new NewBackShortcutBubble(this));
+    prefs->SetInteger(prefs::kBackShortcutBubbleShownCount, shown_count + 1);
+    last_back_shortcut_press_time_ = base::TimeTicks();
+  }
+
+  new_back_shortcut_bubble_->UpdateContent(forward);
+}
 
+void BrowserView::HideNewBackShortcutBubble() {
   if (new_back_shortcut_bubble_)
-    new_back_shortcut_bubble_->UpdateContent(forward);
-  else
-    new_back_shortcut_bubble_.reset(new NewBackShortcutBubble(this, forward));
+    new_back_shortcut_bubble_->Hide();
 }
 
 void BrowserView::RestoreFocus() {