Skip to content

Commit 5fe1e59

Browse files
gahaasChromium LUCI CQ
authored andcommitted
[cleanup] Remove feature WinDelaySpellcheckServiceInit
The feature has landed already years ago, but the feature flag has never been cleaned up. [email protected] Bug: 40681243, 356623689 Change-Id: Id94047197e4d79dd4c19af63de770127fd233a9e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7012087 Reviewed-by: Hidehiko Abe <[email protected]> Reviewed-by: Guillaume Jenkins <[email protected]> Commit-Queue: Andreas Haas <[email protected]> Cr-Commit-Position: refs/heads/main@{#1532372}
1 parent 5a36703 commit 5fe1e59

15 files changed

+66
-359
lines changed

chrome/browser/chrome_browser_main.cc

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,17 +1412,6 @@ void ChromeBrowserMainParts::PostProfileInit(Profile* profile,
14121412
profile->GetPath()));
14131413
}
14141414

1415-
#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
1416-
// Create the spellcheck service. This will asynchronously retrieve the
1417-
// Windows platform spellcheck dictionary language tags used to populate the
1418-
// context menu for editable content.
1419-
if (spellcheck::UseBrowserSpellChecker() &&
1420-
profile->GetPrefs()->GetBoolean(spellcheck::prefs::kSpellCheckEnable) &&
1421-
!base::FeatureList::IsEnabled(
1422-
spellcheck::kWinDelaySpellcheckServiceInit)) {
1423-
SpellcheckServiceFactory::GetForContext(profile);
1424-
}
1425-
#endif
14261415
#endif // BUILDFLAG(IS_WIN)
14271416

14281417
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX) || \

chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -275,21 +275,15 @@ LanguageSettingsPrivateGetLanguageListFunction::Run() {
275275

276276
#if BUILDFLAG(IS_WIN)
277277
if (spellcheck::UseBrowserSpellChecker()) {
278-
if (!base::FeatureList::IsEnabled(
279-
spellcheck::kWinDelaySpellcheckServiceInit)) {
280-
// Platform dictionary support already determined at browser startup.
281-
UpdateSupportedPlatformDictionaries();
282-
} else {
283-
// Asynchronously load the dictionaries to determine platform support.
284-
SpellcheckService* service =
285-
SpellcheckServiceFactory::GetForContext(browser_context());
286-
AddRef(); // Balanced in OnDictionariesInitialized
287-
service->InitializeDictionaries(
288-
base::BindOnce(&LanguageSettingsPrivateGetLanguageListFunction::
289-
OnDictionariesInitialized,
290-
base::Unretained(this)));
291-
return RespondLater();
292-
}
278+
// Asynchronously load the dictionaries to determine platform support.
279+
SpellcheckService* service =
280+
SpellcheckServiceFactory::GetForContext(browser_context());
281+
AddRef(); // Balanced in OnDictionariesInitialized
282+
service->InitializeDictionaries(
283+
base::BindOnce(&LanguageSettingsPrivateGetLanguageListFunction::
284+
OnDictionariesInitialized,
285+
base::Unretained(this)));
286+
return RespondLater();
293287
}
294288
#endif // BUILDFLAG(IS_WIN)
295289

chrome/browser/extensions/api/language_settings_private/language_settings_private_api_unittest.cc

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,6 @@ class LanguageSettingsPrivateApiTest : public ExtensionServiceTestBase {
116116
protected:
117117
void RunGetLanguageListTest();
118118

119-
virtual void InitFeatures() {}
120-
121119
#if BUILDFLAG(IS_WIN)
122120
virtual void AddSpellcheckLanguagesForTesting(
123121
const std::vector<std::string>& spellcheck_languages_for_testing) {
@@ -136,8 +134,6 @@ class LanguageSettingsPrivateApiTest : public ExtensionServiceTestBase {
136134
EventRouterFactory::GetInstance()->SetTestingFactory(
137135
profile(), base::BindRepeating(&BuildEventRouter));
138136

139-
InitFeatures();
140-
141137
LanguageSettingsPrivateDelegateFactory::GetInstance()->SetTestingFactory(
142138
profile(), base::BindRepeating(&BuildLanguageSettingsPrivateDelegate));
143139

@@ -295,28 +291,6 @@ TEST_F(LanguageSettingsPrivateApiTest, GetNeverTranslateLanguagesListTest) {
295291
}
296292
}
297293

298-
class LanguageSettingsPrivateApiGetLanguageListTest
299-
: public LanguageSettingsPrivateApiTest {
300-
public:
301-
LanguageSettingsPrivateApiGetLanguageListTest() = default;
302-
~LanguageSettingsPrivateApiGetLanguageListTest() override = default;
303-
304-
protected:
305-
void InitFeatures() override {
306-
#if BUILDFLAG(IS_WIN)
307-
// Disable the delayed init feature since that case is tested in
308-
// LanguageSettingsPrivateApiTestDelayInit below.
309-
feature_list_.InitAndDisableFeature(
310-
spellcheck::kWinDelaySpellcheckServiceInit);
311-
#endif // BUILDFLAG(IS_WIN)
312-
}
313-
};
314-
315-
TEST_F(LanguageSettingsPrivateApiGetLanguageListTest, GetLanguageList) {
316-
translate::TranslateDownloadManager::GetInstance()->ResetForTesting();
317-
RunGetLanguageListTest();
318-
}
319-
320294
void LanguageSettingsPrivateApiTest::RunGetLanguageListTest() {
321295
struct LanguageToTest {
322296
std::string accept_language;
@@ -726,13 +700,6 @@ class LanguageSettingsPrivateApiTestDelayInit
726700
LanguageSettingsPrivateApiTestDelayInit() = default;
727701

728702
protected:
729-
void InitFeatures() override {
730-
// Force Windows hybrid spellcheck and delayed initialization of the
731-
// spellcheck service to be enabled.
732-
feature_list_.InitAndEnableFeature(
733-
spellcheck::kWinDelaySpellcheckServiceInit);
734-
}
735-
736703
void AddSpellcheckLanguagesForTesting(
737704
const std::vector<std::string>& spellcheck_languages_for_testing)
738705
override {

chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ namespace {
3030
// accesses resources.
3131
class SpellingMenuObserverTest : public InProcessBrowserTest {
3232
public:
33-
SpellingMenuObserverTest();
33+
SpellingMenuObserverTest() = default;
3434

3535
void SetUpOnMainThread() override {
3636
Reset(false);
@@ -54,6 +54,12 @@ class SpellingMenuObserverTest : public InProcessBrowserTest {
5454
content::BrowserContext* context) {
5555
auto spellcheck_service = std::make_unique<SpellcheckService>(context);
5656

57+
// With delayed initialization, we need to initialize dictionaries.
58+
spellcheck_service->InitializeDictionaries(
59+
base::BindOnce(&SpellingMenuObserverTest::OnSuggestionsComplete,
60+
base::Unretained(this)));
61+
RunUntilCallbackReceived();
62+
5763
// Call SetLanguage to assure that the platform spellchecker is initialized.
5864
spellcheck_platform::SetLanguage(
5965
spellcheck_service->platform_spell_checker(), "en-US",
@@ -167,16 +173,6 @@ class SpellingMenuObserverTest : public InProcessBrowserTest {
167173
#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
168174
};
169175

170-
#if BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
171-
SpellingMenuObserverTest::SpellingMenuObserverTest() {
172-
feature_list_.InitWithFeatures(
173-
/*enabled_features=*/{},
174-
/*disabled_features=*/{spellcheck::kWinDelaySpellcheckServiceInit});
175-
}
176-
#else
177-
SpellingMenuObserverTest::SpellingMenuObserverTest() = default;
178-
#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
179-
180176
SpellingMenuObserverTest::~SpellingMenuObserverTest() = default;
181177

182178
} // namespace

chrome/browser/site_isolation/spellcheck_per_process_browsertest.cc

Lines changed: 13 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -133,26 +133,21 @@ class MockSpellCheckHost : spellcheck::mojom::SpellCheckHost {
133133
#if BUILDFLAG(IS_WIN)
134134
void InitializeDictionaries(
135135
InitializeDictionariesCallback callback) override {
136-
if (base::FeatureList::IsEnabled(
137-
spellcheck::kWinDelaySpellcheckServiceInit)) {
138-
SpellcheckService* spellcheck = SpellcheckServiceFactory::GetForContext(
139-
process_host()->GetBrowserContext());
140-
141-
if (!spellcheck) { // Teardown.
142-
std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
143-
/*enable=*/false);
144-
return;
145-
}
146-
147-
dictionaries_loaded_callback_ = std::move(callback);
148-
149-
spellcheck->InitializeDictionaries(
150-
base::BindOnce(&MockSpellCheckHost::OnDictionariesInitialized,
151-
base::Unretained(this)));
136+
SpellcheckService* spellcheck = SpellcheckServiceFactory::GetForContext(
137+
process_host()->GetBrowserContext());
138+
139+
if (!spellcheck) { // Teardown.
140+
std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
141+
/*enable=*/false);
152142
return;
153143
}
154144

155-
NOTREACHED();
145+
dictionaries_loaded_callback_ = std::move(callback);
146+
147+
spellcheck->InitializeDictionaries(
148+
base::BindOnce(&MockSpellCheckHost::OnDictionariesInitialized,
149+
base::Unretained(this)));
150+
return;
156151
}
157152

158153
void OnDictionariesInitialized() {
@@ -264,17 +259,7 @@ class ChromeSitePerProcessSpellCheckTest : public ChromeSitePerProcessTest {
264259
public:
265260
ChromeSitePerProcessSpellCheckTest() = default;
266261

267-
void SetUp() override {
268-
#if BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
269-
// When delayed initialization of the spellcheck service is enabled by
270-
// default, want to maintain test coverage for the older code path that
271-
// initializes spellcheck on browser startup.
272-
feature_list_.InitAndDisableFeature(
273-
spellcheck::kWinDelaySpellcheckServiceInit);
274-
#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
275-
276-
ChromeSitePerProcessTest::SetUp();
277-
}
262+
void SetUp() override { ChromeSitePerProcessTest::SetUp(); }
278263

279264
protected:
280265
// Tests that spelling in out-of-process subframes is checked.
@@ -381,29 +366,3 @@ IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessSpellCheckTest,
381366
EXPECT_TRUE(host->SpellingPanelVisible());
382367
}
383368
#endif // BUILDFLAG(HAS_SPELLCHECK_PANEL)
384-
385-
#if BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
386-
class ChromeSitePerProcessSpellCheckTestDelayInit
387-
: public ChromeSitePerProcessSpellCheckTest {
388-
public:
389-
ChromeSitePerProcessSpellCheckTestDelayInit() = default;
390-
391-
void SetUp() override {
392-
// Don't initialize the SpellcheckService on browser launch.
393-
feature_list_.InitAndEnableFeature(
394-
spellcheck::kWinDelaySpellcheckServiceInit);
395-
396-
ChromeSitePerProcessTest::SetUp();
397-
}
398-
};
399-
400-
IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessSpellCheckTestDelayInit,
401-
OOPIFSpellCheckTest) {
402-
RunOOPIFSpellCheckTest();
403-
}
404-
405-
IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessSpellCheckTestDelayInit,
406-
OOPIFDisabledSpellCheckTest) {
407-
RunOOPIFDisabledSpellCheckTest();
408-
}
409-
#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)

chrome/browser/spellchecker/spell_check_host_chrome_impl.cc

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -186,27 +186,21 @@ void SpellCheckHostChromeImpl::InitializeDictionaries(
186186
InitializeDictionariesCallback callback) {
187187
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
188188

189-
if (base::FeatureList::IsEnabled(
190-
spellcheck::kWinDelaySpellcheckServiceInit)) {
191-
// Initialize the spellcheck service if needed. Initialization must
192-
// happen on UI thread.
193-
SpellcheckService* spellcheck = GetSpellcheckService();
194-
195-
if (!spellcheck) { // Teardown.
196-
std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
197-
/*enable=*/false);
198-
return;
199-
}
200-
201-
dictionaries_loaded_callback_ = std::move(callback);
189+
// Initialize the spellcheck service if needed. Initialization must
190+
// happen on UI thread.
191+
SpellcheckService* spellcheck = GetSpellcheckService();
202192

203-
spellcheck->InitializeDictionaries(
204-
base::BindOnce(&SpellCheckHostChromeImpl::OnDictionariesInitialized,
205-
weak_factory_.GetWeakPtr()));
193+
if (!spellcheck) { // Teardown.
194+
std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
195+
/*enable=*/false);
206196
return;
207197
}
208198

209-
NOTREACHED();
199+
dictionaries_loaded_callback_ = std::move(callback);
200+
201+
spellcheck->InitializeDictionaries(
202+
base::BindOnce(&SpellCheckHostChromeImpl::OnDictionariesInitialized,
203+
weak_factory_.GetWeakPtr()));
210204
}
211205

212206
void SpellCheckHostChromeImpl::OnDictionariesInitialized() {

chrome/browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc

Lines changed: 17 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,7 @@ class SpellCheckHostChromeImplWinBrowserTest : public InProcessBrowserTest {
3232
public:
3333
SpellCheckHostChromeImplWinBrowserTest() = default;
3434

35-
void SetUp() override {
36-
// Don't delay initialization of the SpellcheckService on browser launch.
37-
feature_list_.InitAndDisableFeature(
38-
spellcheck::kWinDelaySpellcheckServiceInit);
39-
InProcessBrowserTest::SetUp();
40-
}
35+
void SetUp() override { InProcessBrowserTest::SetUp(); }
4136

4237
void SetUpOnMainThread() override {
4338
content::BrowserContext* context = browser()->profile();
@@ -55,7 +50,22 @@ class SpellCheckHostChromeImplWinBrowserTest : public InProcessBrowserTest {
5550

5651
void TearDownOnMainThread() override { renderer_.reset(); }
5752

58-
virtual void InitializeSpellcheckService() {}
53+
void InitializeSpellcheckService() {
54+
spell_check_host_->InitializeDictionaries(base::BindOnce(
55+
&SpellCheckHostChromeImplWinBrowserTest::InitializeDictionariesCallback,
56+
base::Unretained(this)));
57+
RunUntilResultReceived();
58+
}
59+
60+
void InitializeDictionariesCallback(
61+
std::vector<spellcheck::mojom::SpellCheckBDictLanguagePtr> dictionaries,
62+
const std::vector<std::string>& custom_words,
63+
bool enable) {
64+
received_result_ = true;
65+
if (quit_) {
66+
std::move(quit_).Run();
67+
}
68+
}
5969

6070
void OnSpellcheckResult(const std::vector<SpellCheckResult>& result) {
6171
received_result_ = true;
@@ -129,41 +139,3 @@ void SpellCheckHostChromeImplWinBrowserTest::RunSpellCheckReturnMessageTest() {
129139
EXPECT_EQ(result_[0].length, 2);
130140
EXPECT_EQ(result_[0].decoration, SpellCheckResult::SPELLING);
131141
}
132-
133-
class SpellCheckHostChromeImplWinBrowserTestDelayInit
134-
: public SpellCheckHostChromeImplWinBrowserTest {
135-
public:
136-
SpellCheckHostChromeImplWinBrowserTestDelayInit() = default;
137-
138-
void SetUp() override {
139-
// Don't initialize the SpellcheckService on browser launch.
140-
feature_list_.InitAndEnableFeature(
141-
spellcheck::kWinDelaySpellcheckServiceInit);
142-
InProcessBrowserTest::SetUp();
143-
}
144-
145-
void InitializeSpellcheckService() override {
146-
// With the kWinDelaySpellcheckServiceInit feature flag set, the spellcheck
147-
// service is not initialized when instantiated. Call InitializeDictionaries
148-
// to load the dictionaries.
149-
spell_check_host_->InitializeDictionaries(
150-
base::BindOnce(&SpellCheckHostChromeImplWinBrowserTestDelayInit::
151-
InitializeDictionariesCallback,
152-
base::Unretained(this)));
153-
RunUntilResultReceived();
154-
}
155-
156-
void InitializeDictionariesCallback(
157-
std::vector<spellcheck::mojom::SpellCheckBDictLanguagePtr> dictionaries,
158-
const std::vector<std::string>& custom_words,
159-
bool enable) {
160-
received_result_ = true;
161-
if (quit_)
162-
std::move(quit_).Run();
163-
}
164-
};
165-
166-
IN_PROC_BROWSER_TEST_F(SpellCheckHostChromeImplWinBrowserTestDelayInit,
167-
SpellCheckReturnMessage) {
168-
RunSpellCheckReturnMessageTest();
169-
}

chrome/browser/spellchecker/spellcheck_service.cc

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,7 @@ SpellcheckService::SpellcheckService(content::BrowserContext* context)
165165
custom_dictionary_->Load();
166166

167167
#if BUILDFLAG(IS_WIN)
168-
if (spellcheck::UseBrowserSpellChecker() &&
169-
base::FeatureList::IsEnabled(
170-
spellcheck::kWinDelaySpellcheckServiceInit)) {
168+
if (spellcheck::UseBrowserSpellChecker()) {
171169
// If initialization of the spellcheck service is on-demand, it is up to the
172170
// instantiator of the spellcheck service to call InitializeDictionaries
173171
// with a callback.
@@ -487,9 +485,7 @@ void SpellcheckService::LoadDictionaries() {
487485
}
488486

489487
#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
490-
if (base::FeatureList::IsEnabled(
491-
spellcheck::kWinDelaySpellcheckServiceInit) &&
492-
spellcheck::UseBrowserSpellChecker()) {
488+
if (spellcheck::UseBrowserSpellChecker()) {
493489
// Only want to fire the callback on first call to LoadDictionaries
494490
// originating from InitializeDictionaries, since supported platform
495491
// dictionaries are cached throughout the browser session and not
@@ -517,9 +513,7 @@ bool SpellcheckService::IsSpellcheckEnabled() const {
517513

518514
bool enable_if_uninitialized = false;
519515
#if BUILDFLAG(IS_WIN)
520-
if (spellcheck::UseBrowserSpellChecker() &&
521-
base::FeatureList::IsEnabled(
522-
spellcheck::kWinDelaySpellcheckServiceInit)) {
516+
if (spellcheck::UseBrowserSpellChecker()) {
523517
// If initialization of the spellcheck service is on-demand, the
524518
// renderer-side SpellCheck object needs to start out as enabled in order
525519
// for a click on editable content to initialize the spellcheck service.

0 commit comments

Comments
 (0)