Offers suggestions for each language that marks a word misspelled.

chrome/renderer/spellchecker/spellcheck.cc

 // Copyright (c) 2012 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 "chrome/renderer/spellchecker/spellcheck.h"
 
 #include <algorithm>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 246 matching lines @@
   // report the word as correctly spelled.)
   if (InitializeIfNeeded())
     return true;
 
   // These are for holding misspelling or skippable word positions and lengths
   // between calls to SpellcheckLanguage::SpellCheckWord.
   int possible_misspelling_start;
   int possible_misspelling_len;
   // The longest sequence of text that all languages agree is skippable.
   int agreed_skippable_len;
+  // A vector of vectors containing spelling suggestions from different
+  // languages.
+  std::vector<std::vector<base::string16>> suggestions_list;
 
   // This loop only advances if all languages agree that a sequence of text is
   // skippable.
   for (; position_in_text <= text_length;
        position_in_text += agreed_skippable_len) {
     // Reseting |agreed_skippable_len| to the worst-case length each time
     // prevents some unnecessary iterations.
     agreed_skippable_len = text_length;
     *misspelling_start = 0;
     *misspelling_len = 0;
-
-    if (optional_suggestions)
-      optional_suggestions->clear();
+    suggestions_list.clear();
 
     for (ScopedVector<SpellcheckLanguage>::iterator language =
              languages_.begin();
          language != languages_.end();) {
+      std::vector<base::string16> language_suggestions;
       SpellcheckLanguage::SpellcheckWordResult result =
-          (*language)->SpellCheckWord(text_begin, position_in_text, text_length,
-                                      tag, &possible_misspelling_start,
-                                      &possible_misspelling_len,
-                                      optional_suggestions);
+          (*language)->SpellCheckWord(
+              text_begin, position_in_text, text_length, tag,
+              &possible_misspelling_start, &possible_misspelling_len,
+              optional_suggestions ? &language_suggestions : nullptr);
 
       switch (result) {
         case SpellcheckLanguage::SpellcheckWordResult::IS_CORRECT:
           *misspelling_start = 0;
           *misspelling_len = 0;
           return true;
         case SpellcheckLanguage::SpellcheckWordResult::IS_SKIPPABLE:
           agreed_skippable_len =
               std::min(agreed_skippable_len, possible_misspelling_len);
           // If true, this means the spellchecker moved past a word that was
           // previously determined to be misspelled or skippable, which means
           // another spellcheck language marked it as correct.
           if (position_in_text != possible_misspelling_start) {
             *misspelling_len = 0;
             position_in_text = possible_misspelling_start;
+            if (optional_suggestions)
+              suggestions_list.clear();

[please use gerrit instead, 2015/08/20 18:40:23]

If optional_suggestions is null, then suggestions_list will be empty already.
Calling suggestions_list on an empty vector is cheap for the computer. Thinking
about why you're checking optional_suggestions for null here is cognitive load
for the reader. Therefore, remove "if (optional_suggestions)". Just
"suggestions_list.clear();" is enough.

[Julius, 2015/08/20 21:52:50]

Done.

             language = languages_.begin();
           } else {
             language++;
           }
           break;
         case SpellcheckLanguage::SpellcheckWordResult::IS_MISSPELLED:
           *misspelling_start = possible_misspelling_start;
           *misspelling_len = possible_misspelling_len;
           // If true, this means the spellchecker moved past a word that was
           // previously determined to be misspelled or skippable, which means
           // another spellcheck language marked it as correct.
-          language = position_in_text != *misspelling_start ? languages_.begin()
-                                                            : language + 1;
-          position_in_text = *misspelling_start;
+          if (position_in_text != *misspelling_start) {
+            suggestions_list.clear();
+            language = languages_.begin();
+            position_in_text = *misspelling_start;
+          } else {
+            // Only add the language's suggestions if this is not a newly
+            // determined misspelling. This prevents adding a list of
+            // suggestions twice.

[please use gerrit instead, 2015/08/20 18:40:23]

I don't think this comment explains much, although the code seems correct.
Remove the comment.

[Julius, 2015/08/20 21:52:50]

Done.

+            if (optional_suggestions)

[please use gerrit instead, 2015/08/20 18:40:23]

Remove the if. Just push_back.

[Julius, 2015/08/20 21:52:50]

Done.

+              suggestions_list.push_back(language_suggestions);
+            language++;
+          }
           break;
       }
     }
 
     // If |*misspelling_len| is non-zero, that means at least one language
     // marked a word misspelled and no other language considered it correct.
-    if (*misspelling_len != 0)
+    if (*misspelling_len != 0) {
+      FillSuggestions(suggestions_list, optional_suggestions);
       return false;
+    }
   }
 
   NOTREACHED();
   return true;
 }
 
+void SpellCheck::FillSuggestions(

[please use gerrit instead, 2015/08/20 18:40:23]

Your function does nothing if |optional_suggestions| is null. This type of
functions is usually called MaybeDoSomething(), but I would avoid this. Instead,
assume that |optinal_suggestions| is never null. When you call this function
call it like so:

if (optional_suggestions)
  FillSuggestions(suggestions_list, optional_suggestions);

[Julius, 2015/08/20 21:52:51]

Done.

+    std::vector<std::vector<base::string16>>& suggestions_list,
+    std::vector<base::string16>* optional_suggestions) {
+  if (!optional_suggestions)
+    return;
+
+  // A vector containing the indices of the current suggestion for each
+  // language's suggestion list.
+  std::vector<size_t> indices(suggestions_list.size(), 0);
+  // Take one suggestion at a time from each language's suggestions and add it
+  // to |optional_suggestions|.
+  for (size_t i = 0, num_empty = 0;
+       num_empty < suggestions_list.size() &&
+       optional_suggestions->size() <
+           chrome::spellcheck_common::kMaxSuggestions;
+       i = (i + 1) % suggestions_list.size()) {
+    if (indices[i] < suggestions_list[i].size()) {
+      base::string16 suggestion = suggestions_list[i][indices[i]];

[please use gerrit instead, 2015/08/20 18:40:23]

Avoid copying |suggestion| by making it const-ref. This is not const-happiness,
because the purpose of const-ref is to avoid copying, not to avoid modifying.
Copying would be bad here, because you're doing it inside of a loop, so possibly
many-many times.

[Julius, 2015/08/20 21:52:51]

Done.

+      // Only add the suggestion if it's unique.
+      if (std::find_if(optional_suggestions->begin(),

[please use gerrit instead, 2015/08/20 18:40:23]

You don't need a lambda and std::find_if. Use std::find() instead.

http://www.cplusplus.com/reference/algorithm/find/

[Julius, 2015/08/20 21:52:51]

Wow, duh.

+                       optional_suggestions->end(),
+                       [&suggestion](base::string16 word) {
+                         return word == suggestion;
+                       }) == optional_suggestions->end()) {
+        optional_suggestions->push_back(suggestion);
+      }
+      indices[i]++;
+      if (indices[i] == suggestions_list[i].size())

[please use gerrit instead, 2015/08/20 18:40:23]

You can reduce lines 373-374 into a single line:

if (++indicies[i] == suggesions_list[i].size())

[Julius, 2015/08/20 21:52:50]

Done.

+        num_empty++;
+    }
+  }
+}
+
 bool SpellCheck::SpellCheckParagraph(
     const base::string16& text,
     WebVector<WebTextCheckingResult>* results) {
 #if !defined(USE_BROWSER_SPELLCHECKER)
   // Mac and Android have their own spell checkers,so this method won't be used
   DCHECK(results);
   std::vector<WebTextCheckingResult> textcheck_results;
   size_t length = text.length();
   size_t position_in_text = 0;
 
@@ skipping 211 matching lines @@
 
 bool SpellCheck::IsSpellcheckEnabled() {
 #if defined(OS_ANDROID)
   if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableAndroidSpellChecker)) {
     return false;
   }
 #endif
   return spellcheck_enabled_;
 }