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

content/browser/renderer_host/render_widget_host_view_base.h

Index: content/browser/renderer_host/render_widget_host_view_base.h
diff --git a/content/browser/renderer_host/render_widget_host_view_base.h b/content/browser/renderer_host/render_widget_host_view_base.h
index 8eb96897ecc8dc2feca83d74474372a6cdcf1550..687f16cd1822360cb0c10228ececfc15c30e3e58 100644
--- a/content/browser/renderer_host/render_widget_host_view_base.h
+++ b/content/browser/renderer_host/render_widget_host_view_base.h
@@ -66,6 +66,7 @@ class BrowserAccessibilityManager;
 class RenderWidgetHostViewBaseObserver;
 class SyntheticGesture;
 class SyntheticGestureTarget;
+class TextInputManager;
 class WebCursor;
 struct DidOverscrollParams;
 struct NativeWebKeyboardEvent;
@@ -126,6 +127,12 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView,
   // changed since the last time.
   bool HasDisplayPropertyChanged(gfx::NativeView view);
 
+  // Called by the TextInputManager to notify the view about being removed from
+  // the list of registered views, i.e., TextInputManager is no longer tracking
+  // TextInputState from this view. The RWHV should reset |text_input_manager_|
+  // to nullptr.
+  void DidUnregisterFromTextInputManager(TextInputManager* text_input_manager);
+
   base::WeakPtr<RenderWidgetHostViewBase> GetWeakPtr();
 
   //----------------------------------------------------------------------------
@@ -240,6 +247,9 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView,
                                                cc::SurfaceId original_surface,
                                                gfx::Point* transformed_point);
 
+  // Updates the state of the input method attached to the view.
+  virtual void TextInputStateChanged(const TextInputState& text_input_state);
+
   //----------------------------------------------------------------------------
   // The following static methods are implemented by each platform.
 
@@ -265,9 +275,6 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView,
   // Indicates whether the page has finished loading.
   virtual void SetIsLoading(bool is_loading) = 0;
 
-  // Updates the state of the input method attached to the view.
-  virtual void TextInputStateChanged(const TextInputState& params) = 0;
-
   // Cancel the ongoing composition of the input method attached to the view.
   virtual void ImeCancelComposition() = 0;
 
@@ -385,6 +392,19 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView,
 
   void NotifyObserversAboutShutdown();
 
+  // Returns a reference to the current instance of TextInputManager. The
+  // reference is obtained from RenderWidgetHostDelegate. The first time a
+  // reference is obtained (, which is not nullptr), its value is cached in

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

nit: Commas go before the open paren.  Also, you can just say "a non-null
reference" to flow more smoothly.

[EhsanK, 2016/05/30 15:06:08]

Acknowledged.

+  // |text_input_manager_|. Then this view is registered with TextInputManager
+  // (,i.e., the instance |text_input_manager_| referes to). The RWHV will

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

nit: No comma before i.e.

[EhsanK, 2016/05/30 15:06:08]

Done.

+  // unregister from the TextInputManager if it is destroyed or if the
+  // TextInputManager itself is destroyed. The unregistration of the RWHV from
+  // TextInputManager is necessary and must be done by explicitly calling
+  // TextInputManager::Unregister.
+  // It is safer to use this method when required rather than dereferencing

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

nit: Drop "when required"

[EhsanK, 2016/05/30 15:06:08]

Done.

+  // |text_input_manager_|.
+  TextInputManager* GetTextInputManager();
+
   // Whether this view is a popup and what kind of popup it is (select,
   // autofill...).
   blink::WebPopupType popup_type_;
@@ -412,7 +432,6 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView,
   // The current selection range relative to the start of the web page.
   gfx::Range selection_range_;
 
- protected:
   // The scale factor of the display the renderer is currently on.
   float current_device_scale_factor_;
 
@@ -423,6 +442,12 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView,
   // renderer.
   bool pinch_zoom_enabled_;
 
+  // A reference to current TextInputManager instance this RWHV is registered
+  // with. This is initially nullptr until the first time the view calls
+  // GetTextInputManager(). It can also become nullptr if the top-level
+  // RenderWidgetHostDelegate owning the TextInputManager is destroyed.

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

Isn't that the WebContents?  When does a RenderWidgetHostView outlive the
WebContents?

[EhsanK, 2016/05/30 15:06:08]

As far as I can tell, yes it is.
Not really. It is just we cannot guarantee the order of destruction. When
WebContentsImpl is deleted, we delete the frame tree and frame tree node for the
root. That is when we delete RenderWidgetHostViewAura. In fact, if I do not
reset text_input_manager_ to null when TextInputManager is destroyed, we get a
UaF when closing the window.

Also, for RenderWidgetHostViewChildFrame we use a DeleteSoon which I think could
actually make it go away after WebContents.

All this being said, I modified the comment to mention only TextInputManager not
the delegate.

[Charlie Reis, 2016/06/02 22:03:28]

Acknowledged.

+  TextInputManager* text_input_manager_;
+
  private:
   void FlushInput();