Showing virtual keyboard when input type changed

components/arc/ime/arc_ime_service.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/arc/ime/arc_ime_service.h"
 
 #include "base/logging.h"
 #include "components/arc/ime/arc_ime_bridge_impl.h"
 #include "components/exo/shell_surface.h"
 #include "components/exo/surface.h"
@@ skipping 108 matching lines @@
     ui::InputMethod* const input_method = GetInputMethod();
     if (input_method)
       input_method->SetFocusedTextInputClient(this);
   }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Overridden from arc::ArcImeBridge::Delegate
 
 void ArcImeService::OnTextInputTypeChanged(ui::TextInputType type) {
-  if (ime_type_ == type)

[kinaba, 2016/06/14 04:22:08]

Although it's a matter of taste, can we keep this early exit logic as-is?

[takaoka, 2016/06/14 05:55:51]

Done.

-    return;
+  ui::TextInputType previous_type = ime_type_;
   ime_type_ = type;
-
   ui::InputMethod* const input_method = GetInputMethod();
-  if (input_method) {
+  if (input_method && ime_type_ != previous_type) {
     input_method->OnTextInputTypeChanged(this);
-    if (input_method->GetTextInputClient() == this &&
-        ime_type_ != ui::TEXT_INPUT_TYPE_NONE) {
-      // TODO(kinaba): crbug.com/581282. This is tentative short-term solution.
-      //
-      // For fully correct implementation, rather than to piggyback the "show"
-      // request with the input type change, we need dedicated IPCs to share the
-      // virtual keyboard show/hide states between Chromium and ARC.
-      input_method->ShowImeIfNeeded();

[kinaba, 2016/06/14 04:22:08]

Removing this code at once causes a regression in New-Chrome + Old-ARC
combination.

Is it possible to leave this code as is for a while (I hope it's harmless),
at least until we get new ARC image rolled to the Chrome OS build.

[takaoka, 2016/06/14 05:55:51]

Done.

-    }
   }
 }
 
 void ArcImeService::OnCursorRectChanged(const gfx::Rect& rect) {
   if (cursor_rect_ == rect)
     return;
   cursor_rect_ = rect;
 
   ui::InputMethod* const input_method = GetInputMethod();
   if (input_method)
     input_method->OnCaretBoundsChanged(this);
 }
 
 void ArcImeService::OnCancelComposition() {
   ui::InputMethod* const input_method = GetInputMethod();
   if (input_method)
     input_method->CancelComposition(this);
 }
 
+void ArcImeService::ShowImeIfNeeded() {
+  ui::InputMethod* const input_method = GetInputMethod();
+  if (input_method && input_method->GetTextInputClient() == this) {
+    input_method->ShowImeIfNeeded();
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Oberridden from ui::TextInputClient:
 
 void ArcImeService::SetCompositionText(
     const ui::CompositionText& composition) {
   has_composition_text_ = !composition.text.empty();
   ime_bridge_->SendSetCompositionText(composition);
 }
 
 void ArcImeService::ConfirmCompositionText() {
@@ skipping 105 matching lines @@
 bool ArcImeService::ChangeTextDirectionAndLayoutAlignment(
     base::i18n::TextDirection direction) {
   return false;
 }
 
 bool ArcImeService::IsEditCommandEnabled(int command_id) {
   return false;
 }
 
 }  // namespace arc