Implement complete lifecycle transition for IdleSpellCheckCallback

third_party/WebKit/Source/core/editing/Editor.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 721 matching lines @@
 }
 
 void Editor::respondToChangedContents(const VisibleSelection& endingSelection) {
   if (frame().settings() && frame().settings()->getAccessibilityEnabled()) {
     Node* node = endingSelection.start().anchorNode();
     if (AXObjectCache* cache = frame().document()->existingAXObjectCache())
       cache->handleEditableTextContentChanged(node);
   }
 
   spellChecker().updateMarkersForWordsAffectedByEditing(true);
+  frame().setNeedsSpellChecking();

[yosin_UTC9, 2017/02/23 05:20:48]

Can we make Editor owns both SpellChecker and IdleTimeSpellCheckCallback to make
LocalFrame simple?
Or make SpellChecker to own IdleTimeSpellCheckCallback?

LocalFrame is yet another kitchen sink, we want to decompose it.

[Xiaocheng, 2017/02/23 20:46:07]

I have the feeling that editing-related classes shouldn't be owned by LocalFrame
at all -- they don't make much sense when the document is inactive. See my
comment in crbug.com/695359

It seems to be a different issue, though. If we decide to move classes, let's
move them in different patches.

[yosin_UTC9, 2017/02/24 02:10:53]

I feel it is strange that SpellChecker tells LocalFrame about spell checking. I
don't imagine LocalFrame care about spell checking.
I think IdleTimeSpellChecking is just implementation details, it should not be
expose outside SpellChecker or at most core/editing/spellcheck

BTW, it seems it is better that SpellChecker is isolate from Editor, yet another
kitchen sink. Because SpellChecker doesn't(should not) depend on Editor and
Editor should not depend on SpellChecker.

   client().respondToChangedContents();
 }
 
 void Editor::removeFormattingAndStyle() {
   DCHECK(frame().document());
   RemoveFormatCommand::create(*frame().document())->apply();
 }
 
 void Editor::registerCommandGroup(CompositeEditCommand* commandGroupWrapper) {
   DCHECK(commandGroupWrapper->isCommandGroupWrapper());
@@ skipping 922 matching lines @@
     return;
 
   m_areMarkedTextMatchesHighlighted = flag;
   frame().document()->markers().repaintMarkers(DocumentMarker::TextMatch);
 }
 
 void Editor::respondToChangedSelection(
     const Position& oldSelectionStart,
     FrameSelection::SetSelectionOptions options) {
   spellChecker().respondToChangedSelection(oldSelectionStart, options);
+  frame().setNeedsSpellChecking();
   client().respondToChangedSelection(&frame(),
                                      frame()
                                          .selection()
                                          .selectionInDOMTree()
                                          .selectionTypeWithLegacyGranularity());
   setStartNewKillRingSequence(true);
 }
 
 SpellChecker& Editor::spellChecker() const {
   return frame().spellChecker();
@@ skipping 65 matching lines @@
 
 DEFINE_TRACE(Editor) {
   visitor->trace(m_frame);
   visitor->trace(m_lastEditCommand);
   visitor->trace(m_undoStack);
   visitor->trace(m_mark);
   visitor->trace(m_typingStyle);
 }
 
 }  // namespace blink