Browser Side TextInputState Tracking 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 3b75d36aca95cf2e63c1f29109057b2f2b796ec4..cf4786e2f34a19676f198585ce17081a0e947a0c 100644
--- a/content/browser/renderer_host/text_input_manager.cc
+++ b/content/browser/renderer_host/text_input_manager.cc
@@ -15,7 +15,7 @@ namespace content {
 
 namespace {
 
-bool AreDifferentTextInputStates(const content::TextInputState& old_state,
+bool ShouldUpdateTextInputState(const content::TextInputState& old_state,
                                  const content::TextInputState& new_state) {
 #if defined(USE_AURA)
   return old_state.type != new_state.type || old_state.mode != new_state.mode ||
@@ -25,8 +25,8 @@ bool AreDifferentTextInputStates(const content::TextInputState& old_state,
   return old_state.type != new_state.type ||
          old_state.can_compose_inline != new_state.can_compose_inline;
 #else

[Shu Chen, 2016/11/21 02:10:53]

Can you please add #elif defined(OS_ANDROID) for this?
And "NOTREACHED();" for #else for other platforms?

[EhsanK, 2016/11/21 20:44:21]

Acknowledged and done. Although I am not sure what platform would be neither
Aura, Mac, or Android and for that platform, what is the best fallback behavior
(return false or true?).

[Charlie Reis, 2016/11/23 23:27:21]

If you really don't want it to be handled, you can put #error so it won't
compile.  But I think there will be other corner case platforms that don't fall
into these buckets.  (I don't know much about Ozone, for example.)  I think it's
fine to just pick a value for now, since people on that platform will have to
decide when they hit your NOTREACHED.

[EhsanK, 2016/11/24 03:59:45]

Acknowledged.

-  // TODO(ekaramad): Implement the logic for other platforms (crbug.com/578168).
-  NOTREACHED();
+  // On Android, TextInputState update is sent only if there is some change in
+  // the state. So the new state is always different.
   return true;
 #endif
 }
@@ -57,8 +57,12 @@ RenderWidgetHostImpl* TextInputManager::GetActiveWidget() const {
                         : nullptr;
 }
 
-const TextInputState* TextInputManager::GetTextInputState() const {
-  return !!active_view_ ? &text_input_state_map_.at(active_view_) : nullptr;
+const TextInputState* TextInputManager::GetTextInputState(
+    RenderWidgetHostViewBase* view) const {
+  DCHECK(!view || IsRegistered(view));
+  if (!view)
+    view = active_view_;
+  return !!view ? &text_input_state_map_.at(view) : nullptr;
 }
 
 const TextInputManager::SelectionRegion* TextInputManager::GetSelectionRegion(
@@ -105,8 +109,8 @@ void TextInputManager::UpdateTextInputState(
 
   // Since |view| is registered, we already have a previous value for its
   // TextInputState.
-  bool changed = AreDifferentTextInputStates(text_input_state_map_[view],
-                                             text_input_state);
+  bool changed = ShouldUpdateTextInputState(text_input_state_map_[view],
+                                            text_input_state);
 
   text_input_state_map_[view] = text_input_state;