Browser Side Text Input State Tracking for OOPIF.

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 2d463287de60180ff5c1c2e3e3f41c96ae7d9f7b..dd34edf211b0d08216315ff7b391274b0dd06f59 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -361,6 +361,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
       bluetooth_device_connected_(false),
       virtual_keyboard_requested_(false),
       page_scale_factor_is_one_(true),
+      view_with_active_text_input_(nullptr),
       loading_weak_factory_(this),
       weak_factory_(this) {
   frame_tree_.SetFrameRemoveListener(
@@ -2240,6 +2241,38 @@ void WebContentsImpl::SendScreenRects() {
     browser_plugin_embedder_->DidSendScreenRects();
 }
 
+TextInputState WebContentsImpl::GetTextInputState() {
+  if (GetOuterWebContents())
+    return GetOuterWebContents()->GetTextInputState();
+  return text_input_state_;
+}
+
+void WebContentsImpl::UpdateTextInputState(RenderWidgetHostViewBase* rwhv,
+                                           bool text_input_state_changed) {
+  // If there is an outer WebContents, let it process this update.
+  if (GetOuterWebContents()) {
+    return GetOuterWebContents()->UpdateTextInputState(
+        rwhv, text_input_state_changed);
+  }
+
+  // If this RWHV has a text input type of NONE, then there is nothing to
+  // report to the tab RWHV.
+  if (view_with_active_text_input_ != rwhv &&

[Charlie Reis, 2016/03/30 21:52:47]

Is this comparison the only reason we need view_with_active_text_input_?  I'm
wondering if there's another way to track this without risking a use-after-free.

For example, suppose we kept the RenderWidgetHost's routing ID and process ID
around instead?  That would let us compare rwhv->GetRenderWidgetHost() against
the pair, without keeping an extra pointer to the RenderWidgetHostView.

Sorry that I'm being really conservative on this, but there's just way too much
code that has to be right to avoid a UaF here.  And even if there aren't many
uses of view_with_active_text_input_ now, I don't want future code in
WebContentsImpl to assume that possibly stale pointer is safe to use.

[EhsanK, 2016/03/30 23:36:33]

This makes a lot of sense...thanks! Following our discussion over hangouts, I
will use a WeakPtr given that RenderWidgetHostViewBase already supports that.

+      rwhv->text_input_state()->type == ui::TEXT_INPUT_TYPE_NONE)
+    return;
+
+  view_with_active_text_input_ =
+      (rwhv->text_input_state()->type != ui::TEXT_INPUT_TYPE_NONE) ? rwhv
+                                                                   : nullptr;
+  text_input_state_ = *rwhv->text_input_state();
+
+  // The top level RWHV should know about the change.
+  RenderWidgetHostViewBase* tab_rwhv =
+      static_cast<RenderWidgetHostViewBase*>(GetRenderWidgetHostView());
+  if (tab_rwhv)
+    tab_rwhv->UpdateInputMethodIfNecessary(text_input_state_changed);
+}
+
 BrowserAccessibilityManager*
     WebContentsImpl::GetRootBrowserAccessibilityManager() {
   RenderFrameHostImpl* rfh = GetMainFrame();