"Ctrl + Alt + /" will do the following:
  // 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.

Can we just use AddAccelerator to add the accelerator, similar to add Ctrl + W [1]?

AddAccelerator(ui::Accelerator(ui::VKEY_OEM_2, ui::EF_CONTROL_DOWN | ui::EF_CONTROL_ALT)?

[1] https://cs.chromium.org/chromium/src/ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc?l=146&rcl=9a374af14a9813c83f766d6d8ab2215e879bcca5

control-alt-/ needs to be a global accelerator. The code you mention in [1] is only used if the ksv has focus. So, ash should deal with control-alt-/.

Right! Thanks!

wutao, any chance you would be interested in owning this bug?

I would like to own this. This will be a follow up cl discussed with msw@ in  https://crrev.com/c/1073101

I am new to mus/WindowService, is there any guidance or examples to do the proposal by sky@?

Looking current code, we already have StartService(kServiceName).
Seems we need functions:
1. FindService(kServiceName).
2. ActiveService/Window(kServiceName).
3. IsActiveService/Window(kServiceName).
4. CloseService(kServiceName).

No, we'll need the app to implement a mojo interface, and then Ash will use that interface to issue commands to activate and close.
Scott suggested using mojom::WindowTree::StackAtTop() to activate/bring-to-front, so the app will need some way to invoke that.
I'm not sure if views::Widget::StackAtTop() would work, it may just ask the client-side aura::Window / NativeWidgetAura parent for stacking changes.
Give that a shot, let me know if you get stuck.

More concretely:
. Create a file ash/components/shortcut_viewer/public/mojom/shortcut_viewer.mojom
. In shortcut_viewer.mojom define a ShortcutViewer interface that has Show/Hide.
. Make ShortcutViewerApplication implement mojom::ShortcutViewer.
. ShortcutViewerApplication will need to register mojom::ShortcutViewer. See AutoclickApplication's OnStart and OnBindInterface. You'll need that logic, but for ShortcutViewer.
. Update ash/components/shortcut_viewer/manifest.json to say it provides mojom::ShortcutViewer.
. Update ash's manifest.json to say it requires mojom::ShortcutViewer.
. When ask connects to ksv it'll now request the specific interface and call the appropriate function.

Calling Widget::StackAtTop() should hopefully just work (I'll wire up the necessary WindowService support for this).

Thanks msw@ and sky@.

From the auto_click example,
In some controller in Shell (this case is AccessibilityController) to bind the interface by kServiceName ("autoclick_app") to get the mojo interface ptr and then call the functions on the interface ptr [1].

Therefore in the KSV case, the call path is:
In keyboard_shortcut_viewer_util::ShowKeyboardShortcutViewer => get the connector =>
1. (already done by msw@) if "shortcut_viewer_app" is not started, call connector->StartService("shortcut_viewer_app") and handle in ShortcutViewerApplication::OnStart().

2. if "shortcut_viewer_app" is started, get mojom::ShortcutViewerPtr and call Show/Hide. In Show/Hide, we will get KSV widget and call StackAtTop().

How to know if a kServiceName has already been Started? Thanks!

[1] https://cs.chromium.org/chromium/src/ash/accessibility/accessibility_controller.cc?l=755&rcl=9ad9c75bf5a90fa025c066c8d11274d5e6eaa1ac

"How to know if a kServiceName has already been Started" I'm not sure what you mean. Why does it matter? Do you need to know that to decide whether to call Show or Hide? I think ash needs to make the decision as to whether to call show or hide. Ash could do that by looking at the active window. If the active window is the KSV, call hide, otherwise show. There are different ways to go about identifying the ksv window, probably easiest is to use a constant for the name (defined in a mojom).

Do we need to decide whether to call StartService or Show or Hide. As you mentioned Show or Hide can be decided by Ash. How about StartService? Do we also let Ash to find out if a KSV window exist, if not then call StartService?

Is there any race condition for StartService, e.g. we press "Ctrl + Alt + /" first time to call connector->StartService("shortcut_viewer_app"). While the KSV window has not been inited, we press the accelerator second time. This time Ash does not see any KSV window yet, then we call connector->StartService("shortcut_viewer_app") again. Will this be a problem.

Only one instance of the service is created. So, there shouldn't be any race conditions.

I guess we do not have an API to query connector->IsStarted(kServiceName).
So we can ask Ash to find out if a KSV window exist by iterating all windows?
I will come up a cl soon.

I'm not sure I entirely understand the overall suggestion, Scott, hopefully you can clarify.
IIUC, the KSV mojo interface should have one function to signal that the shortcut was pressed.
Chrome/Ash will connect and call the function anytime the shortcut is pressed. The function impl in the KSV app will:
1) Create and show the window and bring it to front if none exists.
2) Query Ash somehow about the active window (via Widget::IsActive or wm::IsActiveWindow or some Window Service function?) and:
3) Bring the widget/window to front (via Widget::StackAtTop() or wm::ActivateWindow() or some window Service function?) if it's not active.
4) Close the widget/window if it's not active.

I'm suggesting the following:
1. Create ash/components/shortcut_viewer/public/mojom/shortcut_viewer.mojom with a single interface ShortcutViewer. ShortcutViewer has two functions: Show() and Hide().
2. shortcut_viewer.mojom also has a constant named kWindowName.
3. When keyboard_shortcut_view.cc creates the widget, it sets the name of the widget to mojom::kWindowName.
4. In ash, when the accelerator is pressed it connects to shortcut_viewer asking for the ShortcutViewer interface.
5. Ash looks at the active window, if it's name is shortcut_viewer::mojom::kWindowName it calls shortcut_viewer->Hide(), otherwise it calls shortcut_viewer->Show().

Does that make sense?

Ah yeah, that does make sense to me, thanks for clarifying.

I have a prototype to do 1) and 3) in #14. Thanks msw@ helped offline.

But due to  bug 848883 , I cannot test 4) now.

A side point for 2 and 3) in #14: seems "Widget::IsActive" and "Widget::Activate" just work.

 bug 848883  should be fixed. Let me know if you see otherwise.

Thanks sky@. I uploaded a cl to crrev.com/c/1087838

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb024f15dc64220cf50dc7ceb362ce273697fc12

commit bb024f15dc64220cf50dc7ceb362ce273697fc12
Author: James Cook <jamescook@chromium.org>
Date: Wed Jun 06 17:42:07 2018

chromeos: Add UMA metrics for keyboard shortcut viewer startup time

This will measure the time from Ctrl-Shift-/ keystroke to dialog show.
This CL works with the non-mojo-app shortcut viewer we're using today.
When we switch to the app version we'll need to pass the keystroke
time over mojo to the app, perhaps via a Toggle() method. Adding the
mojo Toggle() interface is tracked in  crbug.com/847615 

Metric is Keyboard.ShortcutViewer.StartupTime

Bug:  847615 ,  847613 
Test: added to ash_unittests, manually checked chrome://histograms
Change-Id: I425f44d0b71543f225d73857365d050be0e1b446
Reviewed-on: https://chromium-review.googlesource.com/1087779
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564952}
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/ash/components/shortcut_viewer/BUILD.gn
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/ash/components/shortcut_viewer/shortcut_viewer_application.cc
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/ash/components/shortcut_viewer/shortcut_viewer_application.h
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/ash/components/shortcut_viewer/views/keyboard_shortcut_view.cc
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/ash/components/shortcut_viewer/views/keyboard_shortcut_view.h
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/ash/components/shortcut_viewer/views/keyboard_shortcut_view_unittest.cc
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.cc
[modify] https://crrev.com/bb024f15dc64220cf50dc7ceb362ce273697fc12/tools/metrics/histograms/histograms.xml

sky@, there is still some bug to send the accelerator to close KSV with patch in #20:

1. Open KSV by "ctrl+alt+/".
2. Press "Tab" key to place the focus from the search box to side menu.
3. Press "Tab" key again to place the focus on the search box again.
4. Press "ctrl+alt+/" will NOT close the app.
5. Put KSV window behind a browser window.
6. Press "ctrl+alt+/" will active the KSV window.
7. Press "ctrl+alt+/" will NOT close the app.

I will take a look shortly.

Fix for case mentioned in comment #22 is out for review here: https://chromium-review.googlesource.com/c/chromium/src/+/1089885 .

Fix fir comment #22 landed. Let me know if you see any other bugs.

Thanks sky@. #22 problem has been fixed.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/357bb3540260175aaea422588ec3c356ae56b90b

commit 357bb3540260175aaea422588ec3c356ae56b90b
Author: wutao <wutao@chromium.org>
Date: Sat Jun 09 04:51:39 2018

cros: Handle accelerator in Shortcut Viewer app

This cl makes accelerator for Keyboard Shortcut Viewer work when running
as a standalone app.

Bug:  847615 
Test: manual.
Change-Id: I55d161896b8268532dbc884cd9d6573fafb66b44
Reviewed-on: https://chromium-review.googlesource.com/1087838
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565861}
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/components/shortcut_viewer/BUILD.gn
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/components/shortcut_viewer/manifest.json
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/components/shortcut_viewer/public/mojom/BUILD.gn
[delete] https://crrev.com/0f4c5db938fc1739e3bb218dd69a43ebb3768250/ash/components/shortcut_viewer/public/mojom/constants.mojom
[add] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/components/shortcut_viewer/public/mojom/shortcut_viewer.mojom
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/components/shortcut_viewer/shortcut_viewer_application.cc
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/components/shortcut_viewer/shortcut_viewer_application.h
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/shell/content/client/shell_content_browser_client.cc
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/ash/shell/content/client/shell_main_delegate.cc
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/chrome/browser/ash_service_registry.cc
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_util.cc
[modify] https://crrev.com/357bb3540260175aaea422588ec3c356ae56b90b/chrome/utility/mash_service_factory.cc