[reland] Browser Side Text Input State Tracking for OOPIF (Aura Only)

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
new file mode 100644
index 0000000000000000000000000000000000000000..9c5bd3ad88d0c2a3d17e0ff043a160310c22b31f
--- /dev/null
+++ b/content/browser/renderer_host/text_input_manager.cc
@@ -0,0 +1,122 @@
+// 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_view_base.h"
+
+namespace content {
+
+namespace {
+
+bool AreDifferentTextInputStates(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 ||
+         old_state.flags != new_state.flags ||
+         old_state.can_compose_inline != new_state.can_compose_inline;
+#else
+  // TODO(ekaramad): Implement this logic for other platforms.

[Charlie Reis, 2016/05/18 20:46:04]

Maybe add a NOTREACHED()?  We don't want to forget about this when adding more
platforms.

[EhsanK, 2016/05/24 20:42:45]

Done. Also, added #ifdef USE_AURA in
RenderWidgetHostViewBase::TextInputStateChanged. Before, we were hitting this
line but it was not affecting IME on Android and Mac since they still override
TextInputManager.

+  return true;
+#endif
+}
+
+}  // namespace
+
+TextInputManager::TextInputManager() : active_view_(nullptr) {}
+
+TextInputManager::~TextInputManager() {
+  FOR_EACH_OBSERVER(Observer, observer_list_,
+                    OnDestroyingTextInputManager(this));
+}
+
+const TextInputState* TextInputManager::GetTextInputState() {
+  return !!active_view_ ? &text_input_state_map_[active_view_] : nullptr;
+}
+
+RenderWidgetHostViewBase* TextInputManager::GetActiveView() const {
+  return active_view_;
+}
+
+void TextInputManager::UpdateTextInputState(
+    RenderWidgetHostViewBase* view,
+    const TextInputState& text_input_state) {
+  DCHECK(IsRegisteredView(view));
+
+  bool changed = AreDifferentTextInputStates(text_input_state_map_[view],

[Charlie Reis, 2016/05/18 20:46:04]

What if text_input_state_map_[view] were null?  AreDifferentTextInputStates
would crash.  Seems like many of the other uses of the map include null checks,
so I'm not sure if there's a reason this is safe.  (If so, please document.)

[EhsanK, 2016/05/24 20:42:45]

On line 45, we verify that the view is registered, i.e., the key exists in the
map. The value is also a struct (not a pointer). Also, when registering the
view, the struct is initialized to TextInputState() (line 67).

So I believe we are fine. I added a comment re Registering.

[Charlie Reis, 2016/05/26 06:22:03]

Hopefully so.  The IsRegistered check is a DCHECK, which means it isn't present
in release builds.  I suppose it'll be easily diagnosed if we see crashes in
AreDifferentTextInputStates, though.

+                                             text_input_state);
+
+  text_input_state_map_[view] = text_input_state;
+
+  // |active_view_| is only updated when the state for |view| is not none.
+  if (text_input_state.type != ui::TEXT_INPUT_TYPE_NONE)
+    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::Register(RenderWidgetHostViewBase* view) {
+  DCHECK(!IsRegisteredView(view));
+
+  text_input_state_map_[view] = TextInputState();
+  view->AddObserver(this);
+}
+
+void TextInputManager::Unregister(RenderWidgetHostViewBase* view) {
+  DCHECK(IsRegisteredView(view));
+
+  text_input_state_map_.erase(view);
+  if (active_view_ == view) {
+    active_view_ = nullptr;
+    NotifyObserversAboutInputStateUpdate(view, true);
+  }
+  view->RemoveObserver(this);
+}
+
+bool TextInputManager::IsRegisteredView(RenderWidgetHostViewBase* view) const {
+  return text_input_state_map_.count(view) == 1;
+}
+
+void TextInputManager::AddObserver(Observer* observer) {
+  observer_list_.AddObserver(observer);
+}
+
+void TextInputManager::RemoveObserver(Observer* observer) {
+  observer_list_.RemoveObserver(observer);
+}
+
+void TextInputManager::OnRenderWidgetHostViewBaseDestroyed(
+    RenderWidgetHostViewBase* view) {
+  DCHECK(IsRegisteredView(view));
+
+  Unregister(view);
+
+  if (observer_list_.HasObserver(view))
+    RemoveObserver(view);
+
+  if (active_view_ == view) {

[Charlie Reis, 2016/05/18 20:46:04]

Didn't we just do this block in Unregister on line 98?  I don't see how we could
hit it again.  RemoveObserver doesn't run observer code, does it?

[EhsanK, 2016/05/24 20:42:45]

You are right and the DCHECK was impossible to hit. However, I have now removed
this method as TextInputManager is no longer observing RenderWidgetHostViewBase.

+    DCHECK(text_input_state_map_[active_view_].type !=
+           ui::TEXT_INPUT_TYPE_NONE);
+    active_view_ = nullptr;
+    NotifyObserversAboutInputStateUpdate(view, true);
+  }
+}
+
+void TextInputManager::NotifyObserversAboutInputStateUpdate(
+    RenderWidgetHostViewBase* updated_view,
+    bool state_changed) {
+  if (state_changed) {
+    FOR_EACH_OBSERVER(Observer, observer_list_,
+                      OnTextInputStateUpdated(this, updated_view));
+  }
+  FOR_EACH_OBSERVER(Observer, observer_list_,
+                    OnTextInputStateUpdateCalled(this));
+}
+
+}  // namespace content