Handle newCursorPosition correctly for Android's commitText()

third_party/WebKit/Source/web/WebViewImpl.cpp

Index: third_party/WebKit/Source/web/WebViewImpl.cpp
diff --git a/third_party/WebKit/Source/web/WebViewImpl.cpp b/third_party/WebKit/Source/web/WebViewImpl.cpp
index 218f70fe9f3dfa61695497b476055adf4a9d0b69..dfa198acd7008ee6b11f6249e40b6071d0667698 100644
--- a/third_party/WebKit/Source/web/WebViewImpl.cpp
+++ b/third_party/WebKit/Source/web/WebViewImpl.cpp
@@ -2368,32 +2368,51 @@ bool WebViewImpl::setComposition(
     return text.isEmpty() || inputMethodController.hasComposition();
 }
 
-bool WebViewImpl::confirmComposition()
+bool WebViewImpl::confirmComposition(int newCursorPosition)
 {
-    return confirmComposition(DoNotKeepSelection);
+    return confirmComposition(DoNotKeepSelection, newCursorPosition);
 }
 
-bool WebViewImpl::confirmComposition(ConfirmCompositionBehavior selectionBehavior)
+bool WebViewImpl::confirmComposition(ConfirmCompositionBehavior selectionBehavior, int newCursorPosition)
 {
-    return confirmComposition(WebString(), selectionBehavior);
+    return confirmComposition(WebString(), selectionBehavior, newCursorPosition);
 }
 
-bool WebViewImpl::confirmComposition(const WebString& text)
+bool WebViewImpl::confirmComposition(const WebString& text, int newCursorPosition)
 {
     UserGestureIndicator gestureIndicator(DefinitelyProcessingNewUserGesture);
-    return confirmComposition(text, DoNotKeepSelection);
+    return confirmComposition(text, DoNotKeepSelection, newCursorPosition);
 }
 
-bool WebViewImpl::confirmComposition(const WebString& text, ConfirmCompositionBehavior selectionBehavior)
+bool WebViewImpl::confirmComposition(const WebString& text, ConfirmCompositionBehavior selectionBehavior, int newCursorPosition)
 {
     LocalFrame* focused = toLocalFrame(focusedCoreFrame());
     if (!focused || !m_imeAcceptEvents)
         return false;
 
+    InputMethodController& inputMethodController = focused->inputMethodController();
+    if (inputMethodController.hasComposition()) {
+        if (newCursorPosition < 1) {
+            newCursorPosition = newCursorPosition + textInputInfo().compositionStart;

[Changwan Ryu, 2016/05/24 08:35:50]

1. Don't call textInputInfo() repeatedly, as it is quite an expensive function.
2. InputMethodController has access to m_compositionRange. Why do you have to
call textInputInfo().compositionStart? Is it because of
WebPlugin::confirmComposition()? How did you test it?

[yabinh, 2016/07/06 11:44:30]

InputMethodController::m_compositionRange doesn't work for multiple nodes,
because the offset in InputMethodController::m_compositionRange is based on its
anchor node, not rootEditableElement.

textInputInfo() works for multiple nodes.

BTW, I'll upload a new patch once another cl is committed:
https://codereview.chromium.org/2020973002/

+        } else {
+            newCursorPosition = newCursorPosition + textInputInfo().compositionEnd - 1;
+        }
+    } else {
+        if (newCursorPosition < 1) {
+            newCursorPosition = newCursorPosition + textInputInfo().selectionStart;
+        } else {
+            newCursorPosition = newCursorPosition + textInputInfo().selectionStart + text.length() - 1;
+        }
+    }
+
     if (WebPlugin* plugin = focusedPluginIfInputMethodSupported(focused))
-        return plugin->confirmComposition(text, selectionBehavior);
+        return plugin->confirmComposition(text, selectionBehavior, newCursorPosition);
+
+    if (selectionBehavior == KeepSelection)

[Changwan Ryu, 2016/05/24 08:35:50]

Why couldn't you pass selectionBehavior to confirmCompositionOrInsertText()? I
think it's always better to put the logic in one place than spread it around.

+        return inputMethodController.confirmCompositionOrInsertText(text, InputMethodController::KeepSelection);
 
-    return focused->inputMethodController().confirmCompositionOrInsertText(text, selectionBehavior == KeepSelection ? InputMethodController::KeepSelection : InputMethodController::DoNotKeepSelection);
+    return inputMethodController.confirmCompositionOrInsertText(text, InputMethodController::DoNotKeepSelection)
+        && inputMethodController.setEditableSelectionOffsetsWithBoundaryCheck(newCursorPosition, newCursorPosition);
 }
 
 bool WebViewImpl::compositionRange(size_t* location, size_t* length)