Unify and change logout/sleep/lock shortcuts

ash/accelerators/exit_warning_handler.cc

 // Copyright 2013 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 "ash/accelerators/exit_warning_handler.h"
 
+#include "ash/accelerators/accelerator_controller.h"
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
 #include "ash/shell_window_ids.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time.h"
 #include "base/timer.h"
 #include "grit/ash_strings.h"
 #include "ui/aura/root_window.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 #include "ui/views/widget/widget_delegate.h"
 
 namespace ash {
 namespace {
 
 const int64 kDoublePressTimeOutMilliseconds = 300;
-const int64 kHoldTimeOutMilliseconds = 1700;
+const int64 kHoldTimeOutMilliseconds = 1000;
 const SkColor kForegroundColor = 0xFFFFFFFF;
 const SkColor kBackgroundColor = 0xE0808080;
 const int kHorizontalMarginAroundText = 100;
 const int kVerticalMarginAroundText = 100;
 
 class ExitWarningWidgetDelegateView : public views::WidgetDelegateView {
  public:
   ExitWarningWidgetDelegateView() : text_width_(0), width_(0), height_(0) {
     text_ = l10n_util::GetStringUTF16(IDS_ASH_EXIT_WARNING_POPUP_TEXT);
     ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
@@ skipping 27 matching lines @@
   int text_width_;
   int width_;
   int height_;
 
   DISALLOW_COPY_AND_ASSIGN(ExitWarningWidgetDelegateView);
 };
 
 } // namespace
 
 ExitWarningHandler::ExitWarningHandler()
-  : state_(IDLE),
-    widget_(NULL),
-    stub_timers_for_test_(false) {
+    : context_(NULL),
+      state_(IDLE),
+      widget_(NULL),
+      stub_timers_for_test_(false) {
 }
 
 ExitWarningHandler::~ExitWarningHandler() {
   // Note: If a timer is outstanding, it is stopped in its destructor.
   Hide();
 }
 
-void ExitWarningHandler::HandleExitKey(bool press) {
+void ExitWarningHandler::Init(AcceleratorControllerContext* context) {

[sky, 2013/05/17 14:53:16]

Why not pass the context into the constructor?

[sschmitz, 2013/05/17 15:53:31]

ExitWarningHandler and AcceleratorControllerContext are both members of
AcceleratorController. I avoided initializing one member with the address of
another in the constructor initialization list.

+  context_ = context;
+}
+
+void ExitWarningHandler::HandleAccelerator() {
+  if (!context_)
+    return;
   switch (state_) {
     case IDLE:
-      if (press) {
-        state_ = WAIT_FOR_QUICK_RELEASE;
-        Show();
-        StartTimers();
-      }
-      break;
-    case WAIT_FOR_QUICK_RELEASE:
-      if (!press)
-        state_ = WAIT_FOR_DOUBLE_PRESS;
+      state_ = WAIT_FOR_DOUBLE_PRESS;
+      accelerator_ = context_->current_accelerator();
+      Show();
+      StartTimers();
       break;
     case WAIT_FOR_DOUBLE_PRESS:
-      if (press) {
-        state_ = EXITING;
-        CancelTimers();
-        Hide();
-        Shell::GetInstance()->delegate()->Exit();
-      }
+      state_ = EXITING;
+      CancelTimers();
+      Hide();
+      Shell::GetInstance()->delegate()->Exit();
       break;
     case WAIT_FOR_LONG_HOLD:
-      if (!press)
-        state_ = CANCELED;
+      state_ = CANCELED;
       break;
     case CANCELED:
     case EXITING:
       break;
     default:
       NOTREACHED();
       break;
   }
 }
 
 void ExitWarningHandler::Timer1Action() {
-  if (state_ == WAIT_FOR_QUICK_RELEASE)
+  if (state_ == WAIT_FOR_DOUBLE_PRESS)
     state_ = WAIT_FOR_LONG_HOLD;
-  else if (state_ == WAIT_FOR_DOUBLE_PRESS)
-    state_ = CANCELED;
 }
 
 void ExitWarningHandler::Timer2Action() {
+  Hide();

[sky, 2013/05/17 14:53:16]

Doesn't this mean you're effectively waiting longer than needed? Seems better to
make Hide() as soon as one of the keys that make up the accelerator is released.

[sschmitz, 2013/05/17 15:53:31]

No. This is done by design. PM did not want a short flash of the popup so the
user can't read it. The popup is meant to stay up for the duration (unless an
expert user performs a quick double press)

   if (state_ == CANCELED) {
     state_ = IDLE;
-    Hide();
-  }
-  else if (state_ == WAIT_FOR_LONG_HOLD) {
-    state_ = EXITING;
-    Hide();
-    Shell::GetInstance()->delegate()->Exit();
+  } else if (state_ == WAIT_FOR_LONG_HOLD) {
+    if (accelerator_ == context_->current_accelerator()) {
+      // We detect if the user has released any one of the keys that
+      // make up the shortcut by comparing "our" accelerator to the one
+      // from the current context and do not exit in that case.
+      state_ = EXITING;
+      Shell::GetInstance()->delegate()->Exit();
+    }
+    else {
+      state_ = IDLE;
+    }
   }
 }
 
 void ExitWarningHandler::StartTimers() {
   if (stub_timers_for_test_)
     return;
   timer1_.Start(FROM_HERE,
                 base::TimeDelta::FromMilliseconds(
                     kDoublePressTimeOutMilliseconds),
                 this,
@@ skipping 40 matching lines @@
 }
 
 void ExitWarningHandler::Hide() {
   if (!widget_)
     return;
   widget_->Close();
   widget_ = NULL;
 }
 
 }  // namespace ash