Merge pull request #11359 from electron/fix_spell_checker_crash

Fixed crash in `atom::api::SpellCheckClient`

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);
-  text_iterator_.SetText(in_word.c_str(), in_word.size());
+  if (!contraction_iterator_.IsInitialized() &&
+      !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) {
-  if (spell_check_.IsEmpty())
-    return true;
+bool SpellCheckClient::SpellCheckWord(
+    const SpellCheckScope& scope,
+    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(
-      provider_.NewHandle(), 1, &word);
+  v8::Local<v8::Value> result =
+      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) {
-  if (!contraction_iterator_.IsInitialized() &&
-      !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 true;
-  }
+bool SpellCheckClient::IsValidContraction(const SpellCheckScope& scope,
+                                          const base::string16& contraction) {
+  DCHECK(contraction_iterator_.IsInitialized());
 
   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);
-
-  // 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);
+  bool SpellCheckWord(const SpellCheckScope& scope,
+                      const base::string16& word_to_check) const;
 
   // 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_;