OOPIF IME: Renderer Side Changes

third_party/WebKit/public/web/WebWidget.h

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 225 matching lines @@
     virtual WebColor backgroundColor() const { return 0xFFFFFFFF; /* SK_ColorWHITE */ }
 
     // The currently open page popup, which are calendar and datalist pickers
     // but not the select popup.
     virtual WebPagePopup* pagePopup() const { return 0; }
 
     // Updates top controls constraints and current state. Allows embedder to
     // 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() { }
 };
 
 } // namespace blink
 
 #endif