ESC accelerator and consistent overview mode for Athena homecard

athena/wm/window_manager_impl.cc

 // Copyright 2014 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 "athena/wm/window_manager_impl.h"
 
 #include <algorithm>
 
 #include "athena/screen/public/screen_manager.h"
 #include "athena/util/container_priorities.h"
@@ skipping 185 matching lines @@
 
 bool WindowManagerImpl::IsOverviewModeActive() {
   return overview_;
 }
 
 void WindowManagerImpl::SetInOverview(bool active) {
   bool in_overview = !!overview_;
   if (active == in_overview)
     return;
 
+  const AcceleratorData esc_accelerator_data = {
+      TRIGGER_ON_PRESS, ui::VKEY_ESCAPE, ui::EF_NONE, CMD_EXIT_OVERVIEW,
+      AF_NONE};
+
   bezel_controller_->set_left_right_delegate(
       active ? NULL : split_view_controller_.get());
   if (active) {
     FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnOverviewModeEnter());
 
     // Note: The window_list_provider_ resembles the exact window list of the
     // container, so no re-stacking is required before showing the OverviewMode.
     overview_ = WindowOverviewMode::Create(
         container_.get(), window_list_provider_.get(),
         split_view_controller_.get(), this);
+    AcceleratorManager::Get()->RegisterAccelerator(esc_accelerator_data, this);
   } else {
     overview_.reset();
     FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnOverviewModeExit());
+    AcceleratorManager::Get()->UnregisterAccelerator(esc_accelerator_data,
+                                                     this);
   }
 }
 
 void WindowManagerImpl::InstallAccelerators() {
   const AcceleratorData accelerator_data[] = {
       {TRIGGER_ON_PRESS, ui::VKEY_F6, ui::EF_NONE, CMD_TOGGLE_OVERVIEW,
        AF_NONE},
       {TRIGGER_ON_PRESS, ui::VKEY_F6, ui::EF_CONTROL_DOWN,
        CMD_TOGGLE_SPLIT_VIEW, AF_NONE},
   };
@@ skipping 61 matching lines @@
   FOR_EACH_OBSERVER(WindowManagerObserver, observers_, OnSplitViewModeEnter());
   split_view_controller_->ActivateSplitMode(left, right);
   wm::ActivateWindow(to_activate);
 }
 
 void WindowManagerImpl::OnWindowDestroying(aura::Window* window) {
   if (window == container_)
     container_.reset();
 }
 
 bool WindowManagerImpl::IsCommandEnabled(int command_id) const {

[Jun Mukai, 2014/09/25 21:53:41]

Isn't it good enough to make this returning false when it's not in overview mode
rather than registering/unregistering the accelerator?

[Jun Mukai, 2014/09/25 22:37:45]

FYI: if you want to keep register/unregister structure, I'd suggest to change
WindowOverviewMode as an accelerator handler, and it registers in ctor and
unregisters in dtor.  That looks more natural to me.

# also, it could be great to use a WeakPtr for AcceleratorHandler in
AcceleratorManager so that manager recognizes the deletion of a handler and
automatically unregisters the accelerators on destruction.

[Greg Levin, 2014/10/02 21:56:09]

Based on our conversation, I'm leaving this alone.  I agree that having
WindowOverviewMode (un)register it is more natural in one way.  But since
WindowManagerImpl is already an AcceleratorHandler, and is already handling one
accelerator to (de)activate overview mode, letting it handle the other
accelerator to deactivate overview mode also seems natural.  Let me know if you
decide you'd like this changed after all.
Not implemented, based on our conversation.

[Greg Levin, 2014/10/02 21:56:09]

Leaving as is based on our conversation with Oshima

   return true;
 }
 
 bool WindowManagerImpl::OnAcceleratorFired(int command_id,
                                            const ui::Accelerator& accelerator) {
   switch (command_id) {
+    case CMD_EXIT_OVERVIEW:
+      if (!IsOverviewModeActive())
+         break; // else fall through & Toggle
     case CMD_TOGGLE_OVERVIEW:
       ToggleOverview();
       break;
     case CMD_TOGGLE_SPLIT_VIEW:
       ToggleSplitview();
       break;
   }
   return true;
 }
 
@@ skipping 108 matching lines @@
   DCHECK(!instance);
 }
 
 // static
 WindowManager* WindowManager::GetInstance() {
   DCHECK(instance);
   return instance;
 }
 
 }  // namespace athena