Elim ash::KeyboardUI

I discovered this class while looking at keyboard::KeyboardUI.
It has the comment:

// KeyboardUI wraps the appropriate keyboard ui depending upon whether
// ash is running in mus or non-mus.

KeyboardUIMash is unused, and there is no longer mus/mash
differentiation, so the class seems unnecessary.

It is also super confusing since there is also keyboard::KeyboardUI
which is effectively unrelated. It's functionality can easily be
directly integrated with the callers.

Bug: 843332
Change-Id: Ib082a0f6f9bede2cef766c006e003dbe512be561
Reviewed-on: https://chromium-review.googlesource.com/1180255
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584507}

ash/BUILD.gn

@@ -187,9 +187,6 @@ component("ash") {
     "ime/ime_controller.h",
     "ime/ime_focus_handler.h",
     "ime/ime_switch_type.h",
-    "keyboard/keyboard_ui.h",
-    "keyboard/keyboard_ui_mash.h",
-    "keyboard/keyboard_ui_observer.h",
     "keyboard/virtual_keyboard_container_layout_manager.h",
     "keyboard/virtual_keyboard_controller.h",
     "laser/laser_pointer_controller.h",
@@ -845,8 +842,6 @@ component("ash") {
     "ime/ime_mode_indicator_view.h",
     "ime/mode_indicator_observer.cc",
     "ime/mode_indicator_observer.h",
-    "keyboard/keyboard_ui.cc",
-    "keyboard/keyboard_ui_mash.cc",
     "keyboard/virtual_keyboard_container_layout_manager.cc",
     "keyboard/virtual_keyboard_controller.cc",
     "laser/laser_pointer_controller.cc",

ash/keyboard/keyboard_ui.cc

-// Copyright 2016 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/keyboard/keyboard_ui.h"
-
-#include <memory>
-
-#include "ash/accessibility/accessibility_controller.h"
-#include "ash/accessibility/accessibility_observer.h"
-#include "ash/keyboard/keyboard_ui_observer.h"
-#include "ash/shell.h"
-#include "ui/keyboard/keyboard_controller.h"
-
-namespace ash {
-
-class KeyboardUIImpl : public KeyboardUI, public AccessibilityObserver {
- public:
-  KeyboardUIImpl() : enabled_(false) {
-    Shell::Get()->accessibility_controller()->AddObserver(this);
-  }
-
-  ~KeyboardUIImpl() override {
-    if (Shell::HasInstance() && Shell::Get()->accessibility_controller())
-      Shell::Get()->accessibility_controller()->RemoveObserver(this);
-  }
-
-  void ShowInDisplay(const display::Display& display) override {
-    auto* controller = keyboard::KeyboardController::Get();
-    // Keyboard may not always be enabled. https://crbug.com/749989
-    if (!controller->enabled())
-      return;
-    controller->ShowKeyboardInDisplay(display);
-  }
-  void Hide() override {
-    // Do nothing as this is called from ash::Shell, which also calls through
-    // to the appropriate keyboard functions.
-  }
-  bool IsEnabled() override {
-    return Shell::Get()->accessibility_controller()->IsVirtualKeyboardEnabled();
-  }
-
-  // AccessibilityObserver:
-  void OnAccessibilityStatusChanged() override {
-    bool enabled = IsEnabled();
-    if (enabled_ == enabled)
-      return;
-
-    enabled_ = enabled;
-    for (auto& observer : *observers())
-      observer.OnKeyboardEnabledStateChanged(enabled);
-  }
-
- private:
-  bool enabled_;
-
-  DISALLOW_COPY_AND_ASSIGN(KeyboardUIImpl);
-};
-
-KeyboardUI::~KeyboardUI() = default;
-
-// static
-std::unique_ptr<KeyboardUI> KeyboardUI::Create() {
-  return std::make_unique<KeyboardUIImpl>();
-}
-
-void KeyboardUI::AddObserver(KeyboardUIObserver* observer) {
-  observers_.AddObserver(observer);
-}
-
-void KeyboardUI::RemoveObserver(KeyboardUIObserver* observer) {
-  observers_.RemoveObserver(observer);
-}
-
-KeyboardUI::KeyboardUI() = default;
-
-}  // namespace ash

ash/keyboard/keyboard_ui.h

-// Copyright 2016 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.
-
-#ifndef ASH_KEYBOARD_KEYBOARD_UI_H_
-#define ASH_KEYBOARD_KEYBOARD_UI_H_
-
-#include <memory>
-
-#include "ash/ash_export.h"
-#include "base/macros.h"
-#include "base/observer_list.h"
-
-namespace display {
-class Display;
-}
-
-namespace ash {
-
-class KeyboardUIObserver;
-
-// KeyboardUI wraps the appropriate keyboard ui depending upon whether ash is
-// running in mus or non-mus.
-class ASH_EXPORT KeyboardUI {
- public:
-  virtual ~KeyboardUI();
-
-  static std::unique_ptr<KeyboardUI> Create();
-
-  virtual void ShowInDisplay(const display::Display& display) = 0;
-  virtual void Hide() = 0;
-
-  // Returns true if the keyboard is enabled.
-  virtual bool IsEnabled() = 0;
-
-  void AddObserver(KeyboardUIObserver* observer);
-  void RemoveObserver(KeyboardUIObserver* observer);
-
-  // Applist also queries this for bounds. If app list remains in ash then
-  // we need to plumb bounds through here too.
-
- protected:
-  KeyboardUI();
-
-  base::ObserverList<KeyboardUIObserver>::Unchecked* observers() {
-    return &observers_;
-  }
-
- private:
-  base::ObserverList<KeyboardUIObserver>::Unchecked observers_;
-};
-
-}  // namespace ash
-
-#endif  // ASH_KEYBOARD_KEYBOARD_UI_H_

ash/keyboard/keyboard_ui_mash.cc

-// Copyright 2016 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/keyboard/keyboard_ui_mash.h"
-
-#include <memory>
-
-#include "ash/keyboard/keyboard_ui_observer.h"
-#include "services/service_manager/public/cpp/connector.h"
-
-namespace ash {
-
-KeyboardUIMash::KeyboardUIMash(service_manager::Connector* connector)
-    : is_enabled_(false), observer_binding_(this) {
-  // TODO: chrome should register the keyboard interface with ash.
-  // http://crbug.com/683289.
-}
-
-KeyboardUIMash::~KeyboardUIMash() = default;
-
-// static
-std::unique_ptr<KeyboardUI> KeyboardUIMash::Create(
-    service_manager::Connector* connector) {
-  return std::make_unique<KeyboardUIMash>(connector);
-}
-
-void KeyboardUIMash::Hide() {
-  if (keyboard_)
-    keyboard_->Hide();
-}
-
-void KeyboardUIMash::ShowInDisplay(const display::Display& display) {
-  // TODO(yhanada): Send display id after adding a display_id argument to
-  // |Keyboard::Show()| in keyboard.mojom. See crbug.com/585253.
-  if (keyboard_)
-    keyboard_->Show();
-}
-
-bool KeyboardUIMash::IsEnabled() {
-  return is_enabled_;
-}
-
-void KeyboardUIMash::OnKeyboardStateChanged(bool is_enabled,
-                                            bool is_visible,
-                                            uint64_t display_id,
-                                            const gfx::Rect& bounds) {
-  if (is_enabled_ == is_enabled)
-    return;
-
-  is_enabled_ = is_enabled;
-  for (auto& observer : *observers())
-    observer.OnKeyboardEnabledStateChanged(is_enabled);
-}
-
-}  // namespace ash

ash/keyboard/keyboard_ui_mash.h

-// Copyright 2016 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.
-
-#ifndef ASH_KEYBOARD_KEYBOARD_UI_MASH_H_
-#define ASH_KEYBOARD_KEYBOARD_UI_MASH_H_
-
-#include <stdint.h>
-
-#include <memory>
-
-#include "ash/keyboard/keyboard_ui.h"
-#include "base/macros.h"
-#include "mojo/public/cpp/bindings/binding.h"
-#include "ui/keyboard/keyboard.mojom.h"
-
-namespace service_manager {
-class Connector;
-}
-namespace display {
-class Display;
-}
-
-namespace ash {
-
-class KeyboardUIMash : public KeyboardUI,
-                       public keyboard::mojom::KeyboardObserver {
- public:
-  // |connector| may be null in tests.
-  explicit KeyboardUIMash(service_manager::Connector* connector);
-  ~KeyboardUIMash() override;
-
-  static std::unique_ptr<KeyboardUI> Create(
-      service_manager::Connector* connector);
-
-  // KeyboardUI:
-  void Hide() override;
-  void ShowInDisplay(const display::Display& display) override;
-  bool IsEnabled() override;
-
-  // keyboard::mojom::KeyboardObserver:
-  void OnKeyboardStateChanged(bool is_enabled,
-                              bool is_visible,
-                              uint64_t display_id,
-                              const gfx::Rect& bounds) override;
-
- private:
-  bool is_enabled_;
-
-  // May be null during tests.
-  keyboard::mojom::KeyboardPtr keyboard_;
-  mojo::Binding<keyboard::mojom::KeyboardObserver> observer_binding_;
-
-  DISALLOW_COPY_AND_ASSIGN(KeyboardUIMash);
-};
-
-}  // namespace ash
-
-#endif  // ASH_KEYBOARD_KEYBOARD_UI_MASH_H_

ash/keyboard/keyboard_ui_observer.h

-// Copyright 2016 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.
-
-#ifndef ASH_KEYBOARD_KEYBOARD_UI_OBSERVER_H_
-#define ASH_KEYBOARD_KEYBOARD_UI_OBSERVER_H_
-
-#include "ash/ash_export.h"
-#include "base/macros.h"
-
-namespace ash {
-
-class ASH_EXPORT KeyboardUIObserver {
- public:
-  virtual void OnKeyboardEnabledStateChanged(bool new_enabled) = 0;
-
- protected:
-  virtual ~KeyboardUIObserver() {}
-};
-
-}  // namespace ash
-
-#endif  // ASH_KEYBOARD_KEYBOARD_UI_OBSERVER_H_

ash/keyboard/virtual_keyboard_controller.cc

@@ -8,7 +8,6 @@
 
 #include "ash/accessibility/accessibility_controller.h"
 #include "ash/ime/ime_controller.h"
-#include "ash/keyboard/keyboard_ui.h"
 #include "ash/public/cpp/config.h"
 #include "ash/root_window_controller.h"
 #include "ash/session/session_controller.h"
@@ -51,7 +50,6 @@ void DisableVirtualKeyboard() {
 void MoveKeyboardToDisplayInternal(const display::Display& display) {
   // Remove the keyboard from curent root window controller
   TRACE_EVENT0("vk", "MoveKeyboardToDisplayInternal");
-  Shell::Get()->keyboard_ui()->Hide();
   RootWindowController::ForWindow(
       keyboard::KeyboardController::Get()->GetRootWindow())
       ->DeactivateKeyboard(keyboard::KeyboardController::Get());

ash/shell.cc

@@ -55,7 +55,6 @@
 #include "ash/host/ash_window_tree_host_init_params.h"
 #include "ash/ime/ime_controller.h"
 #include "ash/ime/ime_focus_handler.h"
-#include "ash/keyboard/keyboard_ui.h"
 #include "ash/keyboard/virtual_keyboard_controller.h"
 #include "ash/laser/laser_pointer_controller.h"
 #include "ash/login/login_screen_controller.h"
@@ -500,7 +499,6 @@ void Shell::EnableKeyboard() {
 }
 
 void Shell::DisableKeyboard() {
-  keyboard_ui_->Hide();
   if (keyboard_controller_->enabled()) {
     for (auto* const controller : GetAllRootWindowControllers())
       controller->DeactivateKeyboard(keyboard_controller_.get());
@@ -1246,8 +1244,6 @@ void Shell::Init(
       display_configurator_.get(), display_manager_.get(),
       std::make_unique<display::DefaultTouchTransformSetter>());
 
-  keyboard_ui_ = KeyboardUI::Create();
-
   // |system_tray_model_| should be available before
   // |system_notification_controller_| is initialized and Shelf is created by
   // WindowTreeHostManager::InitHosts.

ash/shell.h

@@ -133,7 +133,6 @@ class ImmersiveContextAsh;
 class ImmersiveHandlerFactoryAsh;
 class KeyAccessibilityEnabler;
 class KeyboardBrightnessControlDelegate;
-class KeyboardUI;
 class LaserPointerController;
 class LocaleNotificationController;
 class LockStateController;
@@ -423,7 +422,6 @@ class ASH_EXPORT Shell : public SessionObserver,
   KeyboardBrightnessControlDelegate* keyboard_brightness_control_delegate() {
     return keyboard_brightness_control_delegate_.get();
   }
-  KeyboardUI* keyboard_ui() { return keyboard_ui_.get(); }
   LaserPointerController* laser_pointer_controller() {
     return laser_pointer_controller_.get();
   }
@@ -753,7 +751,6 @@ class ASH_EXPORT Shell : public SessionObserver,
   std::unique_ptr<ImmersiveContextAsh> immersive_context_;
   std::unique_ptr<KeyboardBrightnessControlDelegate>
       keyboard_brightness_control_delegate_;
-  std::unique_ptr<KeyboardUI> keyboard_ui_;
   std::unique_ptr<keyboard::KeyboardController> keyboard_controller_;
   std::unique_ptr<LocaleNotificationController> locale_notification_controller_;
   std::unique_ptr<LoginScreenController> login_screen_controller_;

ash/system/virtual_keyboard/virtual_keyboard_tray.cc

@@ -6,7 +6,7 @@
 
 #include <algorithm>
 
-#include "ash/keyboard/keyboard_ui.h"
+#include "ash/accessibility/accessibility_controller.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/session/session_controller.h"
 #include "ash/shelf/shelf.h"
@@ -36,7 +36,7 @@ VirtualKeyboardTray::VirtualKeyboardTray(Shelf* shelf)
 
   // The Shell may not exist in some unit tests.
   if (Shell::HasInstance()) {
-    Shell::Get()->keyboard_ui()->AddObserver(this);
+    Shell::Get()->accessibility_controller()->AddObserver(this);
     Shell::Get()->AddShellObserver(this);
   }
   // Try observing keyboard controller, in case it is already constructed.
@@ -49,7 +49,7 @@ VirtualKeyboardTray::~VirtualKeyboardTray() {
   // The Shell may not exist in some unit tests.
   if (Shell::HasInstance()) {
     Shell::Get()->RemoveShellObserver(this);
-    Shell::Get()->keyboard_ui()->RemoveObserver(this);
+    Shell::Get()->accessibility_controller()->RemoveObserver(this);
   }
 }
 
@@ -66,9 +66,14 @@ void VirtualKeyboardTray::ClickedOutsideBubble() {}
 bool VirtualKeyboardTray::PerformAction(const ui::Event& event) {
   UserMetricsRecorder::RecordUserClickOnTray(
       LoginMetricsRecorder::TrayClickTarget::kVirtualKeyboardTray);
-  Shell::Get()->keyboard_ui()->ShowInDisplay(
-      display::Screen::GetScreen()->GetDisplayNearestWindow(
-          shelf_->GetWindow()));
+
+  auto* keyboard_controller = keyboard::KeyboardController::Get();
+  // Keyboard may not always be enabled. https://crbug.com/749989
+  if (keyboard_controller->enabled()) {
+    keyboard_controller->ShowKeyboardInDisplay(
+        display::Screen::GetScreen()->GetDisplayNearestWindow(
+            shelf_->GetWindow()));
+  }
   // Normally, active status is set when virtual keyboard is shown/hidden,
   // however, showing virtual keyboard happens asynchronously and, especially
   // the first time, takes some time. We need to set active status here to
@@ -78,7 +83,9 @@ bool VirtualKeyboardTray::PerformAction(const ui::Event& event) {
   return true;
 }
 
-void VirtualKeyboardTray::OnKeyboardEnabledStateChanged(bool new_enabled) {
+void VirtualKeyboardTray::OnAccessibilityStatusChanged() {
+  bool new_enabled =
+      Shell::Get()->accessibility_controller()->IsVirtualKeyboardEnabled();
   SetVisible(new_enabled);
   if (new_enabled) {
     // Observe keyboard controller to detect when the virtual keyboard is

ash/system/virtual_keyboard/virtual_keyboard_tray.h

@@ -5,7 +5,7 @@
 #ifndef ASH_SYSTEM_VIRTUAL_KEYBOARD_VIRTUAL_KEYBOARD_TRAY_H_
 #define ASH_SYSTEM_VIRTUAL_KEYBOARD_VIRTUAL_KEYBOARD_TRAY_H_
 
-#include "ash/keyboard/keyboard_ui_observer.h"
+#include "ash/accessibility/accessibility_observer.h"
 #include "ash/session/session_observer.h"
 #include "ash/shell_observer.h"
 #include "ash/system/tray/tray_background_view.h"
@@ -20,7 +20,7 @@ namespace ash {
 
 // TODO(sky): make this visible on non-chromeos platforms.
 class VirtualKeyboardTray : public TrayBackgroundView,
-                            public KeyboardUIObserver,
+                            public AccessibilityObserver,
                             public keyboard::KeyboardControllerObserver,
                             public ShellObserver,
                             public SessionObserver {
@@ -34,8 +34,8 @@ class VirtualKeyboardTray : public TrayBackgroundView,
   void ClickedOutsideBubble() override;
   bool PerformAction(const ui::Event& event) override;
 
-  // KeyboardUIObserver:
-  void OnKeyboardEnabledStateChanged(bool new_enabled) override;
+  // AccessibilityObserver:
+  void OnAccessibilityStatusChanged() override;
 
   // keyboard::KeyboardControllerObserver:
   void OnKeyboardVisibilityStateChanged(bool is_visible) override;