From 7a73b1d5237c2a42ec929b587f33cc9e9332df9a Mon Sep 17 00:00:00 2001 From: Ales Pergl Date: Wed, 6 Dec 2017 12:55:28 +0100 Subject: [PATCH] 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. --- .../api/atom_api_spell_check_client.cc | 65 ++++++++++++------- .../api/atom_api_spell_check_client.h | 23 ++++--- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/atom/renderer/api/atom_api_spell_check_client.cc b/atom/renderer/api/atom_api_spell_check_client.cc index 36561da467ea..3f7676595164 100644 --- a/atom/renderer/api/atom_api_spell_check_client.cc +++ b/atom/renderer/api/atom_api_spell_check_client.cc @@ -41,7 +41,10 @@ SpellCheckClient::SpellCheckClient(const std::string& language, v8::Isolate* isolate, v8::Local 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* 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 word = mate::ConvertToV8(isolate_, word_to_check); - v8::Local result = spell_check_.NewHandle()->Call( - provider_.NewHandle(), 1, &word); + v8::Local 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::New(client.isolate_, client.context_)), + provider_(client.provider_.NewHandle()), + spell_check_(client.spell_check_.NewHandle()) {} + } // namespace api } // namespace atom diff --git a/atom/renderer/api/atom_api_spell_check_client.h b/atom/renderer/api/atom_api_spell_check_client.h index ce0533a5b193..acdb2164437e 100644 --- a/atom/renderer/api/atom_api_spell_check_client.h +++ b/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 provider_; + v8::Local 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* 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 context_; mate::ScopedPersistent provider_; mate::ScopedPersistent spell_check_;