Don't show the virtual keyboard if there's no text input client

content/browser/renderer_host/render_widget_host_view_aura.cc

Index: content/browser/renderer_host/render_widget_host_view_aura.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index 17b34eb542e63671df6b13eacc138b68f475534b..f84742fabde6afdc06de2cf7c74ea349a68ade7f 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -2302,9 +2302,10 @@ void RenderWidgetHostViewAura::OnUpdateTextInputStateCalled(
     GetInputMethod()->OnTextInputTypeChanged(this);
 
   const TextInputState* state = text_input_manager_->GetTextInputState();
-  if (state && state->show_ime_if_needed)
+  if (state && state->show_ime_if_needed &&
+      GetInputMethod()->GetTextInputType() != ui::TEXT_INPUT_TYPE_NONE) {

[sadrul, 2017/01/11 02:46:06]

Two alternate options that I think would be better:

 1. Move this check into InputMethod itself.
 2. Instead of checking the input-type, check that |this| is the current
input-client. This seems to be what ARC does as well [*].

The first option is probably going to be a bit tedious, because we have a few
InputMethod implementations, and you probably have to update
InputMethodObserver::OnShowImeIfNeeded() implementations too. The second option
would be fairly easy, and be consistent with what we do for ARC.

We could unify both farther with an API change like:
 3. Change InputMethod::ShowImeIfNeeded() to
InputMethod::ShowImeIfNeeded(TextInputClient*), so that the input-client lets
the IME know what client is requesting the keyboard to be made visible. The IME
can then make sure it's a valid input-client, and also that the input-client is
the currently active one.

[*] :
https://cs.chromium.org/chromium/src/components/arc/ime/arc_ime_service.cc?q=...

[afakhry, 2017/01/11 17:50:49]

Thank you very much for the detailed suggestions!

For suggestion (2), |this| (which is the RenderWidgetHostViewAura) is never the
current input-client. So this solution breaks showing the virtual keyboard
inside text fields and text areas in the webcontents.

For suggestion (1), when you said "you probably have to update
InputMethodObserver::OnShowImeIfNeeded() implementations too", did you mean to
forward the current input-client to the observers and let them decide? I was
thinking more like:

auto* client = GetTextInputClient();
if (client && client->GetTextInputType() != ui::TEXT_INPUT_TYPE_NONE) {
  for (InputMethodObserver& observer : observer_list_)
    observer.OnShowImeIfNeeded();
} 

[sadrul, 2017/01/11 17:53:27]

This is unexpected to me. RenderWidgetHostViewAura is-a TextInputClient. When it
has focus, what is the 'current' input client?
Yes.
I guess this would work too.

[afakhry, 2017/01/11 18:36:00]

Oops, sorry, I was looking at the *raw* pointer values without casting. Indeed
|this| == GetTextInputClient().

In this case, I opted for solution (2).

     GetInputMethod()->ShowImeIfNeeded();
-
+  }
 
   if (state && state->type != ui::TEXT_INPUT_TYPE_NONE) {
     // Start monitoring the composition information if the focused node is