[Editing][CodeHealth] Refactor SpellChecker::respondToChangedSelection

third_party/WebKit/Source/core/editing/spellcheck/SpellChecker.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 778 matching lines @@
     oldSelection.start().document()->updateStyleAndLayoutIgnorePendingStylesheets();
     return oldSelection.isContentEditable();
 }
 
 void SpellChecker::respondToChangedSelection(const VisibleSelection& oldSelection, FrameSelection::SetSelectionOptions options)
 {
     TRACE_EVENT0("blink", "SpellChecker::respondToChangedSelection");
     if (!isSpellCheckingEnabledFor(oldSelection))
         return;
 
-    bool closeTyping = options & FrameSelection::CloseTyping;
-    bool isContinuousSpellCheckingEnabled = this->isContinuousSpellCheckingEnabled();
-    bool isContinuousGrammarCheckingEnabled = isContinuousSpellCheckingEnabled;
-    if (isContinuousSpellCheckingEnabled && closeTyping && shouldCheckOldSelection(oldSelection)) {
-        VisibleSelection newAdjacentWords;
-        bool caretBrowsing = frame().settings() && frame().settings()->caretBrowsingEnabled();
-        const VisibleSelection newSelection = frame().selection().selection();
-        if (isSelectionInTextFormControl(newSelection)) {
-            Position newStart = newSelection.start();
-            newAdjacentWords.setWithoutValidation(HTMLTextFormControlElement::startOfWord(newStart), HTMLTextFormControlElement::endOfWord(newStart));
-        } else {
-            frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
-            if (newSelection.isContentEditable() || caretBrowsing) {
-                VisiblePosition newStart(newSelection.visibleStart());
-                newAdjacentWords = VisibleSelection(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
-            }
-        }
-
-        // When typing we check spelling elsewhere, so don't redo it here.
-        // If this is a change in selection resulting from a delete operation,
-        // oldSelection may no longer be in the document.
-        // FIXME(http://crbug.com/382809): if oldSelection is on a textarea
-        // element, we cause synchronous layout.
-        spellCheckOldSelection(oldSelection, newAdjacentWords);
+    // When continuous spell checking is off, existing markers disappear after the selection changes.
+    if (!this->isContinuousSpellCheckingEnabled()) {

[yosin_UTC9, 2016/07/01 08:57:09]

nit: s/this->// since we don't have local variable
|isContinuousSpellCheckingEnabled| anymore.

[yoichio, 2016/07/04 02:23:20]

What do you mean?

[yosin_UTC9, 2016/07/04 04:29:30]

Before your patch, L800 has |bool isContinuousSpellCheckingEnabled =
this->isContinuousSpellCheckingEnabled();|.
After your patch, L800 is removed. So, we can use
|SpellChecker::isContinuousSpellCheckingEnabled()| w/o |this->|

[yoichio, 2016/07/04 05:00:44]

I see. Done.

+        frame().document()->markers().removeMarkers(DocumentMarker::Spelling);
+        frame().document()->markers().removeMarkers(DocumentMarker::Grammar);
+        return;
     }
 
-    // When continuous spell checking is off, existing markers disappear after the selection changes.
-    if (!isContinuousSpellCheckingEnabled)
-        frame().document()->markers().removeMarkers(DocumentMarker::Spelling);
-    if (!isContinuousGrammarCheckingEnabled)
-        frame().document()->markers().removeMarkers(DocumentMarker::Grammar);
+    if (!(options & FrameSelection::CloseTyping))
+        return;
+    if (!shouldCheckOldSelection(oldSelection))
+        return;
+
+    VisibleSelection newAdjacentWords;
+    const VisibleSelection newSelection = frame().selection().selection();
+    if (isSelectionInTextFormControl(newSelection)) {
+        const Position newStart = newSelection.start();
+        newAdjacentWords.setWithoutValidation(HTMLTextFormControlElement::startOfWord(newStart), HTMLTextFormControlElement::endOfWord(newStart));
+    } else {
+        frame().document()->updateStyleAndLayoutIgnorePendingStylesheets();
+        const bool caretBrowsing = frame().settings() && frame().settings()->caretBrowsingEnabled();
+        if (newSelection.isContentEditable() || caretBrowsing) {
+            const VisiblePosition newStart(newSelection.visibleStart());
+            newAdjacentWords = VisibleSelection(startOfWord(newStart, LeftWordIfOnBoundary), endOfWord(newStart, RightWordIfOnBoundary));
+        }
+    }
+
+    // When typing we check spelling elsewhere, so don't redo it here.
+    // If this is a change in selection resulting from a delete operation,
+    // oldSelection may no longer be in the document.
+    // FIXME(http://crbug.com/382809): if oldSelection is on a textarea
+    // element, we cause synchronous layout.
+    spellCheckOldSelection(oldSelection, newAdjacentWords);
 }
 
 void SpellChecker::removeSpellingMarkers()
 {
     frame().document()->markers().removeMarkers(DocumentMarker::MisspellingMarkers());
 }
 
 void SpellChecker::removeSpellingMarkersUnderWords(const Vector<String>& words)
 {
     MarkerRemoverPredicate removerPredicate(words);
@@ skipping 117 matching lines @@
     visitor->trace(m_frame);
     visitor->trace(m_spellCheckRequester);
 }
 
 void SpellChecker::prepareForLeakDetection()
 {
     m_spellCheckRequester->prepareForLeakDetection();
 }
 
 } // namespace blink