[Spellcheck] Split WindowsSpellChecker tests

Now that the WindowsSpellChecker class has been reorganized to not rely
on a base::NoDestructor life cycle, it is now safe to re-create an
instance of it every test. The WindowsSpellChecker unit tests were
working around the base::NoDestructor by having a single TEST_F that
tested all methods. This CL splits that test up into multiple tests.
There are no functional changes to the tests.

#Squeegee

Bug: 1035044
Change-Id: I180f8fa6fedec4470a8f8e77c124bf321ba62e1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2057245Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Guillaume Jenkins <gujen@google.com>
Cr-Commit-Position: refs/heads/master@{#742198}

components/spellcheck/browser/windows_spell_checker_unittest.cc

@@ -7,7 +7,6 @@
 #include <stddef.h>
 
 #include "base/bind.h"
-#include "base/no_destructor.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
@@ -32,9 +31,19 @@ class WindowsSpellCheckerTest : public testing::Test {
  public:
   WindowsSpellCheckerTest() {
     if (spellcheck::WindowsVersionSupportsSpellchecker()) {
+      feature_list_.InitAndEnableFeature(
+          spellcheck::kWinUseBrowserSpellChecker);
+
       win_spell_checker_ = std::make_unique<WindowsSpellChecker>(
           base::ThreadTaskRunnerHandle::Get(),
           base::CreateCOMSTATaskRunner({base::ThreadPool(), base::MayBlock()}));
+
+      win_spell_checker_->CreateSpellChecker(
+          "en-US", base::BindOnce(
+                       &WindowsSpellCheckerTest::SetLanguageCompletionCallback,
+                       base::Unretained(this)));
+
+      RunUntilResultReceived();
     }
   }
 
@@ -89,9 +98,33 @@ class WindowsSpellCheckerTest : public testing::Test {
   }
 #endif  // BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK
 
-  // TODO(gujen) Make these methods actual tests. See the task_environment_
-  // comment below.
-  void RequestTextCheckTests() {
+ protected:
+  std::unique_ptr<WindowsSpellChecker> win_spell_checker_;
+
+  bool callback_finished_ = false;
+  base::OnceClosure quit_;
+  base::test::ScopedFeatureList feature_list_;
+
+  bool set_language_result_;
+  std::vector<SpellCheckResult> spell_check_results_;
+#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
+  spellcheck::PerLanguageSuggestions per_language_suggestions_;
+#endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
+#if BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK)
+  std::vector<std::string> preferred_languages_;
+#endif  // BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK
+
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::MainThreadType::UI};
+};
+
+TEST_F(WindowsSpellCheckerTest, RequestTextCheck) {
+  if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
+    return;
+  }
+
+  ASSERT_TRUE(set_language_result_);
+
   static const struct {
     const char* text_to_check;
     const char* expected_suggestion;
@@ -130,14 +163,19 @@ class WindowsSpellCheckerTest : public testing::Test {
                        return suggestion.compare(suggested_word) == 0;
                      });
 
-      ASSERT_NE(suggestions.end(), position)
-          << "RequestTextCheckTests case " << i
+    ASSERT_NE(suggestions.end(), position) << "RequestTextCheckTests case " << i
                                            << ": Expected suggestion not found";
   }
-  }
+}
 
 #if BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK)
-  void GetSupportedWindowsPreferredLanguagesTests() {
+TEST_F(WindowsSpellCheckerTest, GetSupportedWindowsPreferredLanguages) {
+  if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
+    return;
+  }
+
+  ASSERT_TRUE(set_language_result_);
+
   win_spell_checker_->GetSupportedWindowsPreferredLanguages(base::BindOnce(
       &WindowsSpellCheckerTest::GetSupportedWindowsPreferredLanguagesCallback,
       base::Unretained(this)));
@@ -146,13 +184,19 @@ class WindowsSpellCheckerTest : public testing::Test {
 
   ASSERT_LE(1u, preferred_languages_.size());
   ASSERT_NE(preferred_languages_.end(),
-              std::find(preferred_languages_.begin(),
-                        preferred_languages_.end(), "en-US"));
-  }
+            std::find(preferred_languages_.begin(), preferred_languages_.end(),
+                      "en-US"));
+}
 #endif  // BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK
 
 #if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
-  void PerLanguageSuggestionsTests() {
+TEST_F(WindowsSpellCheckerTest, GetPerLanguageSuggestions) {
+  if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
+    return;
+  }
+
+  ASSERT_TRUE(set_language_result_);
+
   win_spell_checker_->GetPerLanguageSuggestions(
       base::ASCIIToUTF16("tihs"),
       base::BindOnce(
@@ -162,65 +206,7 @@ class WindowsSpellCheckerTest : public testing::Test {
 
   ASSERT_EQ(per_language_suggestions_.size(), 1u);
   ASSERT_GT(per_language_suggestions_[0].size(), 0u);
-  }
-#endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
-
- protected:
-  std::unique_ptr<WindowsSpellChecker> win_spell_checker_;
-
-  bool callback_finished_ = false;
-  base::OnceClosure quit_;
-
-  bool set_language_result_;
-  std::vector<SpellCheckResult> spell_check_results_;
-#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
-  spellcheck::PerLanguageSuggestions per_language_suggestions_;
-#endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
-#if BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK)
-  std::vector<std::string> preferred_languages_;
-#endif  // BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK
-
-  // TODO(gujen) (crbug.com/1035044) The WindowsSpellChecker class is
-  // instantiated with static storage using base::NoDestructor (see
-  // GetWindowsSpellChecker) and creates its own task runner. The thread pool is
-  // destroyed together with TaskEnvironment when the test fixture object is
-  // destroyed. Therefore without some elaborate test-only code added to the
-  // WindowsSpellChecker class or a means to keep the TaskEnvironment alive
-  // (which would set off leak detection), easiest approach for now is to add
-  // all test coverage for Windows spellchecking to a single test.
-  base::test::TaskEnvironment task_environment_{
-      base::test::TaskEnvironment::MainThreadType::UI};
-};
-
-// TODO(gujen) Split this test into multiple tests instead of individual
-//             methods. See the task_environment_ comment in the
-//             WindowsSpellCheckerTest class.
-TEST_F(WindowsSpellCheckerTest, SpellCheckAsyncMethods) {
-  if (!spellcheck::WindowsVersionSupportsSpellchecker()) {
-    return;
-  }
-
-  base::test::ScopedFeatureList feature_list;
-  feature_list.InitAndEnableFeature(spellcheck::kWinUseBrowserSpellChecker);
-
-  win_spell_checker_->CreateSpellChecker(
-      "en-US",
-      base::BindOnce(&WindowsSpellCheckerTest::SetLanguageCompletionCallback,
-                     base::Unretained(this)));
-
-  RunUntilResultReceived();
-
-  ASSERT_TRUE(set_language_result_);
-
-  RequestTextCheckTests();
-
-#if BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK)
-  GetSupportedWindowsPreferredLanguagesTests();
-#endif  // BUILDFLAG(USE_WINDOWS_PREFERRED_LANGUAGES_FOR_SPELLCHECK
-
-#if BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
-  PerLanguageSuggestionsTests();
-#endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
 }
+#endif  // BUILDFLAG(USE_WIN_HYBRID_SPELLCHECKER)
 
 }  // namespace