feat: Spellchecker Async Implementation (#14032)

* feat:Spellchecker Async Implementation

* Adhere to chromium style

* Updating dependency to use gh branch

* Update docs and electron-typescript-definitions module

* Fix lint

* Update electron typescript definitions version

* Update spec

* Address review

atom/renderer/api/atom_api_spell_check_client.cc

@@ -4,7 +4,7 @@
 
 #include "atom/renderer/api/atom_api_spell_check_client.h"
 
-#include <algorithm>
+#include <map>
 #include <vector>
 
 #include "atom/common/native_mate_converters/string16_converter.h"
@@ -13,6 +13,7 @@
 #include "chrome/renderer/spellchecker/spellcheck_worditerator.h"
 #include "native_mate/converter.h"
 #include "native_mate/dictionary.h"
+#include "native_mate/function_template.h"
 #include "third_party/blink/public/web/web_text_checking_completion.h"
 #include "third_party/blink/public/web/web_text_checking_result.h"
 #include "third_party/icu/source/common/unicode/uscript.h"
@@ -40,6 +41,10 @@ bool HasWordCharacters(const base::string16& text, int index) {
 
 class SpellCheckClient::SpellcheckRequest {
  public:
+  // Map of individual words to list of occurrences in text
+  using WordMap =
+      std::map<base::string16, std::vector<blink::WebTextCheckingResult>>;
+
   SpellcheckRequest(const base::string16& text,
                     blink::WebTextCheckingCompletion* completion)
       : text_(text), completion_(completion) {
@@ -47,12 +52,13 @@ class SpellCheckClient::SpellcheckRequest {
   }
   ~SpellcheckRequest() {}
 
-  base::string16 text() { return text_; }
+  const base::string16& text() const { return text_; }
   blink::WebTextCheckingCompletion* completion() { return completion_; }
+  WordMap& wordmap() { return word_map_; }
 
  private:
   base::string16 text_;  // Text to be checked in this task.
-
+  WordMap word_map_;     // WordMap to hold distinct words in text
   // The interface to send the misspelled ranges to WebKit.
   blink::WebTextCheckingCompletion* completion_;
 
@@ -60,10 +66,10 @@ class SpellCheckClient::SpellcheckRequest {
 };
 
 SpellCheckClient::SpellCheckClient(const std::string& language,
-                                   bool auto_spell_correct_turned_on,
                                    v8::Isolate* isolate,
                                    v8::Local<v8::Object> provider)
-    : isolate_(isolate),
+    : pending_request_param_(nullptr),
+      isolate_(isolate),
       context_(isolate, isolate->GetCurrentContext()),
       provider_(isolate, provider) {
   DCHECK(!context_.IsEmpty());
@@ -79,19 +85,6 @@ SpellCheckClient::~SpellCheckClient() {
   context_.Reset();
 }
 
-void SpellCheckClient::CheckSpelling(
-    const blink::WebString& text,
-    int& misspelling_start,
-    int& misspelling_len,
-    blink::WebVector<blink::WebString>* optional_suggestions) {
-  std::vector<blink::WebTextCheckingResult> results;
-  SpellCheckText(text.Utf16(), true, &results);
-  if (results.size() == 1) {
-    misspelling_start = results[0].location;
-    misspelling_len = results[0].length;
-  }
-}
-
 void SpellCheckClient::RequestCheckingOfText(
     const blink::WebString& textToCheck,
     blink::WebTextCheckingCompletion* completionCallback) {
@@ -103,7 +96,7 @@ void SpellCheckClient::RequestCheckingOfText(
   }
 
   // Clean up the previous request before starting a new request.
-  if (pending_request_param_.get()) {
+  if (pending_request_param_) {
     pending_request_param_->completion()->DidCancelCheckingText();
   }
 
@@ -111,8 +104,7 @@ void SpellCheckClient::RequestCheckingOfText(
 
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
-      base::BindOnce(&SpellCheckClient::PerformSpellCheck, AsWeakPtr(),
-                     base::Owned(pending_request_param_.release())));
+      base::BindOnce(&SpellCheckClient::SpellCheckText, AsWeakPtr()));
 }
 
 bool SpellCheckClient::IsSpellCheckingEnabled() const {
@@ -128,12 +120,13 @@ bool SpellCheckClient::IsShowingSpellingUI() {
 void SpellCheckClient::UpdateSpellingUIWithMisspelledWord(
     const blink::WebString& word) {}
 
-void SpellCheckClient::SpellCheckText(
-    const base::string16& text,
-    bool stop_at_first_result,
-    std::vector<blink::WebTextCheckingResult>* results) {
-  if (text.empty() || spell_check_.IsEmpty())
+void SpellCheckClient::SpellCheckText() {
+  const auto& text = pending_request_param_->text();
+  if (text.empty() || spell_check_.IsEmpty()) {
+    pending_request_param_->completion()->DidCancelCheckingText();
+    pending_request_param_ = nullptr;
     return;
+  }
 
   if (!text_iterator_.IsInitialized() &&
       !text_iterator_.Initialize(&character_attributes_, true)) {
@@ -153,56 +146,87 @@ void SpellCheckClient::SpellCheckText(
 
   SpellCheckScope scope(*this);
   base::string16 word;
-  int word_start;
-  int word_length;
-  for (auto status =
-           text_iterator_.GetNextWord(&word, &word_start, &word_length);
-       status != SpellcheckWordIterator::IS_END_OF_TEXT;
-       status = text_iterator_.GetNextWord(&word, &word_start, &word_length)) {
+  std::vector<base::string16> words;
+  auto& word_map = pending_request_param_->wordmap();
+  blink::WebTextCheckingResult result;
+  for (;;) {  // Run until end of text
+    const auto status =
+        text_iterator_.GetNextWord(&word, &result.location, &result.length);
+    if (status == SpellcheckWordIterator::IS_END_OF_TEXT)
+      break;
     if (status == SpellcheckWordIterator::IS_SKIPPABLE)
       continue;
 
-    // Found a word (or a contraction) that the spellchecker can check the
-    // spelling of.
-    if (SpellCheckWord(scope, word))
-      continue;
-
     // If the given word is a concatenated word of two or more valid words
     // (e.g. "hello:hello"), we should treat it as a valid word.
-    if (IsValidContraction(scope, word))
-      continue;
+    std::vector<base::string16> contraction_words;
+    if (!IsContraction(scope, word, &contraction_words)) {
+      words.push_back(word);
+      word_map[word].push_back(result);
+    } else {
+      // For a contraction, we want check the spellings of each individual
+      // part, but mark the entire word incorrect if any part is misspelled
+      // Hence, we use the same word_start and word_length values for every
+      // part of the contraction.
+      for (const auto& w : contraction_words) {
+        words.push_back(w);
+        word_map[w].push_back(result);
+      }
+    }
+  }
 
-    blink::WebTextCheckingResult result;
-    result.location = word_start;
-    result.length = word_length;
-    results->push_back(result);
+  // Send out all the words data to the spellchecker to check
+  SpellCheckWords(scope, words);
+}
 
-    if (stop_at_first_result)
-      return;
+void SpellCheckClient::OnSpellCheckDone(
+    const std::vector<base::string16>& misspelled_words) {
+  std::vector<blink::WebTextCheckingResult> results;
+  auto* const completion_handler = pending_request_param_->completion();
+
+  auto& word_map = pending_request_param_->wordmap();
+
+  // Take each word from the list of misspelled words received, find their
+  // corresponding WebTextCheckingResult that's stored in the map and pass
+  // all the results to blink through the completion callback.
+  for (const auto& word : misspelled_words) {
+    auto iter = word_map.find(word);
+    if (iter != word_map.end()) {
+      // Word found in map, now gather all the occurrences of the word
+      // from the map value
+      auto& words = iter->second;
+      results.insert(results.end(), words.begin(), words.end());
+      words.clear();
+    }
   }
+  completion_handler->DidFinishCheckingText(results);
+  pending_request_param_ = nullptr;
 }
 
-bool SpellCheckClient::SpellCheckWord(
+void SpellCheckClient::SpellCheckWords(
     const SpellCheckScope& scope,
-    const base::string16& word_to_check) const {
+    const std::vector<base::string16>& words) {
   DCHECK(!scope.spell_check_.IsEmpty());
 
-  v8::Local<v8::Value> word = mate::ConvertToV8(isolate_, word_to_check);
-  v8::Local<v8::Value> result =
-      scope.spell_check_->Call(scope.provider_, 1, &word);
+  v8::Local<v8::FunctionTemplate> templ = mate::CreateFunctionTemplate(
+      isolate_, base::Bind(&SpellCheckClient::OnSpellCheckDone, AsWeakPtr()));
 
-  if (!result.IsEmpty() && result->IsBoolean())
-    return result->BooleanValue();
-  else
-    return true;
+  v8::Local<v8::Value> args[] = {mate::ConvertToV8(isolate_, words),
+                                 templ->GetFunction()};
+  // Call javascript with the words and the callback function
+  scope.spell_check_->Call(scope.provider_, 2, args);
 }
 
-// Returns whether or not the given string is a valid contraction.
+// Returns whether or not the given string is a contraction.
 // This function is a fall-back when the SpellcheckWordIterator class
 // returns a concatenated word which is not in the selected dictionary
 // (e.g. "in'n'out") but each word is valid.
-bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope,
-                                          const base::string16& contraction) {
+// Output variable contraction_words will contain individual
+// words in the contraction.
+bool SpellCheckClient::IsContraction(
+    const SpellCheckScope& scope,
+    const base::string16& contraction,
+    std::vector<base::string16>* contraction_words) {
   DCHECK(contraction_iterator_.IsInitialized());
 
   contraction_iterator_.SetText(contraction.c_str(), contraction.length());
@@ -210,7 +234,6 @@ bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope,
   base::string16 word;
   int word_start;
   int word_length;
-
   for (auto status =
            contraction_iterator_.GetNextWord(&word, &word_start, &word_length);
        status != SpellcheckWordIterator::IS_END_OF_TEXT;
@@ -219,18 +242,9 @@ bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope,
     if (status == SpellcheckWordIterator::IS_SKIPPABLE)
       continue;
 
-    if (!SpellCheckWord(scope, word))
-      return false;
+    contraction_words->push_back(word);
   }
-  return true;
-}
-
-void SpellCheckClient::PerformSpellCheck(SpellcheckRequest* param) {
-  DCHECK(param);
-
-  std::vector<blink::WebTextCheckingResult> results;
-  SpellCheckText(param->text(), false, &results);
-  param->completion()->DidFinishCheckingText(results);
+  return contraction_words->size() > 1;
 }
 
 SpellCheckClient::SpellCheckScope::SpellCheckScope(

atom/renderer/api/atom_api_spell_check_client.h

@@ -31,7 +31,6 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient,
                          public base::SupportsWeakPtr<SpellCheckClient> {
  public:
   SpellCheckClient(const std::string& language,
-                   bool auto_spell_correct_turned_on,
                    v8::Isolate* isolate,
                    v8::Local<v8::Object> provider);
   ~SpellCheckClient() override;
@@ -39,11 +38,6 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient,
  private:
   class SpellcheckRequest;
   // blink::WebTextCheckClient:
-  void CheckSpelling(
-      const blink::WebString& text,
-      int& misspelledOffset,
-      int& misspelledLength,
-      blink::WebVector<blink::WebString>* optionalSuggestions) override;
   void RequestCheckingOfText(
       const blink::WebString& textToCheck,
       blink::WebTextCheckingCompletion* completionCallback) override;
@@ -65,22 +59,27 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient,
     ~SpellCheckScope();
   };
 
-  // Check the spelling of text.
-  void SpellCheckText(const base::string16& text,
-                      bool stop_at_first_result,
-                      std::vector<blink::WebTextCheckingResult>* results);
+  // Run through the word iterator and send out requests
+  // to the JS API for checking spellings of words in the current
+  // request.
+  void SpellCheckText();
 
   // Call JavaScript to check spelling a word.
-  bool SpellCheckWord(const SpellCheckScope& scope,
-                      const base::string16& word_to_check) const;
+  // The javascript function will callback OnSpellCheckDone
+  // with the results of all the misspelled words.
+  void SpellCheckWords(const SpellCheckScope& scope,
+                       const std::vector<base::string16>& words);
 
   // Returns whether or not the given word is a contraction of valid words
   // (e.g. "word:word").
-  bool IsValidContraction(const SpellCheckScope& scope,
-                          const base::string16& word);
-
-  // Performs spell checking from the request queue.
-  void PerformSpellCheck(SpellcheckRequest* param);
+  // Output variable contraction_words will contain individual
+  // words in the contraction.
+  bool IsContraction(const SpellCheckScope& scope,
+                     const base::string16& word,
+                     std::vector<base::string16>* contraction_words);
+
+  // Callback for the JS API which returns the list of misspelled words.
+  void OnSpellCheckDone(const std::vector<base::string16>& misspelled_words);
 
   // Represents character attributes used for filtering out characters which
   // are not supported by this SpellCheck object.

atom/renderer/api/atom_api_web_frame.cc

@@ -215,15 +215,14 @@ int WebFrame::GetWebFrameId(v8::Local<v8::Value> content_window) {
 
 void WebFrame::SetSpellCheckProvider(mate::Arguments* args,
                                      const std::string& language,
-                                     bool auto_spell_correct_turned_on,
                                      v8::Local<v8::Object> provider) {
   if (!provider->Has(mate::StringToV8(args->isolate(), "spellCheck"))) {
     args->ThrowError("\"spellCheck\" has to be defined");
     return;
   }
 
-  auto client = std::make_unique<SpellCheckClient>(
-      language, auto_spell_correct_turned_on, args->isolate(), provider);
+  auto client =
+      std::make_unique<SpellCheckClient>(language, args->isolate(), provider);
   // Set spellchecker for all live frames in the same process or
   // in the sandbox mode for all live sub frames to this WebFrame.
   FrameSpellChecker spell_checker(

atom/renderer/api/atom_api_web_frame.h

@@ -58,7 +58,6 @@ class WebFrame : public mate::Wrappable<WebFrame> {
   // Set the provider that will be used by SpellCheckClient for spell check.
   void SetSpellCheckProvider(mate::Arguments* args,
                              const std::string& language,
-                             bool auto_spell_correct_turned_on,
                              v8::Local<v8::Object> provider);
 
   void RegisterURLSchemeAsBypassingCSP(const std::string& scheme);

docs/api/web-frame.md

@@ -57,26 +57,34 @@ Sets the maximum and minimum pinch-to-zoom level.
 
 Sets the maximum and minimum layout-based (i.e. non-visual) zoom level.
 
-### `webFrame.setSpellCheckProvider(language, autoCorrectWord, provider)`
+### `webFrame.setSpellCheckProvider(language, provider)`
 
 * `language` String
-* `autoCorrectWord` Boolean
 * `provider` Object
-  * `spellCheck` Function - Returns `Boolean`.
-    * `text` String
+  * `spellCheck` Function.
+    * `words` String[]
+    * `callback` Function
+      * `misspeltWords` String[]
 
 Sets a provider for spell checking in input fields and text areas.
 
-The `provider` must be an object that has a `spellCheck` method that returns
-whether the word passed is correctly spelled.
+The `provider` must be an object that has a `spellCheck` method that accepts
+an array of individual words for spellchecking.
+The `spellCheck` function runs asynchronously and calls the `callback` function
+with an array of misspelt words when complete.
 
 An example of using [node-spellchecker][spellchecker] as provider:
 
 ```javascript
 const { webFrame } = require('electron')
-webFrame.setSpellCheckProvider('en-US', true, {
-  spellCheck (text) {
-    return !(require('spellchecker').isMisspelled(text))
+const spellChecker = require('spellchecker')
+webFrame.setSpellCheckProvider('en-US', {
+  spellCheck (words, callback) {
+    setTimeout(() => {
+      const spellchecker = require('spellchecker')
+      const misspelled = words.filter(x => spellchecker.isMisspelled(x))
+      callback(misspelled)
+    }, 0)
   }
 })
 ```

package-lock.json

@@ -2860,9 +2860,9 @@
       }
     },
     "electron-typescript-definitions": {
-      "version": "2.1.1",
-      "resolved": "https://registry.npmjs.org/electron-typescript-definitions/-/electron-typescript-definitions-2.1.1.tgz",
-      "integrity": "sha512-vrEhi3hhPzUEDLwPGOqScYBLefNKH5r9odp3dy/lqE0nhAmUHBkrwnU5jVga3A2pJW22wzCCB1kwkEoPV7Rq4w==",
+      "version": "3.0.0",
+      "resolved": "https://registry.npmjs.org/electron-typescript-definitions/-/electron-typescript-definitions-3.0.0.tgz",
+      "integrity": "sha512-PQEdLGY9i5IWxQKWCllCun3Lh07YDMeTt/YRbsS+kaCC13vvxoy/LprwkqRUWbzAH704MHtDU9pKrEK2MoqP2w==",
       "dev": true,
       "requires": {
         "@types/node": "^7.0.18",

package.json

@@ -14,7 +14,7 @@
     "dugite": "^1.45.0",
     "electabul": "~0.0.4",
     "electron-docs-linter": "^2.4.0",
-    "electron-typescript-definitions": "^2.1.1",
+    "electron-typescript-definitions": "^3.0.0",
     "eslint": "^5.6.0",
     "eslint-config-standard": "^12.0.0",
     "eslint-plugin-mocha": "^5.2.0",

spec/api-web-frame-spec.js

@@ -152,12 +152,22 @@ describe('webFrame module', function () {
     w.focus()
     await w.webContents.executeJavaScript('document.querySelector("input").focus()', true)
 
-    const spellCheckerFeedback = emittedOnce(ipcMain, 'spec-spell-check')
-    const misspelledWord = 'spleling'
-    for (const keyCode of [...misspelledWord, ' ']) {
+    const spellCheckerFeedback =
+      new Promise((resolve, reject) => {
+        ipcMain.on('spec-spell-check', (e, words, callback) => {
+          if (words.length === 2) {
+            // The promise is resolved only after this event is received twice
+            // Array contains only 1 word first time and 2 the next time
+            resolve([words, callback])
+          }
+        })
+      })
+    const inputText = 'spleling test '
+    for (const keyCode of inputText) {
       w.webContents.sendInputEvent({ type: 'char', keyCode })
     }
-    const [, text] = await spellCheckerFeedback
-    expect(text).to.equal(misspelledWord)
+    const [words, callback] = await spellCheckerFeedback
+    expect(words).to.deep.equal(['spleling', 'test'])
+    expect(callback).to.be.true()
   })
 })

spec/fixtures/pages/webframe-spell-check.html

@@ -2,9 +2,10 @@
 <body>
 <script type="text/javascript" charset="utf-8">
   const {ipcRenderer, webFrame} = require('electron')
-  webFrame.setSpellCheckProvider('en-US', true, {
-    spellCheck: text => {
-      ipcRenderer.send('spec-spell-check', text)
+  webFrame.setSpellCheckProvider('en-US', {
+    spellCheck: (words, callback) => {
+      callback(words)
+      ipcRenderer.send('spec-spell-check', words, callback != undefined)
     }
   })
 </script>