github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-01-08 23:18:06 -05:00
Code Issues Packages Projects Releases Wiki Activity

refactor: remove spellcheck::kWinDelaySpellcheckServiceInit patch (#48843)

Browse Source 
refactor: remove spellcheck::kWinDelaySpellcheckServiceInit patch
This commit is contained in:
Shelley Vohr
2025-11-08 17:26:06 +01:00
committed by GitHub
parent bec1734d63
commit ab0ff5dffc
4 changed files with 1 additions and 770 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
patches/chromium/.patches
View File

@@ -141,5 +141,4 @@ allow_electron_to_depend_on_components_os_crypt_sync.patch
expose_referrerscriptinfo_hostdefinedoptionsindex.patch
chore_disable_protocol_handler_dcheck.patch
fix_check_for_file_existence_before_setting_mtime.patch
revert_cleanup_remove_feature_windelayspellcheckserviceinit.patch
fix_linux_tray_id.patch

765
patches/chromium/revert_cleanup_remove_feature_windelayspellcheckserviceinit.patch
View File

@@ -1,765 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles@charleskerr.com>
Date: Tue, 21 Oct 2025 21:45:33 -0500
Subject: Revert "[cleanup] Remove feature WinDelaySpellcheckServiceInit"
This reverts commit 5fe1e59226f59c4d6fb70e7410d1a0ab83688ae2.
Our codebase currently needs the ability to delay this service.
It was added in c2d7164 (#38248) to fix a crash originally
described in 97b353a (#34993):
> Delaying spell check initialization is causing specs for
> 'custom dictionary word list API' to fail in Electron.
This patch can be removed when we fix that crash.
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index 2034b961d99225ebe9b606af915f5d90fdae913e..a7ee864ae4d14d36bdf5f7f4fb0ba86255dc9c6f 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -1475,6 +1475,17 @@ void ChromeBrowserMainParts::PostProfileInit(Profile* profile,
profile->GetPath()));
}
+#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+ // Create the spellcheck service. This will asynchronously retrieve the
+ // Windows platform spellcheck dictionary language tags used to populate the
+ // context menu for editable content.
+ if (spellcheck::UseBrowserSpellChecker() &&
+ profile->GetPrefs()->GetBoolean(spellcheck::prefs::kSpellCheckEnable) &&
+ !base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit)) {
+ SpellcheckServiceFactory::GetForContext(profile);
+ }
+#endif
#endif // BUILDFLAG(IS_WIN)
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX) || \
diff --git a/chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc b/chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
index 3e617c9bf924e089e27784f960f08a21b98fee0a..8fd91821d588b5d70d1b320b64cc640c77be8d7d 100644
--- a/chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
+++ b/chrome/browser/extensions/api/language_settings_private/language_settings_private_api.cc
@@ -275,15 +275,21 @@ LanguageSettingsPrivateGetLanguageListFunction::Run() {
#if BUILDFLAG(IS_WIN)
if (spellcheck::UseBrowserSpellChecker()) {
- // Asynchronously load the dictionaries to determine platform support.
- SpellcheckService* service =
- SpellcheckServiceFactory::GetForContext(browser_context());
- AddRef(); // Balanced in OnDictionariesInitialized
- service->InitializeDictionaries(
- base::BindOnce(&LanguageSettingsPrivateGetLanguageListFunction::
- OnDictionariesInitialized,
- base::Unretained(this)));
- return RespondLater();
+ if (!base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit)) {
+ // Platform dictionary support already determined at browser startup.
+ UpdateSupportedPlatformDictionaries();
+ } else {
+ // Asynchronously load the dictionaries to determine platform support.
+ SpellcheckService* service =
+ SpellcheckServiceFactory::GetForContext(browser_context());
+ AddRef(); // Balanced in OnDictionariesInitialized
+ service->InitializeDictionaries(
+ base::BindOnce(&LanguageSettingsPrivateGetLanguageListFunction::
+ OnDictionariesInitialized,
+ base::Unretained(this)));
+ return RespondLater();
+ }
}
#endif // BUILDFLAG(IS_WIN)
diff --git a/chrome/browser/extensions/api/language_settings_private/language_settings_private_api_unittest.cc b/chrome/browser/extensions/api/language_settings_private/language_settings_private_api_unittest.cc
index 272d50c9b060a8984a15d2dfbf2297e931482b57..e74b37ed7e596ea57728694eab9a398e94bde08c 100644
--- a/chrome/browser/extensions/api/language_settings_private/language_settings_private_api_unittest.cc
+++ b/chrome/browser/extensions/api/language_settings_private/language_settings_private_api_unittest.cc
@@ -116,6 +116,8 @@ class LanguageSettingsPrivateApiTest : public ExtensionServiceTestBase {
protected:
void RunGetLanguageListTest();
+ virtual void InitFeatures() {}
+
#if BUILDFLAG(IS_WIN)
virtual void AddSpellcheckLanguagesForTesting(
const std::vector<std::string>& spellcheck_languages_for_testing) {
@@ -134,6 +136,8 @@ class LanguageSettingsPrivateApiTest : public ExtensionServiceTestBase {
EventRouterFactory::GetInstance()->SetTestingFactory(
profile(), base::BindRepeating(&BuildEventRouter));
+ InitFeatures();
+
LanguageSettingsPrivateDelegateFactory::GetInstance()->SetTestingFactory(
profile(), base::BindRepeating(&BuildLanguageSettingsPrivateDelegate));
@@ -291,6 +295,28 @@ TEST_F(LanguageSettingsPrivateApiTest, GetNeverTranslateLanguagesListTest) {
}
}
+class LanguageSettingsPrivateApiGetLanguageListTest
+ : public LanguageSettingsPrivateApiTest {
+ public:
+ LanguageSettingsPrivateApiGetLanguageListTest() = default;
+ ~LanguageSettingsPrivateApiGetLanguageListTest() override = default;
+
+ protected:
+ void InitFeatures() override {
+#if BUILDFLAG(IS_WIN)
+ // Disable the delayed init feature since that case is tested in
+ // LanguageSettingsPrivateApiTestDelayInit below.
+ feature_list_.InitAndDisableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+#endif // BUILDFLAG(IS_WIN)
+ }
+};
+
+TEST_F(LanguageSettingsPrivateApiGetLanguageListTest, GetLanguageList) {
+ translate::TranslateDownloadManager::GetInstance()->ResetForTesting();
+ RunGetLanguageListTest();
+}
+
void LanguageSettingsPrivateApiTest::RunGetLanguageListTest() {
struct LanguageToTest {
std::string accept_language;
@@ -700,6 +726,13 @@ class LanguageSettingsPrivateApiTestDelayInit
LanguageSettingsPrivateApiTestDelayInit() = default;
protected:
+ void InitFeatures() override {
+ // Force Windows hybrid spellcheck and delayed initialization of the
+ // spellcheck service to be enabled.
+ feature_list_.InitAndEnableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ }
+
void AddSpellcheckLanguagesForTesting(
const std::vector<std::string>& spellcheck_languages_for_testing)
override {
diff --git a/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc b/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
index bd0d77323277ee422ec4f75cee9f7d4e9bf198b3..d0ed451fe104b9671d6002ca72e8ca3a620cb111 100644
--- a/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
+++ b/chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
@@ -30,7 +30,7 @@ namespace {
// accesses resources.
class SpellingMenuObserverTest : public InProcessBrowserTest {
public:
- SpellingMenuObserverTest() = default;
+ SpellingMenuObserverTest();
void SetUpOnMainThread() override {
Reset(false);
@@ -54,12 +54,6 @@ class SpellingMenuObserverTest : public InProcessBrowserTest {
content::BrowserContext* context) {
auto spellcheck_service = std::make_unique<SpellcheckService>(context);
- // With delayed initialization, we need to initialize dictionaries.
- spellcheck_service->InitializeDictionaries(
- base::BindOnce(&SpellingMenuObserverTest::OnSuggestionsComplete,
- base::Unretained(this)));
- RunUntilCallbackReceived();
-
// Call SetLanguage to assure that the platform spellchecker is initialized.
spellcheck_platform::SetLanguage(
spellcheck_service->platform_spell_checker(), "en-US",
@@ -173,6 +167,16 @@ class SpellingMenuObserverTest : public InProcessBrowserTest {
#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
};
+#if BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+SpellingMenuObserverTest::SpellingMenuObserverTest() {
+ feature_list_.InitWithFeatures(
+ /*enabled_features=*/{},
+ /*disabled_features=*/{spellcheck::kWinDelaySpellcheckServiceInit});
+}
+#else
+SpellingMenuObserverTest::SpellingMenuObserverTest() = default;
+#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+
SpellingMenuObserverTest::~SpellingMenuObserverTest() = default;
} // namespace
diff --git a/chrome/browser/site_isolation/spellcheck_per_process_browsertest.cc b/chrome/browser/site_isolation/spellcheck_per_process_browsertest.cc
index 081e422f9a29fc36eea19ed730b430ba5ceb2861..7817bc7a6f9f2c7fdeb08c9b185d2dd3c8cc61f7 100644
--- a/chrome/browser/site_isolation/spellcheck_per_process_browsertest.cc
+++ b/chrome/browser/site_isolation/spellcheck_per_process_browsertest.cc
@@ -134,21 +134,26 @@ class MockSpellCheckHost : spellcheck::mojom::SpellCheckHost {
#if BUILDFLAG(IS_WIN)
void InitializeDictionaries(
InitializeDictionariesCallback callback) override {
- SpellcheckService* spellcheck = SpellcheckServiceFactory::GetForContext(
- process_host()->GetBrowserContext());
-
- if (!spellcheck) { // Teardown.
- std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
- /*enable=*/false);
+ if (base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit)) {
+ SpellcheckService* spellcheck = SpellcheckServiceFactory::GetForContext(
+ process_host()->GetBrowserContext());
+
+ if (!spellcheck) { // Teardown.
+ std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
+ /*enable=*/false);
+ return;
+ }
+
+ dictionaries_loaded_callback_ = std::move(callback);
+
+ spellcheck->InitializeDictionaries(
+ base::BindOnce(&MockSpellCheckHost::OnDictionariesInitialized,
+ base::Unretained(this)));
return;
}
- dictionaries_loaded_callback_ = std::move(callback);
-
- spellcheck->InitializeDictionaries(
- base::BindOnce(&MockSpellCheckHost::OnDictionariesInitialized,
- base::Unretained(this)));
- return;
+ NOTREACHED();
}
void OnDictionariesInitialized() {
@@ -263,7 +268,17 @@ class ChromeSitePerProcessSpellCheckTest : public ChromeSitePerProcessTest {
blink::features::kRestrictSpellingAndGrammarHighlights);
}
- void SetUp() override { ChromeSitePerProcessTest::SetUp(); }
+ void SetUp() override {
+#if BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+ // When delayed initialization of the spellcheck service is enabled by
+ // default, want to maintain test coverage for the older code path that
+ // initializes spellcheck on browser startup.
+ feature_list_.InitAndDisableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+
+ ChromeSitePerProcessTest::SetUp();
+ }
protected:
// Tests that spelling in out-of-process subframes is checked.
@@ -370,3 +385,29 @@ IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessSpellCheckTest,
EXPECT_TRUE(host->SpellingPanelVisible());
}
#endif // BUILDFLAG(HAS_SPELLCHECK_PANEL)
+
+#if BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
+class ChromeSitePerProcessSpellCheckTestDelayInit
+ : public ChromeSitePerProcessSpellCheckTest {
+ public:
+ ChromeSitePerProcessSpellCheckTestDelayInit() = default;
+
+ void SetUp() override {
+ // Don't initialize the SpellcheckService on browser launch.
+ feature_list_.InitAndEnableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+
+ ChromeSitePerProcessTest::SetUp();
+ }
+};
+
+IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessSpellCheckTestDelayInit,
+ OOPIFSpellCheckTest) {
+ RunOOPIFSpellCheckTest();
+}
+
+IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessSpellCheckTestDelayInit,
+ OOPIFDisabledSpellCheckTest) {
+ RunOOPIFDisabledSpellCheckTest();
+}
+#endif // BUILDFLAG(IS_WIN) && BUILDFLAG(USE_BROWSER_SPELLCHECKER)
diff --git a/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc b/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc
index 4eda6aba66af11df4e8719e075e807564d54baac..29d9399c78fc6b4d597ab1f0dc53571131bffd95 100644
--- a/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc
+++ b/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc
@@ -186,21 +186,27 @@ void SpellCheckHostChromeImpl::InitializeDictionaries(
InitializeDictionariesCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- // Initialize the spellcheck service if needed. Initialization must
- // happen on UI thread.
- SpellcheckService* spellcheck = GetSpellcheckService();
+ if (base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit)) {
+ // Initialize the spellcheck service if needed. Initialization must
+ // happen on UI thread.
+ SpellcheckService* spellcheck = GetSpellcheckService();
+
+ if (!spellcheck) { // Teardown.
+ std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
+ /*enable=*/false);
+ return;
+ }
- if (!spellcheck) { // Teardown.
- std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
- /*enable=*/false);
+ dictionaries_loaded_callback_ = std::move(callback);
+
+ spellcheck->InitializeDictionaries(
+ base::BindOnce(&SpellCheckHostChromeImpl::OnDictionariesInitialized,
+ weak_factory_.GetWeakPtr()));
return;
}
- dictionaries_loaded_callback_ = std::move(callback);
-
- spellcheck->InitializeDictionaries(
- base::BindOnce(&SpellCheckHostChromeImpl::OnDictionariesInitialized,
- weak_factory_.GetWeakPtr()));
+ NOTREACHED();
}
void SpellCheckHostChromeImpl::OnDictionariesInitialized() {
diff --git a/chrome/browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc b/chrome/browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc
index 31ab441aedd69c08c6f1beefa2129e60bd44a7f3..308c90516a9881b82da5cedec5d80208dbccdced 100644
--- a/chrome/browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc
+++ b/chrome/browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc
@@ -32,7 +32,12 @@ class SpellCheckHostChromeImplWinBrowserTest : public InProcessBrowserTest {
public:
SpellCheckHostChromeImplWinBrowserTest() = default;
- void SetUp() override { InProcessBrowserTest::SetUp(); }
+ void SetUp() override {
+ // Don't delay initialization of the SpellcheckService on browser launch.
+ feature_list_.InitAndDisableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ InProcessBrowserTest::SetUp();
+ }
void SetUpOnMainThread() override {
content::BrowserContext* context = browser()->profile();
@@ -50,22 +55,7 @@ class SpellCheckHostChromeImplWinBrowserTest : public InProcessBrowserTest {
void TearDownOnMainThread() override { renderer_.reset(); }
- void InitializeSpellcheckService() {
- spell_check_host_->InitializeDictionaries(base::BindOnce(
- &SpellCheckHostChromeImplWinBrowserTest::InitializeDictionariesCallback,
- base::Unretained(this)));
- RunUntilResultReceived();
- }
-
- void InitializeDictionariesCallback(
- std::vector<spellcheck::mojom::SpellCheckBDictLanguagePtr> dictionaries,
- const std::vector<std::string>& custom_words,
- bool enable) {
- received_result_ = true;
- if (quit_) {
- std::move(quit_).Run();
- }
- }
+ virtual void InitializeSpellcheckService() {}
void OnSpellcheckResult(const std::vector<SpellCheckResult>& result) {
received_result_ = true;
@@ -139,3 +129,41 @@ void SpellCheckHostChromeImplWinBrowserTest::RunSpellCheckReturnMessageTest() {
EXPECT_EQ(result_[0].length, 2);
EXPECT_EQ(result_[0].decoration, SpellCheckResult::SPELLING);
}
+
+class SpellCheckHostChromeImplWinBrowserTestDelayInit
+ : public SpellCheckHostChromeImplWinBrowserTest {
+ public:
+ SpellCheckHostChromeImplWinBrowserTestDelayInit() = default;
+
+ void SetUp() override {
+ // Don't initialize the SpellcheckService on browser launch.
+ feature_list_.InitAndEnableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ InProcessBrowserTest::SetUp();
+ }
+
+ void InitializeSpellcheckService() override {
+ // With the kWinDelaySpellcheckServiceInit feature flag set, the spellcheck
+ // service is not initialized when instantiated. Call InitializeDictionaries
+ // to load the dictionaries.
+ spell_check_host_->InitializeDictionaries(
+ base::BindOnce(&SpellCheckHostChromeImplWinBrowserTestDelayInit::
+ InitializeDictionariesCallback,
+ base::Unretained(this)));
+ RunUntilResultReceived();
+ }
+
+ void InitializeDictionariesCallback(
+ std::vector<spellcheck::mojom::SpellCheckBDictLanguagePtr> dictionaries,
+ const std::vector<std::string>& custom_words,
+ bool enable) {
+ received_result_ = true;
+ if (quit_)
+ std::move(quit_).Run();
+ }
+};
+
+IN_PROC_BROWSER_TEST_F(SpellCheckHostChromeImplWinBrowserTestDelayInit,
+ SpellCheckReturnMessage) {
+ RunSpellCheckReturnMessageTest();
+}
diff --git a/chrome/browser/spellchecker/spellcheck_service.cc b/chrome/browser/spellchecker/spellcheck_service.cc
index 97b8fc0f38650e816bcae00e074543766f71e0d0..83d9e177e9b5e13587655915b9fbbddf5f9f275d 100644
--- a/chrome/browser/spellchecker/spellcheck_service.cc
+++ b/chrome/browser/spellchecker/spellcheck_service.cc
@@ -170,7 +170,9 @@ SpellcheckService::SpellcheckService(content::BrowserContext* context)
custom_dictionary_->Load();
#if BUILDFLAG(IS_WIN)
- if (spellcheck::UseBrowserSpellChecker()) {
+ if (spellcheck::UseBrowserSpellChecker() &&
+ base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit)) {
// If initialization of the spellcheck service is on-demand, it is up to the
// instantiator of the spellcheck service to call InitializeDictionaries
// with a callback.
@@ -493,7 +495,9 @@ void SpellcheckService::LoadDictionaries() {
}
#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
- if (spellcheck::UseBrowserSpellChecker()) {
+ if (base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit) &&
+ spellcheck::UseBrowserSpellChecker()) {
// Only want to fire the callback on first call to LoadDictionaries
// originating from InitializeDictionaries, since supported platform
// dictionaries are cached throughout the browser session and not
@@ -521,7 +525,9 @@ bool SpellcheckService::IsSpellcheckEnabled() const {
bool enable_if_uninitialized = false;
#if BUILDFLAG(IS_WIN)
- if (spellcheck::UseBrowserSpellChecker()) {
+ if (spellcheck::UseBrowserSpellChecker() &&
+ base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit)) {
// If initialization of the spellcheck service is on-demand, the
// renderer-side SpellCheck object needs to start out as enabled in order
// for a click on editable content to initialize the spellcheck service.
diff --git a/chrome/browser/spellchecker/spellcheck_service_browsertest.cc b/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
index d99667b86795215717dbb25dfefdf0a32f7cd089..5a6dd4dcc4467601a3197f5b273231b2c95d8cdf 100644
--- a/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
+++ b/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
@@ -712,6 +712,32 @@ class SpellcheckServiceWindowsHybridBrowserTest
: SpellcheckServiceBrowserTest(/* use_browser_spell_checker=*/true) {}
};
+IN_PROC_BROWSER_TEST_F(SpellcheckServiceWindowsHybridBrowserTest,
+ WindowsHybridSpellcheck) {
+ // This test specifically covers the case where spellcheck delayed
+ // initialization is not enabled, so return early if it is. Other tests
+ // cover the case where delayed initialization is enabled.
+ if (base::FeatureList::IsEnabled(spellcheck::kWinDelaySpellcheckServiceInit))
+ return;
+
+ ASSERT_TRUE(spellcheck::UseBrowserSpellChecker());
+
+ // Note that the base class forces dictionary sync to not be performed, which
+ // on its own would have created a SpellcheckService object. So testing here
+ // that we are still instantiating the SpellcheckService as a browser startup
+ // task to support hybrid spellchecking.
+ SpellcheckService* service = static_cast<SpellcheckService*>(
+ SpellcheckServiceFactory::GetInstance()->GetServiceForBrowserContext(
+ GetContext(), /* create */ false));
+ ASSERT_NE(nullptr, service);
+
+ // The list of Windows spellcheck languages should have been populated by at
+ // least one language. This assures that the spellcheck context menu will
+ // include Windows spellcheck languages that lack Hunspell support.
+ EXPECT_TRUE(service->dictionaries_loaded());
+ EXPECT_FALSE(service->windows_spellcheck_dictionary_map_.empty());
+}
+
class SpellcheckServiceWindowsHybridBrowserTestDelayInit
: public SpellcheckServiceBrowserTest {
public:
@@ -719,6 +745,10 @@ class SpellcheckServiceWindowsHybridBrowserTestDelayInit
: SpellcheckServiceBrowserTest(/* use_browser_spell_checker=*/true) {}
void SetUp() override {
+ // Don't initialize the SpellcheckService on browser launch.
+ feature_list_.InitAndEnableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+
// Add command line switch that forces first run state, to test whether
// primary preferred language has its spellcheck dictionary enabled by
// default for non-Hunspell languages.
@@ -808,9 +838,10 @@ IN_PROC_BROWSER_TEST_F(SpellcheckServiceWindowsHybridBrowserTestDelayInit,
WindowsHybridSpellcheckDelayInit) {
ASSERT_TRUE(spellcheck::UseBrowserSpellChecker());
- // The base class forces dictionary sync to be skipped, so the
- // SpellcheckService object should not have been created on browser startup
- // because. Verify this is the case.
+ // Note that the base class forces dictionary sync to not be performed, and
+ // the kWinDelaySpellcheckServiceInit flag is set, which together should
+ // prevent creation of a SpellcheckService object on browser startup. So
+ // testing here that this is indeed the case.
SpellcheckService* service = static_cast<SpellcheckService*>(
SpellcheckServiceFactory::GetInstance()->GetServiceForBrowserContext(
GetContext(), /* create */ false));
diff --git a/chrome/browser/spellchecker/spellcheck_service_unittest.cc b/chrome/browser/spellchecker/spellcheck_service_unittest.cc
index be073ddd362c6a8e9b93961eba2c5a7d4a978b5e..68c479ca1e8fd93ef661258823122cbc6277ad95 100644
--- a/chrome/browser/spellchecker/spellcheck_service_unittest.cc
+++ b/chrome/browser/spellchecker/spellcheck_service_unittest.cc
@@ -181,11 +181,15 @@ class SpellcheckServiceHybridUnitTestBase
protected:
void SetUp() override {
+ InitFeatures();
+
// Use SetTestingFactoryAndUse to force creation and initialization.
SpellcheckServiceFactory::GetInstance()->SetTestingFactoryAndUse(
&profile_, base::BindRepeating(&BuildSpellcheckService));
}
+ virtual void InitFeatures() {}
+
virtual void InitializeSpellcheckService(
const std::vector<std::string>& spellcheck_languages_for_testing) {
// Fake the presence of Windows spellcheck dictionaries.
@@ -215,6 +219,8 @@ class SpellcheckServiceHybridUnitTestBase
static const std::vector<std::string>
windows_spellcheck_languages_for_testing_;
+ base::test::ScopedFeatureList feature_list_;
+
raw_ptr<SpellcheckService> spellcheck_service_;
};
@@ -327,6 +333,18 @@ const std::vector<std::string> SpellcheckServiceHybridUnitTestBase::
// dictionaries.
};
+class GetDictionariesHybridUnitTestNoDelayInit
+ : public SpellcheckServiceHybridUnitTestBase,
+ public testing::WithParamInterface<TestCase> {
+ protected:
+ void InitFeatures() override {
+ // Disable kWinDelaySpellcheckServiceInit, as the case where it's enabled
+ // is tested in SpellcheckServiceWindowsDictionaryMappingUnitTestDelayInit.
+ feature_list_.InitAndDisableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ }
+};
+
static const TestCase kHybridGetDictionariesParams[] = {
// Galician (gl) has only Windows support, no Hunspell dictionary. Croatian
// (hr) has only Hunspell support, no local Windows dictionary. First
@@ -379,6 +397,16 @@ static const TestCase kHybridGetDictionariesParams[] = {
TestCase("it,it-IT", {"it", "it-IT"}, {"it", "it-IT"}, {"it", "it-IT"}),
};
+INSTANTIATE_TEST_SUITE_P(TestCases,
+ GetDictionariesHybridUnitTestNoDelayInit,
+ testing::ValuesIn(kHybridGetDictionariesParams));
+
+TEST_P(GetDictionariesHybridUnitTestNoDelayInit, GetDictionaries) {
+ RunGetDictionariesTest(GetParam().accept_languages,
+ GetParam().spellcheck_dictionaries,
+ GetParam().expected_dictionaries);
+}
+
struct DictionaryMappingTestCase {
std::string full_tag;
std::string expected_accept_language;
@@ -401,6 +429,18 @@ std::ostream& operator<<(std::ostream& out,
return out;
}
+class SpellcheckServiceWindowsDictionaryMappingUnitTest
+ : public SpellcheckServiceHybridUnitTestBase,
+ public testing::WithParamInterface<DictionaryMappingTestCase> {
+ protected:
+ void InitFeatures() override {
+ // Disable kWinDelaySpellcheckServiceInit, as the case where it's enabled
+ // is tested in SpellcheckServiceWindowsDictionaryMappingUnitTestDelayInit.
+ feature_list_.InitAndDisableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ }
+};
+
static const DictionaryMappingTestCase kHybridDictionaryMappingsParams[] = {
DictionaryMappingTestCase({"en-CA", "en-CA", "en-CA", "en", "en"}),
DictionaryMappingTestCase({"en-PH", "en", "en", "", ""}),
@@ -423,6 +463,18 @@ static const DictionaryMappingTestCase kHybridDictionaryMappingsParams[] = {
DictionaryMappingTestCase({"pt-BR", "pt-BR", "pt-BR", "pt", "pt"}),
};
+INSTANTIATE_TEST_SUITE_P(TestCases,
+ SpellcheckServiceWindowsDictionaryMappingUnitTest,
+ testing::ValuesIn(kHybridDictionaryMappingsParams));
+
+TEST_P(SpellcheckServiceWindowsDictionaryMappingUnitTest, CheckMappings) {
+ RunDictionaryMappingTest(
+ GetParam().full_tag, GetParam().expected_accept_language,
+ GetParam().expected_tag_passed_to_spellcheck,
+ GetParam().expected_accept_language_generic,
+ GetParam().expected_tag_passed_to_spellcheck_generic);
+}
+
class SpellcheckServiceHybridUnitTestDelayInitBase
: public SpellcheckServiceHybridUnitTestBase {
public:
@@ -435,6 +487,12 @@ class SpellcheckServiceHybridUnitTestDelayInitBase
}
protected:
+ void InitFeatures() override {
+ // Don't initialize the SpellcheckService on browser launch.
+ feature_list_.InitAndEnableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ }
+
void InitializeSpellcheckService(
const std::vector<std::string>& spellcheck_languages_for_testing)
override {
diff --git a/components/spellcheck/common/spellcheck_features.cc b/components/spellcheck/common/spellcheck_features.cc
index ab07d966779449efcb0bad95ebe05e6018300048..527fa5d72369bb1194684527312eb093946d41c0 100644
--- a/components/spellcheck/common/spellcheck_features.cc
+++ b/components/spellcheck/common/spellcheck_features.cc
@@ -41,6 +41,8 @@ ScopedDisableBrowserSpellCheckerForTesting::
g_browser_spell_checker_enabled = previous_value_;
}
+BASE_FEATURE(kWinDelaySpellcheckServiceInit, base::FEATURE_ENABLED_BY_DEFAULT);
+
#endif // BUILDFLAG(IS_WIN)
#if BUILDFLAG(IS_ANDROID)
diff --git a/components/spellcheck/common/spellcheck_features.h b/components/spellcheck/common/spellcheck_features.h
index 01e193221c74f0e0bd8620627455f92741448075..7929156c59d078b3d4299bb44ea28bc61bbe0086 100644
--- a/components/spellcheck/common/spellcheck_features.h
+++ b/components/spellcheck/common/spellcheck_features.h
@@ -30,6 +30,25 @@ class ScopedDisableBrowserSpellCheckerForTesting {
const bool previous_value_;
};
+// 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=WinDelaySpellcheckServiceInit
+// and if instantiation of the spellcheck service needs to be completely
+// disabled:
+// chrome
+// --enable-features=WinDelaySpellcheckServiceInit
+// --disable-sync-types="Dictionary"
+BASE_DECLARE_FEATURE(kWinDelaySpellcheckServiceInit);
#endif // BUILDFLAG(IS_WIN)
#if BUILDFLAG(IS_ANDROID)
diff --git a/components/spellcheck/renderer/spellcheck_provider.cc b/components/spellcheck/renderer/spellcheck_provider.cc
index 20e73dd66865f1d7573adc092d8747e3b3252cfd..0bec57f9a7276c3623edd4fcf009d7b35e453df4 100644
--- a/components/spellcheck/renderer/spellcheck_provider.cc
+++ b/components/spellcheck/renderer/spellcheck_provider.cc
@@ -126,7 +126,9 @@ void SpellCheckProvider::RequestTextChecking(
#if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
if (spellcheck::UseBrowserSpellChecker()) {
#if BUILDFLAG(IS_WIN)
- if (!dictionaries_loaded_) {
+ if (base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit) &&
+ !dictionaries_loaded_) {
// Initialize the spellcheck service on demand (this spellcheck request
// could be the result of the first click in editable content), then
// complete the text check request when the dictionaries are loaded.
diff --git a/components/spellcheck/renderer/spellcheck_provider_test.cc b/components/spellcheck/renderer/spellcheck_provider_test.cc
index 04dcb599f5dd93d3e381c243e5ba81fbec8a3790..12c32fd631ff93f1d1ff3c7d2a19924e8909c099 100644
--- a/components/spellcheck/renderer/spellcheck_provider_test.cc
+++ b/components/spellcheck/renderer/spellcheck_provider_test.cc
@@ -188,8 +188,14 @@ void TestingSpellCheckProvider::FillSuggestionList(const std::u16string&,
#if BUILDFLAG(IS_WIN)
void TestingSpellCheckProvider::InitializeDictionaries(
InitializeDictionariesCallback callback) {
- std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
- /*enable=*/false);
+ if (base::FeatureList::IsEnabled(
+ spellcheck::kWinDelaySpellcheckServiceInit)) {
+ std::move(callback).Run(/*dictionaries=*/{}, /*custom_words=*/{},
+ /*enable=*/false);
+ return;
+ }
+
+ NOTREACHED();
}
#endif // BUILDFLAG(IS_WIN)
#endif // BUILDFLAG(USE_BROWSER_SPELLCHECKER)
diff --git a/components/spellcheck/renderer/spellcheck_provider_unittest.cc b/components/spellcheck/renderer/spellcheck_provider_unittest.cc
index 487cdb1f871868a7fed8ae2ba1adc816c581d0e2..6e1da0564754063e78ad69997be71bbc95e27b39 100644
--- a/components/spellcheck/renderer/spellcheck_provider_unittest.cc
+++ b/components/spellcheck/renderer/spellcheck_provider_unittest.cc
@@ -65,12 +65,34 @@ class HybridSpellCheckTest
HybridSpellCheckTest() : provider_(&embedder_provider_) {}
~HybridSpellCheckTest() override = default;
+ void SetUp() override {
+ // Don't delay initialization of the SpellcheckService on browser launch.
+ feature_list_.InitAndDisableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ }
+
+ void RunShouldUseBrowserSpellCheckOnlyWhenNeededTest();
+
protected:
+ base::test::ScopedFeatureList feature_list_;
base::test::SingleThreadTaskEnvironment task_environment_;
spellcheck::EmptyLocalInterfaceProvider embedder_provider_;
TestingSpellCheckProvider provider_;
};
+// Test fixture for testing hybrid check cases with delayed initialization of
+// the spellcheck service.
+class HybridSpellCheckTestDelayInit : public HybridSpellCheckTest {
+ public:
+ HybridSpellCheckTestDelayInit() = default;
+
+ void SetUp() override {
+ // Don't initialize the SpellcheckService on browser launch.
+ feature_list_.InitAndEnableFeature(
+ spellcheck::kWinDelaySpellcheckServiceInit);
+ }
+};
+
// Test fixture for testing combining results from both the native spell checker
// and Hunspell.
class CombineSpellCheckResultsTest
@@ -173,6 +195,10 @@ INSTANTIATE_TEST_SUITE_P(
testing::ValuesIn(kSpellCheckProviderHybridTestsParams));
TEST_P(HybridSpellCheckTest, ShouldUseBrowserSpellCheckOnlyWhenNeeded) {
+ RunShouldUseBrowserSpellCheckOnlyWhenNeededTest();
+}
+
+void HybridSpellCheckTest::RunShouldUseBrowserSpellCheckOnlyWhenNeededTest() {
const auto& test_case = GetParam();
FakeTextCheckingResult completion;
@@ -191,6 +217,20 @@ TEST_P(HybridSpellCheckTest, ShouldUseBrowserSpellCheckOnlyWhenNeeded) {
EXPECT_EQ(completion.cancellation_count_, 0U);
}
+// Tests that the SpellCheckProvider calls into the native spell checker only
+// when needed when the code path through
+// SpellCheckProvider::RequestTextChecking is that used when the spellcheck
+// service is initialized on demand.
+INSTANTIATE_TEST_SUITE_P(
+ SpellCheckProviderHybridTests,
+ HybridSpellCheckTestDelayInit,
+ testing::ValuesIn(kSpellCheckProviderHybridTestsParams));
+
+TEST_P(HybridSpellCheckTestDelayInit,
+ ShouldUseBrowserSpellCheckOnlyWhenNeeded) {
+ RunShouldUseBrowserSpellCheckOnlyWhenNeededTest();
+}
+
// Tests that the SpellCheckProvider can correctly combine results from the
// native spell checker and Hunspell.
INSTANTIATE_TEST_SUITE_P(

1
shell/browser/api/electron_api_session.cc
View File

@@ -586,6 +586,7 @@ Session::Session(v8::Isolate* isolate, ElectronBrowserContext* browser_context)
if (auto* service =
SpellcheckServiceFactory::GetForContext(browser_context)) {
service->SetHunspellObserver(this);
service->InitializeDictionaries(base::DoNothing());
}
#endif
}

4
shell/browser/feature_list.cc
View File

@@ -64,10 +64,6 @@ void InitializeFeatureList() {
std::string(",") + network::features::kLocalNetworkAccessChecks.name;
#if BUILDFLAG(IS_WIN)
disable_features +=
// Delayed spellcheck initialization is causing the
// 'custom dictionary word list API' spec to crash.
std::string(",") + spellcheck::kWinDelaySpellcheckServiceInit.name;
// Refs https://issues.chromium.org/issues/401996981
// TODO(deepak1556): Remove this once test added in
// https://github.com/electron/electron/pull/12904