[refactor] - Move textInputInfo() and textInputType() from WebWidget to WebInputMethodController

content/renderer/render_widget.cc

Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index dc1f14d8a538aa4db7acaddb94aac06061415076..b2eaeff87597db5e71a535d4a6d389cc30ced5fc 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -1038,8 +1038,8 @@ void RenderWidget::UpdateTextInputState(ShowIme show_ime,
     return;  // Not considered as a text input field in WebKit/Chromium.
 
   blink::WebTextInputInfo new_info;
-  if (GetWebWidget())
-    new_info = GetWebWidget()->textInputInfo();
+  if (auto* controller = GetInputMethodController())
+    new_info = controller->textInputInfo();
   const ui::TextInputMode new_mode =
       ConvertWebTextInputMode(new_info.inputMode);
 
@@ -1742,8 +1742,8 @@ ui::TextInputType RenderWidget::GetTextInputType() {
   if (focused_pepper_plugin_)
     return focused_pepper_plugin_->text_input_type();
 #endif
-  if (GetWebWidget())
-    return ConvertWebTextInputType(GetWebWidget()->textInputType());
+  if (auto* controller = GetInputMethodController())
+    return ConvertWebTextInputType(controller->textInputType());
   return ui::TEXT_INPUT_TYPE_NONE;
 }
 
@@ -2172,7 +2172,8 @@ void RenderWidget::didHandleGestureEvent(
     UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME);
   } else if (event.type == WebInputEvent::GestureLongPress) {
     DCHECK(GetWebWidget());
-    if (GetWebWidget()->textInputInfo().value.isEmpty())
+    blink::WebInputMethodController* controller = GetInputMethodController();
+    if (controller && controller->textInputInfo().value.isEmpty())

[Charlie Reis, 2016/11/23 23:35:09]

Is this right?  This looks like it's hiding the IME if the TextInputInfo is
empty.  Maybe we should also be taking that branch if there's no controller? 
(Or is IF_NEEDED the right thing to do in that case?)

Just wondering if this should be:
  if (!controller || controller->textInputInfo...)

[EhsanK, 2016/11/24 02:27:45]

Yes. Apparently, this is because we should show a paste menu when long pressing
<input> and not IME. This is the CL that did this:
https://chromium.googlesource.com/chromium/src/+/07c70d207c4ed217286cdf842365...
That is very good question. The way I see it, there is two scenarios that will
lead to |controller| being null.

1- GetWebWidget() is not a frame widget which is a case we should not update
anything in my opinion. This is probably the case where WebViewImpl is returned
by GetWebWidget(), i.e., through navigating back and forward. But as I
understand RWH for the main frame is destroyed anyway so the TextInputState will
reset and there will be no Ime when it is destroyed.

2- We do have a focused frame, but it does not belong to this local root
(subtree). Which then begs the question, why are we handling a long gesture
here? So |controller| cannot be null.

Given all this, I think (1) is the only scenario were controller can be null and
in that case, we should not even bother sending an IPC back.

       UpdateTextInputState(ShowIme::HIDE_IME, ChangeSource::FROM_NON_IME);
     else
       UpdateTextInputState(ShowIme::IF_NEEDED, ChangeSource::FROM_NON_IME);