Use Windows spell checker unconditionally. [chromium/src : main]
David Bienvenu (Gerrit)
Attention is currently required from: Guillaume Jenkins.
David Bienvenu would like Guillaume Jenkins to review this change.
Use Windows spell checker unconditionally.
Now that Chrome no longer supports Win7, Chrome on Windows can use
the Windows spell checker unconditionally. This CL removes the kWinUseBrowserSpellChecker feature, which launched a while ago, and
makes UseBrowserSpellChecker() always return true on Windows.
In a follow-on CL, I'll remove the calls to UseBrowserSpellChecker in
Windows-only code.
Change-Id: Ib03164cd9335267cfe8c7c637afbe7e9ea28e07a
---
M chrome/browser/extensions/api/language_settings_private/language_settings_private_api_unittest.cc
M chrome/browser/extensions/api/language_settings_private/language_settings_private_delegate_unittest.cc
M chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
M chrome/browser/site_isolation/spellcheck_per_process_browsertest.cc
M chrome/browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc
M chrome/browser/spellchecker/spellcheck_language_policy_handlers_unittest.cc
M chrome/browser/spellchecker/spellcheck_service_browsertest.cc
M chrome/browser/spellchecker/spellcheck_service_unittest.cc
M components/spellcheck/browser/windows_spell_checker_unittest.cc
M components/spellcheck/common/spellcheck_features.cc
M components/spellcheck/common/spellcheck_features.h
M components/spellcheck/renderer/spellcheck_provider_hunspell_unittest.cc
M components/spellcheck/renderer/spellcheck_provider_unittest.cc
13 files changed, 146 insertions(+), 191 deletions(-)
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
David Bienvenu (Gerrit)
Attention is currently required from: Guillaume Jenkins.
1 comment:
Patchset:
PTAL
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
Guillaume Jenkins (Gerrit)
Attention is currently required from: David Bienvenu, Francois Pierre Doray.
David Bienvenu has uploaded this change for review.
Use Windows spell checker unconditionally.
Now that Chrome no longer supports Win7, Chrome on Windows can use
the Windows spell checker unconditionally. This CL removes the kWinUseBrowserSpellChecker feature, which launched a while ago, and
makes UseBrowserSpellChecker() always return true on Windows.
In a follow-on CL, I'll remove the calls to UseBrowserSpellChecker in
Windows-only code.
Change-Id: Ib03164cd9335267cfe8c7c637afbe7e9ea28e07a
---
M chrome/browser/extensions/api/language_settings_private/language_settings_private_api_unittest.cc
M chrome/browser/extensions/api/language_settings_private/language_settings_private_delegate_unittest.cc
M chrome/browser/renderer_context_menu/spelling_menu_observer_browsertest.cc
M chrome/browser/site_isolation/spellcheck_per_process_browsertest.cc
M chrome/browser/spellchecker/spell_check_host_chrome_impl_win_browsertest.cc
M chrome/browser/spellchecker/spellcheck_language_policy_handlers_unittest.cc
M chrome/browser/spellchecker/spellcheck_service_browsertest.cc
M chrome/browser/spellchecker/spellcheck_service_unittest.cc
M components/spellcheck/browser/windows_spell_checker_unittest.cc
M components/spellcheck/common/spellcheck_features.cc
M components/spellcheck/common/spellcheck_features.h
M components/spellcheck/renderer/spellcheck_provider_hunspell_unittest.cc
M components/spellcheck/renderer/spellcheck_provider_unittest.cc
13 files changed, 146 insertions(+), 191 deletions(-)
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
Guillaume Jenkins (Gerrit)
Attention is currently required from: David Bienvenu, Francois Pierre Doray.
1 comment:
Patchset:
Thanks for doing this! @fdo...@chromium.org already sent a CL this week for the same thing:
https://chromium-review.googlesource.com/c/chromium/src/+/4188827
Not sure how we want to proceed?
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
David Bienvenu (Gerrit)
Attention is currently required from: Francois Pierre Doray, Guillaume Jenkins.
1 comment:
Patchset:
Thanks for doing this! @fdo...@chromium.org already sent a CL this week for the same thing: […]
Ha, well, I'd defer to fdoray's CL, since I assume you want the Hunspell dictionary tests on Windows, which my CL defers on.
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
Francois Pierre Doray (Gerrit)
Attention is currently required from: David Bienvenu, Guillaume Jenkins.
1 comment:
File chrome/browser/extensions/api/language_settings_private/language_settings_private_delegate_unittest.cc:
Patch Set #11, Line 98:
#if BUILDFLAG(USE_RENDERER_SPELLCHECKER)The CL I wrote uses a boolean to disable the browser spell checker when running these tests on Windows [1]. gujen@ mentioned that the non-browser path (Hunspell) is still needed when Windows language packs are missing [2]. Is this correct? If so, should we keep testing these code paths?
[1] https://chromium-review.googlesource.com/c/chromium/src/+/4188827/5/chrome/browser/extensions/api/language_settings_private/language_settings_private_delegate_unittest.cc
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=1392145#c2
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
David Bienvenu (Gerrit)
Attention is currently required from: Francois Pierre Doray, Guillaume Jenkins.
1 comment:
File chrome/browser/extensions/api/language_settings_private/language_settings_private_delegate_unittest.cc:
Patch Set #11, Line 98:
#if BUILDFLAG(USE_RENDERER_SPELLCHECKER)
The CL I wrote uses a boolean to disable the browser spell checker when running these tests on Windo […]
My understanding from a comment I ran across in the code is that Hunspell supports languages that Windows does not, or at least it did at some point. I have no idea if it's true, or if for some reason language packs might not be installed. It might be interesting to emit a metric when we do fallback to Hunspell on Windows, along with the locale that required the fallback.
But, for now, I think we should keep testing these code paths.
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
David Bienvenu (Gerrit)
David Bienvenu abandoned this change.
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.
Guillaume Jenkins (Gerrit)
1 comment:
File chrome/browser/extensions/api/language_settings_private/language_settings_private_delegate_unittest.cc:
Patch Set #11, Line 98:
#if BUILDFLAG(USE_RENDERER_SPELLCHECKER)
My understanding from a comment I ran across in the code is that Hunspell supports languages that Wi […]
Yes, Chrome first tries to use the native spell checker, and if the required language packs are not installed, will use Hunspell instead. This choice happens on a per-language basis in multi-language spell checking context, meaning e.g. Chrome can use the native spell checker for English, but Hunspell for French (called "hybrid" spell checking in the code)
The following metrics indicate how many spell check requests are fully handled by native, by Hunspell, or by both (well, they measure the duration, but you can use the unique client count to get an idea of how often Hunspell is needed):
https://uma.googleplex.com/p/chrome/timeline_v2?sid=9cabd4331b2ae05917c3d71d31923c9d
To view, visit change 4205939. To unsubscribe, or for help writing mail filters, visit settings.