Bug 1518514 - Correctly read and parse list.json in search engine manager. r=nalexander
authorMichael Kaply <mozilla@kaply.com>
Thu, 10 Jan 2019 19:07:18 +0000
changeset 513737 b1522f31ad7d8084dcdaddbcb582e2411522a766
parent 513736 5306b81a94f76fc9d867435ff304dbdec9bab203
child 513738 aa50cd5b75d80b2f00b5ff8b337b1f9c88183922
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
bugs1518514
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518514 - Correctly read and parse list.json in search engine manager. r=nalexander Differential Revision: https://phabricator.services.mozilla.com/D15992
mobile/android/base/java/org/mozilla/gecko/search/SearchEngineManager.java
--- a/mobile/android/base/java/org/mozilla/gecko/search/SearchEngineManager.java
+++ b/mobile/android/base/java/org/mozilla/gecko/search/SearchEngineManager.java
@@ -436,46 +436,46 @@ public class SearchEngineManager impleme
         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 region used to fence search engines.
+            // Get the current language
+            final String languageTag = Locales.getLanguageTag(Locale.getDefault());
+
+            // Get the current region
             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();
 
-            if (region != null) {
-                if (json.has(region)) {
-                    final JSONObject regionData = json.getJSONObject(region);
-                    if (regionData.has("searchDefault")) {
-                        Log.d(LOG_TAG, "Found region-specific default engine name in browsersearch.json.");
-                        return regionData.getString("searchDefault");
-                    }
+            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.
+                    region = "default";
+                }
+                final JSONObject regionData = regions.getJSONObject(region);
+                if (regionData.has("searchDefault")) {
+                    return regionData.getString("searchDefault");
                 }
             }
-
-            // Either we have no geoip region, or we didn't find the right region and we are falling back to the default.
-            if (json.has("default")) {
-                final JSONObject defaultData = json.getJSONObject("default");
-                if (defaultData.has("searchDefault")) {
-                  Log.d(LOG_TAG, "Found default engine name in list.json.");
-                  return defaultData.getString("searchDefault");
-                }
-            }
-            // We should never get here
-            Log.e(LOG_TAG, "Error missing defaultSearch in list.json");
+            // 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;
@@ -598,28 +598,54 @@ 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 {
-            String region = GeckoSharedPrefs.forApp(context).getString(PREF_REGION_KEY, null);
+            final String languageTag = Locales.getLanguageTag(Locale.getDefault());
+
+            String region = fetchCountryCode();
+
+            final JSONObject locales = json.getJSONObject("locales");
 
-            JSONArray engines;
-            if (json.has(region)) {
-                engines = json.getJSONObject(region).getJSONArray("visibleDefaultEngines");
+            JSONArray jsonEngines;
+            if (locales.has(languageTag)) {
+                final JSONObject regions = locales.getJSONObject(languageTag);
+                if (!regions.has(region)) {
+                    // Region doesn't exist, use default.
+                    // default always exists.
+                    region = "default";
+                }
+                jsonEngines = regions.getJSONObject(region).getJSONArray("visibleDefaultEngines");
             } else {
-                engines = json.getJSONObject("default").getJSONArray("visibleDefaultEngines");
+                // Falling back to the overall default
+                jsonEngines = locales.getJSONObject("default").getJSONArray("visibleDefaultEngines");
             }
-            for (int i = 0; i < engines.length(); i++) {
-                final InputStream pluginIn = getInputStreamFromSearchPluginsJar(engines.getString(i) + ".xml");
+
+            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);
+                    if (regionOverride.has(engineName)) {
+                        engines.add(regionOverride.getString(engineName));
+                    } else {
+                        engines.add(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.getString(i), pluginIn);
+                    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);
         }
@@ -698,39 +724,18 @@ public class SearchEngineManager impleme
 
     /**
      * Reads a file from the searchplugins directory in the Gecko jar.
      *
      * @param fileName name of the file to read.
      * @return InputStream for file.
      */
     private InputStream getInputStreamFromSearchPluginsJar(String fileName) {
-        final Locale locale = Locale.getDefault();
-
-        // First, try a file path for the full locale.
-        final String languageTag = Locales.getLanguageTag(locale);
-        String url = getSearchPluginsJarURL(context, languageTag, fileName);
-
-        InputStream in = GeckoJarReader.getStream(context, url);
-        if (in != null) {
-            return in;
-        }
-
-        // If that doesn't work, try a file path for just the language.
-        final String language = Locales.getLanguage(locale);
-        if (!languageTag.equals(language)) {
-            url = getSearchPluginsJarURL(context, language, fileName);
-            in = GeckoJarReader.getStream(context, url);
-            if (in != null) {
-                return in;
-            }
-        }
-
-        // Finally, fall back to default locale defined in chrome registry.
-        url = getSearchPluginsJarURL(context, getFallbackLocale(), fileName);
+        final String path = "chrome/chrome/searchplugins/" + fileName;
+        final String url =  GeckoJarReader.getJarURL(context, path);
         return GeckoJarReader.getStream(context, url);
     }
 
     /**
      * Finds a fallback locale in the Gecko chrome registry. If a locale is declared
      * here, we should be guaranteed to find a searchplugins directory for it.
      *
      * This method should only be accessed from the background thread.
@@ -764,28 +769,16 @@ public class SearchEngineManager impleme
                 br.close();
             } catch (IOException e) {
                 // Ignore.
             }
         }
         return fallbackLocale;
     }
 
-    /**
-     * Gets the jar URL for a file in the searchplugins directory.
-     *
-     * @param locale String representing the Gecko locale (e.g. "en-US").
-     * @param fileName The name of the file to read.
-     * @return URL for jar file.
-     */
-    private static String getSearchPluginsJarURL(Context context, String locale, String fileName) {
-        final String path = "chrome/" + locale + "/locale/" + locale + "/browser/searchplugins/" + fileName;
-        return GeckoJarReader.getJarURL(context, path);
-    }
-
     private BufferedReader getBufferedReader(InputStream in) {
         try {
             return new BufferedReader(new InputStreamReader(in, "UTF-8"));
         } catch (UnsupportedEncodingException e) {
             // Cannot happen.
             return null;
         }
     }