Bug 1326101 - 2. Fix some GeckoBundle bugs; r?snorp draft
authorJim Chen <nchen@mozilla.com>
Mon, 29 Jan 2018 11:53:02 -0500
changeset 748319 daab03e932dcfa7fe438a9691587a6b7ee3cc8ce
parent 748318 02696b9ebeae3f44759c8a205eba9f333a9372b1
push id97130
push userbmo:nchen@mozilla.com
push dateMon, 29 Jan 2018 16:54:01 +0000
reviewerssnorp
bugs1326101
milestone60.0a1
Bug 1326101 - 2. Fix some GeckoBundle bugs; r?snorp Fix some GeckoBundle tests uncovered by the new unit test. MozReview-Commit-ID: FEKryW81R2y
mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/GeckoBundle.java
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/EventDispatcher.java
@@ -118,17 +118,17 @@ public final class EventDispatcher exten
                         listenersMap.put(event, listeners);
                     }
                     if (!BuildConfig.RELEASE_OR_BETA && listeners.contains(listener)) {
                         throw new IllegalStateException("Already registered " + event);
                     }
                     listeners.add(listener);
                 }
             }
-        } catch (final IllegalAccessException | InstantiationException e) {
+        } catch (final Exception e) {
             throw new IllegalArgumentException("Invalid new list type", e);
         }
     }
 
     private void checkNotRegisteredElsewhere(final Map<String, ?> allowedMap,
                                              final String[] events) {
         if (BuildConfig.RELEASE_OR_BETA) {
             // for performance reasons, we only check for
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/GeckoBundle.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/GeckoBundle.java
@@ -93,23 +93,24 @@ public final class GeckoBundle implement
     /**
      * Clear all mappings.
      */
     public void clear() {
         mMap.clear();
     }
 
     /**
-     * Returns whether a mapping exists.
+     * Returns whether a mapping exists. Null String, Bundle, or arrays are treated as
+     * nonexistent.
      *
      * @param key Key to look for.
-     * @return True if the specified key exists.
+     * @return True if the specified key exists and the value is not null.
      */
     public boolean containsKey(final String key) {
-        return mMap.containsKey(key) && mMap.get(key) != null;
+        return mMap.get(key) != null;
     }
 
     /**
      * Returns the value associated with a mapping as an Object.
      *
      * @param key Key to look for.
      * @return Mapping value or null if the mapping does not exist.
      */
@@ -246,20 +247,23 @@ public final class GeckoBundle implement
                value instanceof double[] ? getIntArray((double[]) value) : (int[]) value;
     }
 
     /**
      * Returns the value associated with a String mapping, or defaultValue if the mapping
      * does not exist.
      *
      * @param key Key to look for.
-     * @param defaultValue Value to return if mapping does not exist.
+     * @param defaultValue Value to return if mapping value is null or mapping does not exist.
      * @return String value
      */
     public String getString(final String key, final String defaultValue) {
+        // If the key maps to null, technically we should return null because the mapping
+        // exists and null is a valid string value. However, people expect the default
+        // value to be returned instead, so we make an exception to return the default value.
         final Object value = mMap.get(key);
         return value == null ? defaultValue : (String) value;
     }
 
     /**
      * Returns the value associated with a String mapping, or null if the mapping does not
      * exist.
      *
@@ -736,18 +740,18 @@ public final class GeckoBundle implement
             final Object value = mMap.valueAt(i);
             final Object jsonValue;
 
             if (value instanceof GeckoBundle) {
                 jsonValue = ((GeckoBundle) value).toJSONObject();
             } else if (value instanceof GeckoBundle[]) {
                 final GeckoBundle[] array = (GeckoBundle[]) value;
                 final JSONArray jsonArray = new JSONArray();
-                for (int j = 0; j < array.length; j++) {
-                    jsonArray.put(array[j] == null ? JSONObject.NULL : array[j].toJSONObject());
+                for (final GeckoBundle element : array) {
+                    jsonArray.put(element == null ? JSONObject.NULL : element.toJSONObject());
                 }
                 jsonValue = jsonArray;
             } else if (Build.VERSION.SDK_INT >= 19) {
                 final Object wrapped = JSONObject.wrap(value);
                 jsonValue = wrapped != null ? wrapped : value.toString();
             } else if (value == null) {
                 jsonValue = JSONObject.NULL;
             } else if (value.getClass().isArray()) {
@@ -844,17 +848,19 @@ public final class GeckoBundle implement
             }
 
             i++;
         }
         return new GeckoBundle(keys, values);
     }
 
     private static Object fromJSONValue(Object value) throws JSONException {
-        if (value instanceof JSONObject || value == JSONObject.NULL) {
+        if (value == null || value == JSONObject.NULL) {
+            return null;
+        } else if (value instanceof JSONObject) {
             return fromJSONObject((JSONObject) value);
         }
         if (value instanceof JSONArray) {
             final JSONArray array = (JSONArray) value;
             final int len = array.length();
             if (len == 0) {
                 return EMPTY_BOOLEAN_ARRAY;
             }
@@ -888,17 +894,17 @@ public final class GeckoBundle implement
             return value;
         }
         if (value instanceof Byte || value instanceof Short) {
             return ((Number) value).intValue();
         }
         if (value instanceof Float || value instanceof Long) {
             return ((Number) value).doubleValue();
         }
-        return value != null ? value.toString() : null;
+        return value.toString();
     }
 
     public static GeckoBundle fromJSONObject(final JSONObject obj) throws JSONException {
         if (obj == null || obj == JSONObject.NULL) {
             return null;
         }
 
         final String[] keys = new String[obj.length()];