|
| 1 | +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Darren Shen <shend@chromium.org> |
| 3 | +Date: Tue, 15 Oct 2024 04:04:37 +0000 |
| 4 | +Subject: [PATCH] upstream-backport: Guard CheckSpelling mojom method at |
| 5 | + compile time with build flags. |
| 6 | + |
| 7 | +When use_browser_spellchecker && !enable_spelling_service, |
| 8 | +SpellcheckHost::CheckSpelling will trigger a NOTREACHED. |
| 9 | + |
| 10 | +Guard the mojom declaration of the method with the same build |
| 11 | +flags so that it's impossible to hit the NOTREACHED. |
| 12 | + |
| 13 | +Bug: 367216585 |
| 14 | +Change-Id: I840ebf66e371c6968b8cc2195bed3cccfa55fcea |
| 15 | +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5923299 |
| 16 | +Reviewed-by: Sam McNally <sammc@chromium.org> |
| 17 | +Reviewed-by: Michael Cui <mlcui@google.com> |
| 18 | +Auto-Submit: Darren Shen <shend@chromium.org> |
| 19 | +Commit-Queue: Darren Shen <shend@chromium.org> |
| 20 | +Cr-Commit-Position: refs/heads/main@{#1368587} |
| 21 | +--- |
| 22 | + .../spellcheck/browser/spell_check_host_impl.cc | 15 --------------- |
| 23 | + .../spellcheck/browser/spell_check_host_impl.h | 5 ----- |
| 24 | + components/spellcheck/common/BUILD.gn | 3 +++ |
| 25 | + components/spellcheck/common/spellcheck.mojom | 4 ++-- |
| 26 | + .../renderer/platform_spelling_engine.cc | 11 +++++++++++ |
| 27 | + .../renderer/spellcheck_provider_test.cc | 2 ++ |
| 28 | + .../renderer/spellcheck_provider_test.h | 2 ++ |
| 29 | + components/spellcheck/renderer/spelling_engine.h | 3 +++ |
| 30 | + 8 files changed, 23 insertions(+), 22 deletions(-) |
| 31 | + |
| 32 | +diff --git a/components/spellcheck/browser/spell_check_host_impl.cc b/components/spellcheck/browser/spell_check_host_impl.cc |
| 33 | +index c77bff349b65d..6b0af413e546c 100644 |
| 34 | +--- a/components/spellcheck/browser/spell_check_host_impl.cc |
| 35 | ++++ b/components/spellcheck/browser/spell_check_host_impl.cc |
| 36 | +@@ -48,21 +48,6 @@ void SpellCheckHostImpl::RequestTextCheck(const std::u16string& text, |
| 37 | + session_bridge_.RequestTextCheck(text, std::move(callback)); |
| 38 | + } |
| 39 | + |
| 40 | +-void SpellCheckHostImpl::CheckSpelling(const std::u16string& word, |
| 41 | +- CheckSpellingCallback callback) { |
| 42 | +- DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
| 43 | +- NOTREACHED_IN_MIGRATION(); |
| 44 | +- std::move(callback).Run(false); |
| 45 | +-} |
| 46 | +- |
| 47 | +-void SpellCheckHostImpl::FillSuggestionList( |
| 48 | +- const std::u16string& word, |
| 49 | +- FillSuggestionListCallback callback) { |
| 50 | +- DCHECK_CURRENTLY_ON(content::BrowserThread::UI); |
| 51 | +- NOTREACHED_IN_MIGRATION(); |
| 52 | +- std::move(callback).Run({}); |
| 53 | +-} |
| 54 | +- |
| 55 | + #if BUILDFLAG(IS_WIN) |
| 56 | + void SpellCheckHostImpl::InitializeDictionaries( |
| 57 | + InitializeDictionariesCallback callback) { |
| 58 | +diff --git a/components/spellcheck/browser/spell_check_host_impl.h b/components/spellcheck/browser/spell_check_host_impl.h |
| 59 | +index fa79c682478a2..4998ceda29776 100644 |
| 60 | +--- a/components/spellcheck/browser/spell_check_host_impl.h |
| 61 | ++++ b/components/spellcheck/browser/spell_check_host_impl.h |
| 62 | +@@ -45,11 +45,6 @@ class SpellCheckHostImpl : public spellcheck::mojom::SpellCheckHost { |
| 63 | + void RequestTextCheck(const std::u16string& text, |
| 64 | + RequestTextCheckCallback callback) override; |
| 65 | + |
| 66 | +- void CheckSpelling(const std::u16string& word, |
| 67 | +- CheckSpellingCallback callback) override; |
| 68 | +- void FillSuggestionList(const std::u16string& word, |
| 69 | +- FillSuggestionListCallback callback) override; |
| 70 | +- |
| 71 | + #if BUILDFLAG(IS_WIN) |
| 72 | + void InitializeDictionaries(InitializeDictionariesCallback callback) override; |
| 73 | + #endif // BUILDFLAG(IS_WIN) |
| 74 | +diff --git a/components/spellcheck/common/BUILD.gn b/components/spellcheck/common/BUILD.gn |
| 75 | +index 8d4bdec6868d8..65e62a176bebe 100644 |
| 76 | +--- a/components/spellcheck/common/BUILD.gn |
| 77 | ++++ b/components/spellcheck/common/BUILD.gn |
| 78 | +@@ -48,6 +48,9 @@ mojom("interfaces") { |
| 79 | + |
| 80 | + enabled_features = [] |
| 81 | + if (use_browser_spellchecker) { |
| 82 | ++ if (enable_spelling_service) { |
| 83 | ++ enabled_features += [ "USE_BROWSER_SPELLCHECKER_AND_SPELLING_SERVICE" ] |
| 84 | ++ } |
| 85 | + enabled_features += [ "USE_BROWSER_SPELLCHECKER" ] |
| 86 | + } |
| 87 | + |
| 88 | +diff --git a/components/spellcheck/common/spellcheck.mojom b/components/spellcheck/common/spellcheck.mojom |
| 89 | +index 3ae350e2a1ca6..275bdc4d1f8c7 100644 |
| 90 | +--- a/components/spellcheck/common/spellcheck.mojom |
| 91 | ++++ b/components/spellcheck/common/spellcheck.mojom |
| 92 | +@@ -66,13 +66,13 @@ interface SpellCheckHost { |
| 93 | + |
| 94 | + // Checks the correctness of a word with a platform-specific spell checker. |
| 95 | + // TODO(groby): This needs to originate from SpellcheckProvider. |
| 96 | +- [EnableIf=USE_BROWSER_SPELLCHECKER, Sync] |
| 97 | ++ [EnableIf=USE_BROWSER_SPELLCHECKER_AND_SPELLING_SERVICE, Sync] |
| 98 | + CheckSpelling(mojo_base.mojom.String16 word) |
| 99 | + => (bool correct); |
| 100 | + |
| 101 | + // Returns a list of suggestions for a given word with a platform-specific |
| 102 | + // spell checker. |
| 103 | +- [EnableIf=USE_BROWSER_SPELLCHECKER, Sync] |
| 104 | ++ [EnableIf=USE_BROWSER_SPELLCHECKER_AND_SPELLING_SERVICE, Sync] |
| 105 | + FillSuggestionList(mojo_base.mojom.String16 word) => |
| 106 | + (array<mojo_base.mojom.String16> suggestions); |
| 107 | + |
| 108 | +diff --git a/components/spellcheck/renderer/platform_spelling_engine.cc b/components/spellcheck/renderer/platform_spelling_engine.cc |
| 109 | +index b0d30fd559af4..1e8abc5211491 100644 |
| 110 | +--- a/components/spellcheck/renderer/platform_spelling_engine.cc |
| 111 | ++++ b/components/spellcheck/renderer/platform_spelling_engine.cc |
| 112 | +@@ -4,6 +4,7 @@ |
| 113 | + |
| 114 | + #include "components/spellcheck/renderer/platform_spelling_engine.h" |
| 115 | + |
| 116 | ++#include "components/spellcheck/spellcheck_buildflags.h" |
| 117 | + #include "content/public/renderer/render_thread.h" |
| 118 | + #include "services/service_manager/public/cpp/local_interface_provider.h" |
| 119 | + |
| 120 | +@@ -31,9 +32,14 @@ bool PlatformSpellingEngine::IsEnabled() { |
| 121 | + bool PlatformSpellingEngine::CheckSpelling( |
| 122 | + const std::u16string& word_to_check, |
| 123 | + spellcheck::mojom::SpellCheckHost& host) { |
| 124 | ++#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && !BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 125 | ++ return false; |
| 126 | ++#else |
| 127 | + bool word_correct = false; |
| 128 | + host.CheckSpelling(word_to_check, &word_correct); |
| 129 | + return word_correct; |
| 130 | ++#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) && |
| 131 | ++ // !BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 132 | + } |
| 133 | + |
| 134 | + // Synchronously query against the platform's spellchecker. |
| 135 | +@@ -43,5 +49,10 @@ void PlatformSpellingEngine::FillSuggestionList( |
| 136 | + const std::u16string& wrong_word, |
| 137 | + spellcheck::mojom::SpellCheckHost& host, |
| 138 | + std::vector<std::u16string>* optional_suggestions) { |
| 139 | ++#if BUILDFLAG(USE_BROWSER_SPELLCHECKER) && !BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 140 | ++ optional_suggestions->clear(); |
| 141 | ++#else |
| 142 | + host.FillSuggestionList(wrong_word, optional_suggestions); |
| 143 | ++#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER) && |
| 144 | ++ // !BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 145 | + } |
| 146 | +diff --git a/components/spellcheck/renderer/spellcheck_provider_test.cc b/components/spellcheck/renderer/spellcheck_provider_test.cc |
| 147 | +index 1e389bb31354c..e85ec2a2055de 100644 |
| 148 | +--- a/components/spellcheck/renderer/spellcheck_provider_test.cc |
| 149 | ++++ b/components/spellcheck/renderer/spellcheck_provider_test.cc |
| 150 | +@@ -173,6 +173,7 @@ void TestingSpellCheckProvider::RequestTextCheck( |
| 151 | + text_check_requests_.push_back(std::make_pair(text, std::move(callback))); |
| 152 | + } |
| 153 | + |
| 154 | ++#if BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 155 | + void TestingSpellCheckProvider::CheckSpelling(const std::u16string&, |
| 156 | + CheckSpellingCallback) { |
| 157 | + NOTREACHED_IN_MIGRATION(); |
| 158 | +@@ -182,6 +183,7 @@ void TestingSpellCheckProvider::FillSuggestionList(const std::u16string&, |
| 159 | + FillSuggestionListCallback) { |
| 160 | + NOTREACHED_IN_MIGRATION(); |
| 161 | + } |
| 162 | ++#endif // BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 163 | + |
| 164 | + #if BUILDFLAG(IS_WIN) |
| 165 | + void TestingSpellCheckProvider::InitializeDictionaries( |
| 166 | +diff --git a/components/spellcheck/renderer/spellcheck_provider_test.h b/components/spellcheck/renderer/spellcheck_provider_test.h |
| 167 | +index 65f70952d9d7f..1117e33acb3cc 100644 |
| 168 | +--- a/components/spellcheck/renderer/spellcheck_provider_test.h |
| 169 | ++++ b/components/spellcheck/renderer/spellcheck_provider_test.h |
| 170 | +@@ -139,11 +139,13 @@ class TestingSpellCheckProvider : public SpellCheckProvider, |
| 171 | + #if BUILDFLAG(USE_BROWSER_SPELLCHECKER) |
| 172 | + void RequestTextCheck(const std::u16string&, |
| 173 | + RequestTextCheckCallback) override; |
| 174 | ++#if BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 175 | + using SpellCheckProvider::CheckSpelling; |
| 176 | + void CheckSpelling(const std::u16string&, |
| 177 | + CheckSpellingCallback) override; |
| 178 | + void FillSuggestionList(const std::u16string&, |
| 179 | + FillSuggestionListCallback) override; |
| 180 | ++#endif // BUILDFLAG(ENABLE_SPELLING_SERVICE) |
| 181 | + #if BUILDFLAG(IS_WIN) |
| 182 | + void InitializeDictionaries(InitializeDictionariesCallback callback) override; |
| 183 | + #endif // BUILDFLAG(IS_WIN) |
| 184 | +diff --git a/components/spellcheck/renderer/spelling_engine.h b/components/spellcheck/renderer/spelling_engine.h |
| 185 | +index 0300d965196c6..f56973289085a 100644 |
| 186 | +--- a/components/spellcheck/renderer/spelling_engine.h |
| 187 | ++++ b/components/spellcheck/renderer/spelling_engine.h |
| 188 | +@@ -35,6 +35,9 @@ class SpellingEngine { |
| 189 | + |
| 190 | + // Synchronously check spelling. `host` is only valid for the duration of the |
| 191 | + // method call and should not be stored. |
| 192 | ++ // TODO(https://crbug.com/367216585): Replace this method with an asynchronous |
| 193 | ++ // API, since not all platforms (e.g. Android) supports synchronous |
| 194 | ++ // spellchecking. |
| 195 | + virtual bool CheckSpelling(const std::u16string& word_to_check, |
| 196 | + spellcheck::mojom::SpellCheckHost& host) = 0; |
| 197 | + |
0 commit comments