Enables the user to select multiple languages for spellchecking (UI)

chrome/browser/spellchecker/spellcheck_service_browsertest.cc

Index: chrome/browser/spellchecker/spellcheck_service_browsertest.cc
diff --git a/chrome/browser/spellchecker/spellcheck_service_browsertest.cc b/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
index 7c6eed772fc9386dc9219534dee1912c70344bdc..761c11d81eee3360cdc3bb6c28c36a962c0891a8 100644
--- a/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
+++ b/chrome/browser/spellchecker/spellcheck_service_browsertest.cc
@@ -2,15 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/spellchecker/spellcheck_service.h"
+
+#include <string>
+#include <vector>
+
+#include "base/command_line.h"
 #include "base/path_service.h"
+#include "base/prefs/pref_member.h"

[please use gerrit instead, 2015/06/17 17:47:16]

Why this include?

[Julius, 2015/06/17 20:14:45]

Deleted.

+#include "base/prefs/pref_service.h"
 #include "base/synchronization/waitable_event.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/spellchecker/spellcheck_factory.h"
-#include "chrome/browser/spellchecker/spellcheck_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/spellcheck_common.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/user_prefs/user_prefs.h"
 #include "content/public/test/test_utils.h"
 #include "url/gurl.h"
 
@@ -21,26 +31,80 @@ namespace {
 // A corrupted BDICT data used in DeleteCorruptedBDICT. Please do not use this
 // BDICT data for other tests.
 const uint8 kCorruptedBDICT[] = {

[Julius, 2015/06/17 00:59:53]

clang-format reformatted this.

[please use gerrit instead, 2015/06/17 17:47:15]

Acknowledged.

-  0x42, 0x44, 0x69, 0x63, 0x02, 0x00, 0x01, 0x00,
-  0x20, 0x00, 0x00, 0x00, 0x3b, 0x00, 0x00, 0x00,
-  0x65, 0x72, 0xe0, 0xac, 0x27, 0xc7, 0xda, 0x66,
-  0x6d, 0x1e, 0xa6, 0x35, 0xd1, 0xf6, 0xb7, 0x35,
-  0x32, 0x00, 0x00, 0x00, 0x38, 0x00, 0x00, 0x00,
-  0x39, 0x00, 0x00, 0x00, 0x3a, 0x00, 0x00, 0x00,
-  0x0a, 0x0a, 0x41, 0x46, 0x20, 0x30, 0x00, 0x00,
-  0x00, 0x00, 0x00, 0xe6, 0x49, 0x00, 0x68, 0x02,
-  0x73, 0x06, 0x74, 0x0b, 0x77, 0x11, 0x79, 0x15,
+    0x42, 0x44, 0x69, 0x63, 0x02, 0x00, 0x01, 0x00, 0x20, 0x00, 0x00, 0x00,
+    0x3b, 0x00, 0x00, 0x00, 0x65, 0x72, 0xe0, 0xac, 0x27, 0xc7, 0xda, 0x66,
+    0x6d, 0x1e, 0xa6, 0x35, 0xd1, 0xf6, 0xb7, 0x35, 0x32, 0x00, 0x00, 0x00,
+    0x38, 0x00, 0x00, 0x00, 0x39, 0x00, 0x00, 0x00, 0x3a, 0x00, 0x00, 0x00,
+    0x0a, 0x0a, 0x41, 0x46, 0x20, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe6,
+    0x49, 0x00, 0x68, 0x02, 0x73, 0x06, 0x74, 0x0b, 0x77, 0x11, 0x79, 0x15,
 };
 
 }  // namespace
 
-class SpellcheckServiceBrowserTest : public InProcessBrowserTest {
+struct SpellcheckLanguageTestCase {
+  SpellcheckLanguageTestCase(
+      const std::string& spellcheck_dictionary,
+      const std::string& accept_languages,
+      const unsigned long& expected_enabled_spellcheck_languages,

[please use gerrit instead, 2015/06/17 17:47:15]

size_t, no const, no &.

"const var_type& var_name" (called const-ref) is used to avoid copying large
chunks of memory. It's OK to use "var_type var_name" for pointers, int, long,
size_t, char, etc, because they are cheap (fast) to copy.

[Julius, 2015/06/17 20:14:46]

Done.

+      const std::vector<std::string> expected_spellcheck_languages)
+      : spellcheck_dictionary(spellcheck_dictionary),
+        accept_languages(accept_languages),
+        expected_enabled_spellcheck_languages(
+            expected_enabled_spellcheck_languages),
+        expected_spellcheck_languages(expected_spellcheck_languages) {}
+  ~SpellcheckLanguageTestCase() {}
+
+  const std::string spellcheck_dictionary;
+  const std::string accept_languages;
+  const unsigned long expected_enabled_spellcheck_languages;

[please use gerrit instead, 2015/06/17 17:47:15]

size_t

[Julius, 2015/06/17 20:14:45]

Done.

+  const std::vector<std::string> expected_spellcheck_languages;
+};
+
+class SpellcheckServiceBrowserTest
+    : public InProcessBrowserTest,
+      public testing::WithParamInterface<SpellcheckLanguageTestCase> {
  public:
-  Profile* GetProfile() {
-    return browser()->profile();
-  }
+  Profile* GetProfile() { return browser()->profile(); }

[please use gerrit instead, 2015/06/17 17:47:16]

class SpellcheckServiceBrowserTest
    : public InProcessBrowserTest,
      public testing::WithParamInterface<SpellcheckLanguageTestCase> {
 public:
  SpellcheckServiceBrowserTest() {}
  ~SpellcheckServiceBrowserTest() override {}

  BrowserContext* GetContext() { return
static_cast<BrowserContext*>(browser()->profile()); }

 private:
  DISALLOW_COPY_AND_ASSIGN(SpellcheckServiceBrowserTest);
};

[Julius, 2015/06/17 20:14:46]

Done.

 };
 
+INSTANTIATE_TEST_CASE_P(
+    SpellcheckLanguageTestCases,
+    SpellcheckServiceBrowserTest,
+    testing::Values(
+        SpellcheckLanguageTestCase("en-US",
+                                   "en,en-US",
+                                   1UL,
+                                   std::vector<std::string>{"en-US"}),
+        SpellcheckLanguageTestCase("en-US",
+                                   "en-US,en",
+                                   1UL,
+                                   std::vector<std::string>{"en-US"}),
+        SpellcheckLanguageTestCase("en-US",
+                                   "en,fr,en-US,en-AU",
+                                   1UL,
+                                   std::vector<std::string>{"en-US", "fr",

[please use gerrit instead, 2015/06/17 17:47:15]

This is "(Uniform) Initialization Syntax", which is banned in Chromium at the
moment: https://chromium-cpp.appspot.com/.

Only 9 instances of this syntax can be found in the code, but they are all in
third-party libraries that are not cross platform:

https://code.google.com/p/chromium/codesearch#search/&q=std::vector%3C.*%3E%7...

Your code needs to be cross platform. I recommend that you extend the
SpellcheckLanguageTestCase constructor to take three StringPiece objects:

https://code.google.com/p/chromium/codesearch#chromium/src/base/strings/strin...

By the way, the string_piece.h header comment says that it's preferred to pass
StringPiece by value: "base::StringPiece first" etc. This is because StringPiece
does some C++ magic to avoid copying the actual string around. Pretty neat!

Anyway, once you have the three StringPiece objects in your
SpellcheckLanguageTestCase constructor, do something like this:

if (!first.empty())
  expected_spellcheck_languages.push_back(first);

if (!second.empty())
  expected_spellcheck_languages.push_back(second);

if (!third.empty())
  expected_spellcheck_languages.push_back(third);

[Julius, 2015/06/17 20:14:46]

Done.

+                                                            "en-AU"}),
+        SpellcheckLanguageTestCase("fr",
+                                   "en,en-JP,fr,zz,en-US",
+                                   1UL,
+                                   std::vector<std::string>{"fr", "en-US"})));
+
+IN_PROC_BROWSER_TEST_P(SpellcheckServiceBrowserTest, GetSpellcheckLanguages) {
+  BrowserContext* context = static_cast<BrowserContext*>(GetProfile());
+  PrefService* prefs = user_prefs::UserPrefs::Get(context);
+  prefs->SetString(prefs::kSpellCheckDictionary,
+                   GetParam().spellcheck_dictionary);
+  prefs->SetString(prefs::kAcceptLanguages, GetParam().accept_languages);
+
+  std::vector<std::string> spellcheck_languages;
+  size_t enabled_spellcheck_languages =
+      SpellcheckService::GetSpellCheckLanguages(context, &spellcheck_languages);
+
+  EXPECT_EQ(GetParam().expected_enabled_spellcheck_languages,
+            enabled_spellcheck_languages);
+  EXPECT_EQ(GetParam().expected_spellcheck_languages, spellcheck_languages);
+}
+
 // Tests that we can delete a corrupted BDICT file used by hunspell. We do not
 // run this test on Mac because Mac does not use hunspell by default.
 IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest, DeleteCorruptedBDICT) {
@@ -50,8 +114,8 @@ IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest, DeleteCorruptedBDICT) {
   base::FilePath bdict_path =
       chrome::spellcheck_common::GetVersionedFileName("en-US", dict_dir);
 
-  size_t actual = base::WriteFile(bdict_path,
-      reinterpret_cast<const char*>(kCorruptedBDICT),
+  size_t actual = base::WriteFile(
+      bdict_path, reinterpret_cast<const char*>(kCorruptedBDICT),

[Julius, 2015/06/17 00:59:53]

clang-format reformatted this and the lines below.

[please use gerrit instead, 2015/06/17 17:47:15]

Acknowledged.

       arraysize(kCorruptedBDICT));
   EXPECT_EQ(arraysize(kCorruptedBDICT), actual);
 
@@ -60,15 +124,14 @@ IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest, DeleteCorruptedBDICT) {
   base::WaitableEvent event(true, false);
   SpellcheckService::AttachStatusEvent(&event);
 
-  BrowserContext * context = static_cast<BrowserContext*>(GetProfile());
+  BrowserContext* context = static_cast<BrowserContext*>(GetProfile());
 
   // Ensure that the SpellcheckService object does not already exist. Otherwise
   // the next line will not force creation of the SpellcheckService and the
   // test will fail.
   SpellcheckService* service = static_cast<SpellcheckService*>(
       SpellcheckServiceFactory::GetInstance()->GetServiceForBrowserContext(
-          context,
-          false));
+          context, false));
   ASSERT_EQ(NULL, service);
 
   // Getting the spellcheck_service will initialize the SpellcheckService
@@ -89,3 +152,53 @@ IN_PROC_BROWSER_TEST_F(SpellcheckServiceBrowserTest, DeleteCorruptedBDICT) {
     EXPECT_TRUE(base::DeleteFile(bdict_path, true));
   }
 }
+
+class MultilingualSpellcheckServiceBrowserTest
+    : public InProcessBrowserTest,
+      public testing::WithParamInterface<SpellcheckLanguageTestCase> {
+ public:
+  Profile* GetProfile() { return browser()->profile(); }
+  void SetUpCommandLine(base::CommandLine* command_line) override {

[please use gerrit instead, 2015/06/17 17:47:15]

Please also call "InProcBrowserTest::SetUpCommandLine(command_line);"

[Julius, 2015/06/17 20:14:46]

Done.

+    command_line->AppendSwitch(switches::kEnableMultilingualSpellChecker);
+  }
+};
+
+INSTANTIATE_TEST_CASE_P(
+    MultilingualSpellcheckLanguageTestCases,
+    MultilingualSpellcheckServiceBrowserTest,
+    testing::Values(
+        SpellcheckLanguageTestCase("en-US",
+                                   "en,en-US",
+                                   1UL,
+                                   std::vector<std::string>{"en-US"}),
+        SpellcheckLanguageTestCase("en-US",
+                                   "en-US,en",
+                                   1UL,
+                                   std::vector<std::string>{"en-US"}),
+        SpellcheckLanguageTestCase("en-US,fr",
+                                   "en,fr,en-US,en-AU",
+                                   2UL,
+                                   std::vector<std::string>{"en-US", "fr",
+                                                            "en-AU"}),
+        SpellcheckLanguageTestCase("fr",
+                                   "en,en-JP,fr,zz,en-US",
+                                   1UL,
+                                   std::vector<std::string>{"fr", "en-US"})));
+
+IN_PROC_BROWSER_TEST_P(MultilingualSpellcheckServiceBrowserTest,
+                       GetSpellcheckLanguages) {
+  BrowserContext* context = static_cast<BrowserContext*>(GetProfile());
+  PrefService* prefs = user_prefs::UserPrefs::Get(context);
+  prefs->SetString(prefs::kSpellCheckDictionaries,
+                   GetParam().spellcheck_dictionary);
+  prefs->SetString(prefs::kAcceptLanguages, GetParam().accept_languages);
+
+  std::vector<std::string> spellcheck_languages;
+  size_t enabled_spellcheck_languages =
+      SpellcheckService::GetSpellCheckLanguages(context,
+                                                &spellcheck_languages);
+
+  EXPECT_EQ(GetParam().expected_enabled_spellcheck_languages,
+            enabled_spellcheck_languages);
+  EXPECT_EQ(GetParam().expected_spellcheck_languages, spellcheck_languages);
+}