Request composition info from the focused RenderWidget

content/browser/renderer_host/render_widget_host_view_mac.mm

Index: content/browser/renderer_host/render_widget_host_view_mac.mm
diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm
index fa22008815a79b8747088da8dfaeb84b2c7bd229..a947b182a3fb7058125aee17dda09426a14ad435 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -885,13 +885,27 @@ void RenderWidgetHostViewMac::OnUpdateTextInputStateCalled(
   if (!did_update_state)
     return;
 
+  // Only the widget with a focused text field should be computing composition
+  // info, i.e., if the current TextInputState is NONE, then the widget
+  // corresponding to |updated_view| should stop monitoring composition info. If
+  // state is anything other than NONE, the widget should start monitoring the
+  // composition info. TextInputManager guarantees that in two consecutive calls
+  // to this method, if |updated_view| are different, then the first call must
+  // have had a state->type of NONE and the second call must have had a
+  // none-NONE state, i.e., it is safe to set/unset monitoring based on current
+  // state and |updated_view|.

[Charlie Reis, 2016/09/30 22:39:16]

How is this comment related to the null check below?  Is it just a separate
observation?

It also seems like an indirect way to explain why we need to use updated_view's
RWH instead of render_widget_host_.  Is that what the first sentence is saying? 
I'm wondering if there's a way to make this clearer.

[EhsanK, 2016/10/05 16:39:02]

A separate observation. I left a new comment for the null check though. Please
see if it is useful.
Sort of...yes. I modified the explanation a bit. But FWIW, I will explain a few
issues here:

1- We would like to make sure only the RWH with active TextInputState monitor
composition info. For that, when RWH is active we send active IPC and when the
state is NONE we send stop IPC.

2- If TextInputManager calls this method when there is active TextInputState,
the very next call is always for a TextInputState of NONE, and vice versa.

3- When state is Text, we have a TextInputManager::GetActiveWidget() return the
active widget which is the one corresponding to |updated_view|. But when state
becomes NONE, GetActiveWidget() returns nullptr. But the widget which lost its
state (if it still exists) is the one corresponding to |updated_view|. So we
could as well only use |updated_view| to start/stop monitoring.

4- This behavior, to activate and deactivate monitoring was also implemented for
Aura. But we do not use |updated_view| there and rather, try to remember the
last widget which had active TextInputState (due to the fact that when we lose
state, GetActiveWidget() is nullptr so we have no clue which widget should stop
monitoring). So we stored the routing and process IDs to be safe. The reason we
did not use |updated_view| was that
RenderWidgetHostViewBase::GetRenderWidgetHost() is pure virtual. In the
destruction path of RWHVs, and inside RWHVBase dtor we make a call to
TextInputManager:Unregister(this) to free up memory which comes back here is the
|updated_view| had active TextInputState before. That of course was a crash back
then.

So in the comments here I wanted to emphasize that since TextInputManager
behaves as in item 2, item 1 is do able by just sending IPCs to the
|updated_view|.

Sorry for the long response. I hope it helps.

+  RenderWidgetHost* widgetHost = updated_view->GetRenderWidgetHost();
+  if (!widgetHost)
+    return;
+
   // Set the monitor state based on the text input focus state.
   const bool has_focus = HasFocus();
   const TextInputState* state = text_input_manager->GetTextInputState();
   bool need_monitor_composition =
       has_focus && state && state->type != ui::TEXT_INPUT_TYPE_NONE;
-  Send(new InputMsg_RequestCompositionUpdate(
-      render_widget_host_->GetRoutingID(), false /* immediate request */,
+
+  widgetHost->Send(new InputMsg_RequestCompositionUpdate(
+      widgetHost->GetRoutingID(), false /* immediate request */,
       need_monitor_composition));
 
   if (has_focus) {