Stop ColdModeSpellCheckRequester from repeatedly checking in full

In the original design, ColdModeSpellCheckRequester is responsible for
doing spellchecking after the page is idle after a while (>1s) to
ensure that we don't miss anything to check in the hot mode.

It turns out to be too aggressive because it always checks the
content of the editable element in full even after some minor
operations (e.g., moving caret), causing significant input delay when
the content is large.

This patch:

- Makes ColdModeSpellCheckRequester memorize the editable elements
  whose contents have been checked in full, so that when invoked on
  such an element, it won't do any checking if no text change is
  detected, or just check a short chunk around the selection for
  small text changes

- We also maintain the accumulated text change delta after fully
  checking an element. If that exceeds a threshold, we re-allow doing
  full checking on the element.

Bug: 1000616
Fixed: 1326287
Change-Id: I56a48583ed1f63e64e6d313672a0ba1c1052bbd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3822635
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1035181}

third_party/blink/renderer/core/editing/spellcheck/cold_mode_spell_check_requester.cc

@@ -9,6 +9,7 @@
 #include "third_party/blink/renderer/core/editing/editing_utilities.h"
 #include "third_party/blink/renderer/core/editing/ephemeral_range.h"
 #include "third_party/blink/renderer/core/editing/frame_selection.h"
+#include "third_party/blink/renderer/core/editing/iterators/backwards_character_iterator.h"
 #include "third_party/blink/renderer/core/editing/iterators/character_iterator.h"
 #include "third_party/blink/renderer/core/editing/selection_template.h"
 #include "third_party/blink/renderer/core/editing/spellcheck/spell_check_requester.h"
@@ -23,15 +24,26 @@ namespace blink {
 
 namespace {
 
-const int kColdModeChunkSize = 16384;  // in UTF16 code units
+// in UTF16 code units
+const int kColdModeFullCheckingChunkSize = 16384;
+const int kColdModeLocalCheckingSize = 128;
+const int kRecheckThreshold = 1024;
+
 const int kInvalidChunkIndex = -1;
 
+int TotalTextLength(const Element& root_editable) {
+  const EphemeralRange& full_range =
+      EphemeralRange::RangeOfContents(root_editable);
+  return TextIterator::RangeLength(full_range);
+}
+
 }  // namespace
 
 void ColdModeSpellCheckRequester::Trace(Visitor* visitor) const {
   visitor->Trace(window_);
   visitor->Trace(root_editable_);
   visitor->Trace(remaining_check_range_);
+  visitor->Trace(fully_checked_root_editables_);
 }
 
 ColdModeSpellCheckRequester::ColdModeSpellCheckRequester(LocalDOMWindow& window)
@@ -87,21 +99,35 @@ void ColdModeSpellCheckRequester::Invoke(IdleDeadline* deadline) {
     return;
   }
 
-  if (root_editable_ != current_focused) {
+  switch (AccumulateTextDeltaAndComputeCheckingType(*current_focused)) {
+    case CheckingType::kNone:
+      return;
+    case CheckingType::kLocal:
+      return RequestLocalChecking(*current_focused);
+    case CheckingType::kFull:
+      return RequestFullChecking(*current_focused, deadline);
+  }
+}
+
+void ColdModeSpellCheckRequester::RequestFullChecking(
+    const Element& element_to_check,
+    IdleDeadline* deadline) {
+  TRACE_EVENT0("blink", "ColdModeSpellCheckRequester::RequestFullChecking");
+
+  if (root_editable_ != &element_to_check) {
     ClearProgress();
-    root_editable_ = current_focused;
+    root_editable_ = &element_to_check;
     last_chunk_index_ = 0;
     remaining_check_range_ = Range::Create(root_editable_->GetDocument());
     remaining_check_range_->selectNodeContents(
         const_cast<Element*>(root_editable_.Get()), ASSERT_NO_EXCEPTION);
   }
 
   while (deadline->timeRemaining() > 0) {
-    if (FullyChecked()) {
+    if (FullyChecked() || !RequestCheckingForNextChunk()) {
       SetHasFullyChecked();
       return;
     }
-    RequestCheckingForNextChunk();
   }
 }
 
@@ -114,16 +140,26 @@ void ColdModeSpellCheckRequester::ClearProgress() {
   remaining_check_range_ = nullptr;
 }
 
+void ColdModeSpellCheckRequester::Deactivate() {
+  ClearProgress();
+  fully_checked_root_editables_.clear();
+}
+
 void ColdModeSpellCheckRequester::SetHasFullyChecked() {
   DCHECK(root_editable_);
+  DCHECK(!fully_checked_root_editables_.Contains(root_editable_));
+
+  fully_checked_root_editables_.Set(
+      root_editable_,
+      FullyCheckedEditableEntry{TotalTextLength(*root_editable_), 0});
   last_chunk_index_ = kInvalidChunkIndex;
   if (!remaining_check_range_)
     return;
   remaining_check_range_->Dispose();
   remaining_check_range_ = nullptr;
 }
 
-void ColdModeSpellCheckRequester::RequestCheckingForNextChunk() {
+bool ColdModeSpellCheckRequester::RequestCheckingForNextChunk() {
   DCHECK(root_editable_);
   DCHECK(!FullyChecked());
 
@@ -132,16 +168,15 @@ void ColdModeSpellCheckRequester::RequestCheckingForNextChunk() {
       remaining_range,
       // Same behavior used in |CalculateCharacterSubrange()|
       TextIteratorBehavior::EmitsObjectReplacementCharacterBehavior());
-  if (remaining_length == 0) {
-    SetHasFullyChecked();
-    return;
-  }
+  if (remaining_length == 0)
+    return false;
 
   const int chunk_index = last_chunk_index_ + 1;
   const Position chunk_start = remaining_range.StartPosition();
   const Position chunk_end =
-      CalculateCharacterSubrange(remaining_range, 0,
-                                 std::min(remaining_length, kColdModeChunkSize))
+      CalculateCharacterSubrange(
+          remaining_range, 0,
+          std::min(remaining_length, kColdModeFullCheckingChunkSize))
           .EndPosition();
 
   // Chromium spellchecker requires complete sentences to be checked. However,
@@ -158,6 +193,66 @@ void ColdModeSpellCheckRequester::RequestCheckingForNextChunk() {
 
   last_chunk_index_ = chunk_index;
   remaining_check_range_->setStart(check_range.EndPosition());
+  return true;
+}
+
+ColdModeSpellCheckRequester::CheckingType
+ColdModeSpellCheckRequester::AccumulateTextDeltaAndComputeCheckingType(
+    const Element& element_to_check) {
+  // Do full checking if we haven't done that before
+  auto iter = fully_checked_root_editables_.find(&element_to_check);
+  if (iter == fully_checked_root_editables_.end())
+    return CheckingType::kFull;
+
+  int current_text_length = TotalTextLength(element_to_check);
+  int delta =
+      std::abs(current_text_length - iter->value.previous_checked_length);
+
+  // Cold mode checking is not needed without plain text change (for example,
+  // after moving caret, changing text style, etc).
+  if (!delta)
+    return CheckingType::kNone;
+
+  iter->value.accumulated_delta += delta;
+  iter->value.previous_checked_length = current_text_length;
+
+  if (iter->value.accumulated_delta > kRecheckThreshold) {
+    fully_checked_root_editables_.erase(iter);
+    return CheckingType::kFull;
+  }
+
+  return CheckingType::kLocal;
+}
+
+void ColdModeSpellCheckRequester::RequestLocalChecking(
+    const Element& element_to_check) {
+  TRACE_EVENT0("blink", "ColdModeSpellCheckRequester::RequestLocalChecking");
+
+  const EphemeralRange& full_range =
+      EphemeralRange::RangeOfContents(element_to_check);
+  const Position position =
+      window_->GetFrame()->Selection().GetSelectionInDOMTree().Extent();
+  DCHECK(position.IsNotNull());
+
+  TextIteratorBehavior behavior =
+      TextIteratorBehavior::Builder()
+          .SetEmitsObjectReplacementCharacter(true)
+          .SetEmitsPunctuationForReplacedElements(true)
+          .Build();
+  BackwardsCharacterIterator backward_iterator(
+      EphemeralRange(full_range.StartPosition(), position), behavior);
+  if (!backward_iterator.AtEnd())
+    backward_iterator.Advance(kColdModeLocalCheckingSize / 2);
+  const Position& chunk_start = backward_iterator.EndPosition();
+  CharacterIterator forward_iterator(position, full_range.EndPosition(),
+                                     behavior);
+  if (!forward_iterator.AtEnd())
+    forward_iterator.Advance(kColdModeLocalCheckingSize / 2);
+  const Position& chunk_end = forward_iterator.EndPosition();
+  EphemeralRange checking_range =
+      ExpandRangeToSentenceBoundary(EphemeralRange(chunk_start, chunk_end));
+
+  GetSpellCheckRequester().RequestCheckingFor(checking_range);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/editing/spellcheck/cold_mode_spell_check_requester.h

@@ -35,7 +35,13 @@ class ColdModeSpellCheckRequester
 
   void Invoke(IdleDeadline*);
 
+  // Called when code mode checking is currently not needed (due to, e.g., user
+  // has resumed active).
   void ClearProgress();
+
+  // Called when document is detached or spellchecking is globally disabled.
+  void Deactivate();
+
   bool FullyChecked() const;
 
   void Trace(Visitor*) const;
@@ -45,14 +51,24 @@ class ColdModeSpellCheckRequester
 
   const Element* CurrentFocusedEditable() const;
 
-  void RequestCheckingForNextChunk();
+  enum class CheckingType { kNone, kLocal, kFull };
+  CheckingType AccumulateTextDeltaAndComputeCheckingType(
+      const Element& element_to_check);
+
+  void RequestLocalChecking(const Element& element_to_check);
+
+  void RequestFullChecking(const Element& element_to_check, IdleDeadline*);
+
+  // Returns true if there's anything remaining to check, false otherwise
+  bool RequestCheckingForNextChunk();
   void SetHasFullyChecked();
 
   // The window this cold mode checker belongs to.
   const Member<LocalDOMWindow> window_;
 
-  // The root editable element checked in the last invocation. |nullptr| if not
-  // invoked yet or didn't find any root editable element to check.
+  // The root editable element checked in the last invocation for full checking.
+  // |nullptr| if not invoked yet or didn't find any root editable element for
+  // full checking.
   Member<const Element> root_editable_;
 
   // If |root_editable_| is non-null and hasn't been fully checked, the id of
@@ -61,6 +77,17 @@ class ColdModeSpellCheckRequester
   int last_chunk_index_;
   Member<Range> remaining_check_range_;
 
+  // After fully checking an element, we don't want to repeatedly check its
+  // content in full unless a significant amount of change has taken place. We
+  // heuristically measure this as the accumulated length change in the element
+  // since the last time it was fully checked.
+  struct FullyCheckedEditableEntry {
+    int previous_checked_length = 0;
+    int accumulated_delta = 0;
+  };
+  HeapHashMap<Member<const Element>, FullyCheckedEditableEntry>
+      fully_checked_root_editables_;
+
   // A test-only flag for forcing lifecycle advancing.
   mutable bool needs_more_invocation_for_testing_;
 };

third_party/blink/renderer/core/editing/spellcheck/idle_spell_check_controller.cc

@@ -103,7 +103,7 @@ void IdleSpellCheckController::Deactivate() {
   state_ = State::kInactive;
   if (cold_mode_timer_.IsActive())
     cold_mode_timer_.Cancel();
-  cold_mode_requester_->ClearProgress();
+  cold_mode_requester_->Deactivate();
   DisposeIdleCallback();
   spell_check_requeseter_->Deactivate();
 }

third_party/blink/renderer/core/editing/spellcheck/spell_check_requester.cc

@@ -165,9 +165,7 @@ void SpellCheckRequest::SetCheckerAndSequence(SpellCheckRequester* requester,
 }
 
 SpellCheckRequester::SpellCheckRequester(LocalDOMWindow& window)
-    : window_(&window),
-      last_request_sequence_(0),
-      last_processed_sequence_(0) {}
+    : window_(&window) {}
 
 SpellCheckRequester::~SpellCheckRequester() = default;
 
@@ -193,6 +191,8 @@ bool SpellCheckRequester::RequestCheckingFor(const EphemeralRange& range,
   if (!request)
     return false;
 
+  spell_checked_text_length_ += request->GetText().length();
+
   DCHECK_EQ(request->Sequence(),
             SpellCheckRequest::kUnrequestedTextCheckingSequence);
   int sequence = ++last_request_sequence_;

third_party/blink/renderer/core/editing/spellcheck/spell_check_requester.h

@@ -98,6 +98,9 @@ class CORE_EXPORT SpellCheckRequester final
 
   int LastProcessedSequence() const { return last_processed_sequence_; }
 
+  // Returns the total length of all text that has been requested for checking.
+  int SpellCheckedTextLength() const { return spell_checked_text_length_; }
+
   // Called to clean up pending requests when no more checking is needed. For
   // example, when document is closed.
   void Deactivate();
@@ -118,8 +121,9 @@ class CORE_EXPORT SpellCheckRequester final
 
   Member<LocalDOMWindow> window_;
 
-  int last_request_sequence_;
-  int last_processed_sequence_;
+  int last_request_sequence_ = 0;
+  int last_processed_sequence_ = 0;
+  wtf_size_t spell_checked_text_length_ = 0;
 
   TaskHandle timer_to_process_queued_request_;
 

third_party/blink/renderer/core/testing/internals.cc

@@ -2041,6 +2041,20 @@ int Internals::lastSpellCheckProcessedSequence(
   return requester->LastProcessedSequence();
 }
 
+int Internals::spellCheckedTextLength(Document* document,
+                                      ExceptionState& exception_state) {
+  SpellCheckRequester* requester = GetSpellCheckRequester(document);
+
+  if (!requester) {
+    exception_state.ThrowDOMException(
+        DOMExceptionCode::kInvalidAccessError,
+        "No spell check requestor can be obtained for the provided document.");
+    return -1;
+  }
+
+  return requester->SpellCheckedTextLength();
+}
+
 void Internals::cancelCurrentSpellCheckRequest(
     Document* document,
     ExceptionState& exception_state) {

third_party/blink/renderer/core/testing/internals.h

@@ -282,6 +282,7 @@ class Internals final : public ScriptWrappable {
 
   int lastSpellCheckRequestSequence(Document*, ExceptionState&);
   int lastSpellCheckProcessedSequence(Document*, ExceptionState&);
+  int spellCheckedTextLength(Document*, ExceptionState&);
   void cancelCurrentSpellCheckRequest(Document*, ExceptionState&);
   String idleTimeSpellCheckerState(Document*, ExceptionState&);
   void runIdleTimeSpellChecker(Document*, ExceptionState&);

third_party/blink/renderer/core/testing/internals.idl

@@ -142,6 +142,7 @@
 
     [RaisesException] long lastSpellCheckRequestSequence(Document document);
     [RaisesException] long lastSpellCheckProcessedSequence(Document document);
+    [RaisesException] long spellCheckedTextLength(Document document);
     [RaisesException] void cancelCurrentSpellCheckRequest(Document document);
     [RaisesException] DOMString idleTimeSpellCheckerState(Document document);
     [RaisesException] void runIdleTimeSpellChecker(Document document);