Windows Spellcheck: Method to retrieve tags for dictionaries on system

A new method is added to asynchronously retrieve the BCP47 language
tags for registered Windows OS spellcheckers on the system, wrapping
the system API ISpellCheckerFactory::get_SupportedLanguages. The method
is for now only used in a unit test, but upcoming work will rely on
this method to allow languages without Hunspell support to still have
 Windows spellchecking enabled on the languages settings page.

Bug: 1000443
Change-Id: I03507650ec355e8c282695d8f25364bccb91449a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2144234Reviewed-by: Guillaume Jenkins <gujen@google.com>
Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Bruce Long <brlong@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#759306}

components/spellcheck/browser/spellcheck_platform.h

@@ -38,11 +38,26 @@ typedef base::OnceCallback<void(const spellcheck::PerLanguageSuggestions&)>
     GetSuggestionsCallback;
 #endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
 
+typedef base::OnceCallback<void(const std::vector<std::string>& /* results */)>
+    RetrieveSpellcheckLanguagesCompleteCallback;
+
 // Get the languages supported by the platform spellchecker and store them in
 // |spellcheck_languages|. Note that they must be converted to
 // Chromium style codes (en-US not en_US). See spellchecker.cc for a full list.
 void GetAvailableLanguages(std::vector<std::string>* spellcheck_languages);
 
+// Retrieve BCP47 language tags for registered platform spellcheckers
+// on the system. Callback will pass an empty vector of language tags if the OS
+// does not support spellcheck or this functionality is not yet implemented.
+void RetrieveSpellcheckLanguages(
+    PlatformSpellChecker* spell_checker_instance,
+    RetrieveSpellcheckLanguagesCompleteCallback callback);
+
+// Test-only method for adding fake list of platform spellcheck languages.
+void AddSpellcheckLanguagesForTesting(
+    PlatformSpellChecker* spell_checker_instance,
+    const std::vector<std::string>& languages);
+
 // Returns the language used for spellchecking on the platform.
 std::string GetSpellCheckerLanguage();
 

components/spellcheck/browser/spellcheck_platform_android.cc

@@ -6,6 +6,7 @@
 
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/logging.h"
 #include "components/spellcheck/common/spellcheck_features.h"
 
 class PlatformSpellChecker;
@@ -15,6 +16,19 @@ namespace spellcheck_platform {
 void GetAvailableLanguages(std::vector<std::string>* spellcheck_languages) {
 }
 
+void RetrieveSpellcheckLanguages(
+    PlatformSpellChecker* spell_checker_instance,
+    RetrieveSpellcheckLanguagesCompleteCallback callback) {
+  NOTIMPLEMENTED();
+  std::move(callback).Run(std::vector<std::string>());
+}
+
+void AddSpellcheckLanguagesForTesting(
+    PlatformSpellChecker* spell_checker_instance,
+    const std::vector<std::string>& languages) {
+  NOTIMPLEMENTED();
+}
+
 std::string GetSpellCheckerLanguage() {
   return std::string();
 }

components/spellcheck/browser/spellcheck_platform_mac.mm

@@ -109,6 +109,19 @@ void GetAvailableLanguages(std::vector<std::string>* spellcheck_languages) {
   }
 }
 
+void RetrieveSpellcheckLanguages(
+    PlatformSpellChecker* spell_checker_instance,
+    RetrieveSpellcheckLanguagesCompleteCallback callback) {
+  NOTIMPLEMENTED();
+  std::move(callback).Run(std::vector<std::string>());
+}
+
+void AddSpellcheckLanguagesForTesting(
+    PlatformSpellChecker* spell_checker_instance,
+    const std::vector<std::string>& languages) {
+  NOTIMPLEMENTED();
+}
+
 std::string GetSpellCheckerLanguage() {
   return ConvertLanguageCodeFromMac([SharedSpellChecker() language]);
 }

components/spellcheck/browser/spellcheck_platform_win.cc

@@ -92,6 +92,20 @@ void GetAvailableLanguages(std::vector<std::string>* spellcheck_languages) {
   // Not used in Windows
 }
 
+void RetrieveSpellcheckLanguages(
+    PlatformSpellChecker* spell_checker_instance,
+    RetrieveSpellcheckLanguagesCompleteCallback callback) {
+  reinterpret_cast<WindowsSpellChecker*>(spell_checker_instance)
+      ->RetrieveSpellcheckLanguages(std::move(callback));
+}
+
+void AddSpellcheckLanguagesForTesting(
+    PlatformSpellChecker* spell_checker_instance,
+    const std::vector<std::string>& languages) {
+  reinterpret_cast<WindowsSpellChecker*>(spell_checker_instance)
+      ->AddSpellcheckLanguagesForTesting(languages);
+}
+
 int GetDocumentTag() {
   return 1;  // Not used in Windows
 }

components/spellcheck/browser/windows_spell_checker.h

@@ -22,12 +22,16 @@ namespace base {
 class SingleThreadTaskRunner;
 }
 
+namespace windows_spell_checker {
+class BackgroundHelper;
+}
+
 // Class used to store all the COM objects and control their lifetime. The class
 // also provides wrappers for ISpellCheckerFactory and ISpellChecker APIs. All
 // COM calls are executed on the background thread.
 class WindowsSpellChecker : public PlatformSpellChecker {
  public:
-  WindowsSpellChecker(
+  explicit WindowsSpellChecker(
       scoped_refptr<base::SingleThreadTaskRunner> background_task_runner);
 
   WindowsSpellChecker(const WindowsSpellChecker&) = delete;
@@ -66,113 +70,26 @@ class WindowsSpellChecker : public PlatformSpellChecker {
   void IsLanguageSupported(const std::string& lang_tag,
                            base::OnceCallback<void(bool)> callback);
 
- private:
-  // Private inner class that handles calls to the native Windows APIs. All
-  // invocations of these methods must be posted to the same COM
-  // |SingleThreadTaskRunner|. This is enforced by checks that all methods run
-  // on the given |SingleThreadTaskRunner|.
-  class BackgroundHelper {
-   public:
-    BackgroundHelper(
-        scoped_refptr<base::SingleThreadTaskRunner> background_task_runner);
-    ~BackgroundHelper();
-
-    // Creates the native spell check factory, which is the main entry point to
-    // the native spell checking APIs.
-    void CreateSpellCheckerFactory();
-
-    // Creates a native |ISpellchecker| for the given language |lang_tag| and
-    // returns a boolean indicating success.
-    bool CreateSpellChecker(const std::string& lang_tag);
-
-    // Removes the native spell checker for the given language |lang_tag| from
-    // the map of active spell checkers.
-    void DisableSpellChecker(const std::string& lang_tag);
-
-    // Requests spell checking of string |text| for all active spell checkers
-    // (all languages) and returns a vector of |SpellCheckResult| containing the
-    // results.
-    std::vector<SpellCheckResult> RequestTextCheckForAllLanguages(
-        int document_tag,
-        const base::string16& text);
-
-    // Gets spelling suggestions for |word| from all active spell checkers (all
-    // languages), keeping the suggestions separate per language, and returns
-    // the results in a vector of vector of strings.
-    spellcheck::PerLanguageSuggestions GetPerLanguageSuggestions(
-        const base::string16& word);
-
-    // Fills the given vector |optional_suggestions| with a number (up to
-    // kMaxSuggestions) of suggestions for the string |wrong_word| using the
-    // native spell checker for language |lang_tag|.
-    void FillSuggestionList(const std::string& lang_tag,
-                            const base::string16& wrong_word,
-                            std::vector<base::string16>* optional_suggestions);
-
-    // Adds |word| to the native dictionary of all active spell checkers (all
-    // languages).
-    void AddWordForAllLanguages(const base::string16& word);
-
-    // Removes |word| from the native dictionary of all active spell checkers
-    // (all languages). This requires a newer version of the native spell
-    // check APIs, so it may be a no-op on older Windows versions.
-    void RemoveWordForAllLanguages(const base::string16& word);
-
-    // Adds |word| to the ignore list of all active spell checkers (all
-    // languages).
-    void IgnoreWordForAllLanguages(const base::string16& word);
-
-    // Returns |true| if a native spell checker is available for the given
-    // language |lang_tag|. This is based on the installed language packs in the
-    // OS settings.
-    bool IsLanguageSupported(const std::string& lang_tag);
-
-    // Returns |true| if an |ISpellCheckerFactory| has been initialized.
-    bool IsSpellCheckerFactoryInitialized();
-
-    // Returns |true| if an |ISpellChecker| has been initialized for the given
-    // language |lang_tag|.
-    bool SpellCheckerReady(const std::string& lang_tag);
-
-    // Returns the |ISpellChecker| pointer for the given language |lang_tag|.
-    Microsoft::WRL::ComPtr<ISpellChecker> GetSpellChecker(
-        const std::string& lang_tag);
-
-    // Records metrics about spell check support for the user's Chrome locales.
-    void RecordChromeLocalesStats(const std::vector<std::string> chrome_locales,
-                                  SpellCheckHostMetrics* metrics);
-
-    // Records metrics about spell check support for the user's enabled spell
-    // check locales.
-    void RecordSpellcheckLocalesStats(
-        const std::vector<std::string> spellcheck_locales,
-        SpellCheckHostMetrics* metrics);
-
-    // Sorts the given locales into four buckets based on spell check support
-    // (both native and Hunspell, Hunspell only, native only, none).
-    LocalesSupportInfo DetermineLocalesSupport(
-        const std::vector<std::string>& locales);
-
-   private:
-    // The native factory to interact with spell check APIs.
-    Microsoft::WRL::ComPtr<ISpellCheckerFactory> spell_checker_factory_;
-
-    // The map of active spell checkers. Each entry maps a language tag to an
-    // |ISpellChecker| (there is one |ISpellChecker| per language).
-    std::map<std::string, Microsoft::WRL::ComPtr<ISpellChecker>>
-        spell_checker_map_;
-
-    // Task runner only used to enforce valid sequencing.
-    scoped_refptr<base::SingleThreadTaskRunner> background_task_runner_;
-  };  // class BackgroundHelper
+  // Asynchronously retrieve language tags for registered Windows OS
+  // spellcheckers on the system. Callback will pass an empty vector of language
+  // tags if the OS does not support spellcheck.
+  void RetrieveSpellcheckLanguages(
+      spellcheck_platform::RetrieveSpellcheckLanguagesCompleteCallback
+          callback);
 
+  // Test-only method for adding fake list of Windows spellcheck languages.
+  void AddSpellcheckLanguagesForTesting(
+      const std::vector<std::string>& languages);
+
+ private:
   // COM-enabled, single-thread task runner used to post invocations of
   // BackgroundHelper methods to interact with spell check native APIs.
   scoped_refptr<base::SingleThreadTaskRunner> background_task_runner_;
 
   // Instance of the background helper to invoke native APIs on the COM-enabled
-  // background thread.
-  std::unique_ptr<BackgroundHelper> background_helper_;
+  // background thread. |background_helper_| is deleted on the background thread
+  // after all other background tasks complete.
+  std::unique_ptr<windows_spell_checker::BackgroundHelper> background_helper_;
 };
 
 #endif  // COMPONENTS_SPELLCHECK_BROWSER_WINDOWS_SPELL_CHECKER_H_

components/spellcheck/browser/windows_spell_checker_unittest.cc

@@ -7,6 +7,7 @@
 #include <stddef.h>
 
 #include "base/bind.h"
+#include "base/logging.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
@@ -30,20 +31,29 @@ namespace {
 class WindowsSpellCheckerTest : public testing::Test {
  public:
   WindowsSpellCheckerTest() {
-    if (spellcheck::WindowsVersionSupportsSpellchecker()) {
-      feature_list_.InitAndEnableFeature(
-          spellcheck::kWinUseBrowserSpellChecker);
+#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
+    // Force hybrid spellchecking to be enabled.
+    feature_list_.InitWithFeatures(
+        /*enabled_features=*/{spellcheck::kWinUseBrowserSpellChecker,
+                              spellcheck::kWinUseHybridSpellChecker},
+        /*disabled_features=*/{});
+#else
+    feature_list_.InitAndEnableFeature(spellcheck::kWinUseBrowserSpellChecker);
+#endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
 
-      win_spell_checker_ = std::make_unique<WindowsSpellChecker>(
-          base::ThreadPool::CreateCOMSTATaskRunner({base::MayBlock()}));
+    // The WindowsSpellchecker object can be created even on Windows versions
+    // that don't support platform spellchecking. However, the spellcheck
+    // factory won't be instantiated and the result returned in the
+    // CreateSpellChecker callback will be false.
+    win_spell_checker_ = std::make_unique<WindowsSpellChecker>(
+        base::ThreadPool::CreateCOMSTATaskRunner({base::MayBlock()}));
 
-      win_spell_checker_->CreateSpellChecker(
-          "en-US", base::BindOnce(
-                       &WindowsSpellCheckerTest::SetLanguageCompletionCallback,
+    win_spell_checker_->CreateSpellChecker(
+        "en-US",
+        base::BindOnce(&WindowsSpellCheckerTest::SetLanguageCompletionCallback,
                        base::Unretained(this)));
 
-      RunUntilResultReceived();
-    }
+    RunUntilResultReceived();
   }
 
   void RunUntilResultReceived() {
@@ -82,6 +92,17 @@ class WindowsSpellCheckerTest : public testing::Test {
   }
 #endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
 
+  void RetrieveSpellcheckLanguagesCompletionCallback(
+      const std::vector<std::string>& spellcheck_languages) {
+    callback_finished_ = true;
+    spellcheck_languages_ = spellcheck_languages;
+    DVLOG(2) << "RetrieveSpellcheckLanguagesCompletionCallback: Dictionary "
+                "found for following language tags: "
+             << base::JoinString(spellcheck_languages_, ", ");
+    if (quit_)
+      std::move(quit_).Run();
+  }
+
  protected:
   std::unique_ptr<WindowsSpellChecker> win_spell_checker_;
 
@@ -94,17 +115,15 @@ class WindowsSpellCheckerTest : public testing::Test {
 #if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
   spellcheck::PerLanguageSuggestions per_language_suggestions_;
 #endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
+  std::vector<std::string> spellcheck_languages_;
 
   base::test::TaskEnvironment task_environment_{
       base::test::TaskEnvironment::MainThreadType::UI};
 };
 
 TEST_F(WindowsSpellCheckerTest, RequestTextCheck) {
-  if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
-    return;
-  }
-
-  ASSERT_TRUE(set_language_result_);
+  ASSERT_EQ(set_language_result_,
+            spellcheck::WindowsVersionSupportsSpellchecker());
 
   static const struct {
     const char* text_to_check;
@@ -131,6 +150,13 @@ TEST_F(WindowsSpellCheckerTest, RequestTextCheck) {
                        base::Unretained(this)));
     RunUntilResultReceived();
 
+    if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
+      // On Windows versions that don't support platform spellchecking, the
+      // returned vector of results should be empty.
+      ASSERT_TRUE(spell_check_results_.empty());
+      continue;
+    }
+
     ASSERT_EQ(1u, spell_check_results_.size())
         << "RequestTextCheckTests case " << i << ": Wrong number of results";
 
@@ -149,13 +175,52 @@ TEST_F(WindowsSpellCheckerTest, RequestTextCheck) {
   }
 }
 
-#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
-TEST_F(WindowsSpellCheckerTest, GetPerLanguageSuggestions) {
+TEST_F(WindowsSpellCheckerTest, RetrieveSpellcheckLanguages) {
+  // Test retrieval of real dictionary on system (useful for debug logging
+  // other registered dictionaries).
+  win_spell_checker_->RetrieveSpellcheckLanguages(base::BindOnce(
+      &WindowsSpellCheckerTest::RetrieveSpellcheckLanguagesCompletionCallback,
+      base::Unretained(this)));
+
+  RunUntilResultReceived();
+
   if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
+    // On Windows versions that don't support platform spellchecking, the
+    // returned vector of results should be empty.
+    ASSERT_TRUE(spellcheck_languages_.empty());
     return;
   }
 
-  ASSERT_TRUE(set_language_result_);
+  ASSERT_LE(1u, spellcheck_languages_.size());
+  ASSERT_TRUE(base::Contains(spellcheck_languages_, "en-US"));
+}
+
+TEST_F(WindowsSpellCheckerTest, RetrieveSpellcheckLanguagesFakeDictionaries) {
+  // Test retrieval of fake dictionaries added using
+  // AddSpellcheckLanguagesForTesting. If fake dictionaries are used,
+  // instantiation of the spellchecker factory is not required for
+  // RetrieveSpellcheckLanguages, so the test should pass even on Windows
+  // versions that don't support platform spellchecking.
+  std::vector<std::string> spellcheck_languages_for_testing = {
+      "ar-SA", "es-419", "fr-CA"};
+  win_spell_checker_->AddSpellcheckLanguagesForTesting(
+      spellcheck_languages_for_testing);
+
+  DVLOG(2) << "Calling RetrieveSpellcheckLanguages after fake dictionaries "
+              "added using AddSpellcheckLanguagesForTesting...";
+  win_spell_checker_->RetrieveSpellcheckLanguages(base::BindOnce(
+      &WindowsSpellCheckerTest::RetrieveSpellcheckLanguagesCompletionCallback,
+      base::Unretained(this)));
+
+  RunUntilResultReceived();
+
+  ASSERT_EQ(spellcheck_languages_for_testing, spellcheck_languages_);
+}
+
+#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
+TEST_F(WindowsSpellCheckerTest, GetPerLanguageSuggestions) {
+  ASSERT_EQ(set_language_result_,
+            spellcheck::WindowsVersionSupportsSpellchecker());
 
   win_spell_checker_->GetPerLanguageSuggestions(
       base::ASCIIToUTF16("tihs"),
@@ -164,6 +229,13 @@ TEST_F(WindowsSpellCheckerTest, GetPerLanguageSuggestions) {
           base::Unretained(this)));
   RunUntilResultReceived();
 
+  if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
+    // On Windows versions that don't support platform spellchecking, the
+    // returned vector of results should be empty.
+    ASSERT_TRUE(per_language_suggestions_.empty());
+    return;
+  }
+
   ASSERT_EQ(per_language_suggestions_.size(), 1u);
   ASSERT_GT(per_language_suggestions_[0].size(), 0u);
 }