More Spellcheck code cleanup.

- Mark some SpellcheckHunspellDictionary code non-Android.
- Use base::StringPiece a little more.
- Fix nits / style.

Change-Id: Ibe2e7bf14678f7fbfc976d42964091afa138bbec
Reviewed-on: https://chromium-review.googlesource.com/838370Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532198}

chrome/browser/chromeos/extensions/input_method_api.cc

@@ -199,7 +199,7 @@ InputMethodPrivateAddWordToDictionaryFunction::Run() {
   // Invalid words:
   // - Already in the dictionary.
   // - Not a UTF8 string.
-  // - Longer than 99 bytes (MAX_CUSTOM_DICTIONARY_WORD_BYTES).
+  // - Longer than 99 bytes (kMaxCustomDictionaryWordBytes).
   // - Leading/trailing whitespace.
   // - Empty.
   return RespondNow(Error("Unable to add invalid word to dictionary."));

chrome/browser/spellchecker/spellcheck_custom_dictionary.cc

@@ -87,7 +87,7 @@ ChecksumStatus LoadFile(const base::FilePath& file_path,
 bool IsValidWord(const std::string& word) {
   std::string tmp;
   return !word.empty() &&
-         word.size() <= spellcheck::MAX_CUSTOM_DICTIONARY_WORD_BYTES &&
+         word.size() <= spellcheck::kMaxCustomDictionaryWordBytes &&
          base::IsStringUTF8(word) &&
          base::TRIM_NONE ==
              base::TrimWhitespaceASCII(word, base::TRIM_ALL, &tmp);
@@ -340,7 +340,7 @@ syncer::SyncDataList SpellcheckCustomDictionary::GetAllSyncData(
   syncer::SyncDataList data;
   size_t i = 0;
   for (const auto& word : words_) {
-    if (i++ >= spellcheck::MAX_SYNCABLE_DICTIONARY_WORDS)
+    if (i++ >= spellcheck::kMaxSyncableDictionaryWords)
       break;
     sync_pb::EntitySpecifics specifics;
     specifics.mutable_dictionary()->set_word(word);
@@ -483,7 +483,7 @@ syncer::SyncError SpellcheckCustomDictionary::Sync(
   int server_size = static_cast<int>(words_.size()) -
       static_cast<int>(dictionary_change.to_add().size());
   int max_upload_size =
-      std::max(0, static_cast<int>(spellcheck::MAX_SYNCABLE_DICTIONARY_WORDS) -
+      std::max(0, static_cast<int>(spellcheck::kMaxSyncableDictionaryWords) -
                       server_size);
   int upload_size = std::min(
       static_cast<int>(dictionary_change.to_add().size()),
@@ -518,7 +518,7 @@ syncer::SyncError SpellcheckCustomDictionary::Sync(
 
   // Turn off syncing of this dictionary if the server already has the maximum
   // number of words.
-  if (words_.size() > spellcheck::MAX_SYNCABLE_DICTIONARY_WORDS)
+  if (words_.size() > spellcheck::kMaxSyncableDictionaryWords)
     StopSyncing(syncer::DICTIONARY);
 
   return error;

chrome/browser/spellchecker/spellcheck_hunspell_dictionary.cc

@@ -106,9 +106,12 @@ SpellcheckHunspellDictionary::SpellcheckHunspellDictionary(
       language_(language),
       use_browser_spellchecker_(false),
       request_context_getter_(request_context_getter),
+#if !defined(OS_ANDROID)
       spellcheck_service_(spellcheck_service),
+#endif
       download_status_(DOWNLOAD_NONE),
-      weak_ptr_factory_(this) {}
+      weak_ptr_factory_(this) {
+}
 
 SpellcheckHunspellDictionary::~SpellcheckHunspellDictionary() {
   if (dictionary_file_.file.IsValid()) {
@@ -287,18 +290,19 @@ void SpellcheckHunspellDictionary::DownloadDictionary(GURL url) {
   request_context_getter_ = NULL;
 }
 
-// The default_dictionary_file can either come from the standard list of
-// hunspell dictionaries (determined in InitializeDictionaryLocation), or it
-// can be passed in via an extension. In either case, the file is checked for
-// existence so that it's not re-downloaded.
-// For systemwide installations on Windows, the default directory may not
-// have permissions for download. In that case, the alternate directory for
-// download is chrome::DIR_USER_DATA.
-//
+#if !defined(OS_ANDROID)
 // static
 SpellcheckHunspellDictionary::DictionaryFile
 SpellcheckHunspellDictionary::OpenDictionaryFile(const base::FilePath& path) {
   base::AssertBlockingAllowed();
+
+  // The default_dictionary_file can either come from the standard list of
+  // hunspell dictionaries (determined in InitializeDictionaryLocation), or it
+  // can be passed in via an extension. In either case, the file is checked for
+  // existence so that it's not re-downloaded.
+  // For systemwide installations on Windows, the default directory may not
+  // have permissions for download. In that case, the alternate directory for
+  // download is chrome::DIR_USER_DATA.
   DictionaryFile dictionary;
 
 #if defined(OS_WIN)
@@ -313,13 +317,12 @@ SpellcheckHunspellDictionary::OpenDictionaryFile(const base::FilePath& path) {
     dictionary.path = path;
 #else
   dictionary.path = path;
-#endif
+#endif  // defined(OS_WIN)
 
   // Read the dictionary file and scan its data to check for corruption. The
   // scoping closes the memory-mapped file before it is opened or deleted.
   bool bdict_is_valid = false;
 
-#if !defined(OS_ANDROID)
   {
     base::MemoryMappedFile map;
     bdict_is_valid =
@@ -328,7 +331,6 @@ SpellcheckHunspellDictionary::OpenDictionaryFile(const base::FilePath& path) {
         hunspell::BDict::Verify(reinterpret_cast<const char*>(map.data()),
                                 map.length());
   }
-#endif
 
   if (bdict_is_valid) {
     dictionary.file.Initialize(dictionary.path,
@@ -340,15 +342,15 @@ SpellcheckHunspellDictionary::OpenDictionaryFile(const base::FilePath& path) {
   return dictionary;
 }
 
-// The default place where the spellcheck dictionary resides is
-// chrome::DIR_APP_DICTIONARIES.
-//
 // static
 SpellcheckHunspellDictionary::DictionaryFile
 SpellcheckHunspellDictionary::InitializeDictionaryLocation(
     const std::string& language) {
   base::AssertBlockingAllowed();
 
+  // The default place where the spellcheck dictionary resides is
+  // chrome::DIR_APP_DICTIONARIES.
+  //
   // Initialize the BDICT path. Initialization should be on the blocking
   // sequence because it checks if there is a "Dictionaries" directory and
   // create it.
@@ -384,6 +386,7 @@ void SpellcheckHunspellDictionary::InitializeDictionaryLocationComplete(
 
   InformListenersOfInitialization();
 }
+#endif  // !defined(OS_ANDROID)
 
 void SpellcheckHunspellDictionary::SaveDictionaryDataComplete(
     bool dictionary_saved) {

chrome/browser/spellchecker/spellcheck_hunspell_dictionary.h

@@ -15,6 +15,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/sequenced_task_runner.h"
+#include "build/build_config.h"
 #include "components/spellcheck/browser/spellcheck_dictionary.h"
 #include "net/url_request/url_fetcher_delegate.h"
 
@@ -122,6 +123,7 @@ class SpellcheckHunspellDictionary
   // Attempt to download the dictionary.
   void DownloadDictionary(GURL url);
 
+#if !defined(OS_ANDROID)
   // Figures out the location for the dictionary, verifies its contents, and
   // opens it.
   static DictionaryFile OpenDictionaryFile(const base::FilePath& path);
@@ -133,6 +135,7 @@ class SpellcheckHunspellDictionary
   // The reply point for PostTaskAndReplyWithResult, called after the dictionary
   // file has been initialized.
   void InitializeDictionaryLocationComplete(DictionaryFile file);
+#endif
 
   // The reply point for PostTaskAndReplyWithResult, called after the dictionary
   // file has been saved.
@@ -145,10 +148,10 @@ class SpellcheckHunspellDictionary
   void InformListenersOfDownloadFailure();
 
   // Task runner where the file operations takes place.
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  scoped_refptr<base::SequencedTaskRunner> const task_runner_;
 
   // The language of the dictionary file.
-  std::string language_;
+  const std::string language_;
 
   // Whether to use the platform spellchecker instead of Hunspell.
   bool use_browser_spellchecker_;
@@ -160,7 +163,9 @@ class SpellcheckHunspellDictionary
   // Used for downloading the dictionary file.
   std::unique_ptr<net::URLFetcher> fetcher_;
 
-  SpellcheckService* spellcheck_service_;
+#if !defined(OS_ANDROID)
+  SpellcheckService* const spellcheck_service_;
+#endif
 
   // Observers of Hunspell dictionary events.
   base::ObserverList<Observer> observers_;

chrome/browser/spellchecker/spellcheck_language_policy_handler.cc

@@ -24,9 +24,7 @@ bool SpellcheckLanguagePolicyHandler::CheckPolicySettings(
     const policy::PolicyMap& policies,
     policy::PolicyErrorMap* errors) {
   const base::Value* value = nullptr;
-  if (!CheckAndGetValue(policies, errors, &value))
-    return false;
-  return true;
+  return CheckAndGetValue(policies, errors, &value);
 }
 
 void SpellcheckLanguagePolicyHandler::ApplyPolicySettings(
@@ -35,10 +33,8 @@ void SpellcheckLanguagePolicyHandler::ApplyPolicySettings(
   // Ignore this policy if the SpellcheckEnabled policy disables spellcheck.
   const base::Value* spellcheck_enabled_value =
       policies.GetValue(policy::key::kSpellcheckEnabled);
-  if (spellcheck_enabled_value &&
-      spellcheck_enabled_value->GetBool() == false) {
+  if (spellcheck_enabled_value && spellcheck_enabled_value->GetBool() == false)
     return;
-  }
 
   const base::Value* value = policies.GetValue(policy_name());
   if (!value)
@@ -51,8 +47,7 @@ void SpellcheckLanguagePolicyHandler::ApplyPolicySettings(
   for (const base::Value& language : languages) {
     std::string current_language =
         spellcheck::GetCorrespondingSpellCheckLanguage(
-            base::TrimWhitespaceASCII(language.GetString(), base::TRIM_ALL)
-                .as_string());
+            base::TrimWhitespaceASCII(language.GetString(), base::TRIM_ALL));
     if (!current_language.empty()) {
       forced_language_list->GetList().push_back(base::Value(current_language));
     } else {

chrome/browser/spellchecker/spellcheck_service.cc

@@ -170,7 +170,7 @@ bool SpellcheckService::SignalStatusEvent(
 }
 
 void SpellcheckService::StartRecordingMetrics(bool spellcheck_enabled) {
-  metrics_.reset(new SpellCheckHostMetrics());
+  metrics_ = std::make_unique<SpellCheckHostMetrics>();
   metrics_->RecordEnabledStats(spellcheck_enabled);
   OnUseSpellingServiceChanged();
 }

chrome/browser/sync/test/integration/performance/dictionary_sync_perf_test.cc

@@ -30,15 +30,15 @@ IN_PROC_BROWSER_TEST_F(DictionarySyncPerfTest, P0) {
   ASSERT_TRUE(dictionary_helper::DictionariesMatch());
 
   base::TimeDelta dt;
-  for (size_t i = 0; i < spellcheck::MAX_SYNCABLE_DICTIONARY_WORDS; ++i) {
+  for (size_t i = 0; i < spellcheck::kMaxSyncableDictionaryWords; ++i) {
     ASSERT_TRUE(dictionary_helper::AddWord(0, "foo" + base::NumberToString(i)));
   }
   dt = TimeMutualSyncCycle(GetClient(0), GetClient(1));
-  ASSERT_EQ(spellcheck::MAX_SYNCABLE_DICTIONARY_WORDS,
+  ASSERT_EQ(spellcheck::kMaxSyncableDictionaryWords,
             dictionary_helper::GetDictionarySize(1));
   PrintResult("dictionary", "add_words", dt);
 
-  for (size_t i = 0; i < spellcheck::MAX_SYNCABLE_DICTIONARY_WORDS; ++i) {
+  for (size_t i = 0; i < spellcheck::kMaxSyncableDictionaryWords; ++i) {
     ASSERT_TRUE(
         dictionary_helper::RemoveWord(0, "foo" + base::NumberToString(i)));
   }

chrome/browser/sync/test/integration/two_client_dictionary_sync_test.cc

@@ -12,7 +12,7 @@
 #include "components/spellcheck/common/spellcheck_common.h"
 #include "components/sync/base/model_type.h"
 
-using spellcheck::MAX_SYNCABLE_DICTIONARY_WORDS;
+using spellcheck::kMaxSyncableDictionaryWords;
 
 class TwoClientDictionarySyncTest : public SyncTest {
  public:
@@ -117,7 +117,7 @@ IN_PROC_BROWSER_TEST_F(TwoClientDictionarySyncTest,
 }
 
 // Tests the case where a client has more words added than the
-// MAX_SYNCABLE_DICTIONARY_WORDS limit.
+// kMaxSyncableDictionaryWords limit.
 IN_PROC_BROWSER_TEST_F(TwoClientDictionarySyncTest, Limit) {
   ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
   dictionary_helper::LoadDictionaries();
@@ -126,11 +126,11 @@ IN_PROC_BROWSER_TEST_F(TwoClientDictionarySyncTest, Limit) {
   // Disable client #1 before client #0 starts adding anything.
   GetClient(1)->DisableSyncForAllDatatypes();
 
-  // Pick a size between 1/2 and 1/3 of MAX_SYNCABLE_DICTIONARY_WORDS. This will
+  // Pick a size between 1/2 and 1/3 of kMaxSyncableDictionaryWords. This will
   // allow the test to verify that while we crossed the limit the client not
   // actively making changes is still recieving sync updates but stops exactly
   // on the limit.
-  size_t chunk_size = MAX_SYNCABLE_DICTIONARY_WORDS * 2 / 5;
+  size_t chunk_size = kMaxSyncableDictionaryWords * 2 / 5;
 
   ASSERT_TRUE(dictionary_helper::AddWords(0, chunk_size, "foo-0-"));
   ASSERT_EQ(chunk_size, dictionary_helper::GetDictionarySize(0));
@@ -157,8 +157,8 @@ IN_PROC_BROWSER_TEST_F(TwoClientDictionarySyncTest, Limit) {
   ASSERT_TRUE(GetClient(1)->EnableSyncForAllDatatypes());
   ASSERT_TRUE(NumDictionaryEntriesChecker(1, 3 * chunk_size).Wait());
   ASSERT_TRUE(ServerCountMatchStatusChecker(syncer::DICTIONARY,
-                                            MAX_SYNCABLE_DICTIONARY_WORDS)
+                                            kMaxSyncableDictionaryWords)
                   .Wait());
   ASSERT_TRUE(
-      NumDictionaryEntriesChecker(0, MAX_SYNCABLE_DICTIONARY_WORDS).Wait());
+      NumDictionaryEntriesChecker(0, kMaxSyncableDictionaryWords).Wait());
 }

components/spellcheck/common/spellcheck_common.cc

@@ -89,16 +89,16 @@ bool IsValidRegion(const std::string& region) {
 
 // This function returns the language-region version of language name.
 // e.g. returns hi-IN for hi.
-std::string GetSpellCheckLanguageRegion(const std::string& input_language) {
+std::string GetSpellCheckLanguageRegion(base::StringPiece input_language) {
   for (const auto& lang_region : kSupportedSpellCheckerLanguages) {
     if (lang_region.language == input_language)
-      return std::string(lang_region.language_region);
+      return lang_region.language_region;
   }
 
-  return input_language;
+  return input_language.as_string();
 }
 
-base::FilePath GetVersionedFileName(const std::string& input_language,
+base::FilePath GetVersionedFileName(base::StringPiece input_language,
                                     const base::FilePath& dict_dir) {
   // The default dictionary version is 3-0. This version indicates that the bdic
   // file contains a checksum.
@@ -138,13 +138,13 @@ base::FilePath GetVersionedFileName(const std::string& input_language,
   return dict_dir.AppendASCII(versioned_bdict_file_name);
 }
 
-std::string GetCorrespondingSpellCheckLanguage(const std::string& language) {
+std::string GetCorrespondingSpellCheckLanguage(base::StringPiece language) {
   std::string best_match;
   // Look for exact match in the Spell Check language list.
   for (const auto& lang_region : kSupportedSpellCheckerLanguages) {
     // First look for exact match in the language region of the list.
     if (lang_region.language == language)
-      return language;
+      return language.as_string();
 
     // Next, look for exact match in the language_region part of the list.
     if (lang_region.language_region == language) {

components/spellcheck/common/spellcheck_common.h

@@ -10,6 +10,8 @@
 #include <string>
 #include <vector>
 
+#include "base/strings/string_piece.h"
+
 namespace base {
 class FilePath;
 }
@@ -21,14 +23,14 @@ static const int kMaxSuggestions = 5;
 
 // Maximum number of words in the custom spellcheck dictionary that can be
 // synced.
-static const size_t MAX_SYNCABLE_DICTIONARY_WORDS = 1300;
+static const size_t kMaxSyncableDictionaryWords = 1300;
 
 // Maximum number of bytes in a word that can be added to the custom spellcheck
 // dictionary. When changing this value, also change the corresponding value in
-// /src/chrome/browser/resources/settings/languages_page/edit_dictionary_page.js
-static const size_t MAX_CUSTOM_DICTIONARY_WORD_BYTES = 99;
+// chrome/browser/resources/settings/languages_page/edit_dictionary_page.js
+static const size_t kMaxCustomDictionaryWordBytes = 99;
 
-base::FilePath GetVersionedFileName(const std::string& input_language,
+base::FilePath GetVersionedFileName(base::StringPiece input_language,
                                     const base::FilePath& dict_dir);
 
 // Returns the spellcheck language that should be used for |language|. For
@@ -39,7 +41,7 @@ base::FilePath GetVersionedFileName(const std::string& input_language,
 // Returns an empty string if no spellcheck language found. For example, there's
 // no single dictionary for English, so this function returns an empty string
 // for "en".
-std::string GetCorrespondingSpellCheckLanguage(const std::string& language);
+std::string GetCorrespondingSpellCheckLanguage(base::StringPiece language);
 
 // Get SpellChecker supported languages.
 std::vector<std::string> SpellCheckLanguages();