Add heuristics to limit showing of new backspace UI bubble.

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

 // Copyright 2012 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/views/frame/browser_view.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
@@ skipping 940 matching lines @@
     ExclusiveAccessBubbleType bubble_type) {
   // Immersive mode has no exit bubble because it has a visible strip at the
   // top that gives the user a hover target.
   // TODO(jamescook): Figure out what to do with mouse-lock.
   if (bubble_type == EXCLUSIVE_ACCESS_BUBBLE_TYPE_NONE ||
       ShouldUseImmersiveFullscreenForUrl(url)) {
     exclusive_access_bubble_.reset();
     return;
   }
 
+  if (exclusive_access_bubble_) {
+    exclusive_access_bubble_->UpdateContent(url, bubble_type);
+    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));
-  }
+  exclusive_access_bubble_.reset(
+      new ExclusiveAccessBubbleViews(this, url, bubble_type));
 }
 
 void BrowserView::OnExclusiveAccessUserInput() {
   if (exclusive_access_bubble_.get())
     exclusive_access_bubble_->OnUserInput();
 }
 
 bool BrowserView::ShouldHideUIForFullscreen() const {
   // Immersive mode needs UI for the slide-down top panel.
   if (immersive_mode_controller_->IsEnabled())
     return false;
 
   return IsFullscreen();
 }
 
 bool BrowserView::IsFullscreen() const {
   return frame_->IsFullscreen();
 }
 
 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() {
   WebContents* selected_web_contents = GetActiveWebContents();
   if (selected_web_contents)
     selected_web_contents->RestoreFocus();
 }
 
 void BrowserView::FullscreenStateChanged() {
   CHECK(!IsFullscreen());
@@ skipping 1616 matching lines @@
 }
 
 extensions::ActiveTabPermissionGranter*
 BrowserView::GetActiveTabPermissionGranter() {
   content::WebContents* web_contents = GetActiveWebContents();
   if (!web_contents)
     return nullptr;
   return extensions::TabHelper::FromWebContents(web_contents)
       ->active_tab_permission_granter();
 }