Added notification when user presses backspace to use new shortcut.

chrome/browser/ui/views/backspace_new_shortcut_bubble.cc

+// Copyright 2016 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/backspace_new_shortcut_bubble.h"
+
+#include <utility>
+
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
+#include "build/build_config.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/ui/views/exclusive_access_bubble_views_context.h"
+#include "chrome/browser/ui/views/frame/top_container_view.h"
+#include "chrome/browser/ui/views/subtle_notification_view.h"
+#include "chrome/grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/gfx/animation/animation.h"
+#include "ui/gfx/animation/slide_animation.h"
+#include "ui/gfx/geometry/rect.h"
+#include "ui/strings/grit/ui_strings.h"
+#include "ui/views/bubble/bubble_border.h"
+#include "ui/views/view.h"
+#include "ui/views/widget/widget.h"
+
+#if defined(OS_WIN)
+#include "ui/base/l10n/l10n_util_win.h"
+#endif

[Peter Kasting, 2016/05/19 10:08:36]

Doesn't seem like this is used.

Check other #includes too.

[Matt Giuca, 2016/05/23 04:22:56]

Done.

+
+namespace {
+
+const int kPopupTopPx = 45;
+const int kSlideInDurationMs = 350;
+const int kSlideOutDurationMs = 700;
+const int kShowDurationMs = 3800;
+
+}
+
+BackspaceNewShortcutBubble::BackspaceNewShortcutBubble(
+    ExclusiveAccessBubbleViewsContext* context,
+    bool forward)
+    : bubble_view_context_(context),
+      popup_(nullptr),
+      animation_(new gfx::SlideAnimation(this)) {
+  // Initially hide the bubble.
+  double initial_value = 0;
+  animation_->Reset(initial_value);
+
+  // Create the contents view.
+  view_ = new SubtleNotificationView(nullptr);
+  UpdateViewContent(forward);
+
+  // Initialize the popup.
+  popup_ = SubtleNotificationView::CreatePopupWidget(
+      bubble_view_context_->GetBubbleParentView(), view_, false);
+  gfx::Size size = GetPopupRect(true).size();
+  // Bounds are in screen coordinates.
+  popup_->SetBounds(GetPopupRect(false));
+  view_->SetBounds(0, 0, size.width(), size.height());

[Peter Kasting, 2016/05/19 10:08:36]

Nit: Can we just do view_->SetSize(size) like UpdateContent() does, since the
position should be the origin by default?

If so we can also inline |size| into this statement (as suggested below).

[Matt Giuca, 2016/05/23 04:22:56]

Actually this was all duplicated with UpdateContent, so I just call that.

Now UpdateViewContent can be inlined into UpdateContent.

+
+  Show();
+
+  // Do not allow the notification to hide for a few seconds.

[Peter Kasting, 2016/05/19 10:08:36]

Nit: "Wait a few seconds before hiding"?  After all, if our owner deletes us,
we'll disappear immediately, so this doesn't "prevent" anything.

[Matt Giuca, 2016/05/23 04:22:56]

Done.

+  hide_timeout_.Start(FROM_HERE,
+                      base::TimeDelta::FromMilliseconds(kShowDurationMs), this,
+                      &BackspaceNewShortcutBubble::TimerElapsed);
+}
+
+BackspaceNewShortcutBubble::~BackspaceNewShortcutBubble() {
+  // This is tricky.  We may be in an ATL message handler stack, in which case
+  // the popup cannot be deleted yet.  We also can't set the popup's ownership
+  // model to NATIVE_WIDGET_OWNS_WIDGET because if the user closed the last tab
+  // while in fullscreen mode, Windows has already destroyed the popup HWND by

[Peter Kasting, 2016/05/19 10:08:36]

Is this all still relevant, given that this bubble no longer has anything to do
with fullscreen mode?

[Matt Giuca, 2016/05/23 04:22:57]

I have no idea what the consequences of not doing this are (I didn't write it).
I think at least some of it still applies here. I'll leave the code, but
condense the comment to point at ExclusiveAccessBubbleViews as the authority.

+  // the time we get here, and thus either the popup will already have been
+  // deleted (if we set this in our constructor) or the popup will never get
+  // another OnFinalMessage() call (if not, as currently).  So instead, we tell
+  // the popup to synchronously hide, and then asynchronously close and delete
+  // itself.
+  popup_->Close();
+  base::MessageLoop::current()->DeleteSoon(FROM_HERE, popup_);
+}
+
+void BackspaceNewShortcutBubble::UpdateContent(bool forward) {
+  UpdateViewContent(forward);
+
+  gfx::Size size = GetPopupRect(true).size();

[Peter Kasting, 2016/05/19 10:08:36]

Nit: I'd just inline thie into the next line.

[Matt Giuca, 2016/05/23 04:22:56]

Done.

+  view_->SetSize(size);
+  popup_->SetBounds(GetPopupRect(false));
+  Show();
+}
+
+void BackspaceNewShortcutBubble::UpdateBounds() {

[Peter Kasting, 2016/05/19 10:08:36]

Who calls this method?  It seems unused.

[Matt Giuca, 2016/05/23 04:22:56]

Done.

+  gfx::Rect popup_rect(GetPopupRect(false));
+  if (!popup_rect.IsEmpty()) {
+    popup_->SetBounds(popup_rect);
+    view_->SetY(popup_rect.height() - view_->height());
+  }
+}
+
+void BackspaceNewShortcutBubble::UpdateViewContent(bool forward) {
+  base::string16 exit_instruction_text;

[Peter Kasting, 2016/05/19 10:08:36]

Nit: Declare this down as close to initialization as possible.

[Matt Giuca, 2016/05/23 04:22:56]

Done.

+  // Note: The key names are parameters so that we can vary by operating system
+  // or change the direction of the arrow as necessary (see
+  // https://crbug.com/612685).
+
+#if defined(OS_MACOSX)
+  // U+2318 = PLACE OF INTEREST SIGN (Mac Command symbol).
+  base::string16 accelerator = base::WideToUTF16(L"\x2318");
+#else
+  base::string16 accelerator = l10n_util::GetStringUTF16(IDS_APP_ALT_KEY);
+#endif
+
+  if (forward) {
+    // U+2192 = RIGHTWARDS ARROW.
+    exit_instruction_text =
+        l10n_util::GetStringFUTF16(IDS_PRESS_ALT_RIGHT_TO_GO_FORWARD,
+                                   accelerator, base::WideToUTF16(L"\x2192"));
+  } else {
+    // U+2190 = LEFTWARDS ARROW.
+    exit_instruction_text =
+        l10n_util::GetStringFUTF16(IDS_PRESS_ALT_LEFT_TO_GO_BACK, accelerator,
+                                   base::WideToUTF16(L"\x2190"));
+  }

[Peter Kasting, 2016/05/19 10:08:36]

Nit: Shorter:

  // U+2192 = RIGHTWARDS ARROW; U+2190 = LEFTWARDS ARROW.
  base::string16 exit_instruction_text = l10n_util::GetStringFUTF16(
      forward ? IDS_PRESS_ALT_RIGHT_TO_GO_FORWARD
              : IDS_PRESS_ALT_LEFT_TO_GO_BACK,
      accelerator,
      base::WideToUTF16(forward ? L"\x2192" : L"\x2190"));

[Matt Giuca, 2016/05/23 04:22:57]

Not a fan of a 5-line expression with multiple ternary operators. Split out into
a few variables but kept the spirit of what you're asking.

+  view_->UpdateContent(exit_instruction_text, base::string16());
+}
+
+views::View* BackspaceNewShortcutBubble::GetBrowserRootView() const {

[Peter Kasting, 2016/05/19 10:08:36]

Another unused function?

[Matt Giuca, 2016/05/23 04:22:57]

Done.

+  return bubble_view_context_->GetBubbleAssociatedWidget()->GetRootView();
+}
+
+void BackspaceNewShortcutBubble::AnimationProgressed(
+    const gfx::Animation* animation) {
+  int opacity = animation_->CurrentValueBetween(0, 255);
+  if (opacity == 0) {
+    popup_->Hide();
+  } else {
+    popup_->Show();
+    popup_->SetOpacity(opacity);
+  }
+}
+
+void BackspaceNewShortcutBubble::AnimationEnded(
+    const gfx::Animation* animation) {
+  AnimationProgressed(animation);
+}
+
+gfx::Rect BackspaceNewShortcutBubble::GetPopupRect(
+    bool ignore_animation_state) const {
+  gfx::Size size(view_->GetPreferredSize());

[Peter Kasting, 2016/05/19 10:08:36]

Nit: Use = rather than () where possible

[Matt Giuca, 2016/05/23 04:22:56]

Done.

+  gfx::Rect widget_bounds = bubble_view_context_->GetClientAreaBoundsInScreen();
+  int x = widget_bounds.x() + (widget_bounds.width() - size.width()) / 2;
+  int top_container_bottom = widget_bounds.y();
+  // |desired_top| is the top of the bubble area including the shadow.
+  int desired_top = kPopupTopPx - view_->border()->GetInsets().top();
+  int y = top_container_bottom + desired_top;
+  return gfx::Rect(gfx::Point(x, y), size);
+}
+
+void BackspaceNewShortcutBubble::Hide() {
+  animation_->SetSlideDuration(kSlideOutDurationMs);
+  animation_->Hide();
+}
+
+void BackspaceNewShortcutBubble::Show() {
+  animation_->SetSlideDuration(kSlideInDurationMs);
+  animation_->Show();
+}
+
+void BackspaceNewShortcutBubble::TimerElapsed() {
+  Hide();
+}