Offers suggestions for each language that marks a word misspelled.

chrome/renderer/spellchecker/spellcheck.cc

Index: chrome/renderer/spellchecker/spellcheck.cc
diff --git a/chrome/renderer/spellchecker/spellcheck.cc b/chrome/renderer/spellchecker/spellcheck.cc
index e2d505500fe58625388e13c1c045d6e64f2be76b..e798e1e5c83496f4724b0d1da888cba584c03ea5 100644
--- a/chrome/renderer/spellchecker/spellcheck.cc
+++ b/chrome/renderer/spellchecker/spellcheck.cc
@@ -264,6 +264,9 @@ bool SpellCheck::SpellCheckWord(
   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.
@@ -274,18 +277,17 @@ bool SpellCheck::SpellCheckWord(
     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:
@@ -301,6 +303,8 @@ bool SpellCheck::SpellCheckWord(
           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++;
@@ -312,23 +316,67 @@ bool SpellCheck::SpellCheckWord(
           // 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) {