Add ash.mojom.KeyboardController.GetEnableFlags

This CL introduces GetEnableFlags to the mojo API so that
GetKeyboardLayout can be moved from keyboard_util.cc/h to
chrome_virtual_keyboard_delegate.cc (the only place it was used).

It also moves the implementaiton of GetKeyboardWindow to
ChromeKeyboardControllerClient.

BUG=876138

Change-Id: I83bca47c9a038f9be2bb3322c5500996d898f21b
Reviewed-on: https://chromium-review.googlesource.com/c/1330327
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607285}

ash/keyboard/ash_keyboard_controller.cc

@@ -113,6 +113,14 @@ void AshKeyboardController::ClearEnableFlag(KeyboardEnableFlag flag) {
   UpdateEnableFlag(was_enabled);
 }
 
+void AshKeyboardController::GetEnableFlags(GetEnableFlagsCallback callback) {
+  const std::set<keyboard::mojom::KeyboardEnableFlag>& keyboard_enable_flags =
+      keyboard_controller_->keyboard_enable_flags();
+  std::vector<keyboard::mojom::KeyboardEnableFlag> flags(
+      keyboard_enable_flags.begin(), keyboard_enable_flags.end());
+  std::move(callback).Run(std::move(flags));
+}
+
 void AshKeyboardController::ReloadKeyboardIfNeeded() {
   keyboard_controller_->Reload();
 }

ash/keyboard/ash_keyboard_controller.h

@@ -64,6 +64,7 @@ class ASH_EXPORT AshKeyboardController
   void IsKeyboardEnabled(IsKeyboardEnabledCallback callback) override;
   void SetEnableFlag(keyboard::mojom::KeyboardEnableFlag flag) override;
   void ClearEnableFlag(keyboard::mojom::KeyboardEnableFlag flag) override;
+  void GetEnableFlags(GetEnableFlagsCallback callback) override;
   void ReloadKeyboardIfNeeded() override;
   void RebuildKeyboardIfEnabled() override;
   void IsKeyboardVisible(IsKeyboardVisibleCallback callback) override;

ash/keyboard/ash_keyboard_controller_unittest.cc

@@ -9,6 +9,7 @@
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
 #include "base/run_loop.h"
+#include "base/stl_util.h"
 #include "base/test/scoped_task_environment.h"
 #include "mojo/public/cpp/bindings/associated_binding.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -92,6 +93,21 @@ class TestClient {
     keyboard_controller_.FlushForTesting();
   }
 
+  std::vector<keyboard::mojom::KeyboardEnableFlag> GetEnableFlags() {
+    std::vector<keyboard::mojom::KeyboardEnableFlag> enable_flags;
+    base::RunLoop run_loop;
+    keyboard_controller_->GetEnableFlags(base::BindOnce(
+        [](std::vector<keyboard::mojom::KeyboardEnableFlag>* enable_flags,
+           base::OnceClosure callback,
+           const std::vector<keyboard::mojom::KeyboardEnableFlag>& flags) {
+          *enable_flags = flags;
+          std::move(callback).Run();
+        },
+        &enable_flags, run_loop.QuitClosure()));
+    run_loop.Run();
+    return enable_flags;
+  }
+
   void RebuildKeyboardIfEnabled() {
     keyboard_controller_->RebuildKeyboardIfEnabled();
     keyboard_controller_.FlushForTesting();
@@ -215,18 +231,32 @@ TEST_F(AshKeyboardControllerTest, SetKeyboardConfig) {
             test_client()->test_observer()->config_.auto_complete);
 }
 
-TEST_F(AshKeyboardControllerTest, Enabled) {
+TEST_F(AshKeyboardControllerTest, EnableFlags) {
   EXPECT_FALSE(test_client()->IsKeyboardEnabled());
   // Enable the keyboard.
   test_client()->SetEnableFlag(KeyboardEnableFlag::kExtensionEnabled);
+  std::vector<keyboard::mojom::KeyboardEnableFlag> enable_flags =
+      test_client()->GetEnableFlags();
+  EXPECT_TRUE(
+      base::ContainsValue(enable_flags, KeyboardEnableFlag::kExtensionEnabled));
   EXPECT_TRUE(test_client()->IsKeyboardEnabled());
 
   // Set the enable override to disable the keyboard.
   test_client()->SetEnableFlag(KeyboardEnableFlag::kPolicyDisabled);
+  enable_flags = test_client()->GetEnableFlags();
+  EXPECT_TRUE(
+      base::ContainsValue(enable_flags, KeyboardEnableFlag::kExtensionEnabled));
+  EXPECT_TRUE(
+      base::ContainsValue(enable_flags, KeyboardEnableFlag::kPolicyDisabled));
   EXPECT_FALSE(test_client()->IsKeyboardEnabled());
 
   // Clear the enable override; should enable the keyboard.
   test_client()->ClearEnableFlag(KeyboardEnableFlag::kPolicyDisabled);
+  enable_flags = test_client()->GetEnableFlags();
+  EXPECT_TRUE(
+      base::ContainsValue(enable_flags, KeyboardEnableFlag::kExtensionEnabled));
+  EXPECT_FALSE(
+      base::ContainsValue(enable_flags, KeyboardEnableFlag::kPolicyDisabled));
   EXPECT_TRUE(test_client()->IsKeyboardEnabled());
 }
 

ash/public/interfaces/keyboard_controller.mojom

@@ -53,6 +53,9 @@ interface KeyboardController {
   // changes, enables or disables the keyboard to match the new state.
   ClearEnableFlag(keyboard.mojom.KeyboardEnableFlag flag);
 
+  // Gets the current set of keyboard enabled flags.
+  GetEnableFlags() => (array<keyboard.mojom.KeyboardEnableFlag> flags);
+
   // Reloads the virtual keyboard if it is enabled and the URL has changed, e.g.
   // the focus has switched from one type of field to another.
   ReloadKeyboardIfNeeded();

chrome/browser/chromeos/DEPS

@@ -21,6 +21,11 @@ include_rules = [
   # does not run in process in mus.
   "-ui/ozone/public",
   "+ui/ozone/public/ozone_switches.h",
+
+  # keyboard::KeyboardController only exists in ash and should not be accessed
+  # directly from src/chrome. Use ChromeKeyboardControllerClient instead.
+  "-ui/keyboard/keyboard_controller.h",
+  "-ui/keyboard/keyboard_util.h",
 ]
 
 specific_include_rules = {
@@ -34,7 +39,12 @@ specific_include_rules = {
     "+ash/shell.h",
     "+ash/sticky_keys/sticky_keys_controller.h",
   ],
-  "event_rewriter_unittest.cc": [
+  "event_rewriter_unittest\.cc": [
     "+ui/events/devices/device_data_manager.h",
   ],
+  "input_method_manager_impl\.cc": [
+    # For GetInputMethodKeyboardController. TODO(stevenjb/shuchen): Fix this
+    # for Mash. https://crbug.com/756059
+    "+ui/keyboard/keyboard_controller.h",
+  ],
 }

chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc

@@ -56,11 +56,6 @@ namespace {
 // until / unless it is explicitly disabled.
 bool g_hotrod_keyboard_enabled = false;
 
-aura::Window* GetKeyboardWindow() {
-  auto* controller = keyboard::KeyboardController::Get();
-  return controller->IsEnabled() ? controller->GetKeyboardWindow() : nullptr;
-}
-
 std::string GenerateFeatureFlag(const std::string& feature, bool enabled) {
   return feature + (enabled ? "-enabled" : "-disabled");
 }
@@ -150,6 +145,15 @@ bool SendKeyEventImpl(const std::string& type,
   return true;
 }
 
+std::string GetKeyboardLayout() {
+  // TODO(bshe): layout string is currently hard coded. We should use more
+  // standard keyboard layouts.
+  return ChromeKeyboardControllerClient::Get()->IsEnableFlagSet(
+             keyboard::mojom::KeyboardEnableFlag::kAccessibilityEnabled)
+             ? "system-qwerty"
+             : "qwerty";
+}
+
 }  // namespace
 
 namespace extensions {
@@ -242,7 +246,8 @@ bool ChromeVirtualKeyboardDelegate::SendKeyEvent(const std::string& type,
                                                  const std::string& key_name,
                                                  int modifiers) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  aura::Window* window = GetKeyboardWindow();
+  aura::Window* window =
+      ChromeKeyboardControllerClient::Get()->GetKeyboardWindow();
   return window && SendKeyEventImpl(type, char_value, key_code, key_name,
                                     modifiers, window->GetHost());
 }
@@ -341,7 +346,8 @@ void ChromeVirtualKeyboardDelegate::OnHasInputDevices(
     bool has_audio_input_devices) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   std::unique_ptr<base::DictionaryValue> results(new base::DictionaryValue());
-  results->SetString("layout", keyboard::GetKeyboardLayout());
+  results->SetString("layout", GetKeyboardLayout());
+
   // TODO(bshe): Consolidate a11y, hotrod and normal mode into a mode enum. See
   // crbug.com/529474.
   results->SetBoolean("a11ymode", keyboard::GetAccessibilityKeyboardEnabled());

chrome/browser/ui/ash/DEPS

@@ -32,9 +32,14 @@ specific_include_rules = {
   "chrome_keyboard_bounds_observer\.cc": [
     "+ash/root_window_controller.h",
   ],
-  # https://crbug.com/843332
-  "chrome_keyboard_ui\.cc": [
+  # For classic Ash mode.
+  "chrome_keyboard_controller_client\.cc": [
+    "+ui/keyboard",
+  ],
+  # This code is only used in classic Ash mode.
+  "chrome_keyboard_ui.*": [
     "+ash/shell.h",
+    "+ui/keyboard",
   ],
   # https://crbug.com/124222
   "chrome_new_window_client\.cc": [
@@ -57,11 +62,6 @@ specific_include_rules = {
   "(keyboard_controller_browsertest|keyboard_end_to_end_browsertest)\.cc": [
     "+ui/keyboard",
   ],
-  # This code is only used in classic Ash mode.
-  "chrome_keyboard_ui.*": [
-    "+ash/shell",
-    "+ui/keyboard",
-  ],
   # Only used in !mash
   "screen_orientation_delegate_chromeos.cc": [
     "+ash/display/screen_orientation_controller.h",

chrome/browser/ui/ash/chrome_keyboard_controller_client.cc

@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/stl_util.h"
 #include "base/values.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -23,7 +24,9 @@
 #include "ui/base/ime/ime_bridge.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/text_input_client.h"
+#include "ui/base/ui_base_features.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/keyboard/keyboard_controller.h"
 #include "ui/keyboard/public/keyboard_switches.h"
 #include "ui/keyboard/resources/keyboard_resource_util.h"
 
@@ -68,6 +71,11 @@ ChromeKeyboardControllerClient::ChromeKeyboardControllerClient(
       base::BindOnce(&ChromeKeyboardControllerClient::OnKeyboardEnabledChanged,
                      weak_ptr_factory_.GetWeakPtr()));
 
+  // Request the initial set of enable flags.
+  keyboard_controller_ptr_->GetEnableFlags(
+      base::BindOnce(&ChromeKeyboardControllerClient::OnGetEnableFlags,
+                     weak_ptr_factory_.GetWeakPtr()));
+
   // Request the initial visible state.
   keyboard_controller_ptr_->IsKeyboardVisible(base::BindOnce(
       &ChromeKeyboardControllerClient::OnKeyboardVisibilityChanged,
@@ -117,11 +125,22 @@ void ChromeKeyboardControllerClient::GetKeyboardEnabled(
 void ChromeKeyboardControllerClient::SetEnableFlag(
     const keyboard::mojom::KeyboardEnableFlag& flag) {
   keyboard_controller_ptr_->SetEnableFlag(flag);
+  keyboard_controller_ptr_->GetEnableFlags(
+      base::BindOnce(&ChromeKeyboardControllerClient::OnGetEnableFlags,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ChromeKeyboardControllerClient::ClearEnableFlag(
     const keyboard::mojom::KeyboardEnableFlag& flag) {
   keyboard_controller_ptr_->ClearEnableFlag(flag);
+  keyboard_controller_ptr_->GetEnableFlags(
+      base::BindOnce(&ChromeKeyboardControllerClient::OnGetEnableFlags,
+                     weak_ptr_factory_.GetWeakPtr()));
+}
+
+bool ChromeKeyboardControllerClient::IsEnableFlagSet(
+    const keyboard::mojom::KeyboardEnableFlag& flag) {
+  return base::ContainsKey(keyboard_enable_flags_, flag);
 }
 
 void ChromeKeyboardControllerClient::ReloadKeyboardIfNeeded() {
@@ -174,6 +193,15 @@ GURL ChromeKeyboardControllerClient::GetVirtualKeyboardUrl() {
   return input_view_url;
 }
 
+aura::Window* ChromeKeyboardControllerClient::GetKeyboardWindow() const {
+  if (::features::IsUsingWindowService()) {
+    // TODO(stevenjb): When WS support is added, return the Chrome window
+    // hosting the extension instead.
+    return nullptr;
+  }
+  return keyboard::KeyboardController::Get()->GetKeyboardWindow();
+}
+
 void ChromeKeyboardControllerClient::FlushForTesting() {
   keyboard_controller_ptr_.FlushForTesting();
 }
@@ -245,6 +273,12 @@ void ChromeKeyboardControllerClient::OnKeyboardVisibleBoundsChanged(
   router->BroadcastEvent(std::move(event));
 }
 
+void ChromeKeyboardControllerClient::OnGetEnableFlags(
+    const std::vector<keyboard::mojom::KeyboardEnableFlag>& flags) {
+  keyboard_enable_flags_ =
+      std::set<keyboard::mojom::KeyboardEnableFlag>(flags.begin(), flags.end());
+}
+
 Profile* ChromeKeyboardControllerClient::GetProfile() {
   if (profile_for_test_)
     return profile_for_test_;

chrome/browser/ui/ash/chrome_keyboard_controller_client.h

@@ -16,6 +16,10 @@
 
 class Profile;
 
+namespace aura {
+class Window;
+}
+
 namespace service_manager {
 class Connector;
 }
@@ -61,8 +65,11 @@ class ChromeKeyboardControllerClient
   void GetKeyboardEnabled(base::OnceCallback<void(bool)> callback);
 
   // Sets/clears the privided keyboard enable state.
-  void SetEnableFlag(const keyboard::mojom::KeyboardEnableFlag& state);
-  void ClearEnableFlag(const keyboard::mojom::KeyboardEnableFlag& state);
+  void SetEnableFlag(const keyboard::mojom::KeyboardEnableFlag& flag);
+  void ClearEnableFlag(const keyboard::mojom::KeyboardEnableFlag& flag);
+
+  // Returns whether |flag| has been set.
+  bool IsEnableFlagSet(const keyboard::mojom::KeyboardEnableFlag& flag);
 
   // Calls ash.mojom.ReloadKeyboardIfNeeded.
   void ReloadKeyboardIfNeeded();
@@ -82,6 +89,9 @@ class ChromeKeyboardControllerClient
   // Returns the URL to use for the virtual keyboard.
   GURL GetVirtualKeyboardUrl();
 
+  // Returns the keyboard window, or null if the window has not been created.
+  aura::Window* GetKeyboardWindow() const;
+
   bool is_keyboard_enabled() { return is_keyboard_enabled_; }
   bool is_keyboard_visible() { return is_keyboard_visible_; }
 
@@ -100,6 +110,9 @@ class ChromeKeyboardControllerClient
   void OnKeyboardVisibilityChanged(bool visible) override;
   void OnKeyboardVisibleBoundsChanged(const gfx::Rect& bounds) override;
 
+  void OnGetEnableFlags(
+      const std::vector<keyboard::mojom::KeyboardEnableFlag>& flags);
+
   // Returns either the test profile or the active user profile.
   Profile* GetProfile();
 
@@ -110,6 +123,10 @@ class ChromeKeyboardControllerClient
   // Cached copy of the latest config provided by mojom::KeyboardController.
   keyboard::mojom::KeyboardConfigPtr cached_keyboard_config_;
 
+  // Cached copy of the active enabled flags provided by
+  // mojom::KeyboardController
+  std::set<keyboard::mojom::KeyboardEnableFlag> keyboard_enable_flags_;
+
   // Tracks the enabled state of the keyboard.
   bool is_keyboard_enabled_ = false;
 

chrome/browser/ui/ash/chrome_keyboard_controller_client_test_helper.cc

@@ -42,6 +42,9 @@ class ChromeKeyboardControllerClientTestHelper::FakeKeyboardController
   void ClearEnableFlag(keyboard::mojom::KeyboardEnableFlag flag) override {
     keyboard_enable_flags_.erase(flag);
   }
+  void GetEnableFlags(GetEnableFlagsCallback callback) override {
+    std::move(callback).Run(std::vector<keyboard::mojom::KeyboardEnableFlag>());
+  }
   void ReloadKeyboardIfNeeded() override {}
   void RebuildKeyboardIfEnabled() override {}
   void IsKeyboardVisible(IsKeyboardVisibleCallback callback) override {

ui/keyboard/keyboard_controller.h

@@ -138,6 +138,9 @@ class KEYBOARD_EXPORT KeyboardController
   void SetEnableFlag(mojom::KeyboardEnableFlag flag);
   void ClearEnableFlag(mojom::KeyboardEnableFlag flag);
   bool IsEnableFlagSet(mojom::KeyboardEnableFlag flag) const;
+  const std::set<mojom::KeyboardEnableFlag>& keyboard_enable_flags() const {
+    return keyboard_enable_flags_;
+  }
 
   // Returns true if the keyboard should be enabled, i.e. the current result
   // of Set/ClearEnableFlag should cause the keyboard to be enabled.

ui/keyboard/keyboard_util.cc

@@ -63,12 +63,6 @@ bool GetTouchKeyboardEnabled() {
   return GetFlag(mojom::KeyboardEnableFlag::kTouchEnabled);
 }
 
-std::string GetKeyboardLayout() {
-  // TODO(bshe): layout string is currently hard coded. We should use more
-  // standard keyboard layouts.
-  return GetAccessibilityKeyboardEnabled() ? "system-qwerty" : "qwerty";
-}
-
 bool IsKeyboardEnabled() {
   return KeyboardController::Get()->IsKeyboardEnableRequested();
 }

ui/keyboard/keyboard_util.h

@@ -34,9 +34,6 @@ KEYBOARD_EXPORT void SetTouchKeyboardEnabled(bool enabled);
 // Gets the state of the touch onscreen keyboard.
 KEYBOARD_EXPORT bool GetTouchKeyboardEnabled();
 
-// Gets the default keyboard layout.
-KEYBOARD_EXPORT std::string GetKeyboardLayout();
-
 // Returns true if the virtual keyboard is enabled.
 KEYBOARD_EXPORT bool IsKeyboardEnabled();