Remove logic to reset input method more than needed

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

 /*
  * Copyright (C) 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 230 matching lines @@
   frame().selection().setSelection(
       SelectionInDOMTree::Builder().setBaseAndExtent(range).build(), 0);
 }
 
 bool InputMethodController::finishComposingText(
     ConfirmCompositionBehavior confirmBehavior) {
   if (!hasComposition())
     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());
 
     const String& composing = composingText();
     const bool result = replaceComposition(composing);
 
     // TODO(xiaochengh): The use of updateStyleAndLayoutIgnorePendingStylesheets
     // 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);
 
     return result;
   }
 
   return replaceCompositionAndMoveCaret(composingText(), 0,
                                         Vector<CompositionUnderline>());
 }
@@ skipping 162 matching lines @@
     return;
 
   // An open typing command that disagrees about current selection would cause
   // issues with typing later on.
   TypingCommand::closeTyping(m_frame);
 
   // No DOM update after 'compositionend'.
   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) {
   const Position& adjustedPosition = previousPositionOf(
       nextPositionOf(position, PositionMoveType::GraphemeCluster),
       PositionMoveType::GraphemeCluster);
   DCHECK_EQ(position.anchorNode(), adjustedPosition.anchorNode());
   DCHECK_GE(position.computeOffsetInContainerNode(),
             adjustedPosition.computeOffsetInContainerNode());
   return static_cast<size_t>(position.computeOffsetInContainerNode() -
@@ skipping 603 matching lines @@
   frame().chromeClient().resetInputMethod();
 }
 
 DEFINE_TRACE(InputMethodController) {
   visitor->trace(m_frame);
   visitor->trace(m_compositionRange);
   SynchronousMutationObserver::trace(visitor);
 }
 
 }  // namespace blink