Enable Chrome to tweak search engines for some locales

components/search_engines/template_url_prepopulate_data.cc

Index: components/search_engines/template_url_prepopulate_data.cc
diff --git a/components/search_engines/template_url_prepopulate_data.cc b/components/search_engines/template_url_prepopulate_data.cc
index 076dcd62657d762db0fe83031310f29c73930105..5ed20a332f7deb4f8a36a049c1c983fa6df1f629 100644
--- a/components/search_engines/template_url_prepopulate_data.cc
+++ b/components/search_engines/template_url_prepopulate_data.cc
@@ -609,14 +609,20 @@ int GetCountryIDFromPrefs(PrefService* prefs) {
   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'
@@ -967,6 +973,13 @@ void GetPrepopulationSetFromCountryID(PrefService* prefs,
   }
 }
 
+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,
@@ -1104,6 +1117,30 @@ int GetDataVersion(PrefService* prefs) {
       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) {
@@ -1117,7 +1154,7 @@ std::vector<std::unique_ptr<TemplateURLData>> GetPrepopulatedEngines(
 
   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]));
   }