Fixed crash in atom::api::SpellCheckClient

The class didn't save the V8 context for the spell checking JS function. When
it later tried to call the JS function and there was no active context, V8
crashed.

I also optimized the spell checking loop by introducing `SpellCheckScope` and
reusing the V8 handles throughout the whole loop.

atom/renderer/api/atom_api_spell_check_client.cc

@@ -41,7 +41,10 @@ SpellCheckClient::SpellCheckClient(const std::string& language,
                                    v8::Isolate* isolate,
                                    v8::Local<v8::Object> provider)
     : isolate_(isolate),
+      context_(isolate, isolate->GetCurrentContext()),
       provider_(isolate, provider) {
+  DCHECK(!context_.IsEmpty());
+
   character_attributes_.SetDefaultLanguage(language);
 
   // Persistent the method.
@@ -49,7 +52,9 @@ SpellCheckClient::SpellCheckClient(const std::string& language,
   dict.Get("spellCheck", &spell_check_);
 }
 
-SpellCheckClient::~SpellCheckClient() {}
+SpellCheckClient::~SpellCheckClient() {
+  context_.Reset();
+}
 
 void SpellCheckClient::CheckSpelling(
     const blink::WebString& text,
@@ -93,12 +98,9 @@ void SpellCheckClient::SpellCheckText(
     const base::string16& text,
     bool stop_at_first_result,
     std::vector<blink::WebTextCheckingResult>* results) {
-  if (text.length() == 0 || spell_check_.IsEmpty())
+  if (text.empty() || spell_check_.IsEmpty())
     return;
 
-  base::string16 word;
-  int word_start;
-  int word_length;
   if (!text_iterator_.IsInitialized() &&
       !text_iterator_.Initialize(&character_attributes_, true)) {
       // We failed to initialize text_iterator_, return as spelled correctly.
@@ -106,17 +108,28 @@ void SpellCheckClient::SpellCheckText(
       return;
   }
 
-  base::string16 in_word(text);
+  if (!contraction_iterator_.IsInitialized() &&
-  text_iterator_.SetText(in_word.c_str(), in_word.size());
+      !contraction_iterator_.Initialize(&character_attributes_, false)) {
+    // We failed to initialize the word iterator, return as spelled correctly.
+    VLOG(1) << "Failed to initialize contraction_iterator_";
+    return;
+  }
+
+  text_iterator_.SetText(text.c_str(), text.size());
+
+  SpellCheckScope scope(*this);
+  base::string16 word;
+  int word_start;
+  int word_length;
   while (text_iterator_.GetNextWord(&word, &word_start, &word_length)) {
     // Found a word (or a contraction) that the spellchecker can check the
     // spelling of.
-    if (SpellCheckWord(word))
+    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(word))
+    if (IsValidContraction(scope, word))
       continue;
 
     blink::WebTextCheckingResult result;
@@ -129,16 +142,16 @@ void SpellCheckClient::SpellCheckText(
   }
 }
 
-bool SpellCheckClient::SpellCheckWord(const base::string16& word_to_check) {
+bool SpellCheckClient::SpellCheckWord(
-  if (spell_check_.IsEmpty())
+    const SpellCheckScope& scope,
-    return true;
+    const base::string16& word_to_check) const {
+    DCHECK(!scope.spell_check_.IsEmpty());
 
-  v8::HandleScope handle_scope(isolate_);
   v8::Local<v8::Value> word = mate::ConvertToV8(isolate_, word_to_check);
-  v8::Local<v8::Value> result = spell_check_.NewHandle()->Call(
+  v8::Local<v8::Value> result =
-      provider_.NewHandle(), 1, &word);
+      scope.spell_check_->Call(scope.provider_, 1, &word);
 
-  if (result->IsBoolean())
+  if (!result.IsEmpty() && result->IsBoolean())
     return result->BooleanValue();
   else
     return true;
@@ -148,13 +161,9 @@ bool SpellCheckClient::SpellCheckWord(const base::string16& word_to_check) {
 // 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 base::string16& contraction) {
+bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope,
-  if (!contraction_iterator_.IsInitialized() &&
+                                          const base::string16& contraction) {
-      !contraction_iterator_.Initialize(&character_attributes_, false)) {
+  DCHECK(contraction_iterator_.IsInitialized());
-    // We failed to initialize the word iterator, return as spelled correctly.
-    VLOG(1) << "Failed to initialize contraction_iterator_";
-    return true;
-  }
 
   contraction_iterator_.SetText(contraction.c_str(), contraction.length());
 
@@ -163,12 +172,20 @@ bool SpellCheckClient::IsValidContraction(const base::string16& contraction) {
   int word_length;
 
   while (contraction_iterator_.GetNextWord(&word, &word_start, &word_length)) {
-    if (!SpellCheckWord(word))
+    if (!SpellCheckWord(scope, word))
       return false;
   }
   return true;
 }
 
+SpellCheckClient::SpellCheckScope::SpellCheckScope(
+    const SpellCheckClient& client)
+    : handle_scope_(client.isolate_),
+      context_scope_(
+          v8::Local<v8::Context>::New(client.isolate_, client.context_)),
+      provider_(client.provider_.NewHandle()),
+      spell_check_(client.spell_check_.NewHandle()) {}
+
 }  // namespace api
 
 }  // namespace atom

atom/renderer/api/atom_api_spell_check_client.h

@@ -50,22 +50,28 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient,
   void UpdateSpellingUIWithMisspelledWord(
       const blink::WebString& word) override;
 
+  struct SpellCheckScope {
+    v8::HandleScope handle_scope_;
+    v8::Context::Scope context_scope_;
+    v8::Local<v8::Object> provider_;
+    v8::Local<v8::Function> spell_check_;
+
+    explicit SpellCheckScope(const SpellCheckClient& client);
+  };
+
   // Check the spelling of text.
   void SpellCheckText(const base::string16& text,
                       bool stop_at_first_result,
                       std::vector<blink::WebTextCheckingResult>* results);
 
   // Call JavaScript to check spelling a word.
-  bool SpellCheckWord(const base::string16& word_to_check);
+  bool SpellCheckWord(const SpellCheckScope& scope,
-
+                      const base::string16& word_to_check) const;
-  // Find a possible correctly spelled word for a misspelled word. Computes an
-  // empty string if input misspelled word is too long, there is ambiguity, or
-  // the correct spelling cannot be determined.
-  base::string16 GetAutoCorrectionWord(const base::string16& word);
 
   // Returns whether or not the given word is a contraction of valid words
   // (e.g. "word:word").
-  bool IsValidContraction(const base::string16& word);
+  bool IsValidContraction(const SpellCheckScope& scope,
+                          const base::string16& word);
 
   // Represents character attributes used for filtering out characters which
   // are not supported by this SpellCheck object.
@@ -79,9 +85,8 @@ class SpellCheckClient : public blink::WebSpellCheckPanelHostClient,
   SpellcheckWordIterator text_iterator_;
   SpellcheckWordIterator contraction_iterator_;
 
-  bool auto_spell_correct_turned_on_;
-
   v8::Isolate* isolate_;
+  v8::Persistent<v8::Context> context_;
   mate::ScopedPersistent<v8::Object> provider_;
   mate::ScopedPersistent<v8::Function> spell_check_;