Confirm-to-quit: Delay quitting until accelerator is released

On Mac, they discovered long ago that quitting after holding Cmd+Q was a bad
idea since apps underneath Chrome would get the repeated Cmd+Q key event and
would also close [1].  Their solution was to delay quitting until the shortcut
is released.  To give users confirmation that Chrome is quitting, all browser
windows are faded out.

The same issue occurs on Linux and Windows (I've unintentionally closed many
background apps writing CL [2]), so this CL implements the Mac solution.

[1] http://dev.chromium.org/developers/design-documents/confirm-to-quit-experiment
[2] https://chromium.googlesource.com/chromium/src.git/+/6cbd505d3cb04e3e108e61a74f82421b66f0f655

BUG=243164
R=sky

Change-Id: Ia3de4acaf463cd5b202743d44f763c6d52fbe963
Reviewed-on: https://chromium-review.googlesource.com/1066924Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560336}

chrome/browser/ui/views/confirm_quit_bubble_controller.cc

@@ -10,9 +10,13 @@
 #include "base/memory/singleton.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/views/confirm_quit_bubble.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/gfx/animation/animation.h"
+#include "ui/gfx/animation/slide_animation.h"
 
 namespace {
 
@@ -22,6 +26,9 @@ constexpr int kAcceleratorModifiers = ui::EF_SHIFT_DOWN | ui::EF_CONTROL_DOWN;
 constexpr base::TimeDelta kShowDuration =
     base::TimeDelta::FromMilliseconds(1500);
 
+constexpr base::TimeDelta kWindowFadeOutDuration =
+    base::TimeDelta::FromMilliseconds(200);
+
 }  // namespace
 
 // static
@@ -30,54 +37,90 @@ ConfirmQuitBubbleController* ConfirmQuitBubbleController::GetInstance() {
 }
 
 ConfirmQuitBubbleController::ConfirmQuitBubbleController()
-    : view_(std::make_unique<ConfirmQuitBubble>()),
-      hide_timer_(std::make_unique<base::OneShotTimer>()) {}
+    : ConfirmQuitBubbleController(std::make_unique<ConfirmQuitBubble>(),
+                                  std::make_unique<base::OneShotTimer>(),
+                                  std::make_unique<gfx::SlideAnimation>(this)) {
+}
 
 ConfirmQuitBubbleController::ConfirmQuitBubbleController(
     std::unique_ptr<ConfirmQuitBubbleBase> bubble,
-    std::unique_ptr<base::Timer> hide_timer)
-    : view_(std::move(bubble)), hide_timer_(std::move(hide_timer)) {}
+    std::unique_ptr<base::Timer> hide_timer,
+    std::unique_ptr<gfx::SlideAnimation> animation)
+    : view_(std::move(bubble)),
+      state_(State::kWaiting),
+      hide_timer_(std::move(hide_timer)),
+      browser_hide_animation_(std::move(animation)) {
+  browser_hide_animation_->SetSlideDuration(
+      kWindowFadeOutDuration.InMilliseconds());
+}
 
 ConfirmQuitBubbleController::~ConfirmQuitBubbleController() {}
 
 bool ConfirmQuitBubbleController::HandleKeyboardEvent(
     const ui::Accelerator& accelerator) {
+  if (state_ == State::kQuitting)
+    return false;
   if (accelerator.key_code() == kAcceleratorKeyCode &&
       accelerator.modifiers() == kAcceleratorModifiers &&
       accelerator.key_state() == ui::Accelerator::KeyState::PRESSED &&
       !accelerator.IsRepeat()) {
-    if (!hide_timer_->IsRunning()) {
+    if (state_ == State::kWaiting) {
+      state_ = State::kPressed;
       view_->Show();
-      released_key_ = false;
       hide_timer_->Start(FROM_HERE, kShowDuration, this,
                          &ConfirmQuitBubbleController::OnTimerElapsed);
-    } else {
+    } else if (state_ == State::kReleased) {
       // The accelerator was pressed while the bubble was showing.  Consider
       // this a confirmation to quit.
-      view_->Hide();
-      hide_timer_->Stop();
-      Quit();
+      ConfirmQuit();
     }
     return true;
   }
   if (accelerator.key_code() == kAcceleratorKeyCode &&
       accelerator.key_state() == ui::Accelerator::KeyState::RELEASED) {
-    released_key_ = true;
+    if (state_ == State::kPressed)
+      state_ = State::kReleased;
+    else if (state_ == State::kConfirmed)
+      Quit();
     return true;
   }
   return false;
 }
 
-void ConfirmQuitBubbleController::OnTimerElapsed() {
-  view_->Hide();
+void ConfirmQuitBubbleController::AnimationProgressed(
+    const gfx::Animation* animation) {
+  float opacity = static_cast<float>(animation->CurrentValueBetween(1.0, 0.0));
+  for (Browser* browser : *BrowserList::GetInstance()) {
+    BrowserView::GetBrowserViewForBrowser(browser)->GetWidget()->SetOpacity(
+        opacity);
+  }
+}
+
+void ConfirmQuitBubbleController::AnimationEnded(
+    const gfx::Animation* animation) {
+  AnimationProgressed(animation);
+}
 
-  if (!released_key_) {
+void ConfirmQuitBubbleController::OnTimerElapsed() {
+  if (state_ == State::kPressed) {
     // The accelerator was held down the entire time the bubble was showing.
-    Quit();
+    ConfirmQuit();
+  } else if (state_ == State::kReleased) {
+    state_ = State::kWaiting;
+    view_->Hide();
   }
 }
 
+void ConfirmQuitBubbleController::ConfirmQuit() {
+  DCHECK(state_ == State::kPressed || state_ == State::kReleased);
+  state_ = State::kConfirmed;
+  hide_timer_->Stop();
+  browser_hide_animation_->Show();
+}
+
 void ConfirmQuitBubbleController::Quit() {
+  DCHECK_EQ(state_, State::kConfirmed);
+  state_ = State::kQuitting;
   if (quit_action_) {
     std::move(quit_action_).Run();
   } else {

chrome/browser/ui/views/confirm_quit_bubble_controller.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/timer/timer.h"
+#include "ui/gfx/animation/animation_delegate.h"
 
 class ConfirmQuitBubbleBase;
 
@@ -17,17 +18,21 @@ template <typename T>
 struct DefaultSingletonTraits;
 }
 
+namespace gfx {
+class SlideAnimation;
+}
+
 namespace ui {
 class Accelerator;
 }
 
 // Manages showing and hiding the confirm-to-quit bubble.  Requests Chrome to be
 // closed if the quit accelerator is held down or pressed twice in succession.
-class ConfirmQuitBubbleController {
+class ConfirmQuitBubbleController : public gfx::AnimationDelegate {
  public:
   static ConfirmQuitBubbleController* GetInstance();
 
-  ~ConfirmQuitBubbleController();
+  ~ConfirmQuitBubbleController() override;
 
   // Returns true if the event was handled.
   bool HandleKeyboardEvent(const ui::Accelerator& accelerator);
@@ -36,24 +41,52 @@ class ConfirmQuitBubbleController {
   friend struct base::DefaultSingletonTraits<ConfirmQuitBubbleController>;
   friend class ConfirmQuitBubbleControllerTest;
 
+  enum class State {
+    // The accelerator has not been pressed.
+    kWaiting,
+
+    // The accelerator was pressed, but not yet released.
+    kPressed,
+
+    // The accelerator was pressed and released before the timer expired.
+    kReleased,
+
+    // The accelerator was either held down for the entire duration of the
+    // timer, or was pressed a second time.  Either way, the accelerator is
+    // currently held.
+    kConfirmed,
+
+    // The accelerator was released and Chrome is now quitting.
+    kQuitting,
+  };
+
+  // |animation| is used to fade out all browser windows.
   ConfirmQuitBubbleController(std::unique_ptr<ConfirmQuitBubbleBase> bubble,
-                              std::unique_ptr<base::Timer> hide_timer);
+                              std::unique_ptr<base::Timer> hide_timer,
+                              std::unique_ptr<gfx::SlideAnimation> animation);
 
   ConfirmQuitBubbleController();
 
+  // gfx::AnimationDelegate:
+  void AnimationProgressed(const gfx::Animation* animation) override;
+  void AnimationEnded(const gfx::Animation* animation) override;
+
   void OnTimerElapsed();
 
+  void ConfirmQuit();
+
   void Quit();
 
   void SetQuitActionForTest(base::OnceClosure quit_action);
 
   std::unique_ptr<ConfirmQuitBubbleBase> const view_;
 
-  // Indicates if the accelerator was released while the timer was active.
-  bool released_key_ = false;
+  State state_;
 
   std::unique_ptr<base::Timer> hide_timer_;
 
+  std::unique_ptr<gfx::SlideAnimation> const browser_hide_animation_;
+
   base::OnceClosure quit_action_;
 
   DISALLOW_COPY_AND_ASSIGN(ConfirmQuitBubbleController);

chrome/browser/ui/views/confirm_quit_bubble_controller_unittest.cc

@@ -14,6 +14,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/accelerators/accelerator.h"
 #include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/gfx/animation/slide_animation.h"
 
 class TestConfirmQuitBubble : public ConfirmQuitBubbleBase {
  public:
@@ -27,6 +28,21 @@ class TestConfirmQuitBubble : public ConfirmQuitBubbleBase {
   DISALLOW_COPY_AND_ASSIGN(TestConfirmQuitBubble);
 };
 
+class TestSlideAnimation : public gfx::SlideAnimation {
+ public:
+  TestSlideAnimation() : gfx::SlideAnimation(nullptr) {}
+  ~TestSlideAnimation() override {}
+
+  void Reset() override {}
+  void Reset(double value) override {}
+  void Show() override {}
+  void Hide() override {}
+  void SetSlideDuration(int duration) override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestSlideAnimation);
+};
+
 class ConfirmQuitBubbleControllerTest : public testing::Test {
  protected:
   void SetUp() override {
@@ -36,8 +52,9 @@ class ConfirmQuitBubbleControllerTest : public testing::Test {
         std::make_unique<base::MockTimer>(false, false);
     bubble_ = bubble.get();
     timer_ = timer.get();
-    controller_.reset(
-        new ConfirmQuitBubbleController(std::move(bubble), std::move(timer)));
+    controller_.reset(new ConfirmQuitBubbleController(
+        std::move(bubble), std::move(timer),
+        std::make_unique<TestSlideAnimation>()));
 
     quit_called_ = false;
     controller_->SetQuitActionForTest(base::BindOnce(
@@ -85,6 +102,8 @@ TEST_F(ConfirmQuitBubbleControllerTest, PressAndHold) {
   PressQuitAccelerator();
   EXPECT_TRUE(timer_->IsRunning());
   timer_->Fire();
+  EXPECT_FALSE(quit_called_);
+  ReleaseQuitAccelerator();
   EXPECT_TRUE(quit_called_);
 }
 
@@ -95,6 +114,8 @@ TEST_F(ConfirmQuitBubbleControllerTest, DoublePress) {
   EXPECT_TRUE(timer_->IsRunning());
   PressQuitAccelerator();
   EXPECT_FALSE(timer_->IsRunning());
+  EXPECT_FALSE(quit_called_);
+  ReleaseQuitAccelerator();
   EXPECT_TRUE(quit_called_);
 }
 
@@ -126,5 +147,7 @@ TEST_F(ConfirmQuitBubbleControllerTest, OtherKeyPress) {
   EXPECT_TRUE(timer_->IsRunning());
   PressQuitAccelerator();
   EXPECT_FALSE(timer_->IsRunning());
+  EXPECT_FALSE(quit_called_);
+  ReleaseQuitAccelerator();
   EXPECT_TRUE(quit_called_);
 }