Bug 1299939 - Use SafeIntent throughout BrowserApp and delegates r=rnewman
authorGrigory Kruglov <gkruglov@mozilla.com>
Thu, 01 Sep 2016 18:25:05 -0700
changeset 312449 adc017f6ddf8e6bd8604b3cef97d513e95322a64
parent 312448 dbf9f64add05eb4a0d75f2d6a62a37b91ebc5407
child 312450 1bb4fec092645bd00840ec15605444921ffa3749
push id20447
push userkwierso@gmail.com
push dateFri, 02 Sep 2016 20:36:44 +0000
treeherderfx-team@969397f22187 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1299939
milestone51.0a1
Bug 1299939 - Use SafeIntent throughout BrowserApp and delegates r=rnewman MozReview-Commit-ID: gW2gXVWji1
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/GuestSession.java
mobile/android/base/java/org/mozilla/gecko/delegates/BrowserAppDelegate.java
mobile/android/base/java/org/mozilla/gecko/feeds/ContentNotificationsDelegate.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/SafeIntent.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/IntentUtils.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestIntentUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -559,17 +559,17 @@ public class BrowserApp extends GeckoApp
     @Override
     public void onCreate(Bundle savedInstanceState) {
         if (!HardwareUtils.isSupportedSystem()) {
             // This build does not support the Android version of the device; Exit early.
             super.onCreate(savedInstanceState);
             return;
         }
 
-        final Intent intent = getIntent();
+        final SafeIntent intent = new SafeIntent(getIntent());
         final boolean isInAutomation = getIsInAutomationFromEnvironment(intent);
 
         // This has to be prepared prior to calling GeckoApp.onCreate, because
         // widget code and BrowserToolbar need it, and they're created by the
         // layout, which GeckoApp takes care of.
         ((GeckoApplication) getApplication()).prepareLightweightTheme();
         super.onCreate(savedInstanceState);
 
@@ -634,17 +634,17 @@ public class BrowserApp extends GeckoApp
             showTabQueuePromptIfApplicable(intent);
         } else if (ACTION_VIEW_MULTIPLE.equals(action) && savedInstanceState == null) {
             // We only want to handle this intent if savedInstanceState is null. In the case where
             // savedInstanceState is not null this activity is being re-created and we already
             // opened tabs for the URLs the last time. Our session store will take care of restoring
             // them.
             openMultipleTabsFromIntent(intent);
         } else if (GuestSession.NOTIFICATION_INTENT.equals(action)) {
-            GuestSession.handleIntent(this, intent);
+            GuestSession.onNotificationIntentReceived(this);
         } else if (TabQueueHelper.LOAD_URLS_ACTION.equals(action)) {
             Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.NOTIFICATION, "tabqueue");
         }
 
         if (HardwareUtils.isTablet()) {
             mTabStrip = (TabStripInterface) (((ViewStub) findViewById(R.id.tablet_tab_strip)).inflate());
         }
 
@@ -795,35 +795,35 @@ public class BrowserApp extends GeckoApp
      * We need to read environment variables from the intent string
      * extra because environment variables from our test harness aren't set
      * until Gecko is loaded, and we need to know this before then.
      *
      * The return value of this method should be used early since other
      * initialization may depend on its results.
      */
     @CheckResult
-    private boolean getIsInAutomationFromEnvironment(final Intent intent) {
+    private boolean getIsInAutomationFromEnvironment(final SafeIntent intent) {
         final HashMap<String, String> envVars = IntentUtils.getEnvVarMap(intent);
         return !TextUtils.isEmpty(envVars.get(IntentUtils.ENV_VAR_IN_AUTOMATION));
     }
 
     /**
      * Initializes the default Switchboard URLs the first time.
      * @param intent
      */
-    private static void initSwitchboard(final Context context, final Intent intent, final boolean isInAutomation) {
+    private static void initSwitchboard(final Context context, final SafeIntent intent, final boolean isInAutomation) {
         if (isInAutomation) {
             Log.d(LOGTAG, "Switchboard disabled - in automation");
             return;
         } else if (!AppConstants.MOZ_SWITCHBOARD) {
             Log.d(LOGTAG, "Switchboard compile-time disabled");
             return;
         }
 
-        final String serverExtra = IntentUtils.getStringExtraSafe(intent, INTENT_KEY_SWITCHBOARD_SERVER);
+        final String serverExtra = intent.getStringExtra(INTENT_KEY_SWITCHBOARD_SERVER);
         final String serverUrl = TextUtils.isEmpty(serverExtra) ? SWITCHBOARD_SERVER : serverExtra;
         new AsyncConfigLoader(context, serverUrl).execute();
     }
 
     private static void initTelemetryUploader(final boolean isInAutomation) {
         TelemetryUploadService.setDisabled(isInAutomation);
     }
 
@@ -1025,17 +1025,17 @@ public class BrowserApp extends GeckoApp
                 @Override
                 public void run() {
                     showNormalTabs();
                 }
             });
         }
     }
 
-    private void openMultipleTabsFromIntent(final Intent intent) {
+    private void openMultipleTabsFromIntent(final SafeIntent intent) {
         final List<String> urls = intent.getStringArrayListExtra("urls");
         if (urls != null) {
             openUrls(urls);
         }
     }
 
     @Override
     public void onResume() {
@@ -3777,17 +3777,18 @@ public class BrowserApp extends GeckoApp
         return super.onKeyLongPress(keyCode, event);
     }
 
     /*
      * If the app has been launched a certain number of times, and we haven't asked for feedback before,
      * open a new tab with about:feedback when launching the app from the icon shortcut.
      */
     @Override
-    protected void onNewIntent(Intent intent) {
+    protected void onNewIntent(Intent externalIntent) {
+        final SafeIntent intent = new SafeIntent(externalIntent);
         String action = intent.getAction();
 
         final boolean isViewAction = Intent.ACTION_VIEW.equals(action);
         final boolean isBookmarkAction = GeckoApp.ACTION_HOMESCREEN_SHORTCUT.equals(action);
         final boolean isTabQueueAction = TabQueueHelper.LOAD_URLS_ACTION.equals(action);
         final boolean isViewMultipleAction = ACTION_VIEW_MULTIPLE.equals(action);
 
         if (mInitialized && (isViewAction || isBookmarkAction)) {
@@ -3801,26 +3802,27 @@ public class BrowserApp extends GeckoApp
                 // GeckoApp.ACTION_HOMESCREEN_SHORTCUT means we're opening a bookmark that
                 // was added to Android's homescreen.
                 Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.HOMESCREEN);
             }
         }
 
         showTabQueuePromptIfApplicable(intent);
 
-        super.onNewIntent(intent);
+        // GeckoApp will wrap this unsafe external intent in a SafeIntent.
+        super.onNewIntent(externalIntent);
 
         if (AppConstants.MOZ_ANDROID_BEAM && NfcAdapter.ACTION_NDEF_DISCOVERED.equals(action)) {
             String uri = intent.getDataString();
             mLayerView.loadUri(uri, GeckoView.LOAD_NEW_TAB);
         }
 
         // Only solicit feedback when the app has been launched from the icon shortcut.
         if (GuestSession.NOTIFICATION_INTENT.equals(action)) {
-            GuestSession.handleIntent(this, intent);
+            GuestSession.onNotificationIntentReceived(this);
         }
 
         // If the user has clicked the tab queue notification then load the tabs.
         if (TabQueueHelper.TAB_QUEUE_ENABLED && mInitialized && isTabQueueAction) {
             Telemetry.sendUIEvent(TelemetryContract.Event.ACTION, TelemetryContract.Method.NOTIFICATION, "tabqueue");
             ThreadUtils.postToBackgroundThread(new Runnable() {
                 @Override
                 public void run() {
@@ -3876,17 +3878,17 @@ public class BrowserApp extends GeckoApp
             object.put("urls", array);
 
             GeckoAppShell.notifyObservers("Tabs:OpenMultiple", object.toString());
         } catch (JSONException e) {
             Log.e(LOGTAG, "Unable to create JSON for opening multiple URLs");
         }
     }
 
-    private void showTabQueuePromptIfApplicable(final Intent intent) {
+    private void showTabQueuePromptIfApplicable(final SafeIntent intent) {
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
                 // We only want to show the prompt if the browser has been opened from an external url
                 if (TabQueueHelper.TAB_QUEUE_ENABLED && mInitialized
                                                      && Intent.ACTION_VIEW.equals(intent.getAction())
                                                      && !intent.getBooleanExtra(BrowserContract.SKIP_TAB_QUEUE_FLAG, false)
                                                      && TabQueueHelper.shouldShowTabQueuePrompt(BrowserApp.this)) {
--- a/mobile/android/base/java/org/mozilla/gecko/GuestSession.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GuestSession.java
@@ -52,13 +52,13 @@ public final class GuestSession {
         manager.notify(R.id.guestNotification, builder.build());
     }
 
     public static void hideNotification(Context context) {
         final NotificationManager manager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
         manager.cancel(R.id.guestNotification);
     }
 
-    public static void handleIntent(BrowserApp context, Intent intent) {
+    public static void onNotificationIntentReceived(BrowserApp context) {
         context.showGuestModeDialog(BrowserApp.GuestModeDialog.LEAVING);
     }
 
 }
--- a/mobile/android/base/java/org/mozilla/gecko/delegates/BrowserAppDelegate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/delegates/BrowserAppDelegate.java
@@ -4,16 +4,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.delegates;
 
 import android.content.Intent;
 import android.os.Bundle;
 
 import org.mozilla.gecko.BrowserApp;
+import org.mozilla.gecko.mozglue.SafeIntent;
 import org.mozilla.gecko.tabs.TabsPanel;
 
 /**
  * Abstract class for extending the behavior of BrowserApp without adding additional code to the
  * already huge class.
  */
 public abstract class BrowserAppDelegate {
     /**
@@ -49,17 +50,17 @@ public abstract class BrowserAppDelegate
     /**
      * The final call before the BrowserApp activity is destroyed.
      */
     public void onDestroy(BrowserApp browserApp) {}
 
     /**
      * Called when BrowserApp already exists and a new Intent to re-launch it was fired.
      */
-    public void onNewIntent(BrowserApp browserApp, Intent intent) {}
+    public void onNewIntent(BrowserApp browserApp, SafeIntent intent) {}
 
     /**
      * Called when the tabs tray is opened.
      */
     public void onTabsTrayShown(BrowserApp browserApp, TabsPanel tabsPanel) {}
 
     /**
      * Called when the tabs tray is closed.
--- a/mobile/android/base/java/org/mozilla/gecko/feeds/ContentNotificationsDelegate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/feeds/ContentNotificationsDelegate.java
@@ -2,24 +2,26 @@
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.feeds;
 
 import android.content.Intent;
 import android.os.Bundle;
+import android.support.annotation.NonNull;
 import android.support.v4.app.NotificationManagerCompat;
 
 import org.mozilla.gecko.AppConstants;
 import org.mozilla.gecko.BrowserApp;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.Telemetry;
 import org.mozilla.gecko.TelemetryContract;
 import org.mozilla.gecko.delegates.BrowserAppDelegate;
+import org.mozilla.gecko.mozglue.SafeIntent;
 
 import java.util.List;
 
 /**
  * BrowserAppDelegate implementation that takes care of handling intents from content notifications.
  */
 public class ContentNotificationsDelegate extends BrowserAppDelegate {
     // The application is opened from a content notification
@@ -28,37 +30,45 @@ public class ContentNotificationsDelegat
     public static final String EXTRA_READ_BUTTON = "read_button";
     public static final String EXTRA_URLS = "urls";
 
     private static final String TELEMETRY_EXTRA_CONTENT_UPDATE = "content_update";
     private static final String TELEMETRY_EXTRA_READ_NOW_BUTTON = TELEMETRY_EXTRA_CONTENT_UPDATE + "_read_now";
 
     @Override
     public void onCreate(BrowserApp browserApp, Bundle savedInstanceState) {
-        final Intent intent = browserApp.getIntent();
-
         if (savedInstanceState != null) {
             // This activity is getting restored: We do not want to handle the URLs in the Intent again. The browser
             // will take care of restoring the tabs we already created.
             return;
         }
 
-        if (intent != null && ACTION_CONTENT_NOTIFICATION.equals(intent.getAction())) {
+
+        final Intent unsafeIntent = browserApp.getIntent();
+
+        // Nothing to do.
+        if (unsafeIntent == null) {
+            return;
+        }
+
+        final SafeIntent intent = new SafeIntent(unsafeIntent);
+
+        if (ACTION_CONTENT_NOTIFICATION.equals(intent.getAction())) {
             openURLsFromIntent(browserApp, intent);
         }
     }
 
     @Override
-    public void onNewIntent(BrowserApp browserApp, Intent intent) {
-        if (intent != null && ACTION_CONTENT_NOTIFICATION.equals(intent.getAction())) {
+    public void onNewIntent(BrowserApp browserApp, @NonNull final SafeIntent intent) {
+        if (ACTION_CONTENT_NOTIFICATION.equals(intent.getAction())) {
             openURLsFromIntent(browserApp, intent);
         }
     }
 
-    private void openURLsFromIntent(BrowserApp browserApp, final Intent intent) {
+    private void openURLsFromIntent(BrowserApp browserApp, @NonNull final SafeIntent intent) {
         final List<String> urls = intent.getStringArrayListExtra(EXTRA_URLS);
         if (urls != null) {
             browserApp.openUrls(urls);
         }
 
         Telemetry.startUISession(TelemetryContract.Session.EXPERIMENT, FeedService.getEnabledExperiment(browserApp));
 
         Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.INTENT, TELEMETRY_EXTRA_CONTENT_UPDATE);
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/SafeIntent.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/mozglue/SafeIntent.java
@@ -7,16 +7,18 @@
 // This should be in util/, but is here because of build dependency issues.
 package org.mozilla.gecko.mozglue;
 
 import android.content.Intent;
 import android.net.Uri;
 import android.os.Bundle;
 import android.util.Log;
 
+import java.util.ArrayList;
+
 /**
  * External applications can pass values into Intents that can cause us to crash: in defense,
  * we wrap {@link Intent} and catch the exceptions they may force us to throw. See bug 1090385
  * for more.
  */
 public class SafeIntent {
     private static final String LOGTAG = "Gecko" + SafeIntent.class.getSimpleName();
 
@@ -97,16 +99,28 @@ public class SafeIntent {
             Log.w(LOGTAG, "Couldn't get intent data string: OOM. Malformed?");
             return null;
         } catch (RuntimeException e) {
             Log.w(LOGTAG, "Couldn't get intent data string.", e);
             return null;
         }
     }
 
+    public ArrayList<String> getStringArrayListExtra(final String name) {
+        try {
+            return intent.getStringArrayListExtra(name);
+        } catch (OutOfMemoryError e) {
+            Log.w(LOGTAG, "Couldn't get intent data string: OOM. Malformed?");
+            return null;
+        } catch (RuntimeException e) {
+            Log.w(LOGTAG, "Couldn't get intent data string.", e);
+            return null;
+        }
+    }
+
     public Uri getData() {
         try {
             return intent.getData();
         } catch (OutOfMemoryError e) {
             Log.w(LOGTAG, "Couldn't get intent data: OOM. Malformed?");
             return null;
         } catch (RuntimeException e) {
             Log.w(LOGTAG, "Couldn't get intent data.", e);
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/IntentUtils.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/IntentUtils.java
@@ -29,34 +29,33 @@ public class IntentUtils {
      * Returns a list of environment variables and their values. These are parsed from an Intent extra
      * 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) {
+    public static HashMap<String, String> getEnvVarMap(@NonNull final SafeIntent intent) {
         // 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<>();
         int i = 0;
         while (true) {
             final String envKey = "env" + i;
             i += 1;
-            if (!unsafeIntent.hasExtra(envKey)) {
+            if (!intent.hasExtra(envKey)) {
                 break;
             }
 
-            maybeAddEnvVarToEnvVarMap(out, safeIntent, envKey, matcher);
+            maybeAddEnvVarToEnvVarMap(out, intent, 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
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestIntentUtils.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/util/TestIntentUtils.java
@@ -6,16 +6,17 @@
 
 package org.mozilla.gecko.util;
 
 import android.content.Intent;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
+import org.mozilla.gecko.mozglue.SafeIntent;
 
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
 import static org.junit.Assert.*;
 
 /**
@@ -54,17 +55,17 @@ public class TestIntentUtils {
             out.putExtra("env" + i, value);
             i += 1;
         }
         return out;
    }
 
     @Test
     public void testGetEnvVarMap() throws Exception {
-        final HashMap<String, String> actual = IntentUtils.getEnvVarMap(testIntent);
+        final HashMap<String, String> actual = IntentUtils.getEnvVarMap(new SafeIntent(testIntent));
         for (final String actualEnvVarName : actual.keySet()) {
             assertTrue("Actual key exists in test data: " + actualEnvVarName,
                     TEST_ENV_VAR_MAP.containsKey(actualEnvVarName));
 
             final String expectedValue = TEST_ENV_VAR_MAP.get(actualEnvVarName);
             final String actualValue = actual.get(actualEnvVarName);
             assertEquals("Actual env var value matches test data", expectedValue, actualValue);
         }