[VK] Don't disable keyboard in IME tray due to temporary hide.

When we use the IME menu tray in laptop mode, we temporarily enable the
accessibility virtual keyboard. When the keyboard is dismissed, we
disable the accessibility keyboard again. However, when we change the
keyboard between docked and floating mode, the keyboard has to hide
first before showing again. This causes the accessibility keyboard to be
disabled and the keyboard doesn't show up again.

We fix this by adding an additional parameter to OnKeyboardHidden
which indicates whether the keyboard is hidden temporarily.

TBR=sky@chromium.org

Bug: 858953
Change-Id: I162917e5e32e6081be49127a3e7e9cf5fbeeb11c
Reviewed-on: https://chromium-review.googlesource.com/1137803
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Blake O'Hare <blakeo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576400}

ash/keyboard/virtual_keyboard_controller.cc

@@ -303,7 +303,12 @@ void VirtualKeyboardController::OnKeyboardDisabled() {
       chromeos::input_method::mojom::ImeKeyset::kNone);
 }
 
-void VirtualKeyboardController::OnKeyboardHidden() {
+void VirtualKeyboardController::OnKeyboardHidden(bool is_temporary_hide) {
+  // The keyboard may temporarily hide (e.g. to change container behaviors).
+  // The keyset should not be reset in this case.
+  if (is_temporary_hide)
+    return;
+
   Shell::Get()->ime_controller()->OverrideKeyboardKeyset(
       chromeos::input_method::mojom::ImeKeyset::kNone);
 

ash/keyboard/virtual_keyboard_controller.h

@@ -56,7 +56,7 @@ class ASH_EXPORT VirtualKeyboardController
 
   // keyboard::KeyboardControllerObserver:
   void OnKeyboardDisabled() override;
-  void OnKeyboardHidden() override;
+  void OnKeyboardHidden(bool is_temporary_hide) override;
 
   // SessionObserver
   void OnActiveUserSessionChanged(const AccountId& account_id) override;

ash/keyboard/virtual_keyboard_controller_unittest.cc

@@ -138,7 +138,8 @@ TEST_F(VirtualKeyboardControllerTest,
 
   // Simulate the keyboard hiding.
   if (GetKeyboardController()->HasObserver(GetVirtualKeyboardController())) {
-    GetVirtualKeyboardController()->OnKeyboardHidden();
+    GetVirtualKeyboardController()->OnKeyboardHidden(
+        false /* is_temporary_hide */);
   }
   base::RunLoop().RunUntilIdle();
 
@@ -182,7 +183,8 @@ TEST_F(VirtualKeyboardControllerTest,
 
   // Simulate the keyboard hiding.
   if (GetKeyboardController()->HasObserver(GetVirtualKeyboardController())) {
-    GetVirtualKeyboardController()->OnKeyboardHidden();
+    GetVirtualKeyboardController()->OnKeyboardHidden(
+        false /* is_temporary_hide */);
   }
   base::RunLoop().RunUntilIdle();
 
@@ -195,6 +197,47 @@ TEST_F(VirtualKeyboardControllerTest,
             client.last_keyset_);
 }
 
+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.
+  GetVirtualKeyboardController()->ForceShowKeyboardWithKeyset(
+      chromeos::input_method::mojom::ImeKeyset::kEmoji);
+  Shell::Get()->ime_controller()->FlushMojoForTesting();
+  EXPECT_TRUE(accessibility_controller->IsVirtualKeyboardEnabled());
+
+  // Keyset should be emoji.
+  EXPECT_EQ(chromeos::input_method::mojom::ImeKeyset::kEmoji,
+            client.last_keyset_);
+
+  // Simulate the keyboard hiding temporarily.
+  if (GetKeyboardController()->HasObserver(GetVirtualKeyboardController())) {
+    GetVirtualKeyboardController()->OnKeyboardHidden(
+        true /* is_temporary_hide */);
+  }
+  base::RunLoop().RunUntilIdle();
+
+  // The keyboard should still be enabled.
+  EXPECT_TRUE(accessibility_controller->IsVirtualKeyboardEnabled());
+
+  // Keyset should still be emoji.
+  EXPECT_EQ(chromeos::input_method::mojom::ImeKeyset::kEmoji,
+            client.last_keyset_);
+}
+
 class VirtualKeyboardControllerAutoTest : public VirtualKeyboardControllerTest,
                                           public VirtualKeyboardObserver {
  public:

ash/system/ime_menu/ime_menu_tray.cc

@@ -504,7 +504,7 @@ void ImeMenuTray::HideBubble(const views::TrayBubbleView* bubble_view) {
   HideBubbleWithView(bubble_view);
 }
 
-void ImeMenuTray::OnKeyboardHidden() {
+void ImeMenuTray::OnKeyboardHidden(bool is_temporary_hide) {
   if (show_bubble_after_keyboard_hidden_) {
     show_bubble_after_keyboard_hidden_ = false;
     auto* keyboard_controller = keyboard::KeyboardController::Get();

ash/system/ime_menu/ime_menu_tray.h

@@ -73,7 +73,7 @@ class ASH_EXPORT ImeMenuTray : public TrayBackgroundView,
   void HideBubble(const views::TrayBubbleView* bubble_view) override;
 
   // keyboard::KeyboardControllerObserver:
-  void OnKeyboardHidden() override;
+  void OnKeyboardHidden(bool is_temporary_hide) override;
 
   // VirtualKeyboardObserver:
   void OnKeyboardSuppressionChanged(bool suppressed) override;

ui/keyboard/keyboard_controller.cc

@@ -415,7 +415,7 @@ void KeyboardController::HideKeyboard(HideReason reason) {
       ChangeState(KeyboardControllerState::HIDDEN);
 
       for (KeyboardControllerObserver& observer : observer_list_)
-        observer.OnKeyboardHidden();
+        observer.OnKeyboardHidden(reason == HIDE_REASON_SYSTEM_TEMPORARY);
       ui_->EnsureCaretInWorkArea(gfx::Rect());
 
       break;

ui/keyboard/keyboard_controller_observer.h

@@ -59,7 +59,9 @@ class KEYBOARD_EXPORT KeyboardControllerObserver {
 
   // Called when the keyboard has been hidden and the hiding animation finished
   // successfully. This is same as |state| == HIDDEN on OnStateChanged.
-  virtual void OnKeyboardHidden() {}
+  // When |is_temporary_hide| is true, this hide is immediately followed by a
+  // show (e.g. when changing to floating keyboard)
+  virtual void OnKeyboardHidden(bool is_temporary_hide) {}
 
   // Called when the state changed.
   virtual void OnStateChanged(KeyboardControllerState state) {}