cros: Toggle shortcut view by the same shortcut

This cl adds the support to toggle the shortcut view by the same
shortcut: Ctrl + Alt + /:
1. If the KSH window is active, close it.
2. Otherwise, open it (if it's not open already) or activate it (if it's
   open but not active).

Bug: 819815
Test: New unit test KeyboardShortcutViewTest.ToggleWindow
Change-Id: Icbc9fba5644208750d1632b7473c66f53b923a80
Reviewed-on: https://chromium-review.googlesource.com/1073101Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562018}

ash/components/shortcut_viewer/shortcut_viewer_application.cc

@@ -30,7 +30,7 @@ void ShortcutViewerApplication::OnStart() {
   DCHECK(ui::InputDeviceManager::HasInstance());
   if (ui::InputDeviceManager::GetInstance()->AreDeviceListsComplete()) {
     // TODO(crbug.com/841020): Place the new app window on the correct display.
-    KeyboardShortcutView::Show(nullptr);
+    KeyboardShortcutView::Toggle(nullptr);
   } else {
     ui::InputDeviceManager::GetInstance()->AddObserver(this);
   }
@@ -39,7 +39,7 @@ void ShortcutViewerApplication::OnStart() {
 void ShortcutViewerApplication::OnDeviceListsComplete() {
   ui::InputDeviceManager::GetInstance()->RemoveObserver(this);
   // TODO(crbug.com/841020): Place the new app window on the correct display.
-  KeyboardShortcutView::Show(nullptr);
+  KeyboardShortcutView::Toggle(nullptr);
 }
 
 }  // namespace keyboard_shortcut_viewer

ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc

@@ -97,11 +97,12 @@ KeyboardShortcutView::~KeyboardShortcutView() {
 }
 
 // static
-views::Widget* KeyboardShortcutView::Show(gfx::NativeWindow context) {
+views::Widget* KeyboardShortcutView::Toggle(gfx::NativeWindow context) {
   if (g_ksv_view) {
-    // If there is a KeyboardShortcutView window open already, just activate
-    // it.
-    g_ksv_view->GetWidget()->Activate();
+    if (g_ksv_view->GetWidget()->IsActive())
+      g_ksv_view->GetWidget()->Close();
+    else
+      g_ksv_view->GetWidget()->Activate();
   } else {
     base::RecordAction(
         base::UserMetricsAction("KeyboardShortcutViewer.CreateWindow"));

ash/components/shortcut_viewer/views/keyboard_shortcut_view.h

@@ -31,8 +31,11 @@ class KeyboardShortcutView : public views::WidgetDelegateView,
  public:
   ~KeyboardShortcutView() override;
 
-  // Shows the Keyboard Shortcut Viewer window, or re-activates an existing one.
-  static views::Widget* Show(gfx::NativeWindow context);
+  // Toggle the Keyboard Shortcut Viewer window.
+  // 1. Show the window if it is not open.
+  // 2. Activate the window if it is open but not active.
+  // 3. Close the window if it is open and active.
+  static views::Widget* Toggle(gfx::NativeWindow context);
 
   // views::View:
   bool AcceleratorPressed(const ui::Accelerator& accelerator) override;

ash/components/shortcut_viewer/views/keyboard_shortcut_view_unittest.cc

@@ -74,8 +74,8 @@ class KeyboardShortcutViewTest : public ash::AshTestBase {
 
 // Shows and closes the widget for KeyboardShortcutViewer.
 TEST_F(KeyboardShortcutViewTest, ShowAndClose) {
-  // Showing the widget.
-  views::Widget* widget = KeyboardShortcutView::Show(CurrentContext());
+  // Show the widget.
+  views::Widget* widget = KeyboardShortcutView::Toggle(CurrentContext());
   EXPECT_TRUE(widget);
 
   // Cleaning up.
@@ -84,8 +84,8 @@ TEST_F(KeyboardShortcutViewTest, ShowAndClose) {
 
 // KeyboardShortcutViewer window should be centered in screen.
 TEST_F(KeyboardShortcutViewTest, CenterWindowInScreen) {
-  // Showing the widget.
-  views::Widget* widget = KeyboardShortcutView::Show(CurrentContext());
+  // Show the widget.
+  views::Widget* widget = KeyboardShortcutView::Toggle(CurrentContext());
   EXPECT_TRUE(widget);
 
   gfx::Rect root_window_bounds =
@@ -105,8 +105,8 @@ TEST_F(KeyboardShortcutViewTest, CenterWindowInScreen) {
 
 // Test that the number of side tabs equals to the number of categories.
 TEST_F(KeyboardShortcutViewTest, SideTabsCount) {
-  // Showing the widget.
-  views::Widget* widget = KeyboardShortcutView::Show(CurrentContext());
+  // Show the widget.
+  views::Widget* widget = KeyboardShortcutView::Toggle(CurrentContext());
 
   int category_number = 0;
   ShortcutCategory current_category = ShortcutCategory::kUnknown;
@@ -126,8 +126,8 @@ TEST_F(KeyboardShortcutViewTest, SideTabsCount) {
 
 // Test that the top line in two views should be center aligned.
 TEST_F(KeyboardShortcutViewTest, TopLineCenterAlignedInItemView) {
-  // Showing the widget.
-  views::Widget* widget = KeyboardShortcutView::Show(CurrentContext());
+  // Show the widget.
+  views::Widget* widget = KeyboardShortcutView::Toggle(CurrentContext());
 
   for (const auto& item_view : GetShortcutViews()) {
     DCHECK(item_view->child_count() == 2);
@@ -151,8 +151,8 @@ TEST_F(KeyboardShortcutViewTest, TopLineCenterAlignedInItemView) {
 
 // Test that the focus is on search box when window inits and exits search mode.
 TEST_F(KeyboardShortcutViewTest, FocusOnSearchBox) {
-  // Showing the widget.
-  views::Widget* widget = KeyboardShortcutView::Show(CurrentContext());
+  // Show the widget.
+  views::Widget* widget = KeyboardShortcutView::Toggle(CurrentContext());
 
   // Case 1: when window creates. The focus should be on search box.
   EXPECT_TRUE(GetSearchBoxView()->search_box()->HasFocus());
@@ -188,8 +188,8 @@ TEST_F(KeyboardShortcutViewTest, FocusOnSearchBox) {
 
 // Test that the window can be closed by accelerator.
 TEST_F(KeyboardShortcutViewTest, CloseWindowByAccelerator) {
-  // Showing the widget.
-  views::Widget* widget = KeyboardShortcutView::Show(CurrentContext());
+  // Show the widget.
+  views::Widget* widget = KeyboardShortcutView::Toggle(CurrentContext());
   EXPECT_FALSE(widget->IsClosed());
 
   ui::test::EventGenerator& event_generator = GetEventGenerator();
@@ -197,4 +197,22 @@ TEST_F(KeyboardShortcutViewTest, CloseWindowByAccelerator) {
   EXPECT_TRUE(widget->IsClosed());
 }
 
+// Test that the window can be activated or closed by toggling.
+TEST_F(KeyboardShortcutViewTest, ToggleWindow) {
+  // Show the widget.
+  views::Widget* widget = KeyboardShortcutView::Toggle(CurrentContext());
+  EXPECT_FALSE(widget->IsClosed());
+
+  // Call |Toggle()| to activate the inactive widget.
+  EXPECT_TRUE(widget->IsActive());
+  widget->Deactivate();
+  EXPECT_FALSE(widget->IsActive());
+  KeyboardShortcutView::Toggle(CurrentContext());
+  EXPECT_TRUE(widget->IsActive());
+
+  // Call |Toggle()| to close the active widget.
+  KeyboardShortcutView::Toggle(CurrentContext());
+  EXPECT_TRUE(widget->IsClosed());
+}
+
 }  // namespace keyboard_shortcut_viewer

chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.cc

@@ -22,7 +22,7 @@ void ShowKeyboardShortcutViewer() {
     connector->StartService(shortcut_viewer::mojom::kServiceName);
   } else {
     // TODO(https://crbug.com/833673): Remove the dependency on aura::Window.
-    keyboard_shortcut_viewer::KeyboardShortcutView::Show(
+    keyboard_shortcut_viewer::KeyboardShortcutView::Toggle(
         ash::Shell::HasInstance() ? ash::Shell::GetRootWindowForNewWindows()
                                   : nullptr);
   }