[Mojo-IMF] Fixes the engine side issues.

The InputMethodEngine needs to work with mojo-based & non-mojo-based clients in
a hybrid mode. For example, the app_list window seems not using
WindowTreeHostMus. Therefore, the engine needs to:
 - track the mojo client breakage, and can switch back to legacy mode to use
   IMEBridge.
 - mutes the FinishInput call as necessary when a non-mojo client calls FocusIn
   before the mojo client calls FinishInput. Otherwise it will confuse the IME
   extension (e.g. receives "onfocus" and later "onblur").

Bug: 937167
Change-Id: I9dc54156b6075d1636e8984d941948a680b365e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1559414
Commit-Queue: Shu Chen <shuchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#649727}

chrome/browser/chromeos/input_method/input_method_engine.cc

@@ -72,8 +72,8 @@ class MojoHelper : public ime::mojom::ImeEngine,
     ime::mojom::ImeEngineFactoryPtr factory_ptr;
     factory_binding_.Close();
     factory_binding_.Bind(mojo::MakeRequest(&factory_ptr));
-    factory_binding_.set_connection_error_handler(
-        base::BindOnce(&MojoHelper::OnConnectionLost, base::Unretained(this)));
+    factory_binding_.set_connection_error_handler(base::BindOnce(
+        &MojoHelper::OnFactoryConnectionLost, base::Unretained(this)));
 
     if (registry) {
       registry_ = std::move(registry);
@@ -86,12 +86,16 @@ class MojoHelper : public ime::mojom::ImeEngine,
     registry_->ActivateFactory(std::move(factory_ptr));
   }
 
+  void set_allow_finish_input(bool allow) { allow_finish_input_ = allow; }
+
   // ime::mojom::ImeEngineFactory overrides:
   void CreateEngine(ime::mojom::ImeEngineRequest engine_request,
                     ime::mojom::ImeEngineClientPtr client) override {
     engine_binding_.Close();
     engine_binding_.Bind(std::move(engine_request));
     engine_client_ = std::move(client);
+    engine_client_.set_connection_error_handler(base::BindOnce(
+        &MojoHelper::OnClientConnectionLost, base::Unretained(this)));
   }
 
   // ime::mojom::ImeEngine overrides:
@@ -100,8 +104,17 @@ class MojoHelper : public ime::mojom::ImeEngine,
         info->type, info->mode, info->flags, info->focus_reason,
         info->should_do_learning);
     engine_->FocusIn(context);
+    allow_finish_input_ = true;
+  }
+  void FinishInput() override {
+    // Only allows the call of FocusOut() when the FocusIn() was caused from a
+    // mojo-based client. Please see the comments for |allow_finish_input_| for
+    // the details.
+    if (allow_finish_input_) {
+      engine_->FocusOut();
+      allow_finish_input_ = false;
+    }
   }
-  void FinishInput() override { engine_->FocusOut(); }
   void CancelInput() override { engine_->Reset(); }
   void ProcessKeyEvent(
       std::unique_ptr<ui::Event> key_event,
@@ -132,13 +145,15 @@ class MojoHelper : public ime::mojom::ImeEngine,
   }
 
  private:
-  void OnConnectionLost() {
+  void OnFactoryConnectionLost() {
     // After the connection to |ImeEngineFactoryRegistry| is broken, notifies
     // the client to reconnect through Window Service.
     if (engine_client_)
       engine_client_->Reconnect();
   }
 
+  void OnClientConnectionLost() { engine_client_.reset(); }
+
   InputMethodEngine* engine_;
   mojo::Binding<ime::mojom::ImeEngineFactory> factory_binding_;
   mojo::Binding<ime::mojom::ImeEngine> engine_binding_;
@@ -146,6 +161,14 @@ class MojoHelper : public ime::mojom::ImeEngine,
   ime::mojom::ImeEngineClientPtr engine_client_;
   ime::mojom::ImeEngineFactoryRegistryPtr registry_;
 
+  // Whether mutes the call of FinishInput().
+  // This is to guard the mis-ordered calls of FocusIn() & FocusOut() calls when
+  // switching between mojo-based and non-mojo-based clients.
+  // e.g. app_list window is non-mojo-based client, so need to guard the
+  // FocusOut() call from the mojo-based client because the app_list window's
+  // FocusIn() comes in first.
+  bool allow_finish_input_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(MojoHelper);
 };
 
@@ -185,6 +208,12 @@ bool InputMethodEngine::IsActive() const {
   return !active_component_id_.empty();
 }
 
+void InputMethodEngine::FocusIn(
+    const ui::IMEEngineHandlerInterface::InputContext& input_context) {
+  InputMethodEngineBase::FocusIn(input_context);
+  mojo_helper_->set_allow_finish_input(false);
+}
+
 void InputMethodEngine::PropertyActivate(const std::string& property_name) {
   observer_->OnMenuItemActivated(active_component_id_, property_name);
 }

chrome/browser/chromeos/input_method/input_method_engine.h

@@ -92,6 +92,8 @@ class InputMethodEngine : public ::input_method::InputMethodEngineBase {
   // InputMethodEngineBase overrides.
   void Enable(const std::string& component_id) override;
   bool IsActive() const override;
+  void FocusIn(const ui::IMEEngineHandlerInterface::InputContext& input_context)
+      override;
 
   // ui::IMEEngineHandlerInterface overrides.
   void PropertyActivate(const std::string& property_name) override;

chrome/browser/chromeos/input_method/input_method_engine_unittest.cc

@@ -424,6 +424,22 @@ TEST_F(InputMethodEngineTest, TestMojoInteractions) {
   engine_->CommitText(context, "input", &error);
   engine_->FlushForTesting();
   EXPECT_TRUE(client.commit_text_called());
+
+  engine_ptr->FinishInput();
+  engine_ptr.FlushForTesting();
+  EXPECT_EQ(ONBLUR, observer_->GetCallsBitmapAndReset());
+
+  // Switches from a mojo-based client to a non-mojo-based client.
+  engine_ptr->StartInput(ime::mojom::EditorInfo::New(
+      ui::TEXT_INPUT_TYPE_TEXT, ui::TEXT_INPUT_MODE_DEFAULT,
+      ui::TEXT_INPUT_FLAG_NONE, ui::TextInputClient::FOCUS_REASON_MOUSE,
+      false));
+  engine_ptr.FlushForTesting();
+  engine_ptr->FinishInput();
+  FocusIn(ui::TEXT_INPUT_TYPE_TEXT);
+  engine_ptr.FlushForTesting();
+  // Verifies no ONBLUR is called.
+  EXPECT_EQ(ONFOCUS, observer_->GetCallsBitmapAndReset());
 }
 
 }  // namespace input_method