Enable Chrome to tweak search engines for some locales

components/search_engines/template_url_prepopulate_data.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/search_engines/template_url_prepopulate_data.h"
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
 #include <locale.h>
 #endif
 
@@ skipping 591 matching lines @@
   // Cache first run Country ID value in prefs, and use it afterwards.  This
   // ensures that just because the user moves around, we won't automatically
   // make major changes to their available search providers, which would feel
   // surprising.
   if (!prefs->HasPrefPath(prefs::kCountryIDAtInstall)) {
     prefs->SetInteger(prefs::kCountryIDAtInstall, GetCurrentCountryID());
   }
   return prefs->GetInteger(prefs::kCountryIDAtInstall);
 }
 
-void GetPrepopulationSetFromCountryID(PrefService* prefs,
+int CountryStringToCountryID(std::string country) {
+  if (country.length() != 2)
+    return kCountryIDUnknown;
+
+  return CountryCharsToCountryIDWithUpdate(country[0], country[1]);
+}

[Peter Kasting, 2016/09/19 21:06:23]

I added this in https://codereview.chromium.org/2349213004/ ; you may want to
mark your patch as dependent on that (I added it in a slightly different place
FWIW)

[Ian Wen, 2016/09/20 04:48:03]

Done.

+
+void GetPrepopulationSetFromCountryID(int country_id,
                                       const PrepopulatedEngine*** engines,
                                       size_t* num_engines) {
   // NOTE: This function should ALWAYS set its outparams.
 
   // If you add a new country make sure to update the unit test for coverage.
-  switch (GetCountryIDFromPrefs(prefs)) {
-
+  switch (country_id) {
 #define CHAR_A 'A'
 #define CHAR_B 'B'
 #define CHAR_C 'C'
 #define CHAR_D 'D'
 #define CHAR_E 'E'
 #define CHAR_F 'F'
 #define CHAR_G 'G'
 #define CHAR_H 'H'
 #define CHAR_I 'I'
 #define CHAR_J 'J'
@@ skipping 330 matching lines @@
     UNHANDLED_COUNTRY(V, C)  // Saint Vincent and the Grenadines
     UNHANDLED_COUNTRY(V, U)  // Vanuatu
     UNHANDLED_COUNTRY(W, S)  // Samoa
     UNHANDLED_COUNTRY(Z, M)  // Zambia
     case kCountryIDUnknown:
     default:                // Unhandled location
     END_UNHANDLED_COUNTRIES(def, ault)
   }
 }
 
+void GetPrepopulationSetFromPrefs(PrefService* prefs,

[Peter Kasting, 2016/09/19 21:06:23]

Nit: You only call this helper once, and it's simple, so I would just inline
this into the one place below and not add the helper.

[Ian Wen, 2016/09/20 04:48:03]

Done.

+                                  const PrepopulatedEngine*** engines,
+                                  size_t* num_engines) {
+  GetPrepopulationSetFromCountryID(GetCountryIDFromPrefs(prefs), engines,
+                                   num_engines);
+}
+
 std::unique_ptr<TemplateURLData> MakePrepopulatedTemplateURLData(
     const base::string16& name,
     const base::string16& keyword,
     const base::StringPiece& search_url,
     const base::StringPiece& suggest_url,
     const base::StringPiece& instant_url,
     const base::StringPiece& image_url,
     const base::StringPiece& new_tab_url,
     const base::StringPiece& contextual_search_url,
     const base::StringPiece& search_url_post_params,
@@ skipping 117 matching lines @@
   registry->RegisterIntegerPref(prefs::kSearchProviderOverridesVersion, -1);
 }
 
 int GetDataVersion(PrefService* prefs) {
   // Allow tests to override the local version.
   return (prefs && prefs->HasPrefPath(prefs::kSearchProviderOverridesVersion)) ?
       prefs->GetInteger(prefs::kSearchProviderOverridesVersion) :
       kCurrentDataVersion;
 }
 
+std::vector<std::unique_ptr<TemplateURLData>> GetLocalPrepopulatedEngines(

[Peter Kasting, 2016/09/19 21:06:23]

Nit: Function definition order should match .h declaration order.

[Ian Wen, 2016/09/20 04:48:03]

Done.

+    const std::string& locale,
+    PrefService* prefs,
+    bool* need_to_load) {
+  std::vector<std::unique_ptr<TemplateURLData>> t_urls;
+
+  int country_id = CountryStringToCountryID(locale);
+  DCHECK(country_id != kCountryIDUnknown);

[Peter Kasting, 2016/09/19 21:06:23]

This DCHECK makes me nervous.  Are you sure the caller has sanitized the
incoming value?  I wonder if we should just modify the conditional below to
treat this case like the "same as current country" case.

[Ian Wen, 2016/09/20 04:48:03]

Done.

+
+  if (country_id == GetCountryIDFromPrefs(prefs)) {
+    *need_to_load = false;
+    return t_urls;
+  }
+  *need_to_load = true;
+
+  const PrepopulatedEngine** engines;
+  size_t num_engines;
+  GetPrepopulationSetFromCountryID(country_id, &engines, &num_engines);

[Peter Kasting, 2016/09/19 21:06:23]

Nit: I suggest modifying GetPrepopulationSetFromCountryID() so that instead of
taking these two outparams, it constructs the vector of TemplateURLs internally.
 (This just involves changing the "return;" to a "break;" in the macro
definition for END_UNHANDLED_COUNTRIES and then adding some code on either side
of the switch.)

This will simplify the two functions here.

+  for (size_t i = 0; i != num_engines; ++i) {
+    t_urls.push_back(MakeTemplateURLDataFromPrepopulatedEngine(*engines[i]));
+  }
+  return t_urls;
+}
+
 std::vector<std::unique_ptr<TemplateURLData>> GetPrepopulatedEngines(
     PrefService* prefs,
     size_t* default_search_provider_index) {
   // If there is a set of search engines in the preferences file, it overrides
   // the built-in set.
   *default_search_provider_index = 0;
   std::vector<std::unique_ptr<TemplateURLData>> t_urls =
       GetPrepopulatedTemplateURLData(prefs);
   if (!t_urls.empty())
     return t_urls;
 
   const PrepopulatedEngine** engines;
   size_t num_engines;
-  GetPrepopulationSetFromCountryID(prefs, &engines, &num_engines);
+  GetPrepopulationSetFromPrefs(prefs, &engines, &num_engines);
   for (size_t i = 0; i != num_engines; ++i) {
     t_urls.push_back(MakeTemplateURLDataFromPrepopulatedEngine(*engines[i]));
   }
   return t_urls;
 }
 
 std::vector<const PrepopulatedEngine*> GetAllPrepopulatedEngines() {
   return std::vector<const PrepopulatedEngine*>(std::begin(kAllEngines),
                                                 std::end(kAllEngines));
 }
@@ skipping 127 matching lines @@
         base::ToUpperASCII(locale_str[begin]),
         base::ToUpperASCII(locale_str[begin + 1]));
   }
 
   return kCountryIDUnknown;
 }
 
 #endif  // OS_*
 
 }  // namespace TemplateURLPrepopulateData