Bug scrub: Is this relevant with views-based login? Is it used in OOBE?

I believe post-70 this will still be used for OOBE.

Yes, I believe we will need to fix at least some of this. I am waiting for us to deprecate the non Views login WebUI so we can focus on what we definitely need to fix.

WebUILoginView's usage of src/ash looks mostly for customizing the focus cycle and its handling; maybe we should move those focus customization to Ash entirely.

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

commit d96b4d7452ef2d065851d8a423ab0fc3c8a4196d
Author: Jun Mukai <mukai@chromium.org>
Date: Fri Nov 09 00:37:23 2018

Remove ash dependency on WebUILoginView::MoveFocusToSystemTray

LoginScreen::FocusLoginShelf() mojo API can be reused.

BUG= 854346 
TEST=trybot

Change-Id: Ide7dad98e1da57826d37d5dff4c3e269aee07c14
Reviewed-on: https://chromium-review.googlesource.com/c/1325689
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606662}
[modify] https://crrev.com/d96b4d7452ef2d065851d8a423ab0fc3c8a4196d/ash/login/login_screen_controller.cc
[modify] https://crrev.com/d96b4d7452ef2d065851d8a423ab0fc3c8a4196d/chrome/browser/chromeos/login/ui/DEPS
[modify] https://crrev.com/d96b4d7452ef2d065851d8a423ab0fc3c8a4196d/chrome/browser/chromeos/login/ui/login_display_host_common.cc
[modify] https://crrev.com/d96b4d7452ef2d065851d8a423ab0fc3c8a4196d/chrome/browser/chromeos/login/ui/webui_login_view.cc

note: I chatted with jdufault@ about the status of WebUILoginView, and learnt that it will continue to exist for a while (maybe at least a few quarters) for the OOBE screen. So this needs to be done at least.

There are two references of ash code in this class:
- ash/accelerators/accelerator_controller: registering OOBE-specific system accelerators.
- SystemTrayFocusObserver: getting notifications of system-tray focus changes

The latter would need to be done through mojo APIs, possibly on LoginScreen. Not sure about the accelerators -- maybe we want to move the accelerators to ash/login first, and then allows registering them through mojo APIs.

In all likelihood WebUILoginView/LoginDisplayHostWebUI will be around for years more unless we greatly rework how OOBE is shown.

Re accelerators, ash/login is not used for OOBE.

Here[1] are my plans for cleaning up OOBE so you can have some insight for what kind of cleanups will happen in the foreseeable future.

1: https://docs.google.com/document/d/1J4Mlt3mI8IBZWdaOqxmMxs_eQ4eSibJj8NyBmoaXX20/edit

Thanks for sharing the document.

Re accelerators:

Our interest is to remove the source code references to ash from chrome/browser files. In other words, I want to remove the line of #include "ash/accelerators/accelerator_controller.h" from chrome/browser/chromeos/login/ui/webui_login_view.cc.

Since WebUILoginView uses accelerator_controller for registering global accelerators for OOBE (like enrollment, kiosk, debugging), we can't simply remove this. I'm currently thinking to move this part of code from the webui_login_view.cc to a new file within ash/login, and then create a mojo API to the access to the new file, so that we can remove the include line but keep the same functionality.

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

commit 103f57b124f5f687241d6108b9512ce3171ccc3d
Author: Jun Mukai <mukai@chromium.org>
Date: Thu Nov 15 01:03:01 2018

Fix typo in LoginScreenController::FocusLoginShelf

My previous CL (crrev.com/606662) made a few typos in this function.

Bug:  854346 
Test: manually
Change-Id: If9f1c94a28e3f88cbff58ae5b1549ed09aa3c2e6
Reviewed-on: https://chromium-review.googlesource.com/c/1336469
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608203}
[modify] https://crrev.com/103f57b124f5f687241d6108b9512ce3171ccc3d/ash/login/login_screen_controller.cc

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

commit 5c7b5b419bbebff6d8fa35e191716464e083fe7a
Author: Jun Mukai <mukai@chromium.org>
Date: Fri Nov 30 00:08:50 2018

Remove ash/system references from WebUILoginView

Bug:  854346 
Test: the new test case
Change-Id: I5a78d4037ffcf2daf995c2fc04ef44459abbd21c
Reviewed-on: https://chromium-review.googlesource.com/c/1338187
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612451}
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/BUILD.gn
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/login/login_screen_controller.cc
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/login/login_screen_controller.h
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/login/login_screen_controller_unittest.cc
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/login/mock_login_screen_client.h
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/login/ui/lock_contents_view.h
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/public/cpp/BUILD.gn
[add] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/public/cpp/system_tray_focus_observer.h
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/public/interfaces/login_screen.mojom
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/shell.cc
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/system/status_area_widget_unittest.cc
[delete] https://crrev.com/2cbdaffe8c43ead16028b3d425ef92ef0aeb977b/ash/system/system_tray_focus_observer.h
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/ash/system/tray/system_tray_notifier.cc
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/chrome/browser/chromeos/login/ui/DEPS
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/chrome/browser/chromeos/login/ui/webui_login_view.cc
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/chrome/browser/chromeos/login/ui/webui_login_view.h
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/chrome/browser/ui/ash/login_screen_client.cc
[modify] https://crrev.com/5c7b5b419bbebff6d8fa35e191716464e083fe7a/chrome/browser/ui/ash/login_screen_client.h

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

commit f82a62ac5051a86d85b0bb20b149c6ab171838da
Author: Jun Mukai <mukai@chromium.org>
Date: Fri Dec 14 18:21:45 2018

Remove the reference of ash from webui_login_view.cc

This file refers ash::Shell to register global accelerators,
but I believe this is not necessary. This CL removes that code
entirely.

Bug:  854346 
Test: none
Change-Id: Ibd72da382d28e136daf571a7292fad7e875facfe
Reviewed-on: https://chromium-review.googlesource.com/c/1375876
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616750}
[modify] https://crrev.com/f82a62ac5051a86d85b0bb20b149c6ab171838da/chrome/browser/chromeos/login/ui/DEPS
[modify] https://crrev.com/f82a62ac5051a86d85b0bb20b149c6ab171838da/chrome/browser/chromeos/login/ui/webui_login_view.cc