Bug 1277214 - Stop reading env vars when the key no longer exists. r=grisha
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 01 Jun 2016 16:29:26 -0700
changeset 339105 0262393b42771cc570b55e5b6f31ef437cc6ea4f
parent 339104 ed3c76c1685c4332ba58eeee2a8dc2e0cf70d380
child 339106 21517143f9fce2334df480d58ecca8847e49c952
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgrisha
bugs1277214
milestone49.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 1277214 - Stop reading env vars when the key no longer exists. r=grisha In the previous implementation, we'd stop reading when the value would return null, however, this breaks with SafeIntents where null is returned if a value throws a runtime exception - i.e. we might stop reading at env2 if it throws even though env3+ exist. MozReview-Commit-ID: 7iZgUAjBSmB
mobile/android/base/java/org/mozilla/gecko/util/IntentUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/util/IntentUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/IntentUtils.java
@@ -28,44 +28,58 @@ public class IntentUtils {
      * with the key -> value format:
      *   env# -> ENV_VAR=VALUE
      *
      * # in env# is expected to be increasing from 0.
      *
      * @return A Map of environment variable name to value, e.g. ENV_VAR -> VALUE
      */
     public static HashMap<String, String> getEnvVarMap(@NonNull final Intent unsafeIntent) {
+        // Optimization: get matcher for re-use. Pattern.matcher creates a new object every time so it'd be great
+        // to avoid the unnecessary allocation, particularly because we expect to be called on the startup path.
+        final Pattern envVarPattern = Pattern.compile(ENV_VAR_REGEX);
+        final Matcher matcher = envVarPattern.matcher(""); // argument does not matter here.
+
         // This is expected to be an external intent so we should use SafeIntent to prevent crashing.
         final SafeIntent safeIntent = new SafeIntent(unsafeIntent);
         final HashMap<String, String> out = new HashMap<>();
-        final Pattern envVarPattern = Pattern.compile(ENV_VAR_REGEX);
-        Matcher matcher = null;
-
-        String envValue = safeIntent.getStringExtra("env0");
-        int i = 1;
-        while (envValue != null) {
-            // Optimization to re-use matcher rather than creating new
-            // objects because this is used in the startup path.
-            if (matcher == null) {
-                matcher = envVarPattern.matcher(envValue);
-            } else {
-                matcher.reset(envValue);
+        int i = 0;
+        while (true) {
+            final String envKey = "env" + i;
+            i += 1;
+            if (!unsafeIntent.hasExtra(envKey)) {
+                break;
             }
 
-            if (matcher.matches()) {
-                final String envVarName = matcher.group(1);
-                final String envVarValue = matcher.group(2);
-                out.put(envVarName, envVarValue);
-            }
-            envValue = safeIntent.getStringExtra("env" + i);
-            i += 1;
+            maybeAddEnvVarToEnvVarMap(out, safeIntent, envKey, matcher);
         }
         return out;
     }
 
+    /**
+     * @param envVarMap the map to add the env var to
+     * @param intent the intent from which to extract the env var
+     * @param envKey the key at which the env var resides
+     * @param envVarMatcher a matcher initialized with the env var pattern to extract
+     */
+    private static void maybeAddEnvVarToEnvVarMap(@NonNull final HashMap<String, String> envVarMap,
+            @NonNull final SafeIntent intent, @NonNull final String envKey, @NonNull final Matcher envVarMatcher) {
+        final String envValue = intent.getStringExtra(envKey);
+        if (envValue == null) {
+            return; // nothing to do here!
+        }
+
+        envVarMatcher.reset(envValue);
+        if (envVarMatcher.matches()) {
+            final String envVarName = envVarMatcher.group(1);
+            final String envVarValue = envVarMatcher.group(2);
+            envVarMap.put(envVarName, envVarValue);
+        }
+    }
+
     public static Bundle getBundleExtraSafe(final Intent intent, final String name) {
         return new SafeIntent(intent).getBundleExtra(name);
     }
 
     public static String getStringExtraSafe(final Intent intent, final String name) {
         return new SafeIntent(intent).getStringExtra(name);
     }