Confirm-to-quit: Close bubble when browser loses focus

Sometimes, another app can grab the focus while the confirm-to-quit bubble is
showing. When this happens, Chrome won't get the key release event and the
bubble will stay open forever.  The only way to close it would be to press
Ctrl+Shift+Q again, but this would close Chrome.

This CL closes the confirm-to-quit bubble and resets the state of the controller
when this happens.

BUG=243164
R=sky

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

chrome/browser/ui/views/confirm_quit_bubble_controller.cc

@@ -52,9 +52,12 @@ ConfirmQuitBubbleController::ConfirmQuitBubbleController(
       browser_hide_animation_(std::move(animation)) {
   browser_hide_animation_->SetSlideDuration(
       kWindowFadeOutDuration.InMilliseconds());
+  BrowserList::AddObserver(this);
 }
 
-ConfirmQuitBubbleController::~ConfirmQuitBubbleController() {}
+ConfirmQuitBubbleController::~ConfirmQuitBubbleController() {
+  BrowserList::RemoveObserver(this);
+}
 
 bool ConfirmQuitBubbleController::HandleKeyboardEvent(
     const ui::Accelerator& accelerator) {
@@ -66,6 +69,7 @@ bool ConfirmQuitBubbleController::HandleKeyboardEvent(
       !accelerator.IsRepeat()) {
     if (state_ == State::kWaiting) {
       state_ = State::kPressed;
+      browser_ = BrowserList::GetInstance()->GetLastActive();
       view_->Show();
       hide_timer_->Start(FROM_HERE, kShowDuration, this,
                          &ConfirmQuitBubbleController::OnTimerElapsed);
@@ -101,6 +105,17 @@ void ConfirmQuitBubbleController::AnimationEnded(
   AnimationProgressed(animation);
 }
 
+void ConfirmQuitBubbleController::OnBrowserNoLongerActive(Browser* browser) {
+  if (browser != browser_ || state_ == State::kWaiting ||
+      state_ == State::kQuitting) {
+    return;
+  }
+  state_ = State::kWaiting;
+  view_->Hide();
+  hide_timer_->Stop();
+  browser_hide_animation_->Hide();
+}
+
 void ConfirmQuitBubbleController::OnTimerElapsed() {
   if (state_ == State::kPressed) {
     // The accelerator was held down the entire time the bubble was showing.

chrome/browser/ui/views/confirm_quit_bubble_controller.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/timer/timer.h"
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "ui/gfx/animation/animation_delegate.h"
 
 class ConfirmQuitBubbleBase;
@@ -28,7 +29,8 @@ 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 : public gfx::AnimationDelegate {
+class ConfirmQuitBubbleController : public gfx::AnimationDelegate,
+                                    public BrowserListObserver {
  public:
   static ConfirmQuitBubbleController* GetInstance();
 
@@ -71,6 +73,9 @@ class ConfirmQuitBubbleController : public gfx::AnimationDelegate {
   void AnimationProgressed(const gfx::Animation* animation) override;
   void AnimationEnded(const gfx::Animation* animation) override;
 
+  // BrowserListObserver:
+  void OnBrowserNoLongerActive(Browser* browser) override;
+
   void OnTimerElapsed();
 
   void ConfirmQuit();
@@ -83,6 +88,9 @@ class ConfirmQuitBubbleController : public gfx::AnimationDelegate {
 
   State state_;
 
+  // The last active browser when the accelerator was pressed.
+  Browser* browser_ = nullptr;
+
   std::unique_ptr<base::Timer> hide_timer_;
 
   std::unique_ptr<gfx::SlideAnimation> const browser_hide_animation_;

chrome/browser/ui/views/confirm_quit_bubble_controller_unittest.cc

@@ -86,6 +86,8 @@ class ConfirmQuitBubbleControllerTest : public testing::Test {
 
   void ReleaseOtherAccelerator() { SendAccelerator(false, false, false); }
 
+  void DeactivateBrowser() { controller_->OnBrowserNoLongerActive(nullptr); }
+
   std::unique_ptr<ConfirmQuitBubbleController> controller_;
 
   // Owned by |controller_|.
@@ -151,3 +153,31 @@ TEST_F(ConfirmQuitBubbleControllerTest, OtherKeyPress) {
   ReleaseQuitAccelerator();
   EXPECT_TRUE(quit_called_);
 }
+
+// The controller state should be reset when the browser loses focus.
+TEST_F(ConfirmQuitBubbleControllerTest, BrowserLosesFocus) {
+  // Press but don't release the accelerator.
+  PressQuitAccelerator();
+  EXPECT_TRUE(timer_->IsRunning());
+  DeactivateBrowser();
+  EXPECT_FALSE(timer_->IsRunning());
+  EXPECT_FALSE(quit_called_);
+  ReleaseQuitAccelerator();
+
+  // Press and release the accelerator.
+  PressQuitAccelerator();
+  ReleaseQuitAccelerator();
+  EXPECT_TRUE(timer_->IsRunning());
+  DeactivateBrowser();
+  EXPECT_FALSE(timer_->IsRunning());
+  EXPECT_FALSE(quit_called_);
+
+  // Press and hold the accelerator.
+  PressQuitAccelerator();
+  EXPECT_TRUE(timer_->IsRunning());
+  timer_->Fire();
+  EXPECT_FALSE(timer_->IsRunning());
+  DeactivateBrowser();
+  ReleaseQuitAccelerator();
+  EXPECT_FALSE(quit_called_);
+}