Implement ThreadedInputConnection.deleteSurroundingTextInCodePoints()

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 4debf150004b75497f83efa480177e71d5c8a50a..e01fa4f435e3948bf3434306b9cddab1b97cbdad 100644
--- a/third_party/WebKit/Source/core/editing/InputMethodController.cpp
+++ b/third_party/WebKit/Source/core/editing/InputMethodController.cpp
@@ -184,6 +184,83 @@ AtomicString getInputModeAttribute(Element* element) {
   return element->fastGetAttribute(HTMLNames::inputmodeAttr).lower();
 }
 
+bool hasInvalidSurrogatePair(const UChar* text,
+                             const int start,
+                             const int end) {
+  for (int i = start; i < end; i++) {
+    if (U_IS_SURROGATE(text[i])) {
+      const bool isValidSurrogatePair =
+          U16_IS_LEAD(text[i]) && i + 1 < end && U16_IS_TRAIL(text[i + 1]);
+      if (!isValidSurrogatePair)
+        return true;
+      i++;
+    }
+  }
+  return false;
+}
+
+bool hasInvalidSurrogatePair(const LChar* text,

[Changwan Ryu, 2017/01/11 06:44:33]

This is somewhat weird. UTF-8 does not have surrogate pairs.
UTF-8 may have up to 4 bytes.

Is it possible to remove UTF-8 logic here and change test to use UTF-16?

[yabinh, 2017/01/24 11:39:56]

Done. This function has been removed.

We can simply remove some InputMethodController tests that use UTF8. Anyway,
they are duplicate with the tests for deleteSurroundingText().
But for ImeTest#testInputTextEvents_DeleteSurroundingTextInCodePoints, somehow
it uses UTF8. So we still need the logic to handle UTF8.

+                             const int start,
+                             const int end) {
+  for (int i = start; i < end; i++) {
+    if (!U8_IS_SINGLE(text[i])) {
+      const bool isValidSurrogatePair =
+          U8_IS_LEAD(text[i]) && i + 1 < end && U8_IS_TRAIL(text[i + 1]);
+      if (!isValidSurrogatePair)
+        return true;
+      i++;
+    }
+  }
+  return false;
+}
+
+std::pair<int, int> invalidDeletionLength() {
+  return std::make_pair(-1, -1);
+}
+
+bool isInvalidDeletionLength(const std::pair<int, int>& lengthPair) {
+  return lengthPair.first == -1 && lengthPair.second == -1;
+}
+
+std::pair<int, int> convertDeletionLengthForDeleteSurroundingTextInCodePoints(
+    const String& text,
+    const int beforeLengthInCodePoints,
+    const int afterLengthInCodePoints,
+    const int selectionStart,
+    const int selectionEnd) {
+  int deletionStart = selectionStart;

[Changwan Ryu, 2017/01/11 06:44:33]

Are selectionStart / selectionEnd in code points?

If not, you may need to do another conversion.

int32_t deletionStart = u_countChar32(UText, selectionStart);
U16_BACK_N(UText, 0, deletionStart, beforeLengthInCodePoints);

[yabinh, 2017/01/24 11:39:56]

No, it's in Java length.
But we don't need convert, because the offset here is also in Java length. See
the definition of U16_BACK_N:

'''
The input offset may be the same as the string length.
'''

BTW, this case is covered in
InputMethodControllerTest#DeleteSurroundingTextInCodePointsWithMultiCodeTextOnTheLeft.

+  int deletionEnd = selectionEnd;
+  const int length = static_cast<int>(text.length());
+
+  if (text.is8Bit()) {
+    const LChar* LText = text.characters8();
+    U8_BACK_N(LText, 0, deletionStart, beforeLengthInCodePoints);
+
+    // Return an invalid value if there are one or more invalid surrogate pairs
+    // in the requested range
+    if (hasInvalidSurrogatePair(LText, deletionStart, selectionStart))
+      return invalidDeletionLength();
+
+    U8_FWD_N(LText, deletionEnd, length, afterLengthInCodePoints);
+    if (hasInvalidSurrogatePair(LText, selectionEnd, deletionEnd))
+      return invalidDeletionLength();
+  } else {
+    const UChar* UText = text.characters16();
+
+    U16_BACK_N(UText, 0, deletionStart, beforeLengthInCodePoints);
+    if (hasInvalidSurrogatePair(UText, deletionStart, selectionStart))
+      return invalidDeletionLength();

[Changwan Ryu, 2017/01/11 06:44:33]

I think it should be ok to delete the range even when surrogate pairs were
invalid. Or is there any case this can be critical?

[yabinh, 2017/01/24 11:39:56]

This is to follow the specification:

'''
This method does nothing if there are one or more invalid surrogate pairs in the
requested range.
'''

+
+    U16_FWD_N(UText, deletionEnd, length, afterLengthInCodePoints);
+    if (hasInvalidSurrogatePair(UText, selectionEnd, deletionEnd))
+      return invalidDeletionLength();
+  }
+
+  const int beforeLengthInJavaChars = selectionStart - deletionStart;

[Changwan Ryu, 2017/01/11 02:00:27]

WebKit should not know about JavaChars. Also it does not apply to UTF-8 cases. I
think beforeLength / afterLength should be fine.

[yabinh, 2017/01/24 11:39:56]

Done.

+  const int afterLengthInJavaChars = deletionEnd - selectionEnd;
+  return std::make_pair(beforeLengthInJavaChars, afterLengthInJavaChars);
+}
+
 }  // anonymous namespace
 
 InputMethodController* InputMethodController::create(LocalFrame& frame) {
@@ -849,6 +926,35 @@ void InputMethodController::deleteSurroundingText(int before, int after) {
   setSelectionOffsets(PlainTextRange(selectionStart, selectionEnd));
 }
 
+void InputMethodController::deleteSurroundingTextInCodePoints(int before,
+                                                              int after) {
+  DCHECK_GE(before, 0);
+  DCHECK_GE(after, 0);
+  if (!editor().canEdit())
+    return;
+  const PlainTextRange selectionOffsets(getSelectionOffsets());
+  if (selectionOffsets.isNull())
+    return;
+  Element* const rootEditableElement =
+      frame().selection().rootEditableElement();
+  if (!rootEditableElement)
+    return;
+
+  const int selectionStart = static_cast<int>(selectionOffsets.start());
+  const int selectionEnd = static_cast<int>(selectionOffsets.end());
+  const String& text =
+      plainText(EphemeralRange::rangeOfContents(*rootEditableElement),
+                TextIteratorEmitsObjectReplacementCharacter);
+  std::pair<int, int> deletionLengthPair =
+      convertDeletionLengthForDeleteSurroundingTextInCodePoints(
+          text, before, after, selectionStart, selectionEnd);
+  if (isInvalidDeletionLength(deletionLengthPair))
+    return;
+
+  return deleteSurroundingText(deletionLengthPair.first,
+                               deletionLengthPair.second);
+}
+
 WebTextInputInfo InputMethodController::textInputInfo() const {
   WebTextInputInfo info;
   if (!isAvailable())