Bug 1522151 - Use correct region for regionOverrides. r=nalexander a=lizzard
☠☠ backed out by 5cd3206e4d85 ☠ ☠
authorDorel Luca <dluca@mozilla.com>
Fri, 22 Feb 2019 11:16:12 +0200
changeset 516081 6d4e20ba3b7f989b1ecbc4aab91df34a3c973c1b
parent 516080 7f2efed32196f18855012fe561c2ed86fdace352
child 516082 c636ac25be612857323adbef5c22e22fe94989ce
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnalexander, lizzard
bugs1522151
milestone66.0
Bug 1522151 - Use correct region for regionOverrides. r=nalexander a=lizzard Reviewers: nalexander Reviewed By: nalexander Bug #: 1522151 Differential Revision: https://phabricator.services.mozilla.com/D17379
mobile/android/app/src/test/java/org/mozilla/gecko/search/TestSearchEngineManager.java
mobile/android/base/java/org/mozilla/gecko/search/SearchEngineManager.java
new file mode 100644
--- /dev/null
+++ b/mobile/android/app/src/test/java/org/mozilla/gecko/search/TestSearchEngineManager.java
@@ -0,0 +1,67 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+package org.mozilla.gecko.search;
+
+import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import org.junit.runner.RunWith;
+
+import org.json.JSONException;
+import org.json.JSONObject;
+import org.mozilla.gecko.search.SearchEngineManager;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+
+import java.util.ArrayList;
+
+
+/**
+ * Unit tests for date utilities.
+ */
+
+@RunWith(RobolectricTestRunner.class)
+public class TestSearchEngineManager {
+    private static String listJSON = "" +
+            "{" +
+              "'default': {" +
+                "'searchDefault': 'Google'" +
+              "}," +
+              "'regionOverrides': {" +
+                "'US': {" +
+                  "'google-b-d': 'google-b-1-d'" +
+                "}" +
+              "}," +
+              "'locales': {" +
+                "'en-US': {" +
+                  "'default': {" +
+                    "'visibleDefaultEngines': [" +
+                      "'google-b-d', 'amazondotcom', 'bing', 'ddg', 'ebay', 'twitter', 'wikipedia'" +
+                    "]" +
+                  "}" +
+                "}" +
+              "}" +
+            "}".replace("'", "\"");
+
+    @Test
+    public void testGetDefaultEngineNameFromJSON() throws Exception {
+        final JSONObject json = new JSONObject(listJSON);
+        final String defaultEngine = SearchEngineManager.getDefaultEngineNameFromJSON("US", json);
+        assertEquals("Incorrect engine", "Google", defaultEngine);
+    }
+
+    @Test
+    public void testGetEngineListFromJSON() throws Exception {
+        ArrayList<String> correctEngineList = new ArrayList<String>();
+        correctEngineList.add("google-b-1-d");
+        correctEngineList.add("amazondotcom");
+        correctEngineList.add("bing");
+        correctEngineList.add("ddg");
+        correctEngineList.add("ebay");
+        correctEngineList.add("twitter");
+        correctEngineList.add("wikipedia");
+        final JSONObject json = new JSONObject(listJSON);
+        final ArrayList<String> engines = SearchEngineManager.getEngineListFromJSON("US", json);
+        assertEquals("Incorrect engine list", correctEngineList, engines);
+    }
+}
--- a/mobile/android/base/java/org/mozilla/gecko/search/SearchEngineManager.java
+++ b/mobile/android/base/java/org/mozilla/gecko/search/SearchEngineManager.java
@@ -435,29 +435,40 @@ public class SearchEngineManager impleme
     private String getDefaultEngineNameFromLocale() {
         final InputStream in = getInputStreamFromSearchPluginsJar("list.json");
         if (in == null) {
             Log.e(LOG_TAG, "Error missing list.json");
             return null;
         }
         try {
             final JSONObject json = new JSONObject(FileUtils.readStringFromInputStreamAndCloseStream(in, MAX_LISTJSON_SIZE));
-
-            // Get the current language
-            final String languageTag = Locales.getLanguageTag(Locale.getDefault());
-
-            // Get the current region
-            String region = fetchCountryCode();
+            final String region = fetchCountryCode();
 
             // Store the result, even if it's empty. If we fail to get a region, we never
             // try to get it again, and we will always fallback to the non-region engine.
             GeckoSharedPrefs.forApp(context)
-                            .edit()
-                            .putString(PREF_REGION_KEY, (region == null ? "" : region))
-                            .apply();
+                    .edit()
+                    .putString(PREF_REGION_KEY, (region == null ? "" : region))
+                    .apply();
+
+            return getDefaultEngineNameFromJSON(region, json);
+        } catch (IOException e) {
+            Log.e(LOG_TAG, "Error getting search engine name from list.json", e);
+        } catch (JSONException e) {
+            Log.e(LOG_TAG, "Error parsing list.json", e);
+        } finally {
+            IOUtils.safeStreamClose(in);
+        }
+        return null;
+    }
+
+    public static String getDefaultEngineNameFromJSON(String region, JSONObject json) {
+        try {
+            // Get the current language
+            final String languageTag = Locales.getLanguageTag(Locale.getDefault());
 
             final JSONObject locales = json.getJSONObject("locales");
 
             if (locales.has(languageTag)) {
                 final JSONObject regions = locales.getJSONObject(languageTag);
                 if (!regions.has(region)) {
                     // Region doesn't exist, use default.
                     // default always exists.
@@ -466,22 +477,18 @@ public class SearchEngineManager impleme
                 final JSONObject regionData = regions.getJSONObject(region);
                 if (regionData.has("searchDefault")) {
                     return regionData.getString("searchDefault");
                 }
             }
             // Falling back to the overall default
             final JSONObject defaultData = json.getJSONObject("default");
             return defaultData.getString("searchDefault");
-        } catch (IOException e) {
-            Log.e(LOG_TAG, "Error getting search engine name from list.json", e);
         } catch (JSONException e) {
             Log.e(LOG_TAG, "Error parsing list.json", e);
-        } finally {
-            IOUtils.safeStreamClose(in);
         }
         return null;
     }
 
     /**
      * Creates a SearchEngine instance from an engine name.
      *
      * To create the engine, we first try to find the search plugin in the distribution
@@ -598,61 +605,76 @@ public class SearchEngineManager impleme
             return null;
         } catch (JSONException e) {
             Log.e(LOG_TAG, "Error parsing list.json", e);
             return null;
         } finally {
             IOUtils.safeStreamClose(in);
         }
         try {
+
+            ArrayList<String> engines = getEngineListFromJSON(fetchCountryCode(), json);
+
+            for (int i = 0; i < engines.size(); i++) {
+                final InputStream pluginIn = getInputStreamFromSearchPluginsJar(engines.get(i) + ".xml");
+                if (pluginIn != null) {
+                    final SearchEngine engine = createEngineFromInputStream(engines.get(i), pluginIn);
+                    if (engine != null && engine.getName().equals(name)) {
+                        return engine;
+                    }
+                }
+            }
+        } catch (Throwable e) {
+            Log.e(LOG_TAG, "Error creating shipped search engine from name: " + name, e);
+        }
+        return null;
+    }
+
+    public static ArrayList<String> getEngineListFromJSON(String originalRegion, JSONObject json) {
+        try {
             final String languageTag = Locales.getLanguageTag(Locale.getDefault());
 
-            String region = fetchCountryCode();
+            String region = originalRegion;
 
             final JSONObject locales = json.getJSONObject("locales");
 
             JSONArray jsonEngines;
             if (locales.has(languageTag)) {
                 final JSONObject regions = locales.getJSONObject(languageTag);
-                if (!regions.has(region)) {
+                if (!regions.has(originalRegion)) {
                     // Region doesn't exist, use default.
                     // default always exists.
                     region = "default";
                 }
                 jsonEngines = regions.getJSONObject(region).getJSONArray("visibleDefaultEngines");
             } else {
                 // Falling back to the overall default
                 jsonEngines = locales.getJSONObject("default").getJSONArray("visibleDefaultEngines");
             }
 
             ArrayList<String> engines = new ArrayList<String>();
 
-            if (json.getJSONObject("regionOverrides").has(region)) {
-                final JSONObject regionOverride = json.getJSONObject("regionOverrides").getJSONObject(fetchCountryCode());
-                for (int i = 0; i < jsonEngines.length(); i++) {
-                    final String engineName = jsonEngines.getString(i);
+            for (int i = 0; i < jsonEngines.length(); i++) {
+                engines.add(jsonEngines.getString(i));
+            }
+
+            if (json.getJSONObject("regionOverrides").has(originalRegion)) {
+                final JSONObject regionOverride = json.getJSONObject("regionOverrides").getJSONObject(originalRegion);
+                for (int i = 0; i < engines.size(); i++) {
+                    final String engineName = engines.get(i);
                     if (regionOverride.has(engineName)) {
-                        engines.add(regionOverride.getString(engineName));
-                    } else {
-                        engines.add(engineName);
+                        engines.set(i, regionOverride.getString(engineName));
                     }
                 }
             }
 
-            for (int i = 0; i < engines.size(); i++) {
-                final InputStream pluginIn = getInputStreamFromSearchPluginsJar(engines.get(i) + ".xml");
-                if (pluginIn != null) {
-                    final SearchEngine engine = createEngineFromInputStream(engines.get(i), pluginIn);
-                    if (engine != null && engine.getName().equals(name)) {
-                        return engine;
-                    }
-                }
-            }
+            return engines;
+
         } catch (Throwable e) {
-            Log.e(LOG_TAG, "Error creating shipped search engine from name: " + name, e);
+            Log.e(LOG_TAG, "Error getting engine list", e);
         }
         return null;
     }
 
     /**
      * Creates a SearchEngine instance for a search plugin in the profile directory.
      *
      * This method iterates through the profile searchplugins directory, creating