In TextInputManager, reset input type to none before switching active widgets.

content/browser/renderer_host/text_input_manager.cc

 // Copyright 2016 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/text_input_manager.h"
 
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/common/view_messages.h"
 #include "ui/gfx/geometry/rect.h"
@@ skipping 66 matching lines @@
     const TextInputState& text_input_state) {
   DCHECK(IsRegistered(view));
 
   // Since |view| is registgered, we already have a previous value for its
   // TextInputState.
   bool changed = AreDifferentTextInputStates(text_input_state_map_[view],
                                              text_input_state);
 
   text_input_state_map_[view] = text_input_state;
 
   // |active_view_| is only updated when the state for |view| is not none.

[Charlie Reis, 2016/07/21 23:48:18]

Let's update this as well.  Maybe that it "only needs to be updated if it is
changing and the state for |view| is not NONE."

[EhsanK, 2016/07/25 21:09:57]

Acknowledged.

-  if (text_input_state.type != ui::TEXT_INPUT_TYPE_NONE)
+  if (text_input_state.type != ui::TEXT_INPUT_TYPE_NONE &&
+      active_view_ != view) {
+    // |view| will become the new |active_view_|, which means the IPC to reset
+    // the TextInputState for |active_view_| has not arrived yet (and will be
+    // ignored later). So we should reset the TextInputState for |active_view_|
+    // manually.
+    if (active_view_)
+      UpdateTextInputState(active_view_, TextInputState());

[Charlie Reis, 2016/07/21 23:48:18]

This has a small risk of becoming an infinite recursion if we ever change the
default |type| on TextInputState.  It would be safer (and clearer) if you set
the state to TEXT_INPUT_TYPE_NONE explicitly.

[EhsanK, 2016/07/25 21:09:57]

The only reason I used recursion like this was to 1) reduce lines of code and 2)
let initialization of TextInputState happen organically through ctor.

I actually did not think of potential recursion if default type changes which is
an interesting point. But another reason for NOT having recursion could be the
discussion in this CL:
https://codereview.chromium.org/1894473002/

Where by calling TextInputStateChanged we end up sending an IPC to renderer
which is a waste of resources; or we could avoid that and endorse a crash on
input thread in android which is even worse.

I will try to implement this explicitly and avoid recursion for the reasons
above.

     active_view_ = view;
+  }
 
   // If the state for |active_view_| is none, then we no longer have an
   // |active_view_|.
   if (active_view_ == view && text_input_state.type == ui::TEXT_INPUT_TYPE_NONE)
     active_view_ = nullptr;
 
   NotifyObserversAboutInputStateUpdate(view, changed);
 }
 
 void TextInputManager::ImeCancelComposition(RenderWidgetHostViewBase* view) {
@@ skipping 126 matching lines @@
 TextInputManager::SelectionRegion::SelectionRegion(
     const SelectionRegion& other) = default;
 
 TextInputManager::CompositionRangeInfo::CompositionRangeInfo() {}
 
 TextInputManager::CompositionRangeInfo::CompositionRangeInfo(
     const CompositionRangeInfo& other) = default;
 
 TextInputManager::CompositionRangeInfo::~CompositionRangeInfo() {}
 
-}  // namespace content
+}  // namespace content