Implement ThreadedInputConnection.deleteSurroundingTextInCodePoints()

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 165 matching lines @@
   }
 
   if (!queryAttribute)
     return AtomicString();
 
   // TODO(dtapuska): We may wish to restrict this to a yet to be proposed
   // <contenteditable> or <richtext> element Mozilla discussed at TPAC 2016.
   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;
+}
+
+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(

[Changwan Ryu, 2017/01/25 07:30:56]

The name is a bit unnecessarily lengthy. How about renaming this to
calculateDeletionLengthsInCodePoints()?

[yabinh, 2017/02/08 12:32:41]

Done.

+    const String& text,
+    const int beforeLengthInCodePoints,
+    const int afterLengthInCodePoints,
+    const int selectionStart,
+    const int selectionEnd) {
+  int deletionStart = selectionStart;
+  int deletionEnd = selectionEnd;
+  const int length = static_cast<int>(text.length());
+
+  const UChar* UText;
+  if (text.is8Bit()) {

[Changwan Ryu, 2017/01/25 07:30:56]

I really hope that we don't do the conversion here because this will only be
used for testing anyways.

Could you instead add CHECK(!text.is8Bit()); here?

Blink tests may need to use String::FromUTF8() to do the conversion into UTF-16.
Could you investigate ImeTest case? It's a bit concerning that it's being
converted to UTF-8 somewhere in the pipeline.

[yabinh, 2017/02/08 12:32:41]

Actually we convert utf16 to utf8 in RenderWidget::OnImeCommitText():
|text| is utf16, but |WebString::fromUTF16(text)| is utf8.
(WebString::fromUTF16() tries to create 8 bits if possible.)

So we need to convert it back for
ImeTest#testInputTextEvents_DeleteSurroundingTextInCodePoints.
As for another similar test
testContentEditableEvents_DeleteSurroundingTextInCodePoints, we don't have to do
that because there is already some text (utf16) in contenteditable, so when
inserting "hello"(utf8), blink will covert it to utf16 and then insert. (in
String::insert()).

[Changwan Ryu, 2017/02/09 01:06:34]

Thanks for looking into it. StringImpl::create8BitIfPossible() seems to compress
the data to 8 bit whenever possible, and not do the actual conversion, which is
very smart and does not break assumptions in other parts of the file.

Now that we understand in what cases we have UTF-8, I prefer the following
structure:

if (text.is8Bit()) {
  U8_BACK_N(text.characters8(), 0, deletionStart, beforeLengthInCodePoints);
  U8_FWD_N(text.characters8(), deletionEnd, length, afterLengthInCodePoints);
} else {
  U16_BACK_N(...);
  // Required by Android's InputConnection spec.
  if (hasInvalidSurrogatePair(...))
    return invalidDeletionLength();
  U16_FWD_N(...);
  // Required by Android's InputConnection spec.
  if (hasInvalidSurrogatePair(...))
    return invalidDeletionLength();
}

...

This way we can avoid unnecessary conversion and still rest assured that both
cases are covered by tests.

[yabinh, 2017/02/09 05:21:37]

Done.

+    const String& text16 = String::make16BitFrom8BitSource(
+        text.characters8(), static_cast<size_t>(length));
+    UText = text16.characters16();
+  } else {
+    UText = text.characters16();
+  }
+
+  U16_BACK_N(UText, 0, deletionStart, beforeLengthInCodePoints);
+  if (hasInvalidSurrogatePair(UText, deletionStart, selectionStart))
+    return invalidDeletionLength();
+
+  U16_FWD_N(UText, deletionEnd, length, afterLengthInCodePoints);
+  if (hasInvalidSurrogatePair(UText, selectionEnd, deletionEnd))
+    return invalidDeletionLength();
+
+  const int beforeLength = selectionStart - deletionStart;
+  const int afterLength = deletionEnd - selectionEnd;

[Changwan Ryu, 2017/01/25 07:30:56]

could you add the following?
DCHECK_GE(beforeLength, 0);
DCHECK_GE(afterLength, 0);

[yabinh, 2017/02/08 12:32:42]

Done.

+  return std::make_pair(beforeLength, afterLength);
+}
+
 }  // anonymous namespace
 
 InputMethodController* InputMethodController::create(LocalFrame& frame) {
   return new InputMethodController(frame);
 }
 
 InputMethodController::InputMethodController(LocalFrame& frame)
     : m_frame(&frame), m_hasComposition(false) {}
 
 InputMethodController::~InputMethodController() = default;
@@ skipping 650 matching lines @@
     const size_t diff = computeDistanceToRightGraphemeBoundary(position);
     const int adjustedEnd = end + static_cast<int>(diff);
     if (!setSelectionOffsets(PlainTextRange(selectionEnd, adjustedEnd)))
       return;
     TypingCommand::deleteSelection(document());
   }
 
   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())
     return info;
 
   if (!frame().selection().isAvailable()) {
     // plugins/mouse-capture-inside-shadow.html reaches here.
     return info;
   }
   Element* element = frame().selection().rootEditableElement();
@@ skipping 201 matching lines @@
   frame().chromeClient().resetInputMethod();
 }
 
 DEFINE_TRACE(InputMethodController) {
   visitor->trace(m_frame);
   visitor->trace(m_compositionRange);
   SynchronousMutationObserver::trace(visitor);
 }
 
 }  // namespace blink