From 4ba74d932823f6d9f9d606d1565dafa84a91e636 Mon Sep 17 00:00:00 2001 From: "rouslan@chromium.org" Date: Wed, 20 Feb 2013 12:09:53 +0000 Subject: [PATCH] Ignore custom spellcheck dictionary words when using server-side spellcheck Chrome has been storing custom spellcheck dictionary words in the platform spelling engine (Hunspell), which does not interact with the server-side spellcheck. This has caused server-side spellcheck to not take into account user's custom spellcheck dictionary. This CL moves handling of custom words from platform spelling engine to spellchecker that uses both the platform spelling engine and the server-side spellcheck. BUG=169629 Review URL: https://chromiumcodereview.appspot.com/12303011 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@183497 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/chrome_renderer.gypi | 2 + .../spellchecker/cocoa_spelling_engine_mac.cc | 9 +-- .../spellchecker/cocoa_spelling_engine_mac.h | 6 +- .../spellchecker/custom_dictionary_engine.cc | 52 +++++++++++++++ .../spellchecker/custom_dictionary_engine.h | 44 +++++++++++++ .../renderer/spellchecker/hunspell_engine.cc | 64 +------------------ .../renderer/spellchecker/hunspell_engine.h | 14 +--- chrome/renderer/spellchecker/spellcheck.cc | 49 +++++++------- chrome/renderer/spellchecker/spellcheck.h | 5 +- .../spellchecker/spellcheck_language.cc | 16 ++--- .../spellchecker/spellcheck_language.h | 9 +-- .../renderer/spellchecker/spelling_engine.h | 6 +- 12 files changed, 139 insertions(+), 137 deletions(-) create mode 100644 chrome/renderer/spellchecker/custom_dictionary_engine.cc create mode 100644 chrome/renderer/spellchecker/custom_dictionary_engine.h diff --git a/chrome/chrome_renderer.gypi b/chrome/chrome_renderer.gypi index 1bf394011e79f5..8659e81397efa6 100644 --- a/chrome/chrome_renderer.gypi +++ b/chrome/chrome_renderer.gypi @@ -265,6 +265,8 @@ 'renderer/security_filter_peer.h', 'renderer/spellchecker/cocoa_spelling_engine_mac.cc', 'renderer/spellchecker/cocoa_spelling_engine_mac.h', + 'renderer/spellchecker/custom_dictionary_engine.cc', + 'renderer/spellchecker/custom_dictionary_engine.h', 'renderer/spellchecker/hunspell_engine.cc', 'renderer/spellchecker/hunspell_engine.h', 'renderer/spellchecker/spellcheck_provider.cc', diff --git a/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.cc b/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.cc index 15f416bd30ddc0..1f487940131126 100644 --- a/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.cc +++ b/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.cc @@ -13,8 +13,7 @@ SpellingEngine* CreateNativeSpellingEngine() { return new CocoaSpellingEngine(); } -void CocoaSpellingEngine::Init(base::PlatformFile bdict_file, - const std::vector&) { +void CocoaSpellingEngine::Init(base::PlatformFile bdict_file) { DCHECK(bdict_file == base::kInvalidPlatformFileValue); } @@ -46,9 +45,3 @@ void CocoaSpellingEngine::FillSuggestionList( RenderThread::Get()->Send(new SpellCheckHostMsg_FillSuggestionList( wrong_word, optional_suggestions)); } - -void CocoaSpellingEngine::OnCustomDictionaryChanged( - const std::vector& words_added, - const std::vector& words_removed) { - // OSX doesn't support the custom dictionary yet. -} diff --git a/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.h b/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.h index d00a8d5a44b87d..6b197ac729c8fe 100644 --- a/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.h +++ b/chrome/renderer/spellchecker/cocoa_spelling_engine_mac.h @@ -10,16 +10,12 @@ class CocoaSpellingEngine : public SpellingEngine { public: - virtual void Init(base::PlatformFile bdict_file, - const std::vector& custom_words) OVERRIDE; + virtual void Init(base::PlatformFile bdict_file) OVERRIDE; virtual bool InitializeIfNeeded() OVERRIDE; virtual bool IsEnabled() OVERRIDE; virtual bool CheckSpelling(const string16& word_to_check, int tag) OVERRIDE; virtual void FillSuggestionList(const string16& wrong_word, std::vector* optional_suggestions) OVERRIDE; - virtual void OnCustomDictionaryChanged( - const std::vector& words_added, - const std::vector& words_removed) OVERRIDE; }; #endif // CHROME_RENDERER_SPELLCHECKER_NSSPELLCHECKER_ENGINE_H_ diff --git a/chrome/renderer/spellchecker/custom_dictionary_engine.cc b/chrome/renderer/spellchecker/custom_dictionary_engine.cc new file mode 100644 index 00000000000000..8557e9cb455499 --- /dev/null +++ b/chrome/renderer/spellchecker/custom_dictionary_engine.cc @@ -0,0 +1,52 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/renderer/spellchecker/custom_dictionary_engine.h" + +#include "base/logging.h" +#include "base/utf_string_conversions.h" + +CustomDictionaryEngine::CustomDictionaryEngine() { +} + +CustomDictionaryEngine::~CustomDictionaryEngine() { +} + +void CustomDictionaryEngine::Init( + const std::vector& custom_words) { + // SpellingMenuOberver calls UTF16ToUTF8(word) to convert words for storage, + // synchronization, and use in the custom dictionary engine. Since + // (UTF8ToUTF16(UTF16ToUTF8(word)) == word) holds, the engine does not need to + // normalize the strings. + for (std::vector::const_iterator it = custom_words.begin(); + it != custom_words.end(); + ++it) { + dictionary_.insert(UTF8ToUTF16(*it)); + } +} + +void CustomDictionaryEngine::OnCustomDictionaryChanged( + const std::vector& words_added, + const std::vector& words_removed) { + for (std::vector::const_iterator it = words_added.begin(); + it != words_added.end(); + ++it) { + dictionary_.insert(UTF8ToUTF16(*it)); + } + for (std::vector::const_iterator it = words_removed.begin(); + it != words_removed.end(); + ++it) { + dictionary_.erase(UTF8ToUTF16(*it)); + } +} + +bool CustomDictionaryEngine::SpellCheckWord( + const char16* text, + int misspelling_start, + int misspelling_len) { + DCHECK(text); + return misspelling_start >= 0 && + misspelling_len > 0 && + dictionary_.count(string16(text, misspelling_start, misspelling_len)) > 0; +} diff --git a/chrome/renderer/spellchecker/custom_dictionary_engine.h b/chrome/renderer/spellchecker/custom_dictionary_engine.h new file mode 100644 index 00000000000000..d2dc8caebf76c8 --- /dev/null +++ b/chrome/renderer/spellchecker/custom_dictionary_engine.h @@ -0,0 +1,44 @@ +// Copyright (c) 2012 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_RENDERER_SPELLCHECKER_CUSTOM_DICTIONARY_ENGINE_H_ +#define CHROME_RENDERER_SPELLCHECKER_CUSTOM_DICTIONARY_ENGINE_H_ + +#include +#include +#include + +#include "base/string16.h" + +// Custom spellcheck dictionary. Words in this dictionary are always correctly +// spelled. Words that are not in this dictionary may or may not be correctly +// spelled. +class CustomDictionaryEngine { + public: + CustomDictionaryEngine(); + ~CustomDictionaryEngine(); + + // Initialize the custom dictionary engine. + void Init(const std::vector& words); + + // Spellcheck |text|. Assumes that another spelling engine has set + // |misspelling_start| and |misspelling_len| to indicate a misspelling. + // Returns true if there are no misspellings, otherwise returns false. + bool SpellCheckWord(const char16* text, + int misspelling_start, + int misspelling_len); + + // Update custom dictionary words. + void OnCustomDictionaryChanged( + const std::vector& words_added, + const std::vector& words_removed); + + private: + // Correctly spelled words. + std::set dictionary_; + + DISALLOW_COPY_AND_ASSIGN(CustomDictionaryEngine); +}; + +#endif // CHROME_RENDERER_SPELLCHECKER_CUSTOM_DICTIONARY_ENGINE_H_ diff --git a/chrome/renderer/spellchecker/hunspell_engine.cc b/chrome/renderer/spellchecker/hunspell_engine.cc index 6cb17550434952..34004f741a47e9 100644 --- a/chrome/renderer/spellchecker/hunspell_engine.cc +++ b/chrome/renderer/spellchecker/hunspell_engine.cc @@ -46,16 +46,11 @@ HunspellEngine::HunspellEngine() HunspellEngine::~HunspellEngine() { } -void HunspellEngine::Init(base::PlatformFile file, - const std::vector& custom_words) { +void HunspellEngine::Init(base::PlatformFile file) { initialized_ = true; hunspell_.reset(); bdict_file_.reset(); file_ = file; - - custom_words_.insert(custom_words_.end(), - custom_words.begin(), custom_words.end()); - // Delay the actual initialization of hunspell until it is needed. } @@ -71,9 +66,6 @@ void HunspellEngine::InitializeHunspell() { hunspell_.reset( new Hunspell(bdict_file_->data(), bdict_file_->length())); - // Add custom words to Hunspell. - AddWordsToHunspell(custom_words_); - DHISTOGRAM_TIMES("Spellcheck.InitTime", base::Histogram::DebugNow() - debug_start_time); } else { @@ -81,35 +73,6 @@ void HunspellEngine::InitializeHunspell() { } } -void HunspellEngine::AddWordsToHunspell(const std::vector& words) { - std::string word; - for (chrome::spellcheck_common::WordList::const_iterator it = words.begin(); - it != words.end(); - ++it) { - word = *it; - if (!word.empty() && - word.length() <= - chrome::spellcheck_common::MAX_CUSTOM_DICTIONARY_WORD_BYTES) { - hunspell_->add(word.c_str()); - } - } -} - -void HunspellEngine::RemoveWordsFromHunspell( - const std::vector& words) { - std::string word; - for (std::vector::const_iterator it = words.begin(); - it != words.end(); - ++it) { - word = *it; - if (!word.empty() && - word.length() <= - chrome::spellcheck_common::MAX_CUSTOM_DICTIONARY_WORD_BYTES) { - hunspell_->remove(word.c_str()); - } - } -} - bool HunspellEngine::CheckSpelling(const string16& word_to_check, int tag) { // Assume all words that cannot be checked are valid. Since Chrome can't // offer suggestions on them, either, there's no point in flagging them to @@ -157,31 +120,6 @@ void HunspellEngine::FillSuggestionList( free(suggestions); } -void HunspellEngine::OnCustomDictionaryChanged( - const std::vector& words_added, - const std::vector& words_removed) { - if (!hunspell_.get()) { - // Save it for later---add it when hunspell is initialized. - custom_words_.insert(custom_words_.end(), - words_added.begin(), - words_added.end()); - // Remove words. - std::vector words_removed_copy(words_removed); - std::sort(words_removed_copy.begin(), words_removed_copy.end()); - std::sort(custom_words_.begin(), custom_words_.end()); - std::vector updated_custom_words; - std::set_difference(custom_words_.begin(), - custom_words_.end(), - words_removed_copy.begin(), - words_removed_copy.end(), - std::back_inserter(updated_custom_words)); - std::swap(custom_words_, updated_custom_words); - } else { - AddWordsToHunspell(words_added); - RemoveWordsFromHunspell(words_removed); - } -} - bool HunspellEngine::InitializeIfNeeded() { if (!initialized_ && !dictionary_requested_) { // RenderThread will not exist in test. diff --git a/chrome/renderer/spellchecker/hunspell_engine.h b/chrome/renderer/spellchecker/hunspell_engine.h index 5b26265a79d96c..26d3a39ce14fae 100644 --- a/chrome/renderer/spellchecker/hunspell_engine.h +++ b/chrome/renderer/spellchecker/hunspell_engine.h @@ -22,37 +22,25 @@ class HunspellEngine : public SpellingEngine { HunspellEngine(); virtual ~HunspellEngine(); - virtual void Init(base::PlatformFile file, - const std::vector& custom_words) OVERRIDE; + virtual void Init(base::PlatformFile file) OVERRIDE; virtual bool InitializeIfNeeded() OVERRIDE; virtual bool IsEnabled() OVERRIDE; virtual bool CheckSpelling(const string16& word_to_check, int tag) OVERRIDE; virtual void FillSuggestionList(const string16& wrong_word, std::vector* optional_suggestions) OVERRIDE; - virtual void OnCustomDictionaryChanged( - const std::vector& words_added, - const std::vector& words_removed) OVERRIDE; private: // Initializes the Hunspell dictionary, or does nothing if |hunspell_| is // non-null. This blocks. void InitializeHunspell(); - // Add the given custom words to |hunspell_|. - void AddWordsToHunspell(const std::vector& words); - - // Remove the given custom words from |hunspell_|. - void RemoveWordsFromHunspell(const std::vector& words); - // We memory-map the BDict file. scoped_ptr bdict_file_; // The hunspell dictionary in use. scoped_ptr hunspell_; - chrome::spellcheck_common::WordList custom_words_; - base::PlatformFile file_; // This flags is true if we have been initialized. diff --git a/chrome/renderer/spellchecker/spellcheck.cc b/chrome/renderer/spellchecker/spellcheck.cc index 4fa7949ecaea63..0fadda7fa420f5 100644 --- a/chrome/renderer/spellchecker/spellcheck.cc +++ b/chrome/renderer/spellchecker/spellcheck.cc @@ -6,6 +6,7 @@ #include "base/bind.h" #include "base/message_loop_proxy.h" +#include "base/utf_string_conversions.h" #include "chrome/common/render_messages.h" #include "chrome/common/spellcheck_common.h" #include "chrome/common/spellcheck_messages.h" @@ -119,7 +120,7 @@ void SpellCheck::OnInit(IPC::PlatformFileForTransit bdict_file, void SpellCheck::OnCustomDictionaryChanged( const std::vector& words_added, const std::vector& words_removed) { - spellcheck_.OnCustomDictionaryChanged(words_added, words_removed); + custom_dictionary_.OnCustomDictionaryChanged(words_added, words_removed); } void SpellCheck::OnEnableAutoSpellCorrect(bool enable) { @@ -137,7 +138,8 @@ void SpellCheck::OnEnableSpellCheck(bool enable) { void SpellCheck::Init(base::PlatformFile file, const std::vector& custom_words, const std::string& language) { - spellcheck_.Init(file, custom_words, language); + spellcheck_.Init(file, language); + custom_dictionary_.Init(custom_words); } bool SpellCheck::SpellCheckWord( @@ -159,17 +161,15 @@ bool SpellCheck::SpellCheckWord( tag, misspelling_start, misspelling_len, optional_suggestions); - - return true; } bool SpellCheck::SpellCheckParagraph( const string16& text, - WebKit::WebVector* results) { + WebVector* results) { #if !defined(OS_MACOSX) // Mac has its own spell checker, so this method will not be used. DCHECK(results); - std::vector textcheck_results; + std::vector textcheck_results; size_t length = text.length(); size_t offset = 0; @@ -191,12 +191,15 @@ bool SpellCheck::SpellCheckParagraph( return true; } - string16 replacement; - textcheck_results.push_back(WebKit::WebTextCheckingResult( - WebKit::WebTextCheckingTypeSpelling, - misspelling_start + offset, - misspelling_length, - replacement)); + if (!custom_dictionary_.SpellCheckWord( + &text[offset], misspelling_start, misspelling_length)) { + string16 replacement; + textcheck_results.push_back(WebTextCheckingResult( + WebKit::WebTextCheckingTypeSpelling, + misspelling_start + offset, + misspelling_length, + replacement)); + } offset += misspelling_start + misspelling_length; } results->assign(textcheck_results); @@ -300,7 +303,7 @@ void SpellCheck::PerformSpellCheck(SpellcheckRequest* param) { if (!spellcheck_.IsEnabled()) { param->completion()->didCancelCheckingText(); } else { - WebKit::WebVector results; + WebVector results; SpellCheckParagraph(param->text(), &results); param->completion()->didFinishCheckingText(results); } @@ -317,26 +320,28 @@ void SpellCheck::CreateTextCheckingResults( // markers to them if our spellchecker tells they are correct words, i.e. they // are probably contextually-misspelled words. const char16* text = line_text.c_str(); - WebVector list(spellcheck_results.size()); + std::vector list; for (size_t i = 0; i < spellcheck_results.size(); ++i) { WebTextCheckingType type = static_cast(spellcheck_results[i].type); int word_location = spellcheck_results[i].location; int word_length = spellcheck_results[i].length; + int misspelling_start = 0; + int misspelling_length = 0; if (type == WebKit::WebTextCheckingTypeSpelling && filter == USE_NATIVE_CHECKER) { - int misspelling_start = 0; - int misspelling_length = 0; if (SpellCheckWord(text + word_location, word_length, 0, &misspelling_start, &misspelling_length, NULL)) { type = WebKit::WebTextCheckingTypeGrammar; } } - list[i] = WebKit::WebTextCheckingResult(type, - word_location + line_offset, - word_length, - spellcheck_results[i].replacement); + if (!custom_dictionary_.SpellCheckWord(text, word_location, word_length)) { + list.push_back(WebTextCheckingResult( + type, + word_location + line_offset, + word_length, + spellcheck_results[i].replacement)); + } } - textcheck_results->swap(list); + textcheck_results->assign(list); } - diff --git a/chrome/renderer/spellchecker/spellcheck.h b/chrome/renderer/spellchecker/spellcheck.h index 48f4637854fe8a..5812986eb20562 100644 --- a/chrome/renderer/spellchecker/spellcheck.h +++ b/chrome/renderer/spellchecker/spellcheck.h @@ -13,6 +13,7 @@ #include "base/memory/weak_ptr.h" #include "base/platform_file.h" #include "base/string16.h" +#include "chrome/renderer/spellchecker/custom_dictionary_engine.h" #include "chrome/renderer/spellchecker/spellcheck_language.h" #include "content/public/renderer/render_process_observer.h" #include "ipc/ipc_platform_file.h" @@ -149,9 +150,11 @@ class SpellCheck : public content::RenderProcessObserver, scoped_ptr pending_request_param_; #endif - private: SpellcheckLanguage spellcheck_; // Language-specific spellchecking code. + // Custom dictionary spelling engine. + CustomDictionaryEngine custom_dictionary_; + // Remember state for auto spell correct. bool auto_spell_correct_turned_on_; diff --git a/chrome/renderer/spellchecker/spellcheck_language.cc b/chrome/renderer/spellchecker/spellcheck_language.cc index 55fcbb07e1d364..01c81590fa471e 100644 --- a/chrome/renderer/spellchecker/spellcheck_language.cc +++ b/chrome/renderer/spellchecker/spellcheck_language.cc @@ -16,11 +16,11 @@ SpellcheckLanguage::SpellcheckLanguage() SpellcheckLanguage::~SpellcheckLanguage() { } -void SpellcheckLanguage::Init(base::PlatformFile file, - const std::vector& custom_words, - const std::string& language) { +void SpellcheckLanguage::Init( + base::PlatformFile file, + const std::string& language) { DCHECK(platform_spelling_engine_.get()); - platform_spelling_engine_->Init(file, custom_words); + platform_spelling_engine_->Init(file); character_attributes_.SetDefaultLanguage(language); text_iterator_.Reset(); @@ -32,14 +32,6 @@ bool SpellcheckLanguage::InitializeIfNeeded() { return platform_spelling_engine_->InitializeIfNeeded(); } -void SpellcheckLanguage::OnCustomDictionaryChanged( - const std::vector& words_added, - const std::vector& words_removed) { - DCHECK(platform_spelling_engine_.get()); - platform_spelling_engine_->OnCustomDictionaryChanged(words_added, - words_removed); -} - bool SpellcheckLanguage::SpellCheckWord( const char16* in_word, int in_word_len, diff --git a/chrome/renderer/spellchecker/spellcheck_language.h b/chrome/renderer/spellchecker/spellcheck_language.h index 511382e34a0828..a6063c6091a13f 100644 --- a/chrome/renderer/spellchecker/spellcheck_language.h +++ b/chrome/renderer/spellchecker/spellcheck_language.h @@ -21,14 +21,7 @@ class SpellcheckLanguage { SpellcheckLanguage(); ~SpellcheckLanguage(); - void Init(base::PlatformFile file, - const std::vector& custom_words, - const std::string& language); - - // Update custom dictionary. - void OnCustomDictionaryChanged( - const std::vector& words_added, - const std::vector& words_removed); + void Init(base::PlatformFile file, const std::string& language); // SpellCheck a word. // Returns true if spelled correctly, false otherwise. diff --git a/chrome/renderer/spellchecker/spelling_engine.h b/chrome/renderer/spellchecker/spelling_engine.h index 9d40b07c1c8a0d..455f3050b99dfa 100644 --- a/chrome/renderer/spellchecker/spelling_engine.h +++ b/chrome/renderer/spellchecker/spelling_engine.h @@ -21,17 +21,13 @@ class SpellingEngine { // Initialize spelling engine with browser-side info. Must be called before // any other functions are called. - virtual void Init(base::PlatformFile bdict_file, - const std::vector& custom_words) = 0; + virtual void Init(base::PlatformFile bdict_file) = 0; virtual bool InitializeIfNeeded() = 0; virtual bool IsEnabled() = 0; virtual bool CheckSpelling(const string16& word_to_check, int tag) = 0; virtual void FillSuggestionList( const string16& wrong_word, std::vector* optional_suggestions) = 0; - virtual void OnCustomDictionaryChanged( - const std::vector& words_added, - const std::vector& words_removed) = 0; }; #endif // CHROME_RENDERER_SPELLCHECKER_SPELLING_ENGINE_H_