From ab0ff5dffc167ac3a35ea207fa5b6901f6b172db Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sat, 8 Nov 2025 17:26:06 +0100 Subject: [PATCH] refactor: remove `spellcheck::kWinDelaySpellcheckServiceInit` patch (#48843) refactor: remove spellcheck::kWinDelaySpellcheckServiceInit patch --- patches/chromium/.patches | 1 - ...eature_windelayspellcheckserviceinit.patch | 765 ------------------ shell/browser/api/electron_api_session.cc | 1 + shell/browser/feature_list.cc | 4 - 4 files changed, 1 insertion(+), 770 deletions(-) delete mode 100644 patches/chromium/revert_cleanup_remove_feature_windelayspellcheckserviceinit.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 0c203ab91c..5881ab1840 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -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 diff --git a/patches/chromium/revert_cleanup_remove_feature_windelayspellcheckserviceinit.patch b/patches/chromium/revert_cleanup_remove_feature_windelayspellcheckserviceinit.patch deleted file mode 100644 index b71eca6167..0000000000 --- a/patches/chromium/revert_cleanup_remove_feature_windelayspellcheckserviceinit.patch +++ /dev/null @@ -1,765 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Charles Kerr -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& 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& 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(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 dictionaries, -- const std::vector& custom_words, -- bool enable) { -- received_result_ = true; -- if (quit_) { -- std::move(quit_).Run(); -- } -- } -+ virtual void InitializeSpellcheckService() {} - - void OnSpellcheckResult(const std::vector& 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 dictionaries, -+ const std::vector& 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( -+ 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( - 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& spellcheck_languages_for_testing) { - // Fake the presence of Windows spellcheck dictionaries. -@@ -215,6 +219,8 @@ class SpellcheckServiceHybridUnitTestBase - static const std::vector - windows_spellcheck_languages_for_testing_; - -+ base::test::ScopedFeatureList feature_list_; -+ - raw_ptr spellcheck_service_; - }; - -@@ -327,6 +333,18 @@ const std::vector SpellcheckServiceHybridUnitTestBase:: - // dictionaries. - }; - -+class GetDictionariesHybridUnitTestNoDelayInit -+ : public SpellcheckServiceHybridUnitTestBase, -+ public testing::WithParamInterface { -+ 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 { -+ 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& 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( diff --git a/shell/browser/api/electron_api_session.cc b/shell/browser/api/electron_api_session.cc index e96a8962cb..229bd23d74 100644 --- a/shell/browser/api/electron_api_session.cc +++ b/shell/browser/api/electron_api_session.cc @@ -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 } diff --git a/shell/browser/feature_list.cc b/shell/browser/feature_list.cc index 178c101336..e5c5c957f5 100644 --- a/shell/browser/feature_list.cc +++ b/shell/browser/feature_list.cc @@ -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