Remove logic to reset input method more than needed

third_party/WebKit/Source/core/editing/InputMethodController.cpp

Index: third_party/WebKit/Source/core/editing/InputMethodController.cpp
diff --git a/third_party/WebKit/Source/core/editing/InputMethodController.cpp b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
index 1b11d96ae61cb76ff1d74a0a80537ee9d18c8bf5..b329eecad3110e8891d2862f63706a89b83229a2 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -248,6 +248,10 @@ bool InputMethodController::finishComposingText(
     return false;
 
   if (confirmBehavior == KeepSelection) {
+    // Do not dismiss handles if we are not moving selection.
+    int selectionOptions = frame().selection().isHandleVisible()
+                               ? FrameSelection::HandleVisible
+                               : 0;

[Changwan Ryu, 2017/01/19 09:13:39]

yosin@, could you take another look at this part? Without this, insertion
handles get dismissed when you compose and then long-press text.

[yosin_UTC9, 2017/01/19 09:29:51]

This is yet another case |FrameSeleciton.isHandleVisible()| causes bug.
We should change FrameSeleciton::setSelection() to have default to make handle
visible.

See https://crrev.com/2642913003 Select All should show handles if they were
already present

[Changwan Ryu, 2017/01/19 11:42:59]

Thanks for the pointer.
I considered fixing FrameSelection::setSelectionAlgorithm() but I didn't because
of the complexity of the alternative solution.

In most cases we do want to dismiss handles when selection change occurs. This
may be default value (options without HandleVisible) or more explicit option
such as HandleVisibleIfSelectionDoesNotChange . Or maybe we should change
HandleVisible to something like ShowHandles.

But in the case of finishComposingText(), replaceComposition moves selection to
one place, and setSelectionOffsets moves selection back to the original offsets.
But we still don't want to dismiss handles.
So we may need something like SelectionHandleScope such that handle visibility
can be kept throughout the changes. In this sense, this may be something
perpendicular to what Pedro is trying to solve.

What do you think?

     PlainTextRange oldOffsets = getSelectionOffsets();
     Editor::RevealSelectionScope revealSelectionScope(&editor());
 
@@ -258,7 +262,7 @@ bool InputMethodController::finishComposingText(
     // needs to be audited. see http://crbug.com/590369 for more details.
     document().updateStyleAndLayoutIgnorePendingStylesheets();
 
-    setSelectionOffsets(oldOffsets);
+    setSelectionOffsets(oldOffsets, selectionOptions);
 
     // No DOM update after 'compositionend'.
     dispatchCompositionEndEvent(frame(), composing);
@@ -441,22 +445,6 @@ void InputMethodController::cancelComposition() {
   dispatchCompositionEndEvent(frame(), emptyString());
 }
 
-void InputMethodController::cancelCompositionIfSelectionIsInvalid() {
-  if (!hasComposition() || editor().preventRevealSelection())
-    return;
-
-  // Check if selection start and selection end are valid.
-  FrameSelection& selection = frame().selection();
-  if (!selection.isNone() && !m_compositionRange->collapsed()) {
-    if (selection.start().compareTo(m_compositionRange->startPosition()) >= 0 &&
-        selection.end().compareTo(m_compositionRange->endPosition()) <= 0)
-      return;
-  }
-
-  cancelComposition();
-  frame().chromeClient().didCancelCompositionOnSelectionChange();
-}
-
 // If current position is at grapheme boundary, return 0; otherwise, return the
 // distance to its nearest left grapheme boundary.
 static size_t computeDistanceToLeftGraphemeBoundary(const Position& position) {