vk: Fix not showing emoji keyboard when triggered from right click menu.

There's a race condition with the right-click emoji menu:

1. User clicks "emoji".
2. We change the input_view_url to be the emoji url.
3. We then force the keyboard to show by enabling it temporarily.
4. This will wake up the extension JS code if it's been suspended.
5. Extension JS will listen to 'onActivate' which triggers the engine to
   re-enable.
6. Enabling will reset the input_view_url to be the url specified in the XKB
   manifest and we overwrite the emoji URL set in step 2.

Hence, we need to prevent step 6 from overidding. A crude way to do this to add
a boolean flag to indicate whether the inputview url is currently overridden.
If it's overridden, then enabling the engine should not reset the inputview url.

This is not an ideal solution, but it's the simplest with lowest impact.

Bug: 887612
Change-Id: I5530bb09ab9127bd0c56e86f8e08c89db84b0047
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2198856
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Shu Chen <shuchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769588}

chrome/browser/chromeos/input_method/input_method_manager_impl.cc

@@ -845,7 +845,9 @@ bool InputMethodManagerImpl::StateImpl::InputMethodIsActivated(
 }
 
 void InputMethodManagerImpl::StateImpl::EnableInputView() {
-  input_view_url = current_input_method.input_view_url();
+  if (!input_view_url_overridden) {
+    input_view_url = current_input_method.input_view_url();
+  }
 }
 
 void InputMethodManagerImpl::StateImpl::DisableInputView() {
@@ -856,6 +858,16 @@ const GURL& InputMethodManagerImpl::StateImpl::GetInputViewUrl() const {
   return input_view_url;
 }
 
+void InputMethodManagerImpl::StateImpl::OverrideInputViewUrl(const GURL& url) {
+  input_view_url = url;
+  input_view_url_overridden = true;
+}
+
+void InputMethodManagerImpl::StateImpl::ResetInputViewUrl() {
+  input_view_url = current_input_method.input_view_url();
+  input_view_url_overridden = false;
+}
+
 void InputMethodManagerImpl::StateImpl::ConnectMojoManager(
     mojo::PendingReceiver<chromeos::ime::mojom::InputEngineManager> receiver) {
   if (!ime_service_connector_) {
@@ -1334,7 +1346,7 @@ void InputMethodManagerImpl::OverrideKeyboardKeyset(
   if (keyset == chromeos::input_method::ImeKeyset::kNone) {
     // Resets the url as the input method default url and notify the hash
     // changed to VK.
-    state_->input_view_url = state_->current_input_method.input_view_url();
+    state_->ResetInputViewUrl();
     ReloadKeyboard();
     return;
   }
@@ -1378,8 +1390,7 @@ void InputMethodManagerImpl::OverrideKeyboardKeyset(
 
   GURL::Replacements replacements;
   replacements.SetRefStr(overridden_ref);
-  state_->input_view_url = url.ReplaceComponents(replacements);
-
+  state_->OverrideInputViewUrl(url.ReplaceComponents(replacements));
   ReloadKeyboard();
 }
 

chrome/browser/chromeos/input_method/input_method_manager_impl.h

@@ -116,6 +116,12 @@ class InputMethodManagerImpl : public InputMethodManager,
     void DisableInputView() override;
     const GURL& GetInputViewUrl() const override;
 
+    // Override the input view URL used to explicitly display some keyset.
+    void OverrideInputViewUrl(const GURL& url);
+
+    // Reset the input view URL to the default url of the current input method.
+    void ResetInputViewUrl();
+
     // Connect to an InputEngineManager instance in an IME Mojo service.
     void ConnectMojoManager(
         mojo::PendingReceiver<chromeos::ime::mojom::InputEngineManager>
@@ -149,10 +155,6 @@ class InputMethodManagerImpl : public InputMethodManager,
     // True if the opt-in IME menu is activated.
     bool menu_activated;
 
-    // The URL of the input view of the active ime with parameters (e.g. layout,
-    // keyset).
-    GURL input_view_url;
-
    protected:
     friend base::RefCounted<chromeos::input_method::InputMethodManager::State>;
     ~StateImpl() override;
@@ -168,6 +170,14 @@ class InputMethodManagerImpl : public InputMethodManager,
     // allowed input method, if no hardware input method is allowed.
     std::string GetAllowedFallBackKeyboardLayout() const;
 
+    // The URL of the input view of the active ime with parameters (e.g. layout,
+    // keyset).
+    GURL input_view_url;
+
+    // Whether the input view URL has been forcibly overridden e.g. to show a
+    // specific keyset.
+    bool input_view_url_overridden = false;
+
     std::unique_ptr<ImeServiceConnector> ime_service_connector_;
   };
 

chrome/browser/chromeos/input_method/input_method_manager_impl_unittest.cc

@@ -1313,11 +1313,27 @@ TEST_F(InputMethodManagerImplTest, MigrateInputMethodTest) {
 TEST_F(InputMethodManagerImplTest, OverrideKeyboardUrlRefWithKeyset) {
   InitComponentExtension();
 
+  // Create an input method with a input view URL for testing.
   const GURL inputview_url(
       "chrome-extension://"
       "inputview.html#id=us.compact.qwerty&language=en-US&passwordLayout=us."
       "compact.qwerty&name=keyboard_us");
-  GetActiveIMEState()->input_view_url = inputview_url;
+
+  const auto ime_id =
+      extension_ime_util::GetInputMethodID(kExtensionId1, "test_engine_id");
+  InputMethodDescriptors descriptors;
+  descriptors.push_back(InputMethodDescriptor(ime_id, "test", "TE", {}, {},
+                                              /*is_login_keyboard=*/false,
+                                              GURL(), inputview_url));
+
+  MockInputMethodEngine engine;
+  std::vector<std::string> enabled_imes = {ime_id};
+  GetActiveIMEState()->SetEnabledExtensionImes(&enabled_imes);
+  GetActiveIMEState()->AddInputMethodExtension(kExtensionId1, descriptors,
+                                               &engine);
+  GetActiveIMEState()->ChangeInputMethod(ime_id, false);
+
+  GetActiveIMEState()->EnableInputView();
   EXPECT_THAT(GetActiveIMEState()->GetInputViewUrl().spec(),
               ::testing::StartsWith(inputview_url.spec()));
 
@@ -1331,7 +1347,6 @@ TEST_F(InputMethodManagerImplTest, OverrideKeyboardUrlRefWithKeyset) {
               ::testing::StartsWith(overridden_url_emoji.spec()));
 
   // Override the keyboard url ref with 'hwt'.
-  GetActiveIMEState()->input_view_url = inputview_url;
   const GURL overridden_url_hwt(
       "chrome-extension://"
       "inputview.html#id=us.compact.qwerty.hwt&language=en-US&passwordLayout="
@@ -1342,7 +1357,6 @@ TEST_F(InputMethodManagerImplTest, OverrideKeyboardUrlRefWithKeyset) {
               ::testing::StartsWith(overridden_url_hwt.spec()));
 
   // Override the keyboard url ref with 'voice'.
-  GetActiveIMEState()->input_view_url = inputview_url;
   const GURL overridden_url_voice(
       "chrome-extension://"
       "inputview.html#id=us.compact.qwerty.voice&language=en-US"
@@ -1354,15 +1368,66 @@ TEST_F(InputMethodManagerImplTest, OverrideKeyboardUrlRefWithKeyset) {
 
 TEST_F(InputMethodManagerImplTest, OverrideDefaultKeyboardUrlRef) {
   InitComponentExtension();
+
   const GURL default_url("chrome://inputview.html");
-  GetActiveIMEState()->input_view_url = default_url;
 
-  EXPECT_EQ(default_url, GetActiveIMEState()->GetInputViewUrl());
+  const auto ime_id =
+      extension_ime_util::GetInputMethodID(kExtensionId1, "test_engine_id");
+  InputMethodDescriptors descriptors;
+  descriptors.push_back(InputMethodDescriptor(ime_id, "test", "TE", {}, {},
+                                              /*is_login_keyboard=*/false,
+                                              GURL(), default_url));
+
+  MockInputMethodEngine engine;
+  std::vector<std::string> enabled_imes = {ime_id};
+  GetActiveIMEState()->SetEnabledExtensionImes(&enabled_imes);
+  GetActiveIMEState()->AddInputMethodExtension(kExtensionId1, descriptors,
+                                               &engine);
+  GetActiveIMEState()->ChangeInputMethod(ime_id, false);
+  GetActiveIMEState()->EnableInputView();
 
   manager_->OverrideKeyboardKeyset(chromeos::input_method::ImeKeyset::kEmoji);
   EXPECT_EQ(default_url, GetActiveIMEState()->GetInputViewUrl());
 }
 
+TEST_F(InputMethodManagerImplTest, DoesNotResetInputViewUrlWhenOverridden) {
+  InitComponentExtension();
+
+  // Create an input method with a input view URL for testing.
+  const GURL inputview_url(
+      "chrome-extension://"
+      "inputview.html#id=us.compact.qwerty&language=en-US&passwordLayout=us."
+      "compact.qwerty&name=keyboard_us");
+
+  const auto ime_id =
+      extension_ime_util::GetInputMethodID(kExtensionId1, "test_engine_id");
+  InputMethodDescriptors descriptors;
+  descriptors.push_back(InputMethodDescriptor(ime_id, "test", "TE", {}, {},
+                                              /*is_login_keyboard=*/false,
+                                              GURL(), inputview_url));
+
+  MockInputMethodEngine engine;
+  std::vector<std::string> enabled_imes = {ime_id};
+  GetActiveIMEState()->SetEnabledExtensionImes(&enabled_imes);
+  GetActiveIMEState()->AddInputMethodExtension(kExtensionId1, descriptors,
+                                               &engine);
+  GetActiveIMEState()->ChangeInputMethod(ime_id, false);
+  GetActiveIMEState()->EnableInputView();
+
+  const GURL overridden_url_emoji(
+      "chrome-extension://"
+      "inputview.html#id=us.compact.qwerty.emoji&language=en-US&passwordLayout="
+      "us.compact.qwerty&name=keyboard_us");
+
+  manager_->OverrideKeyboardKeyset(chromeos::input_method::ImeKeyset::kEmoji);
+  EXPECT_THAT(GetActiveIMEState()->GetInputViewUrl().spec(),
+              ::testing::StartsWith(overridden_url_emoji.spec()));
+
+  GetActiveIMEState()->EnableInputView();
+  EXPECT_THAT(GetActiveIMEState()->GetInputViewUrl().spec(),
+              ::testing::StartsWith(overridden_url_emoji.spec()));
+}
+
 TEST_F(InputMethodManagerImplTest, AllowedKeyboardLayoutsValid) {
   InitComponentExtension();