diff options
Diffstat (limited to 'chromium/components/spellcheck')
28 files changed, 383 insertions, 226 deletions
diff --git a/chromium/components/spellcheck/BUILD.gn b/chromium/components/spellcheck/BUILD.gn index 5e71df2f932..3e4ae90ae86 100644 --- a/chromium/components/spellcheck/BUILD.gn +++ b/chromium/components/spellcheck/BUILD.gn @@ -11,7 +11,6 @@ buildflag_header("buildflags") { "ENABLE_SPELLCHECK=$enable_spellcheck", "USE_BROWSER_SPELLCHECKER=$use_browser_spellchecker", "USE_RENDERER_SPELLCHECKER=$use_renderer_spellchecker", - "USE_WIN_HYBRID_SPELLCHECKER=$use_win_hybrid_spellchecker", "ENABLE_SPELLING_SERVICE=$enable_spelling_service", "HAS_SPELLCHECK_PANEL=$has_spellcheck_panel", ] diff --git a/chromium/components/spellcheck/browser/spell_check_host_impl.cc b/chromium/components/spellcheck/browser/spell_check_host_impl.cc index 0d600fa9d0b..d585f5d2ae5 100644 --- a/chromium/components/spellcheck/browser/spell_check_host_impl.cc +++ b/chromium/components/spellcheck/browser/spell_check_host_impl.cc @@ -67,10 +67,8 @@ void SpellCheckHostImpl::FillSuggestionList( NOTREACHED(); std::move(callback).Run({}); } -#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) && - // !BUILDFLAG(ENABLE_SPELLING_SERVICE) -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) void SpellCheckHostImpl::GetPerLanguageSuggestions( const base::string16& word, GetPerLanguageSuggestionsCallback callback) { @@ -79,7 +77,10 @@ void SpellCheckHostImpl::GetPerLanguageSuggestions( // This API requires Chrome-only features. std::move(callback).Run(std::vector<std::vector<base::string16>>()); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) + +#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) && + // !BUILDFLAG(ENABLE_SPELLING_SERVICE) #if defined(OS_ANDROID) void SpellCheckHostImpl::DisconnectSessionBridge() { diff --git a/chromium/components/spellcheck/browser/spell_check_host_impl.h b/chromium/components/spellcheck/browser/spell_check_host_impl.h index 0e847af45a7..efade8ac2bf 100644 --- a/chromium/components/spellcheck/browser/spell_check_host_impl.h +++ b/chromium/components/spellcheck/browser/spell_check_host_impl.h @@ -50,14 +50,15 @@ class SpellCheckHostImpl : public spellcheck::mojom::SpellCheckHost { CheckSpellingCallback callback) override; void FillSuggestionList(const base::string16& word, FillSuggestionListCallback callback) override; -#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) && - // !BUILDFLAG(ENABLE_SPELLING_SERVICE) -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) void GetPerLanguageSuggestions( const base::string16& word, GetPerLanguageSuggestionsCallback callback) override; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) + +#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) && + // !BUILDFLAG(ENABLE_SPELLING_SERVICE) #if defined(OS_ANDROID) // spellcheck::mojom::SpellCheckHost: diff --git a/chromium/components/spellcheck/browser/spellcheck_platform.h b/chromium/components/spellcheck/browser/spellcheck_platform.h index ad11d9e34e9..be1e5e7f32a 100644 --- a/chromium/components/spellcheck/browser/spellcheck_platform.h +++ b/chromium/components/spellcheck/browser/spellcheck_platform.h @@ -20,9 +20,9 @@ #include "components/spellcheck/browser/spellcheck_host_metrics.h" #endif // defined(OS_WIN) -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) #include "components/spellcheck/common/spellcheck_common.h" -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) class PlatformSpellChecker; @@ -33,10 +33,10 @@ namespace spellcheck_platform { typedef base::OnceCallback<void(const std::vector<SpellCheckResult>&)> TextCheckCompleteCallback; -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) typedef base::OnceCallback<void(const spellcheck::PerLanguageSuggestions&)> GetSuggestionsCallback; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) typedef base::OnceCallback<void(const std::vector<std::string>& /* results */)> RetrieveSpellcheckLanguagesCompleteCallback; @@ -135,12 +135,12 @@ void RequestTextCheck(PlatformSpellChecker* spell_checker_instance, const base::string16& text, TextCheckCompleteCallback callback); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) // Finds the replacement suggestions for each language for the given word. void GetPerLanguageSuggestions(PlatformSpellChecker* spell_checker_instance, const base::string16& word, GetSuggestionsCallback callback); -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) #if defined(OS_WIN) // Records statistics about spell check support for the user's Chrome locales. diff --git a/chromium/components/spellcheck/browser/spellcheck_platform_win.cc b/chromium/components/spellcheck/browser/spellcheck_platform_win.cc index 1cc03aef4f8..8e66c3ca1bc 100644 --- a/chromium/components/spellcheck/browser/spellcheck_platform_win.cc +++ b/chromium/components/spellcheck/browser/spellcheck_platform_win.cc @@ -61,14 +61,14 @@ void RequestTextCheck(PlatformSpellChecker* spell_checker_instance, ->RequestTextCheck(document_tag, text, std::move(callback)); } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) void GetPerLanguageSuggestions(PlatformSpellChecker* spell_checker_instance, const base::string16& word, GetSuggestionsCallback callback) { reinterpret_cast<WindowsSpellChecker*>(spell_checker_instance) ->GetPerLanguageSuggestions(word, std::move(callback)); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) void AddWord(PlatformSpellChecker* spell_checker_instance, const base::string16& word) { diff --git a/chromium/components/spellcheck/browser/windows_spell_checker.cc b/chromium/components/spellcheck/browser/windows_spell_checker.cc index e27ac54526f..ec32eb49206 100644 --- a/chromium/components/spellcheck/browser/windows_spell_checker.cc +++ b/chromium/components/spellcheck/browser/windows_spell_checker.cc @@ -19,6 +19,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/callback.h" +#include "base/logging.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/task/post_task.h" diff --git a/chromium/components/spellcheck/browser/windows_spell_checker_unittest.cc b/chromium/components/spellcheck/browser/windows_spell_checker_unittest.cc index df5731d6717..75a2c2db691 100644 --- a/chromium/components/spellcheck/browser/windows_spell_checker_unittest.cc +++ b/chromium/components/spellcheck/browser/windows_spell_checker_unittest.cc @@ -31,15 +31,7 @@ namespace { class WindowsSpellCheckerTest : public testing::Test { public: WindowsSpellCheckerTest() { -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) - // Force hybrid spellchecking to be enabled. - feature_list_.InitWithFeatures( - /*enabled_features=*/{spellcheck::kWinUseBrowserSpellChecker, - spellcheck::kWinUseHybridSpellChecker}, - /*disabled_features=*/{}); -#else feature_list_.InitAndEnableFeature(spellcheck::kWinUseBrowserSpellChecker); -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) // The WindowsSpellchecker object can be created even on Windows versions // that don't support platform spellchecking. However, the spellcheck @@ -82,7 +74,6 @@ class WindowsSpellCheckerTest : public testing::Test { std::move(quit_).Run(); } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) void PerLanguageSuggestionsCompletionCallback( const spellcheck::PerLanguageSuggestions& suggestions) { callback_finished_ = true; @@ -90,7 +81,6 @@ class WindowsSpellCheckerTest : public testing::Test { if (quit_) std::move(quit_).Run(); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) void RetrieveSpellcheckLanguagesCompletionCallback( const std::vector<std::string>& spellcheck_languages) { @@ -112,9 +102,7 @@ class WindowsSpellCheckerTest : public testing::Test { bool set_language_result_; std::vector<SpellCheckResult> spell_check_results_; -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) spellcheck::PerLanguageSuggestions per_language_suggestions_; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) std::vector<std::string> spellcheck_languages_; base::test::TaskEnvironment task_environment_{ @@ -217,7 +205,6 @@ TEST_F(WindowsSpellCheckerTest, RetrieveSpellcheckLanguagesFakeDictionaries) { ASSERT_EQ(spellcheck_languages_for_testing, spellcheck_languages_); } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) TEST_F(WindowsSpellCheckerTest, GetPerLanguageSuggestions) { ASSERT_EQ(set_language_result_, spellcheck::WindowsVersionSupportsSpellchecker()); @@ -239,6 +226,5 @@ TEST_F(WindowsSpellCheckerTest, GetPerLanguageSuggestions) { ASSERT_EQ(per_language_suggestions_.size(), 1u); ASSERT_GT(per_language_suggestions_[0].size(), 0u); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) } // namespace diff --git a/chromium/components/spellcheck/common/BUILD.gn b/chromium/components/spellcheck/common/BUILD.gn index 77e39c803d7..a7b429c323e 100644 --- a/chromium/components/spellcheck/common/BUILD.gn +++ b/chromium/components/spellcheck/common/BUILD.gn @@ -54,8 +54,4 @@ mojom("interfaces") { if (use_renderer_spellchecker) { enabled_features += [ "USE_RENDERER_SPELLCHECKER" ] } - - if (use_win_hybrid_spellchecker) { - enabled_features += [ "USE_WIN_HYBRID_SPELLCHECKER" ] - } } diff --git a/chromium/components/spellcheck/common/spellcheck.mojom b/chromium/components/spellcheck/common/spellcheck.mojom index f5f5b8966af..184169ca5ed 100644 --- a/chromium/components/spellcheck/common/spellcheck.mojom +++ b/chromium/components/spellcheck/common/spellcheck.mojom @@ -77,7 +77,7 @@ interface SpellCheckHost { // Returns a list of suggestions for each spellcheck language for a given word // with a platform-specific spell checker. - [EnableIf=USE_WIN_HYBRID_SPELLCHECKER, Sync] + [EnableIf=is_win, Sync] GetPerLanguageSuggestions(mojo_base.mojom.String16 word) => (array<array<mojo_base.mojom.String16>> suggestions); }; diff --git a/chromium/components/spellcheck/common/spellcheck_features.cc b/chromium/components/spellcheck/common/spellcheck_features.cc index 0732801e2f2..445941b1a2a 100644 --- a/chromium/components/spellcheck/common/spellcheck_features.cc +++ b/chromium/components/spellcheck/common/spellcheck_features.cc @@ -13,11 +13,6 @@ namespace spellcheck { #if BUILDFLAG(ENABLE_SPELLCHECK) -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) -const base::Feature kWinUseHybridSpellChecker{ - "WinUseHybridSpellChecker", base::FEATURE_DISABLED_BY_DEFAULT}; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) - bool UseBrowserSpellChecker() { #if !BUILDFLAG(USE_BROWSER_SPELLCHECKER) return false; @@ -33,19 +28,13 @@ bool UseBrowserSpellChecker() { const base::Feature kWinUseBrowserSpellChecker{ "WinUseBrowserSpellChecker", base::FEATURE_DISABLED_BY_DEFAULT}; +const base::Feature kWinDelaySpellcheckServiceInit{ + "WinDelaySpellcheckServiceInit", base::FEATURE_DISABLED_BY_DEFAULT}; + bool WindowsVersionSupportsSpellchecker() { return base::win::GetVersion() > base::win::Version::WIN7 && base::win::GetVersion() < base::win::Version::WIN_LAST; } - -bool UseWinHybridSpellChecker() { -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) - return base::FeatureList::IsEnabled(spellcheck::kWinUseHybridSpellChecker) && - UseBrowserSpellChecker(); -#else - return false; -#endif -} #endif // defined(OS_WIN) #if defined(OS_ANDROID) diff --git a/chromium/components/spellcheck/common/spellcheck_features.h b/chromium/components/spellcheck/common/spellcheck_features.h index ed2936f88de..282bd74cbb8 100644 --- a/chromium/components/spellcheck/common/spellcheck_features.h +++ b/chromium/components/spellcheck/common/spellcheck_features.h @@ -13,17 +13,32 @@ namespace spellcheck { #if BUILDFLAG(ENABLE_SPELLCHECK) -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) -extern const base::Feature kWinUseHybridSpellChecker; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) - bool UseBrowserSpellChecker(); #if defined(OS_WIN) extern const base::Feature kWinUseBrowserSpellChecker; +// If the kWinDelaySpellcheckServiceInit feature flag is enabled, don't +// initialize the spellcheck dictionaries when the SpellcheckService is +// instantiated. With this flag set: (1) Completing the initialization of the +// spellcheck service is on-demand, invoked by calling +// SpellcheckService::InitializeDictionaries with a callback to indicate when +// the operation completes. (2) The call to create the spellcheck service in +// ChromeBrowserMainParts::PreMainMessageLoopRunImpl will be skipped. Chromium +// will still by default instantiate the spellcheck service on startup for +// custom dictionary synchronization, but will not load Windows spellcheck +// dictionaries. The command line for launching the browser with Windows hybrid +// spellchecking enabled but no initialization of the spellcheck service is: +// chrome +// --enable-features=WinUseBrowserSpellChecker,WinDelaySpellcheckServiceInit +// and if instantiation of the spellcheck service needs to be completely +// disabled: +// chrome +// --enable-features=WinUseBrowserSpellChecker,WinDelaySpellcheckServiceInit +// --disable-sync-types="Dictionary" +extern const base::Feature kWinDelaySpellcheckServiceInit; + bool WindowsVersionSupportsSpellchecker(); -bool UseWinHybridSpellChecker(); #endif // defined(OS_WIN) #if defined(OS_ANDROID) diff --git a/chromium/components/spellcheck/renderer/spellcheck.cc b/chromium/components/spellcheck/renderer/spellcheck.cc index 53f87d5caaf..9a036faa65e 100644 --- a/chromium/components/spellcheck/renderer/spellcheck.cc +++ b/chromium/components/spellcheck/renderer/spellcheck.cc @@ -297,7 +297,7 @@ bool SpellCheck::SpellCheckWord( suggestions_list.clear(); for (auto language = languages_.begin(); language != languages_.end();) { -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) if (!(*language)->IsEnabled()) { // In the case of hybrid spell checking on Windows, languages that are // handled on the browser side are marked as disabled on the renderer @@ -306,7 +306,7 @@ bool SpellCheck::SpellCheckWord( language++; continue; } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) language_suggestions.clear(); SpellcheckLanguage::SpellcheckWordResult result = @@ -363,18 +363,18 @@ bool SpellCheck::SpellCheckWord( return false; } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) // If we're performing a hybrid spell check, we're only interested in // knowing whether some Hunspell languages considered this text range as // correctly spelled. If no misspellings were found, but the entire text was // skipped, it means that no Hunspell language considered this text // correct, so we should return false here. - if (spellcheck::UseWinHybridSpellChecker() && + if (spellcheck::UseBrowserSpellChecker() && EnabledLanguageCount() != LanguageCount() && agreed_skippable_len == text_length) { return false; } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) } NOTREACHED(); @@ -470,11 +470,11 @@ void SpellCheck::PerformSpellCheck(SpellcheckRequest* param) { WebVector<blink::WebTextCheckingResult> results; SpellCheckParagraph(param->text(), &results); param->completion()->DidFinishCheckingText(results); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) spellcheck_renderer_metrics::RecordSpellcheckDuration( base::TimeTicks::Now() - param->start_ticks(), /*used_hunspell=*/true, /*used_native=*/false); -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) } } #endif @@ -501,6 +501,15 @@ void SpellCheck::CreateTextCheckingResults( spellcheck_result.replacements; SpellCheckResult::Decoration decoration = spellcheck_result.decoration; +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) + // Ignore words that are in a script not supported by any of the enabled + // spellcheck languages. + if (spellcheck::UseBrowserSpellChecker() && + !IsWordInSupportedScript(misspelled_word)) { + continue; + } +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) + // Ignore words in custom dictionary. if (custom_dictionary_.SpellCheckWord(misspelled_word, 0, misspelled_word.length())) { @@ -530,9 +539,9 @@ void SpellCheck::CreateTextCheckingResults( decoration = SpellCheckResult::GRAMMAR; } } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) else if (filter == USE_HUNSPELL_FOR_HYBRID_CHECK && - spellcheck::UseWinHybridSpellChecker() && + spellcheck::UseBrowserSpellChecker() && EnabledLanguageCount() > 0) { // Remove the suggestions that were generated by the native spell checker, // otherwise Blink will cache them without asking for the suggestions @@ -562,7 +571,7 @@ void SpellCheck::CreateTextCheckingResults( } } } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) results.push_back( WebTextCheckingResult(static_cast<WebTextDecorationType>(decoration), @@ -607,3 +616,10 @@ void SpellCheck::NotifyDictionaryObservers( observer.OnDictionaryUpdated(words_added); } } + +bool SpellCheck::IsWordInSupportedScript(const base::string16& word) const { + return std::find_if(languages_.begin(), languages_.end(), + [word](const auto& language) { + return language->IsTextInSameScript(word); + }) != languages_.end(); +} diff --git a/chromium/components/spellcheck/renderer/spellcheck.h b/chromium/components/spellcheck/renderer/spellcheck.h index 2fc497918cc..7327f78e750 100644 --- a/chromium/components/spellcheck/renderer/spellcheck.h +++ b/chromium/components/spellcheck/renderer/spellcheck.h @@ -192,6 +192,10 @@ class SpellCheck : public base::SupportsWeakPtr<SpellCheck>, void NotifyDictionaryObservers( const blink::WebVector<blink::WebString>& words_added); + // Returns whether a word is in the script of one of the enabled spellcheck + // languages. + bool IsWordInSupportedScript(const base::string16& word) const; + #if BUILDFLAG(USE_RENDERER_SPELLCHECKER) // Posts delayed spellcheck task and clear it if any. // Takes ownership of |request|. diff --git a/chromium/components/spellcheck/renderer/spellcheck_language.cc b/chromium/components/spellcheck/renderer/spellcheck_language.cc index 391e36a9481..32ff29257ad 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_language.cc +++ b/chromium/components/spellcheck/renderer/spellcheck_language.cc @@ -148,3 +148,7 @@ bool SpellcheckLanguage::IsEnabled() { DCHECK(platform_spelling_engine_); return platform_spelling_engine_->IsEnabled(); } + +bool SpellcheckLanguage::IsTextInSameScript(const base::string16& text) const { + return character_attributes_.IsTextInSameScript(text); +} diff --git a/chromium/components/spellcheck/renderer/spellcheck_language.h b/chromium/components/spellcheck/renderer/spellcheck_language.h index ed909b10bc0..6e35078a95e 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_language.h +++ b/chromium/components/spellcheck/renderer/spellcheck_language.h @@ -75,6 +75,10 @@ class SpellcheckLanguage { // Return true if the underlying spellcheck engine is enabled. bool IsEnabled(); + // Returns true if all the characters in a text string are in the script + // associated with this spellcheck language. + bool IsTextInSameScript(const base::string16& text) const; + private: friend class SpellCheckTest; friend class FakeSpellCheck; diff --git a/chromium/components/spellcheck/renderer/spellcheck_provider.cc b/chromium/components/spellcheck/renderer/spellcheck_provider.cc index 58e2434a7ce..76f4452ab02 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_provider.cc +++ b/chromium/components/spellcheck/renderer/spellcheck_provider.cc @@ -134,8 +134,10 @@ void SpellCheckProvider::RequestTextChecking( last_results_.Assign(blink::WebVector<blink::WebTextCheckingResult>()); last_identifier_ = text_check_completions_.Add(std::move(completion)); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) - if (spellcheck::UseWinHybridSpellChecker()) { +#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) + if (spellcheck::UseBrowserSpellChecker()) { +#if defined(OS_WIN) + // Determine whether a hybrid check is needed. bool use_hunspell = spellcheck_->EnabledLanguageCount() > 0; bool use_native = spellcheck_->EnabledLanguageCount() != spellcheck_->LanguageCount(); @@ -145,33 +147,26 @@ void SpellCheckProvider::RequestTextChecking( return; } - if (use_native) { - // Some languages can be handled by the native spell checker. Use the - // regular browser spell check code path. If hybrid spell check is - // required (i.e. some locales must be checked by Hunspell), misspellings - // from the native spell checker will be double-checked with Hunspell in - // the |OnRespondTextCheck| callback. - hybrid_requests_info_[last_identifier_] = {/*used_hunspell=*/use_hunspell, - /*used_native=*/use_native, - base::TimeTicks::Now()}; - GetSpellCheckHost().RequestTextCheck( - text, routing_id(), - base::BindOnce(&SpellCheckProvider::OnRespondTextCheck, - weak_factory_.GetWeakPtr(), last_identifier_, text)); - } else { + if (!use_native) { // No language can be handled by the native spell checker. Use the regular // Hunspell code path. GetSpellCheckHost().CallSpellingService( text, base::BindOnce(&SpellCheckProvider::OnRespondSpellingService, weak_factory_.GetWeakPtr(), last_identifier_, text)); + return; } - return; - } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) -#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) - if (spellcheck::UseBrowserSpellChecker()) { + // Some languages can be handled by the native spell checker. Use the + // regular browser spell check code path. If hybrid spell check is + // required (i.e. some locales must be checked by Hunspell), misspellings + // from the native spell checker will be double-checked with Hunspell in + // the |OnRespondTextCheck| callback. + hybrid_requests_info_[last_identifier_] = {/*used_hunspell=*/use_hunspell, + /*used_native=*/use_native, + base::TimeTicks::Now()}; +#endif // defined(OS_WIN) + // Text check (unified request for grammar and spell check) is only // available for browser process, so we ask the system spellchecker // over mojo or return an empty result if the checker is not available. @@ -221,16 +216,16 @@ void SpellCheckProvider::CheckSpelling( const int kWordStart = 0; if (optional_suggestions) { -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) base::TimeTicks suggestions_start = base::TimeTicks::Now(); -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) spellcheck::PerLanguageSuggestions per_language_suggestions; spellcheck_->SpellCheckWord(word.c_str(), kWordStart, word.size(), routing_id(), &offset, &length, &per_language_suggestions); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) - if (spellcheck::UseWinHybridSpellChecker() && +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) + if (spellcheck::UseBrowserSpellChecker() && spellcheck_->EnabledLanguageCount() < spellcheck_->LanguageCount()) { // Also fetch suggestions from the browser process (native spellchecker). // This is a synchronous Mojo call, because this method must return @@ -249,7 +244,7 @@ void SpellCheckProvider::CheckSpelling( spellcheck_renderer_metrics::RecordHunspellSuggestionDuration( base::TimeTicks::Now() - suggestions_start); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) std::vector<base::string16> suggestions; spellcheck::FillSuggestions(per_language_suggestions, &suggestions); @@ -342,29 +337,29 @@ void SpellCheckProvider::OnRespondTextCheck( SpellCheck::ResultFilter result_filter = SpellCheck::DO_NOT_MODIFY; -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) const auto& request_info = hybrid_requests_info_.find(identifier); - if (spellcheck::UseWinHybridSpellChecker() && + if (spellcheck::UseBrowserSpellChecker() && request_info != hybrid_requests_info_.end() && request_info->second.used_hunspell && request_info->second.used_native) { // Not all locales could be checked by the native spell checker. Verify each // mistake against Hunspell in the locales that weren't checked. result_filter = SpellCheck::USE_HUNSPELL_FOR_HYBRID_CHECK; } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) spellcheck_->CreateTextCheckingResults(result_filter, /*line_offset=*/0, line, results, &textcheck_results); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) if (request_info != hybrid_requests_info_.end()) { spellcheck_renderer_metrics::RecordSpellcheckDuration( base::TimeTicks::Now() - request_info->second.request_start_ticks, request_info->second.used_hunspell, request_info->second.used_native); hybrid_requests_info_.erase(request_info); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) completion->DidFinishCheckingText(textcheck_results); diff --git a/chromium/components/spellcheck/renderer/spellcheck_provider.h b/chromium/components/spellcheck/renderer/spellcheck_provider.h index 8e852264682..43e6dd2a46b 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_provider.h +++ b/chromium/components/spellcheck/renderer/spellcheck_provider.h @@ -18,9 +18,9 @@ #include "mojo/public/cpp/bindings/remote.h" #include "third_party/blink/public/web/web_text_check_client.h" -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) #include <unordered_map> -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) class SpellCheck; struct SpellCheckResult; @@ -46,14 +46,14 @@ class SpellCheckProvider : public content::RenderFrameObserver, using WebTextCheckCompletions = base::IDMap<std::unique_ptr<blink::WebTextCheckingCompletion>>; -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) // A struct to hold information related to hybrid spell check requests. struct HybridSpellCheckRequestInfo { bool used_hunspell; bool used_native; base::TimeTicks request_start_ticks; }; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) SpellCheckProvider( content::RenderFrame* render_frame, @@ -155,9 +155,9 @@ class SpellCheckProvider : public content::RenderFrameObserver, // Dictionary updated observer. std::unique_ptr<DictionaryUpdateObserverImpl> dictionary_update_observer_; -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) std::unordered_map<int, HybridSpellCheckRequestInfo> hybrid_requests_info_; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) base::WeakPtrFactory<SpellCheckProvider> weak_factory_{this}; diff --git a/chromium/components/spellcheck/renderer/spellcheck_provider_test.cc b/chromium/components/spellcheck/renderer/spellcheck_provider_test.cc index c2d6414b1ba..9683a995a80 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_provider_test.cc +++ b/chromium/components/spellcheck/renderer/spellcheck_provider_test.cc @@ -16,13 +16,11 @@ #include "components/spellcheck/renderer/spellcheck_language.h" #include "components/spellcheck/spellcheck_buildflags.h" -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/path_service.h" -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) namespace { base::FilePath GetHunspellDirectory() { base::FilePath hunspell_directory; @@ -34,7 +32,7 @@ base::FilePath GetHunspellDirectory() { return hunspell_directory; } } // namespace -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) FakeTextCheckingResult::FakeTextCheckingResult() = default; FakeTextCheckingResult::~FakeTextCheckingResult() = default; @@ -67,26 +65,31 @@ void FakeSpellCheck::SetFakeLanguageCounts(size_t language_count, enabled_language_count_ = enabled_count; } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) -void FakeSpellCheck::InitializeRendererSpellCheckForLocale( - const std::string& language) { - base::FilePath hunspell_directory = GetHunspellDirectory(); - EXPECT_FALSE(hunspell_directory.empty()); - base::FilePath hunspell_file_path = - spellcheck::GetVersionedFileName(language, hunspell_directory); - base::File file(hunspell_file_path, - base::File::FLAG_OPEN | base::File::FLAG_READ); - EXPECT_TRUE(file.IsValid()) << hunspell_file_path << " is not valid" - << file.ErrorToString(file.GetLastFileError()); +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) +void FakeSpellCheck::InitializeSpellCheckForLocale(const std::string& language, + bool use_hunspell) { + // Non-Hunspell case is passed invalid file to SpellcheckLanguage::Init. + base::File file; + + if (use_hunspell) { + base::FilePath hunspell_directory = GetHunspellDirectory(); + EXPECT_FALSE(hunspell_directory.empty()); + base::FilePath hunspell_file_path = + spellcheck::GetVersionedFileName(language, hunspell_directory); + file.Initialize(hunspell_file_path, + base::File::FLAG_OPEN | base::File::FLAG_READ); + EXPECT_TRUE(file.IsValid()) << hunspell_file_path << " is not valid" + << file.ErrorToString(file.GetLastFileError()); + } - // Add the SpellcheckLanguage manually to force the use of Hunspell. + // Add the SpellcheckLanguage manually to the SpellCheck object. SpellCheck::languages_.push_back( std::make_unique<SpellcheckLanguage>(embedder_provider_)); - SpellCheck::languages_.front()->platform_spelling_engine_ = + SpellCheck::languages_.back()->platform_spelling_engine_ = std::make_unique<HunspellEngine>(embedder_provider_); - SpellCheck::languages_.front()->Init(std::move(file), language); + SpellCheck::languages_.back()->Init(std::move(file), language); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) size_t FakeSpellCheck::LanguageCount() { return use_fake_counts_ ? language_count_ : SpellCheck::LanguageCount(); @@ -180,15 +183,16 @@ void TestingSpellCheckProvider::FillSuggestionList(const base::string16&, FillSuggestionListCallback) { NOTREACHED(); } -#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) void TestingSpellCheckProvider::GetPerLanguageSuggestions( const base::string16& word, GetPerLanguageSuggestionsCallback callback) { NOTREACHED(); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) + +#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) #if defined(OS_ANDROID) void TestingSpellCheckProvider::DisconnectSessionBridge() { @@ -209,7 +213,7 @@ bool TestingSpellCheckProvider::SatisfyRequestFromCache( return SpellCheckProvider::SatisfyRequestFromCache(text, completion); } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) int TestingSpellCheckProvider::AddCompletionForTest( std::unique_ptr<FakeTextCheckingCompletion> completion, SpellCheckProvider::HybridSpellCheckRequestInfo request_info) { @@ -226,7 +230,7 @@ void TestingSpellCheckProvider::OnRespondTextCheck( SpellCheckProvider::OnRespondTextCheck(identifier, line, results); base::RunLoop().RunUntilIdle(); } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) SpellCheckProviderTest::SpellCheckProviderTest() : provider_(&embedder_provider_) {} diff --git a/chromium/components/spellcheck/renderer/spellcheck_provider_test.h b/chromium/components/spellcheck/renderer/spellcheck_provider_test.h index 3ed4bdf61cf..7d5516639d1 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_provider_test.h +++ b/chromium/components/spellcheck/renderer/spellcheck_provider_test.h @@ -53,10 +53,11 @@ class FakeSpellCheck : public SpellCheck { // Test-only method to set the fake language counts void SetFakeLanguageCounts(size_t language_count, size_t enabled_count); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) - // Test-only method to initialize Hunspell for the given locale. - void InitializeRendererSpellCheckForLocale(const std::string& language); -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) + // Test-only method to initialize SpellCheck object for the given locale. + void InitializeSpellCheckForLocale(const std::string& language, + bool use_hunspell); +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) // Returns the current number of spell check languages. size_t LanguageCount() override; @@ -91,7 +92,7 @@ class TestingSpellCheckProvider : public SpellCheckProvider, bool SatisfyRequestFromCache(const base::string16& text, blink::WebTextCheckingCompletion* completion); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) int AddCompletionForTest( std::unique_ptr<FakeTextCheckingCompletion> completion, SpellCheckProvider::HybridSpellCheckRequestInfo request_info); @@ -99,7 +100,7 @@ class TestingSpellCheckProvider : public SpellCheckProvider, void OnRespondTextCheck(int identifier, const base::string16& line, const std::vector<SpellCheckResult>& results); -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) #if BUILDFLAG(USE_RENDERER_SPELLCHECKER) void ResetResult(); @@ -109,14 +110,10 @@ class TestingSpellCheckProvider : public SpellCheckProvider, size_t spelling_service_call_count_ = 0; #endif // BUILDFLAG(USE_RENDERER_SPELLCHECKER) -#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) || \ - BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) using RequestTextCheckParams = std::pair<base::string16, RequestTextCheckCallback>; -#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) || - // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) -#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) // Variables logging RequestTextCheck() mojo calls. std::vector<RequestTextCheckParams> text_check_requests_; #endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) @@ -147,13 +144,14 @@ class TestingSpellCheckProvider : public SpellCheckProvider, CheckSpellingCallback) override; void FillSuggestionList(const base::string16&, FillSuggestionListCallback) override; -#endif -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) void GetPerLanguageSuggestions( const base::string16& word, GetPerLanguageSuggestionsCallback callback) override; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) + +#endif #if defined(OS_ANDROID) void DisconnectSessionBridge() override; diff --git a/chromium/components/spellcheck/renderer/spellcheck_provider_unittest.cc b/chromium/components/spellcheck/renderer/spellcheck_provider_unittest.cc index cdc3995a487..8629702ac57 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_provider_unittest.cc +++ b/chromium/components/spellcheck/renderer/spellcheck_provider_unittest.cc @@ -18,7 +18,7 @@ namespace { -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) struct HybridSpellCheckTestCase { size_t language_count; size_t enabled_language_count; @@ -27,13 +27,23 @@ struct HybridSpellCheckTestCase { }; struct CombineSpellCheckResultsTestCase { + std::string browser_locale; std::string renderer_locale; const wchar_t* text; std::vector<SpellCheckResult> browser_results; bool use_spelling_service; blink::WebVector<blink::WebTextCheckingResult> expected_results; }; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) + +std::ostream& operator<<(std::ostream& out, + const CombineSpellCheckResultsTestCase& test_case) { + out << "browser_locale=" << test_case.browser_locale + << ", renderer_locale=" << test_case.renderer_locale << ", text=\"" + << test_case.text + << "\", use_spelling_service=" << test_case.use_spelling_service; + return out; +} +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) class SpellCheckProviderCacheTest : public SpellCheckProviderTest { protected: @@ -47,7 +57,7 @@ class SpellCheckProviderCacheTest : public SpellCheckProviderTest { } }; -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) // Test fixture for testing hybrid check cases. class HybridSpellCheckTest : public testing::TestWithParam<HybridSpellCheckTestCase> { @@ -74,7 +84,7 @@ class CombineSpellCheckResultsTest spellcheck::EmptyLocalInterfaceProvider embedder_provider_; TestingSpellCheckProvider provider_; }; -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) TEST_F(SpellCheckProviderCacheTest, SubstringWithoutMisspellings) { FakeTextCheckingResult result; @@ -128,7 +138,7 @@ TEST_F(SpellCheckProviderCacheTest, ResetCacheOnCustomDictionaryUpdate) { EXPECT_EQ(result.completion_count_, 0U); } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) // Tests that the SpellCheckProvider does not call into the native spell checker // on Windows when the native spell checker flags are disabled. TEST_F(SpellCheckProviderTest, ShouldNotUseBrowserSpellCheck) { @@ -146,29 +156,6 @@ TEST_F(SpellCheckProviderTest, ShouldNotUseBrowserSpellCheck) { EXPECT_EQ(completion.cancellation_count_, 0U); } -// Tests that the SpellCheckProvider calls into the native spell checker when -// the browser spell checker flag is enabled, but the hybrid spell checker flag -// isn't. -TEST_F(SpellCheckProviderTest, ShouldUseBrowserSpellCheck) { - if (!spellcheck::WindowsVersionSupportsSpellchecker()) { - return; - } - - base::test::ScopedFeatureList local_features; - local_features.InitWithFeatures({spellcheck::kWinUseBrowserSpellChecker}, - {spellcheck::kWinUseHybridSpellChecker}); - - FakeTextCheckingResult completion; - base::string16 text = base::ASCIIToUTF16("This is a test"); - provider_.RequestTextChecking( - text, std::make_unique<FakeTextCheckingCompletion>(&completion)); - - EXPECT_EQ(provider_.spelling_service_call_count_, 0U); - EXPECT_EQ(provider_.text_check_requests_.size(), 1U); - EXPECT_EQ(completion.completion_count_, 0U); - EXPECT_EQ(completion.cancellation_count_, 0U); -} - // Tests that the SpellCheckProvider calls into the native spell checker only // when needed. INSTANTIATE_TEST_SUITE_P( @@ -197,10 +184,7 @@ TEST_P(HybridSpellCheckTest, ShouldUseBrowserSpellCheckOnlyWhenNeeded) { const auto& test_case = GetParam(); base::test::ScopedFeatureList local_features; - local_features.InitWithFeatures( - /*enabled_features=*/{spellcheck::kWinUseBrowserSpellChecker, - spellcheck::kWinUseHybridSpellChecker}, - /*disabled_features=*/{}); + local_features.InitAndEnableFeature(spellcheck::kWinUseBrowserSpellChecker); FakeTextCheckingResult completion; provider_.spellcheck()->SetFakeLanguageCounts( @@ -225,7 +209,8 @@ INSTANTIATE_TEST_SUITE_P( CombineSpellCheckResultsTest, testing::Values( // Browser-only check, no browser results - CombineSpellCheckResultsTestCase{"", + CombineSpellCheckResultsTestCase{"en-US", + "", L"This has no misspellings", {}, false, @@ -233,6 +218,7 @@ INSTANTIATE_TEST_SUITE_P( // Browser-only check, no spelling service, browser results CombineSpellCheckResultsTestCase{ + "en-US", "", L"Tihs has soem misspellings", {SpellCheckResult(SpellCheckResult::SPELLING, @@ -256,8 +242,56 @@ INSTANTIATE_TEST_SUITE_P( 9, 4)})}, + // Browser-only check, no spelling service, browser results, some words + // in character sets with no dictionaries enabled. + CombineSpellCheckResultsTestCase{ + "en-US", + "", + L"Tihs has soem \x3053\x3093\x306B\x3061\x306F \x4F60\x597D " + L"\xC548\xB155\xD558\xC138\xC694 " + L"\x0930\x093E\x091C\x0927\x093E\x0928 words in different " + L"character sets " + L"(Japanese, Chinese, Korean, Hindi)", + {SpellCheckResult(SpellCheckResult::SPELLING, + 0, + 4, + {base::ASCIIToUTF16("foo")}), + SpellCheckResult(SpellCheckResult::SPELLING, + 9, + 4, + {base::ASCIIToUTF16("foo")}), + SpellCheckResult(SpellCheckResult::SPELLING, + 14, + 5, + {base::ASCIIToUTF16("foo")}), + SpellCheckResult(SpellCheckResult::SPELLING, + 20, + 2, + {base::ASCIIToUTF16("foo")}), + SpellCheckResult(SpellCheckResult::SPELLING, + 23, + 5, + {base::ASCIIToUTF16("foo")}), + SpellCheckResult(SpellCheckResult::SPELLING, + 29, + 6, + {base::ASCIIToUTF16("foo")})}, + false, + blink::WebVector<blink::WebTextCheckingResult>( + {blink::WebTextCheckingResult( + blink::WebTextDecorationType:: + kWebTextDecorationTypeSpelling, + 0, + 4), + blink::WebTextCheckingResult( + blink::WebTextDecorationType:: + kWebTextDecorationTypeSpelling, + 9, + 4)})}, + // Browser-only check, spelling service, spelling-only browser results CombineSpellCheckResultsTestCase{ + "en-US", "", L"Tihs has soem misspellings", {SpellCheckResult(SpellCheckResult::SPELLING, @@ -284,6 +318,7 @@ INSTANTIATE_TEST_SUITE_P( // Browser-only check, spelling service, spelling and grammar browser // results CombineSpellCheckResultsTestCase{ + "en-US", "", L"Tihs has soem misspellings", {SpellCheckResult(SpellCheckResult::SPELLING, @@ -308,6 +343,7 @@ INSTANTIATE_TEST_SUITE_P( // Hybrid check, no browser results CombineSpellCheckResultsTestCase{"en-US", + "en-US", L"This has no misspellings", {}, false, @@ -317,6 +353,7 @@ INSTANTIATE_TEST_SUITE_P( // with Hunspell results CombineSpellCheckResultsTestCase{ "en-US", + "en-US", L"Tihs has soem misspellings", {SpellCheckResult(SpellCheckResult::SPELLING, 0, @@ -343,6 +380,7 @@ INSTANTIATE_TEST_SUITE_P( // coincide with Hunspell results CombineSpellCheckResultsTestCase{ "en-US", + "en-US", L"Tihs has soem misspellings", { SpellCheckResult(SpellCheckResult::SPELLING, @@ -379,6 +417,7 @@ INSTANTIATE_TEST_SUITE_P( // coincide with Hunspell results CombineSpellCheckResultsTestCase{ "en-US", + "en-US", L"Tihs has soem misspellings", {SpellCheckResult(SpellCheckResult::SPELLING, 5, @@ -392,10 +431,40 @@ INSTANTIATE_TEST_SUITE_P( blink::WebVector<blink::WebTextCheckingResult>()}, // Hybrid check, no spelling service, browser results with some that - // are skipped by Hunspell because of character set mismatch + // that are in character set that does not have dictionary support (so + // (are considered spelled correctly) CombineSpellCheckResultsTestCase{ "en-US", - L"Tihs word is misspelled in Russian: " + "fr", + L"Tihs mot is misspelled in Russian: " + L"\x043C\x0438\x0440\x0432\x043E\x0439", + {SpellCheckResult(SpellCheckResult::SPELLING, + 0, + 4, + {base::ASCIIToUTF16("foo")}), + SpellCheckResult(SpellCheckResult::SPELLING, + 5, + 3, + {base::ASCIIToUTF16("foo")}), + SpellCheckResult(SpellCheckResult::SPELLING, + 35, + 6, + {base::ASCIIToUTF16("foo")})}, + false, + blink::WebVector<blink::WebTextCheckingResult>( + std::vector<blink::WebTextCheckingResult>( + {blink::WebTextCheckingResult( + blink::WebTextDecorationType:: + kWebTextDecorationTypeSpelling, + 0, + 4)}))}, + + // Hybrid check, no spelling service, text with some words in a + // character set that only has a Hunspell dictionary enabled. + CombineSpellCheckResultsTestCase{ + "en-US", + "ru", + L"Tihs \x0432\x0441\x0435\x0445 is misspelled in Russian: " L"\x043C\x0438\x0440\x0432\x043E\x0439", {SpellCheckResult(SpellCheckResult::SPELLING, 0, @@ -426,6 +495,7 @@ INSTANTIATE_TEST_SUITE_P( // that all coincide with Hunspell results CombineSpellCheckResultsTestCase{ "en-US", + "en-US", L"Tihs has soem misspellings", {SpellCheckResult(SpellCheckResult::SPELLING, 0, @@ -452,6 +522,7 @@ INSTANTIATE_TEST_SUITE_P( // locales CombineSpellCheckResultsTestCase{ "en-US", + "en-US", L"This has soem misspellings", {SpellCheckResult(SpellCheckResult::SPELLING, 0, @@ -480,16 +551,18 @@ TEST_P(CombineSpellCheckResultsTest, ShouldCorrectlyCombineHybridResults) { const auto& test_case = GetParam(); base::test::ScopedFeatureList local_features; - local_features.InitWithFeatures( - /*enabled_features=*/{spellcheck::kWinUseBrowserSpellChecker, - spellcheck::kWinUseHybridSpellChecker}, - /*disabled_features=*/{}); - bool has_renderer_check = !test_case.renderer_locale.empty(); + local_features.InitAndEnableFeature(spellcheck::kWinUseBrowserSpellChecker); + const bool has_browser_check = !test_case.browser_locale.empty(); + const bool has_renderer_check = !test_case.renderer_locale.empty(); + + if (has_browser_check) { + provider_.spellcheck()->InitializeSpellCheckForLocale( + test_case.browser_locale, /*use_hunspell*/ false); + } if (has_renderer_check) { - provider_.spellcheck()->InitializeRendererSpellCheckForLocale( - test_case.renderer_locale); - provider_.spellcheck()->SetFakeLanguageCounts(2u, 1u); + provider_.spellcheck()->InitializeSpellCheckForLocale( + test_case.renderer_locale, /*use_hunspell*/ true); } if (test_case.use_spelling_service) { @@ -501,7 +574,7 @@ TEST_P(CombineSpellCheckResultsTest, ShouldCorrectlyCombineHybridResults) { FakeTextCheckingResult completion; SpellCheckProvider::HybridSpellCheckRequestInfo request_info = { /*used_hunspell=*/has_renderer_check, - /*used_native=*/true, base::TimeTicks::Now()}; + /*used_native=*/has_browser_check, base::TimeTicks::Now()}; int check_id = provider_.AddCompletionForTest( std::make_unique<FakeTextCheckingCompletion>(&completion), request_info); @@ -536,6 +609,6 @@ TEST_P(CombineSpellCheckResultsTest, ShouldCorrectlyCombineHybridResults) { } } } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) } // namespace diff --git a/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.cc b/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.cc index bae7b795136..44d29923868 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.cc +++ b/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.cc @@ -12,7 +12,7 @@ #include "build/build_config.h" #include "components/spellcheck/spellcheck_buildflags.h" -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) namespace { // Records the duration of a spell check request. This variation is for when @@ -37,7 +37,7 @@ void RecordNativeSpellcheckDuration(base::TimeDelta duration) { } } // anonymous namespace -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) namespace spellcheck_renderer_metrics { @@ -53,7 +53,7 @@ void RecordCheckedTextLengthWithSuggestions(int length) { UMA_HISTOGRAM_COUNTS_1M("SpellCheck.api.check.suggestions", length); } -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) void RecordHunspellSuggestionDuration(base::TimeDelta duration) { UMA_HISTOGRAM_TIMES( "Spellcheck.Windows.SuggestionGatheringDuration.HunspellOnly", duration); @@ -75,7 +75,7 @@ void RecordSpellcheckDuration(base::TimeDelta duration, RecordNativeSpellcheckDuration(duration); } } -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) } // namespace spellcheck_renderer_metrics diff --git a/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.h b/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.h index 9ad71c4f923..a052bf7ff0b 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.h +++ b/chromium/components/spellcheck/renderer/spellcheck_renderer_metrics.h @@ -25,7 +25,7 @@ void RecordCheckedTextLengthNoSuggestions(int length); // requested. void RecordCheckedTextLengthWithSuggestions(int length); -#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#if defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) // Records the duration of gathering spelling suggestions. This variation is for // when spell check is performed only by Hunspell. void RecordHunspellSuggestionDuration(base::TimeDelta duration); @@ -41,7 +41,7 @@ void RecordHybridSuggestionDuration(base::TimeDelta duration); void RecordSpellcheckDuration(base::TimeDelta duration, bool used_hunspell, bool used_native); -#endif // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER) +#endif // defined(OS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER) } // namespace spellcheck_renderer_metrics diff --git a/chromium/components/spellcheck/renderer/spellcheck_worditerator.cc b/chromium/components/spellcheck/renderer/spellcheck_worditerator.cc index 8fe8a6df381..ec16fefbebc 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_worditerator.cc +++ b/chromium/components/spellcheck/renderer/spellcheck_worditerator.cc @@ -35,6 +35,24 @@ void SpellcheckCharAttribute::SetDefaultLanguage(const std::string& language) { CreateRuleSets(language); } +bool SpellcheckCharAttribute::IsTextInSameScript( + const base::string16& text) const { + const base::char16* data = text.data(); + const size_t length = text.length(); + for (size_t index = 0; index < length; /* U16_NEXT post-increments */) { + uint32_t code = 0; + U16_NEXT(data, index, length, code); + UErrorCode error = U_ZERO_ERROR; + UScriptCode script = uscript_getScript(code, &error); + if (U_SUCCESS(error) && (script != USCRIPT_COMMON) && + (script != USCRIPT_INHERITED)) { + if (script != script_code_) + return false; + } + } + return true; +} + base::string16 SpellcheckCharAttribute::GetRuleSet( bool allow_contraction) const { return allow_contraction ? diff --git a/chromium/components/spellcheck/renderer/spellcheck_worditerator.h b/chromium/components/spellcheck/renderer/spellcheck_worditerator.h index cf3d6cf1efb..a58901cba81 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_worditerator.h +++ b/chromium/components/spellcheck/renderer/spellcheck_worditerator.h @@ -40,6 +40,10 @@ class SpellcheckCharAttribute { // GetRuleSet() returns the rule-sets created in this function. void SetDefaultLanguage(const std::string& language); + // Returns true if all the characters in a text string are in the script + // associated with the spellcheck language. + bool IsTextInSameScript(const base::string16& text) const; + // Returns a custom rule-set string used by the ICU break iterator. This class // has two rule-sets, one splits a contraction and the other does not, so we // can split a concaticated word (e.g. "seven-year-old") into words (e.g. diff --git a/chromium/components/spellcheck/renderer/spellcheck_worditerator_unittest.cc b/chromium/components/spellcheck/renderer/spellcheck_worditerator_unittest.cc index 4ab4c4f3ac3..f18f7ff3c6e 100644 --- a/chromium/components/spellcheck/renderer/spellcheck_worditerator_unittest.cc +++ b/chromium/components/spellcheck/renderer/spellcheck_worditerator_unittest.cc @@ -505,3 +505,75 @@ TEST(SpellcheckWordIteratorTest, FindSkippableWordsKhmer) { EXPECT_EQ(iter.GetWordBreakStatus(), BreakIterator::IS_SKIPPABLE_WORD); EXPECT_FALSE(iter.Advance()); } + +TEST(SpellcheckCharAttributeTest, IsTextInSameScript) { + struct LanguageWithSampleText { + const char* language; + const wchar_t* sample_text; + }; + + static const std::vector<LanguageWithSampleText> kLanguagesWithSampleText = { + // Latin + {"fr", L"Libert\x00e9, \x00e9galitt\x00e9, fraternit\x00e9."}, + // Greek + {"el", L"\x03B3\x03B5\x03B9\x03AC\x0020\x03C3\x03BF\x03C5"}, + // Cyrillic + {"ru", + L"\x0437\x0434\x0440\x0430\x0432\x0441\x0442\x0432\x0443\x0439\x0442" + L"\x0435"}, + // Hebrew + {"he", L"\x05e9\x05c1\x05b8\x05dc\x05d5\x05b9\x05dd"}, + // Arabic + {"ar", + L"\x0627\x064e\x0644\x0633\x064e\x0651\x0644\x0627\x0645\x064f\x0639" + L"\x064e\x0644\x064e\x064a\x0652\x0643\x064f\x0645\x0652 "}, + // Hindi + {"hi", L"\x0930\x093E\x091C\x0927\x093E\x0928"}, + // Thai + {"th", + L"\x0e2a\x0e27\x0e31\x0e2a\x0e14\x0e35\x0020\x0e04\x0e23\x0e31\x0e1a"}, + // Hiragata + {"jp-Hira", L"\x3053\x3093\x306B\x3061\x306F"}, + // Katakana + {"jp-Kana", L"\x30b3\x30de\x30fc\x30b9"}, + // CJKV ideographs + {"zh-Hani", L"\x4F60\x597D"}, + // Hangul Syllables + {"ko", L"\xC548\xB155\xD558\xC138\xC694"}, + }; + + for (const auto& testcase : kLanguagesWithSampleText) { + SpellcheckCharAttribute attribute; + attribute.SetDefaultLanguage(testcase.language); + base::string16 sample_text(base::WideToUTF16(testcase.sample_text)); + EXPECT_TRUE(attribute.IsTextInSameScript(sample_text)) + << "Language \"" << testcase.language + << "\" fails to identify that sample text in same language is in same " + "script."; + + // All other scripts in isolatation or mixed with current script should + // return false. + for (const auto& other_script : kLanguagesWithSampleText) { + if (testcase.language == other_script.language) + continue; + base::string16 other_sample_text( + base::WideToUTF16(other_script.sample_text)); + EXPECT_FALSE(attribute.IsTextInSameScript(other_sample_text)) + << "Language \"" << testcase.language + << "\" fails to identify that sample text in language \"" + << other_script.language << "\" is in different script."; + EXPECT_FALSE( + attribute.IsTextInSameScript(sample_text + other_sample_text)) + << "Language \"" << testcase.language + << "\" fails to identify that sample text in language \"" + << other_script.language + << "\" is in different script when appended to text in this script."; + EXPECT_FALSE( + attribute.IsTextInSameScript(other_sample_text + sample_text)) + << "Language \"" << testcase.language + << "\" fails to identify that sample text in language \"" + << other_script.language + << "\" is in different script when prepended to text in this script."; + } + } +} diff --git a/chromium/components/spellcheck/renderer/spelling_engine.cc b/chromium/components/spellcheck/renderer/spelling_engine.cc index 5d27e2cbede..c2b048bab47 100644 --- a/chromium/components/spellcheck/renderer/spelling_engine.cc +++ b/chromium/components/spellcheck/renderer/spelling_engine.cc @@ -20,29 +20,19 @@ SpellingEngine* CreateNativeSpellingEngine( service_manager::LocalInterfaceProvider* embedder_provider) { DCHECK(embedder_provider); -#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) #if defined(OS_WIN) - if (spellcheck::UseWinHybridSpellChecker()) { - // On Windows, when using the hybrid spellchecker, both Hunspell and the - // platform are used for spellchecking. Ideally we'd want to know which - // languages are supported by the platform spellchecker, and only create a - // HunspellEngine for languages that aren't. Unfortunately, performing that - // check must be done asynchronously on the browser side, while this - // function must return synchronously. However, because the platform - // spellchecker uses a code path that does not involve a SpellingEngine when - // performing spellchecking, a solution is to create a HunspellEngine for - // every language, even those that the platform supports. Because the - // languages that the platform spellchecker supports are initialized on the - // browser side, the HunspellEngine returned here will remain disabled for - // these languages, so they will be skipped when performing the Hunspell - // check. - return new HunspellEngine(embedder_provider); - } else if (spellcheck::UseBrowserSpellChecker()) { - return new PlatformSpellingEngine(embedder_provider); - } -#else - return new PlatformSpellingEngine(embedder_provider); + // On Windows, always return a HunspellEngine. This is a simplification to + // avoid needing an async Mojo call to the browser process to determine which + // languages are supported by the native spell checker. Returning a + // HunspellEngine for languages supported by the native spell checker is + // harmless because the SpellingEngine object returned here is never used + // during native spell checking. It also doesn't affect Hunspell, since these + // languages are skipped during the Hunspell check. + return new HunspellEngine(embedder_provider); #endif // defined(OS_WIN) + +#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) + return new PlatformSpellingEngine(embedder_provider); #endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) #if BUILDFLAG(USE_RENDERER_SPELLCHECKER) diff --git a/chromium/components/spellcheck/spellcheck.md b/chromium/components/spellcheck/spellcheck.md index a127223c15f..b8f197fa6bf 100644 --- a/chromium/components/spellcheck/spellcheck.md +++ b/chromium/components/spellcheck/spellcheck.md @@ -22,7 +22,7 @@ We also need to create the runtime feature flag kWinUseBrowserSpellChecker for Windows OS. The feature flag is used to choose between the platform or Hunspell spellchecker at runtime. -## Note on the Windows Hybrid spellchecker +### Note on the Windows Hybrid spellchecker On Windows, the platform spell checker relies on user-installed language packs. It is possible for a user to enable spell checking in Chrome for a language while not having installed the necessary language pack in the Windows settings. @@ -30,9 +30,3 @@ while not having installed the necessary language pack in the Windows settings. For those situations, the spell checker on Windows will first try to use the platform spell checker. For languages where this is not possible, Hunspell will be used instead. - -To control this fallback logic, a separate build flag -use_win_hybrid_spellchecker is used. In addition, for experiment purposes, a -feature flag kWinUseHybridSpellChecker is used to control this behavior at -runtime. Finally, this fallback logic requires the platform spell checker to be -available, so it also uses the browser spell checker checks described above. diff --git a/chromium/components/spellcheck/spellcheck_build_features.gni b/chromium/components/spellcheck/spellcheck_build_features.gni index 2727993fe91..7e23676bfd7 100644 --- a/chromium/components/spellcheck/spellcheck_build_features.gni +++ b/chromium/components/spellcheck/spellcheck_build_features.gni @@ -14,13 +14,6 @@ use_browser_spellchecker = is_android || is_mac || is_win # Therefore, include Windows in both build flags. use_renderer_spellchecker = !use_browser_spellchecker || is_win -# Use both Hunspell and the OS's spellchecker. Try to use the OS, but if -# a locale is not supported (i.e. no Windows language pack installed by -# the user), fall back to Hunspell. Requires Windows, and requires both -# use_browser_spellchecker and use_renderer_spellchecker. -use_win_hybrid_spellchecker = - is_win && use_browser_spellchecker && use_renderer_spellchecker - # Whether the enhanced spellcheck service is available on the platform. This is # effectively equal to all desktop platforms. enable_spelling_service = use_renderer_spellchecker || is_mac |