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

content/browser/renderer_host/render_widget_host_view_aura.cc

 // Copyright (c) 2012 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 "content/browser/renderer_host/render_widget_host_view_aura.h"
 
 #include <set>
 #include <utility>
 
 #include "base/auto_reset.h"
@@ skipping 2284 matching lines @@
     bool did_update_state) {
   DCHECK_EQ(text_input_manager_, text_input_manager);
 
   if (!GetInputMethod())
     return;
 
   if (did_update_state)
     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
     // editable.
     RenderWidgetHostImpl* last_active_widget =
         text_input_manager_->GetActiveWidget();
     last_active_widget_routing_id_ = last_active_widget->GetRoutingID();
     last_active_widget_process_id_ = last_active_widget->GetProcess()->GetID();
     last_active_widget->Send(new InputMsg_RequestCompositionUpdate(
         last_active_widget->GetRoutingID(), false /* immediate request */,
@@ skipping 65 matching lines @@
 
 void RenderWidgetHostViewAura::SetPopupChild(
     RenderWidgetHostViewAura* popup_child_host_view) {
   popup_child_host_view_ = popup_child_host_view;
   event_handler_->SetPopupChild(
       popup_child_host_view,
       popup_child_host_view ? popup_child_host_view->event_handler() : nullptr);
 }
 
 }  // namespace content