[VK] Use non-accessibility keyboard when opening keyboard from tray.

When in laptop mode, we sometimes need to show the virtual keyboard (
e.g. from the IME tray menu). Currently we just temporarily enable the
accessibility virtual keyboard in order to show the keyboard.

However, this has two downsides:
1) It shows the accessibility keyboard, which has a different layout /
   size than the non-accessibility keyboard.
2) Sometimes, there are bugs where the accessibility keyboard is
   accidentaly enabled/disabled permanently.

We add a new switch in keyboard_util to show the virtual keyboard. We
then enable the switch when showing the VK in laptop mode. This solves
both problems.

Bug: 875096
Change-Id: I712fbe42dbeec99dfea4f1aeddbe1db0cc50ed45
Reviewed-on: https://chromium-review.googlesource.com/1186284
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586172}

ash/keyboard/virtual_keyboard_controller.cc

@@ -36,17 +36,10 @@ bool IsVirtualKeyboardEnabled() {
       keyboard::switches::kEnableVirtualKeyboard);
 }
 
-void ResetVirtualKeyboard(bool disable_accessibility_keyboard) {
-  if (disable_accessibility_keyboard) {
-    // Reset the accessibility keyboard settings.
-    AccessibilityController* accessibility_controller =
-        Shell::Get()->accessibility_controller();
-    if (!accessibility_controller)
-      return;
-
-    DCHECK(accessibility_controller->IsVirtualKeyboardEnabled());
-    accessibility_controller->SetVirtualKeyboardEnabled(false);
-  }
+void ResetVirtualKeyboard() {
+  keyboard::SetKeyboardEnabledFromShelf(false);
+  if (!keyboard::IsKeyboardEnabled())
+    Shell::Get()->DisableKeyboard();
 
   // Reset the keyset after disabling the virtual keyboard to prevent the IME
   // extension from accidentally loading the default keyset while it's shutting
@@ -280,27 +273,13 @@ void VirtualKeyboardController::ForceShowKeyboard() {
     return;
   }
 
-  // Otherwise, force enable the virtual keyboard by turning on the
-  // accessibility keyboard.
-  // TODO(https://crbug.com/818567): This is risky as enabling accessibility
-  // keyboard is a persistent setting, so we have to ensure that we disable it
-  // again when the keyboard is closed.
-  AccessibilityController* accessibility_controller =
-      Shell::Get()->accessibility_controller();
-  DCHECK(!accessibility_controller->IsVirtualKeyboardEnabled());
-
-  // TODO(mash): Turning on accessibility keyboard does not create a valid
-  // KeyboardController under MASH. See https://crbug.com/646565.
-  if (Shell::GetAshConfig() == Config::MASH_DEPRECATED)
-    return;
+  // Otherwise, temporarily enable the virtual keyboard until it is dismissed.
+  DCHECK(!keyboard::GetKeyboardEnabledFromShelf());
+  keyboard::SetKeyboardEnabledFromShelf(true);
+  Shell::Get()->EnableKeyboard();
 
-  // Onscreen keyboard has not been enabled yet, forces to bring out the
-  // keyboard for one time.
-  accessibility_controller->SetVirtualKeyboardEnabled(true);
-  keyboard_enabled_using_accessibility_prefs_ = true;
   keyboard_controller = keyboard::KeyboardController::Get();
   DCHECK(keyboard_controller->enabled());
-
   keyboard_controller->ShowKeyboard(false);
 }
 
@@ -317,9 +296,7 @@ void VirtualKeyboardController::OnKeyboardHidden(bool is_temporary_hide) {
 
   // Post a task to reset the virtual keyboard to its original state.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(ResetVirtualKeyboard,
-                                keyboard_enabled_using_accessibility_prefs_));
-  keyboard_enabled_using_accessibility_prefs_ = false;
+      FROM_HERE, base::BindOnce(ResetVirtualKeyboard));
 }
 
 void VirtualKeyboardController::OnActiveUserSessionChanged(

ash/keyboard/virtual_keyboard_controller.h

@@ -83,11 +83,6 @@ class ASH_EXPORT VirtualKeyboardController
   // True if the presence of an external keyboard should be ignored.
   bool ignore_external_keyboard_;
 
-  // Whether the keyboard was forced to be enabled using accessibility prefs.
-  // Used to determine whether we need to disable the accessibility keyboard
-  // when the keyboard closes.
-  bool keyboard_enabled_using_accessibility_prefs_ = false;
-
   DISALLOW_COPY_AND_ASSIGN(VirtualKeyboardController);
 };
 

ash/keyboard/virtual_keyboard_controller_unittest.cc

@@ -156,26 +156,21 @@ TEST_F(VirtualKeyboardControllerTest,
 }
 
 TEST_F(VirtualKeyboardControllerTest,
-       ForceToShowKeyboardWithKeysetWhenAccessibilityKeyboardIsDisabled) {
-  // TODO(mash): Turning on accessibility keyboard does not create a valid
-  // KeyboardController under MASH. See https://crbug.com/646565.
-  if (Shell::GetAshConfig() == Config::MASH_DEPRECATED)
-    return;
-
-  AccessibilityController* accessibility_controller =
-      Shell::Get()->accessibility_controller();
-  accessibility_controller->SetVirtualKeyboardEnabled(false);
-  ASSERT_FALSE(accessibility_controller->IsVirtualKeyboardEnabled());
-
+       ForceToShowKeyboardWithKeysetWhenKeyboardIsDisabled) {
   // Set up a mock ImeControllerClient to test keyset changes.
   TestImeControllerClient client;
   Shell::Get()->ime_controller()->SetClient(client.CreateInterfacePtr());
 
-  // Should show the keyboard by turning on the accesibility keyboard.
+  // Should show the keyboard by enabling it temporarily.
+  EXPECT_FALSE(keyboard::IsKeyboardEnabled());
+  EXPECT_FALSE(keyboard::GetKeyboardEnabledFromShelf());
+
   GetVirtualKeyboardController()->ForceShowKeyboardWithKeyset(
       chromeos::input_method::mojom::ImeKeyset::kEmoji);
   Shell::Get()->ime_controller()->FlushMojoForTesting();
-  EXPECT_TRUE(accessibility_controller->IsVirtualKeyboardEnabled());
+
+  EXPECT_TRUE(keyboard::GetKeyboardEnabledFromShelf());
+  EXPECT_TRUE(keyboard::IsKeyboardEnabled());
 
   // Keyset should be emoji.
   EXPECT_EQ(chromeos::input_method::mojom::ImeKeyset::kEmoji,
@@ -189,7 +184,8 @@ TEST_F(VirtualKeyboardControllerTest,
   base::RunLoop().RunUntilIdle();
 
   // The keyboard should still be disabled again.
-  EXPECT_FALSE(accessibility_controller->IsVirtualKeyboardEnabled());
+  EXPECT_FALSE(keyboard::IsKeyboardEnabled());
+  EXPECT_FALSE(keyboard::GetKeyboardEnabledFromShelf());
 
   // Keyset should be reset to none.
   Shell::Get()->ime_controller()->FlushMojoForTesting();
@@ -199,25 +195,17 @@ TEST_F(VirtualKeyboardControllerTest,
 
 TEST_F(VirtualKeyboardControllerTest,
        ForceToShowKeyboardWithKeysetTemporaryHide) {
-  // TODO(mash): Turning on accessibility keyboard does not create a valid
-  // KeyboardController under MASH. See https://crbug.com/646565.
-  if (Shell::GetAshConfig() == Config::MASH_DEPRECATED)
-    return;
-
-  AccessibilityController* accessibility_controller =
-      Shell::Get()->accessibility_controller();
-  accessibility_controller->SetVirtualKeyboardEnabled(false);
-  ASSERT_FALSE(accessibility_controller->IsVirtualKeyboardEnabled());
-
   // Set up a mock ImeControllerClient to test keyset changes.
   TestImeControllerClient client;
   Shell::Get()->ime_controller()->SetClient(client.CreateInterfacePtr());
 
-  // Should show the keyboard by turning on the accesibility keyboard.
+  // Should show the keyboard by enabling it temporarily.
   GetVirtualKeyboardController()->ForceShowKeyboardWithKeyset(
       chromeos::input_method::mojom::ImeKeyset::kEmoji);
   Shell::Get()->ime_controller()->FlushMojoForTesting();
-  EXPECT_TRUE(accessibility_controller->IsVirtualKeyboardEnabled());
+
+  EXPECT_TRUE(keyboard::GetKeyboardEnabledFromShelf());
+  EXPECT_TRUE(keyboard::IsKeyboardEnabled());
 
   // Keyset should be emoji.
   EXPECT_EQ(chromeos::input_method::mojom::ImeKeyset::kEmoji,
@@ -231,7 +219,8 @@ TEST_F(VirtualKeyboardControllerTest,
   base::RunLoop().RunUntilIdle();
 
   // The keyboard should still be enabled.
-  EXPECT_TRUE(accessibility_controller->IsVirtualKeyboardEnabled());
+  EXPECT_TRUE(keyboard::GetKeyboardEnabledFromShelf());
+  EXPECT_TRUE(keyboard::IsKeyboardEnabled());
 
   // Keyset should still be emoji.
   EXPECT_EQ(chromeos::input_method::mojom::ImeKeyset::kEmoji,

ui/keyboard/keyboard_util.cc

@@ -57,6 +57,8 @@ bool g_accessibility_keyboard_enabled = false;
 
 bool g_hotrod_keyboard_enabled = false;
 
+bool g_keyboard_enabled_from_shelf = false;
+
 bool g_touch_keyboard_enabled = false;
 
 KeyboardState g_requested_keyboard_state = KEYBOARD_STATE_AUTO;
@@ -98,6 +100,14 @@ bool GetHotrodKeyboardEnabled() {
   return g_hotrod_keyboard_enabled;
 }
 
+void SetKeyboardEnabledFromShelf(bool enabled) {
+  g_keyboard_enabled_from_shelf = enabled;
+}
+
+bool GetKeyboardEnabledFromShelf() {
+  return g_keyboard_enabled_from_shelf;
+}
+
 void SetTouchKeyboardEnabled(bool enabled) {
   g_touch_keyboard_enabled = enabled;
 }
@@ -124,6 +134,9 @@ bool IsKeyboardEnabled() {
   // Accessibility setting prioritized over policy setting.
   if (g_accessibility_keyboard_enabled)
     return true;
+  // Keyboard can be enabled temporarily by the shelf.
+  if (g_keyboard_enabled_from_shelf)
+    return true;
   // Policy strictly disables showing a virtual keyboard.
   if (g_keyboard_show_override == KEYBOARD_SHOW_OVERRIDE_DISABLED)
     return false;

ui/keyboard/keyboard_util.h

@@ -88,6 +88,12 @@ KEYBOARD_EXPORT void SetHotrodKeyboardEnabled(bool enabled);
 // Gets the state of the hotrod onscreen keyboard.
 KEYBOARD_EXPORT bool GetHotrodKeyboardEnabled();
 
+// Sets whether the keyboard is enabled from the shelf.
+KEYBOARD_EXPORT void SetKeyboardEnabledFromShelf(bool enabled);
+
+// Gets whether the keyboard is enabled from the shelf.
+KEYBOARD_EXPORT bool GetKeyboardEnabledFromShelf();
+
 // Sets the state of the touch onscreen keyboard.
 KEYBOARD_EXPORT void SetTouchKeyboardEnabled(bool enabled);