Fix TextInputManager for Android

content/browser/renderer_host/text_input_manager.cc

Index: content/browser/renderer_host/text_input_manager.cc
diff --git a/content/browser/renderer_host/text_input_manager.cc b/content/browser/renderer_host/text_input_manager.cc
index c472b34ce0e8c6a4d4ab5af005e1d56b55882a83..6a96396af7686cc5de3898159785027a0511e725 100644
--- a/content/browser/renderer_host/text_input_manager.cc
+++ b/content/browser/renderer_host/text_input_manager.cc
@@ -102,7 +102,11 @@ void TextInputManager::UpdateTextInputState(
     // already synthesized the loss of TextInputState for the |view| before (see
     // below). So we can forget about this method ever being called (no observer
     // calls necessary).
+    // NOTE: Android requires state to be returned even when the current state
+    // is/becomes NONE. Otherwise IME may become irresponsive.
+#if !defined(OS_ANDROID)
     return;

[EhsanK, 2017/03/10 15:43:16]

This should be good since we will not have more than one RWHV on a page to
overwrite each other's state to begin with. And as I understand we were throwing
away some state updates from renderer when the type was NONE, which seems
incorrect.

(If everything is working fine for normal Android then ignore the rest of this
comment pls).

But wouldn't we also have to change the code inside
RenderWidgetHostViewAndroid::OnUpdateTextInputStateCalled() a bit?
Right now the state it is reporting to content_view_core_ is the one from active
widget, which is the one with TextInputState.type != NONE. Don't we need to
sometimes update the ime_adapter_ with states where the type is NONE?

If that is the case then I suggest changing the signature of GetTextInputState
to pass an optional parameter for view and get the state from the view, e.g.:

void TextInputState::GetTextInputState(RenderWidgetHostViewBase* view = nullptr)
{
if (view == nullptr) view = active_view_;
return view ? text_input_state_map_[view] : nullptr;
}

And inside RenderWidgetHostViewAndroid::OnUpdateTextInputStateCalled do:

const TextInputState* state = text_input_manager_->IsRegisteredView(view) ?
text_input_manager_->GetTextInputState(view) : nullptr.

The last line is to make sure we are not in destruction (view is still
registered) because when a RWHV is destroyed the TextInputManager does not hold
its state anymore (not doing this causes crash https:://crbug.com/669375).

I also need to rework this anyway and base it on focused RenderWidgetHost since
this currently might break things for OOPIF Android (https://crbug.com/631798).

[Changwan Ryu, 2017/03/10 16:01:38]

As long as it returns something, it should work correctly.

[EhsanK, 2017/03/10 16:46:18]

Right. So here:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_wid...

if active_view_= nullptr (which is the case in the beginning or might be set at
line 142 here), we will be updating content view core with TextInputState()
default and seems to be OK. Then I think this is fine.

Also I will soon upload a patch to possibly change this whole method to use the
focused widget. I don't think relying on just TextInputState.type is good enough
(not for android). I will add you as reviewer to that CL.

+#endif
   }
 
   // Since |view| is registered, we already have a previous value for its