Add support for Android spellcheck menu in Chrome/WebViews

third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.cpp

Index: third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.cpp
diff --git a/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.cpp b/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..7a94d7e98f813fd4c2fe1259a87239c22c1c04e8
--- /dev/null
+++ b/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.cpp
@@ -0,0 +1,223 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/editing/suggestion/TextSuggestionController.h"
+
+#include "core/editing/EditingUtilities.h"
+#include "core/editing/Editor.h"
+#include "core/editing/FrameSelection.h"
+#include "core/editing/PlainTextRange.h"
+#include "core/editing/markers/DocumentMarkerController.h"
+#include "core/editing/markers/SpellCheckMarker.h"
+#include "core/editing/spellcheck/SpellChecker.h"
+#include "core/frame/FrameView.h"
+#include "core/frame/LocalFrame.h"
+#include "public/platform/InterfaceProvider.h"
+
+namespace blink {
+
+namespace {
+
+const double kSuggestionUnderlineAlphaMultiplier = 0.4;
+
+}  // anonymous namespace
+
+TextSuggestionController::TextSuggestionController(LocalFrame& frame)
+    : suggestion_menu_is_open_(false), frame_(&frame) {}
+
+bool TextSuggestionController::IsMenuOpen() const {
+  return suggestion_menu_is_open_;
+}
+
+void TextSuggestionController::HandlePotentialMisspelledWordTap() {
+  Optional<std::pair<Node*, SpellCheckMarker*>> node_and_marker =
+      GetFrame().GetSpellChecker().GetSpellCheckMarkerTouchingSelection();
+  if (!node_and_marker)
+    return;
+
+  if (!text_suggestion_host_) {
+    GetFrame().GetInterfaceProvider()->GetInterface(
+        mojo::MakeRequest(&text_suggestion_host_));
+  }
+
+  text_suggestion_host_->StartSpellCheckMenuTimer();
+}
+
+DEFINE_TRACE(TextSuggestionController) {
+  visitor->Trace(frame_);
+}
+
+void TextSuggestionController::ApplySpellCheckSuggestion(
+    const String& suggestion) {
+  GetFrame().GetSpellChecker().ReplaceMisspelledRange(suggestion);
+  SuggestionMenuClosed();
+}
+
+namespace {

[Xiaocheng, 2017/06/13 18:17:42]

nit: Shouldn't have two anonymous namespaces.

+
+bool ShouldDeleteNextCharacter(const Node* marker_text_node,
+                               const DocumentMarker* marker) {
+  // If the character immediately following the range to be deleted is a space,
+  // delete it if either of these conditions holds:
+  // - We're deleting at the beginning of the editable text (to avoid ending up
+  //   with a space at the beginning)
+  // - The character immediately before the range being deleted is also a space
+  //   (to avoid ending up with two adjacent spaces)
+  const EphemeralRange next_character_range =
+      PlainTextRange(marker->EndOffset(), marker->EndOffset() + 1)
+          .CreateRange(*marker_text_node->parentNode());
+  // No character immediately following the range (so it can't be a space)
+  if (next_character_range.IsNull())
+    return false;
+
+  const String next_character_str = PlainText(
+      next_character_range,
+      TextIteratorBehavior::Builder().SetEmitsSpaceForNbsp(true).Build());

[Xiaocheng, 2017/06/13 18:17:42]

|EmitsSpaceForNbsp| worsens the performance, as it needs to copy and modify the
string from LayoutText:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/i...

I think it's better to check space and nbsp explicitly. It does't make code more
complicated.

+  // Character immediately following the range is not a space
+  if (!WTF::IsASCIISpace(next_character_str[0]))
+    return false;
+
+  // First case: we're deleting at the beginning of the editable text
+  if (marker->StartOffset() == 0)
+    return true;
+
+  const EphemeralRange prev_character_range =
+      PlainTextRange(marker->StartOffset() - 1, marker->StartOffset())
+          .CreateRange(*marker_text_node->parentNode());
+  // Not at beginning, but there's no character immediately before the range
+  // being deleted (so it can't be a space)
+  if (prev_character_range.IsNull())
+    return false;
+
+  const String prev_character_str = PlainText(
+      prev_character_range,
+      TextIteratorBehavior::Builder().SetEmitsSpaceForNbsp(true).Build());

[Xiaocheng, 2017/06/13 18:17:42]

Ditto.

+  // Return true if the character immediately before the range is space, false
+  // otherwise
+  return WTF::IsASCIISpace(prev_character_str[0]);
+}
+
+}  // namespace
+
+void TextSuggestionController::DeleteActiveSuggestionRange() {
+  Optional<std::pair<Node*, SpellCheckMarker*>> node_and_marker =
+      GetFrame().GetSpellChecker().GetSpellCheckMarkerTouchingSelection();
+  if (!node_and_marker)
+    return;
+
+  Node* const marker_text_node = node_and_marker.value().first;
+  const DocumentMarker* const marker = node_and_marker.value().second;
+
+  const bool delete_next_char =
+      ShouldDeleteNextCharacter(marker_text_node, marker);
+
+  const EphemeralRange range_to_delete = EphemeralRange(
+      Position(marker_text_node, marker->StartOffset()),
+      Position(marker_text_node, marker->EndOffset() + delete_next_char));
+
+  GetFrame().Selection().SetSelection(

[yosin_UTC9, 2017/06/13 06:29:31]

FrameSelection::SetSelection() can make current frame gone.
We should check whether current frame available or not after FS::SetSelection()
like L136 for after dispatching "beforeinput"

+      SelectionInDOMTree::Builder().SetBaseAndExtent(range_to_delete).Build());
+
+  Document& current_document = *GetFrame().GetDocument();
+
+  // Dispatch 'beforeinput'.
+  Element* const target = GetFrame().GetEditor().FindEventTargetFromSelection();
+  DataTransfer* const data_transfer = DataTransfer::Create(
+      DataTransfer::DataTransferType::kInsertReplacementText,
+      DataTransferAccessPolicy::kDataTransferReadable,
+      DataObject::CreateFromString(""));
+
+  const bool cancel = DispatchBeforeInputDataTransfer(
+                          target, InputEvent::InputType::kInsertReplacementText,
+                          data_transfer) != DispatchEventResult::kNotCanceled;
+
+  // 'beforeinput' event handler may destroy target frame.
+  if (current_document != GetFrame().GetDocument())
+    return;
+
+  // TODO(editing-dev): The use of updateStyleAndLayoutIgnorePendingStylesheets
+  // needs to be audited.  See http://crbug.com/590369 for more details.
+  GetFrame().GetDocument()->UpdateStyleAndLayoutIgnorePendingStylesheets();
+
+  if (cancel)
+    return;
+  GetFrame().GetEditor().ReplaceSelectionWithText(
+      "", false, false, InputEvent::InputType::kInsertReplacementText);
+
+  SuggestionMenuClosed();
+}
+
+void TextSuggestionController::NewWordAddedToDictionary(const String& word) {
+  // Android pops up a dialog to let the user confirm they actually want to add
+  // the word to the dictionary; this method gets called as soon as the dialog
+  // is shown. So the word isn't actually in the dictionary here, even if the
+  // user will end up confirming the dialog, and we shouldn't try to re-run
+  // spellcheck here.
+
+  // Note: this actually matches the behavior in native Android text boxes
+  GetDocument().Markers().RemoveSpellingMarkersUnderWords(
+      Vector<String>({word}));
+  SuggestionMenuClosed();
+}
+
+void TextSuggestionController::SpellCheckMenuTimeoutCallback() {
+  Optional<std::pair<Node*, SpellCheckMarker*>> const node_and_marker =
+      GetFrame().GetSpellChecker().GetSpellCheckMarkerTouchingSelection();
+  if (!node_and_marker)
+    return;
+
+  Node* const container_node = node_and_marker.value().first;
+  SpellCheckMarker* const marker = node_and_marker.value().second;
+
+  const EphemeralRange marker_range =
+      EphemeralRange(Position(container_node, marker->StartOffset()),
+                     Position(container_node, marker->EndOffset()));
+  const String& misspelled_word = PlainText(marker_range);
+  const String& description = marker->Description();
+
+  suggestion_menu_is_open_ = true;
+  GetFrame().Selection().SetCaretVisible(false);
+  GetDocument().Markers().AddActiveSuggestionMarker(
+      marker_range, SK_ColorTRANSPARENT, StyleableMarker::Thickness::kThin,
+      LayoutTheme::GetTheme()
+          .PlatformSpellingMarkerUnderlineColor()

[yosin_UTC9, 2017/06/13 06:29:31]

Could you introduce |PlatformActiveSuggestionUnderlineColor()|?
to move |kSuggestionUnderlineAlphaMultiplier| to platform specific part?

Note: Color[1] can have RGBA32.

BTW, do we really use alpha != 1.0? I guess painting alpha=1.0 is faster than
alpha != 1.0.


[1]
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph...

[rlanday, 2017/06/13 06:59:27]

I'm basically porting over the code Android uses to pick the background color:
https://android.googlesource.com/platform/frameworks/base/+/master/core/java/...

I think the reason it's set up the way it is (the alpha is added separately) is
because the SuggestionSpan underline color can be either gray (for a Voice IME
suggestion) or red (for a spellcheck suggestion), and then we just add some
transparency to get the highlight color (so we do use alpha != 1.0).

So we can either end up with:

PlatformSpellingMarkerUnderlineColor()
PlatformSpellingMarkerHighlightColor()
PlatformActiveSuggestionUnderlineColor()
PlatformActiveSuggestionHighlightColor()

or we can have:

PlatformSpellingMarkerUnderlineColor()
PlatformActiveSuggestionUnderlineColor()

and stick the 0.4 alpha multiplier constant somewhere (this might be a little
awkward). I guess it's probably better to just have the four colors?

[yosin_UTC9, 2017/06/13 07:12:15]

Let's provide four colors in LayoutTheme to provide freedom of design to UX
designer.
Other platform UX designer, may want to use different color scheme.

+          .CombineWithAlpha(kSuggestionUnderlineAlphaMultiplier));
+
+  Vector<String> suggestions;
+  description.Split('\n', suggestions);
+
+  Vector<mojom::blink::SpellCheckSuggestionPtr> suggestion_ptrs;
+  for (const String& suggestion : suggestions) {
+    mojom::blink::SpellCheckSuggestionPtr info_ptr(
+        mojom::blink::SpellCheckSuggestion::New());
+    info_ptr->suggestion = suggestion;
+    suggestion_ptrs.push_back(std::move(info_ptr));
+  }
+
+  const IntRect& absolute_bounds = GetFrame().Selection().AbsoluteCaretBounds();
+  const IntRect& viewport_bounds =
+      GetFrame().View()->ContentsToViewport(absolute_bounds);
+
+  text_suggestion_host_->ShowSpellCheckSuggestionMenu(
+      viewport_bounds.X(), viewport_bounds.MaxY(), std::move(misspelled_word),
+      std::move(suggestion_ptrs));
+}
+
+void TextSuggestionController::SuggestionMenuClosed() {

[Xiaocheng, 2017/06/13 18:17:42]

The frame may be already detached when called from ApplySpellCheckSuggestion()
and DeleteActiveSuggestionRange().

We should check that before accessing DMC and FrameSelection.

+  GetDocument().Markers().RemoveMarkersOfTypes(
+      DocumentMarker::kActiveSuggestion);
+  GetFrame().Selection().SetCaretVisible(true);
+  suggestion_menu_is_open_ = false;
+}
+
+Document& TextSuggestionController::GetDocument() const {
+  DCHECK(IsAvailable());
+  return *GetFrame().GetDocument();
+}
+
+bool TextSuggestionController::IsAvailable() const {
+  return GetFrame().GetDocument();
+}
+
+}  // namespace blink