Implement hot mode invocation for idle time spell checker

third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp

Index: third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp
diff --git a/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp b/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp
index b9fa99b65a4ccbe7ba81799e9421a167ab920e64..cd638acee27564d9bf569216c251867a5f09a613 100644
--- a/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp
+++ b/third_party/WebKit/Source/core/editing/spellcheck/IdleSpellCheckCallback.cpp
@@ -7,10 +7,13 @@
 #include "core/dom/IdleRequestOptions.h"
 #include "core/dom/TaskRunnerHelper.h"
 #include "core/editing/EditingUtilities.h"
+#include "core/editing/Editor.h"
 #include "core/editing/FrameSelection.h"
 #include "core/editing/VisibleSelection.h"
 #include "core/editing/VisibleUnits.h"
 #include "core/editing/commands/CompositeEditCommand.h"
+#include "core/editing/commands/TypingCommand.h"
+#include "core/editing/commands/UndoStack.h"
 #include "core/editing/commands/UndoStep.h"
 #include "core/editing/iterators/BackwardsCharacterIterator.h"
 #include "core/editing/iterators/CharacterIterator.h"
@@ -28,6 +31,7 @@ namespace blink {
 
 namespace {
 
+const int kHotModeChunkSize = 1024;
 const int kColdModeTimerIntervalMS = 1000;
 const int kConsecutiveColdModeTimerIntervalMS = 200;
 const int kRequestTimeoutMS = 200;
@@ -35,6 +39,17 @@ const int kInvalidHandle = -1;
 const int kDummyHandleForForcedInvocation = -2;
 const double kForcedInvocationDeadlineSeconds = 10;
 
+bool isInOrAdjacentToWord(const Position& pos) {
+  const VisiblePosition& visiblePos = createVisiblePosition(pos);
+  const VisiblePosition& wordStart = previousWordPosition(visiblePos);
+  if (wordStart.isNull())
+    return false;
+  const VisiblePosition& wordEnd = endOfWord(wordStart);
+  if (wordEnd.isNull())
+    return false;
+  return comparePositions(visiblePos, wordEnd) <= 0;
+}
+
 }  // namespace
 
 IdleSpellCheckCallback::~IdleSpellCheckCallback() {}
@@ -54,6 +69,7 @@ IdleSpellCheckCallback::IdleSpellCheckCallback(LocalFrame& frame)
       m_idleCallbackHandle(kInvalidHandle),
       m_needsMoreColdModeInvocationForTesting(false),
       m_frame(frame),
+      m_lastProcessedUndoStepSequence(0),
       m_coldModeTimer(TaskRunnerHelper::get(TaskType::UnspecedTimer, &frame),
                       this,
                       &IdleSpellCheckCallback::coldModeTimerFired) {}
@@ -132,8 +148,103 @@ void IdleSpellCheckCallback::coldModeTimerFired(TimerBase*) {
   m_state = State::kColdModeRequested;
 }
 
+EphemeralRange IdleSpellCheckCallback::calculateHotModeCheckingRange(
+    const Element& editable,
+    const Position& refPosition) const {
+  const EphemeralRange& fullRange = EphemeralRange::rangeOfContents(editable);
+  const int fullLength = TextIterator::rangeLength(fullRange.startPosition(),
+                                                   fullRange.endPosition());
+  if (fullLength <= kHotModeChunkSize)
+    return fullRange;
+
+  TextIteratorBehavior behavior = TextIteratorBehavior::Builder()
+                                      .setEmitsObjectReplacementCharacter(true)
+                                      .build();
+  BackwardsCharacterIterator backwardIterator(fullRange.startPosition(),
+                                              refPosition, behavior);
+  backwardIterator.advance(kHotModeChunkSize / 2);
+  const Position& chunkStart = backwardIterator.endPosition();
+  CharacterIterator forwardIterator(refPosition, fullRange.endPosition(),
+                                    behavior);
+  forwardIterator.advance(kHotModeChunkSize / 2);
+  const Position& chunkEnd = forwardIterator.endPosition();
+  return expandRangeToSentenceBoundary(EphemeralRange(chunkStart, chunkEnd));
+}
+
+bool IdleSpellCheckCallback::isTypingInPartialWord(
+    const Element& editable) const {
+  const SelectionInDOMTree& selection =
+      frame().selection().selectionInDOMTree();
+  if (!selection.isCaret())
+    return false;
+  if (rootEditableElementOf(selection.base()) != &editable)
+    return false;
+
+  CompositeEditCommand* typingCommand =
+      frame().editor().lastTypingCommandIfStillOpenForTyping();
+  if (!typingCommand)
+    return false;
+  if (typingCommand->endingSelection().asSelection() != selection)
+    return false;
+  return isInOrAdjacentToWord(selection.base());
+}
+
+bool IdleSpellCheckCallback::shouldCheckRootEditableInHotMode(
+    const Element& editable,
+    const Position& refPosition) const {
+  if (!editable.isSpellCheckingEnabled() &&
+      !SpellChecker::isSpellCheckingEnabledAt(refPosition))
+    return false;
+  if (editable.visibleBoundsInVisualViewport().isEmpty())
+    return false;
+  // TODO(xiaochengh): Design more aggressive strategies to reduce checking when
+  // we are just moving selection around without modifying anything.
+  return !isTypingInPartialWord(editable);
+}
+
+void IdleSpellCheckCallback::checkRootEditableInHotMode(
+    Element* rootEditable,
+    const Position& refPosition,
+    HeapVector<Member<Element>>* processedRootEditables) {

[yosin_UTC9, 2017/03/03 08:55:18]

Let's use HeapHashSet<Member<Element>>

I prefer to have a class SpellCheckRequestScope, or another good name, which
doesn't request for already request element,
rather than pass processed element list.

class SpellCheckRequestScope {
  STACK_ALLOCATE()
 public:
  explicit SpellCheckRequestScope(SpellCheckeRequester*);
  void requestCheckingForRootEditableInHotMode(...) { 
   ...
   if (m_requester.count(element)) return;
   ...
   m_spellCheckRequeset.requestCheckingFor(request);
 }
 private:
  SpellCheckReqter& m_requester;
  HeapHashSet<Member<Element>> m_requestedEleemnt;
}

[Xiaocheng, 2017/03/03 14:31:52]

I don't expect checking a lot of root editables in hot mode. In such case, a
vector shouldn't cause any performance issue, but is much lighter than a hash
set.

Anyway, talk is cheap. I prefer adding a UMA of the number of distinct root
editables checked in each hot mode invocation, and let data decide whether the
de-duplication process should be more scalable.
Sorry I don't get it...

What's the benefit of introducing a scope here? In my understanding, we
introduce scope only in two scenarios:
- To pass local variables deep into the call stack
- To do some work in the dtor of the scope

We are in neither case here.

[yosin_UTC9, 2017/03/06 04:17:31]

This is another use case of scope. In this case, scope manages list of requested
elements to avoid multiple request
on element.

In your itemized list, dtor of this scope frees memory of list of element,
HeapVector/HeapSet.

[yosin_UTC9, 2017/03/06 04:39:15]

Naming wise? If "Scope" makes you confusing. We call
|UniqueSpellCheckeRequester| or another name which tells we don't
make multiple requests for same element.

For me passing "temporary" list to function is implementation details, like:

// Implement recursive function as tail-recursive.
int fact(n, int acc =1) {
  if (n <= 1) return acc;
  return fact(n - 1, acc * n);
}

where |acc| is an implementation detail, we don't want callers don't care.

+  if (!rootEditable || !rootEditable->isConnected())
+    return;
+
+  if (processedRootEditables->contains(rootEditable))
+    return;
+  processedRootEditables->push_back(rootEditable);
+
+  if (!shouldCheckRootEditableInHotMode(*rootEditable, refPosition))
+    return;
+
+  SpellCheckRequest* request = SpellCheckRequest::create(
+      calculateHotModeCheckingRange(*rootEditable, refPosition));
+  spellCheckRequester().requestCheckingFor(request);
+}
+
 void IdleSpellCheckCallback::hotModeInvocation(IdleDeadline* deadline) {
-  // TODO(xiaochengh): Implementation.
+  TRACE_EVENT0("blink", "IdleSpellCheckCallback::hotModeInvocation");
+
+  // TODO(xiaochengh): Figure out if this has any performance impact.
+  frame().document()->updateStyleAndLayout();
+
+  HeapVector<Member<Element>> processedRootEditables;
+
+  const Position& extent = frame().selection().selectionInDOMTree().extent();
+  checkRootEditableInHotMode(rootEditableElementOf(extent), extent,
+                             &processedRootEditables);
+
+  const uint64_t watermark = m_lastProcessedUndoStepSequence;
+  for (const UndoStep* step : frame().editor().undoStack().undoSteps()) {
+    if (step->sequenceNumber() <= watermark)
+      break;
+    m_lastProcessedUndoStepSequence =
+        std::max(step->sequenceNumber(), m_lastProcessedUndoStepSequence);
+    if (deadline->timeRemaining() < 0)
+      break;
+    checkRootEditableInHotMode(step->endingRootEditableElement(),
+                               step->endingSelection().extent(),
+                               &processedRootEditables);
+  }
 }
 
 void IdleSpellCheckCallback::coldModeInvocation(IdleDeadline* deadline) {