OOPIF IME: Renderer Side Changes

third_party/WebKit/public/web/WebWidget.h

Index: third_party/WebKit/public/web/WebWidget.h
diff --git a/third_party/WebKit/public/web/WebWidget.h b/third_party/WebKit/public/web/WebWidget.h
index bfaa2ee40911e73d342ea3ac7e874c1ba99ccbe1..e8525edc6f0cbf7c508e212ac9e6bf8ca9a46ff6 100644
--- a/third_party/WebKit/public/web/WebWidget.h
+++ b/third_party/WebKit/public/web/WebWidget.h
@@ -243,6 +243,14 @@ public:
     // control what are valid states for top controls and if it should animate.
     virtual void updateTopControlsState(WebTopControlsState constraints, WebTopControlsState current, bool animate) { }
 
+    // Populate |bounds| with the composition character bounds for the ongoing
+    // composition. Returns false if there is no focused input or any ongoing
+    // composition.
+    virtual bool getCompositionCharacterBounds(WebVector<WebRect>& bounds) { return false; }
+
+    // Applies the range from |start| to |start + length| on the foucsed frame so that the text will later be replaced.
+    virtual void applyReplacementRange(int start, int length) {}

[dcheng, 2016/07/12 14:43:21]

Does it make sense to move these to WebFrameWidget instead? I took a look at the
WebWidget implementations. It doesn't seem like anything other than frames is
going to bother implementing this interface anyway.

(Ideally, I would also like to hide the pre-existing methods on WebView and
force callers to go through WebViewFrameWidget, but that is somewhat outside the
scope of this change)

[EhsanK, 2016/07/12 15:13:24]

Sorry if the question is simplistic: I am making these calls from inside
RenderWidget which only knows a WebWidget, right? Can we know about
WebFrameWidget from inside content/renderer?

Also, another approach could be to remove applyReplacement() and add a  range
argument to both WebWidget::setComposition and WebWidget::confirmComposition.

All this being said, if we go for helper class, say TextInputHelper obtained
from WebWidget::getTextInputHelper() and then add all these methods to that,
would it be a better solution?

[dcheng, 2016/07/18 16:38:33]

WebLocalFrame has a frameWidget method (which should probably return a
WebFrameWidget, not a WebWidget). Maybe RenderWidget itself should know when it
has a frame widget associated with it though... either way, this is fine for a
followup.
I don't really know enough about IME to say what the right design here is. I
guess this is fine (since my comment applies to all the methods added here)
I think making this WebFrameWidget specific would be better for now.

[EhsanK, 2016/07/19 05:37:32]

I think moving common logic in IME to a base class would be good. Later when the
separation is complete, we can remove the base class and move methods back to
Widget. I too am not certain of creating a helper class for Ime. It might be
neater.

The problem with RenderWidget is that, right now, it has little code (if any)
using RenderFrame or WebFrame. I will ask avi@ about that again since he was
working on separating RenderViewImpl and RenderWidget.

+
 protected:
     ~WebWidget() { }
 };