Bug 1269407 - Return early from all lifecycle methods if not supported system. r=sebastian a=ritu
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 12 May 2016 15:59:34 -0700
changeset 332946 a9a6da099c5967ec3a751686aee954371484f45d
parent 332945 133e9db008eadefc4426a3731d30f60f0e290a76
child 332947 9ba18aae69e626a3f83560df321d91e42316083a
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian, ritu
bugs1269407
milestone48.0a2
Bug 1269407 - Return early from all lifecycle methods if not supported system. r=sebastian a=ritu This avoids a problem where a lifecycle method may assume a previous lifecycle method initialized some values but we returned early (e.g. on a not supported system) so these values were never initialized and the application may crash. I tested this patch by forcing HardwareUtils.isSupportedSystem to return either true or false (but both were in a supported device configuration). MozReview-Commit-ID: 1WvOId8CLjP
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -1073,16 +1073,19 @@ public class BrowserApp extends GeckoApp
         if (urls != null) {
             openUrls(urls);
         }
     }
 
     @Override
     public void onResume() {
         super.onResume();
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
 
         // Needed for Adjust to get accurate session measurements
         AdjustConstants.getAdjustHelper().onResume();
 
         final String args = ContextUtils.getStringExtra(getIntent(), "args");
         // If an external intent tries to start Fennec in guest mode, and it's not already
         // in guest mode, this will change modes before opening the url.
         // NOTE: OnResume is called twice sometimes when showing on the lock screen.
@@ -1101,32 +1104,46 @@ public class BrowserApp extends GeckoApp
         mScreenshotObserver.start();
 
         mAddToHomeScreenPromotion.resume();
     }
 
     @Override
     public void onPause() {
         super.onPause();
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
 
         // Needed for Adjust to get accurate session measurements
         AdjustConstants.getAdjustHelper().onPause();
 
         // Register for Prompt:ShowTop so we can foreground this activity even if it's hidden.
         EventDispatcher.getInstance().registerGeckoThreadListener((GeckoEventListener) this,
-            "Prompt:ShowTop");
+                "Prompt:ShowTop");
 
         mScreenshotObserver.stop();
 
         mAddToHomeScreenPromotion.pause();
     }
 
     @Override
+    public void onRestart() {
+        super.onRestart();
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
+    }
+
+    @Override
     public void onStart() {
         super.onStart();
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
 
         // Queue this work so that the first launch of the activity doesn't
         // trigger profile init too early.
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
                 final GeckoProfile profile = getProfile();
                 if (profile.inGuestMode()) {
@@ -1153,16 +1170,19 @@ public class BrowserApp extends GeckoApp
         //
         // So we're left with onStart/onStop.
         searchEngineManager.getEngine(new UploadTelemetryCallback(BrowserApp.this));
     }
 
     @Override
     public void onStop() {
         super.onStop();
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
 
         // We only show the guest mode notification when our activity is in the foreground.
         GuestSession.hideNotification(this);
     }
 
     @Override
     public void onWindowFocusChanged(boolean hasFocus) {
         super.onWindowFocusChanged(hasFocus);
@@ -1444,18 +1464,17 @@ public class BrowserApp extends GeckoApp
     @Override
     public void setAccessibilityEnabled(boolean enabled) {
         super.setAccessibilityEnabled(enabled);
         mDynamicToolbar.setAccessibilityEnabled(enabled);
     }
 
     @Override
     public void onDestroy() {
-        if (!HardwareUtils.isSupportedSystem()) {
-            // This build does not support the Android version of the device; Exit early.
+        if (mIsAbortingAppLaunch) {
             super.onDestroy();
             return;
         }
 
         mDynamicToolbar.destroy();
 
         if (mBrowserToolbar != null)
             mBrowserToolbar.onDestroy();
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -173,16 +173,19 @@ public abstract class GeckoApp
     protected RelativeLayout mGeckoLayout;
     private View mCameraView;
     private OrientationEventListener mCameraOrientationEventListener;
     public List<GeckoAppShell.AppStateListener> mAppStateListeners = new LinkedList<GeckoAppShell.AppStateListener>();
     protected MenuPanel mMenuPanel;
     protected Menu mMenu;
     protected boolean mIsRestoringActivity;
 
+    /** Tells if we're aborting app launch, e.g. if this is an unsupported device configuration. */
+    protected boolean mIsAbortingAppLaunch;
+
     private ContactService mContactService;
     private PromptService mPromptService;
     protected TextSelection mTextSelection;
 
     protected DoorHangerPopup mDoorHangerPopup;
     protected FormAssistPopup mFormAssistPopup;
 
 
@@ -1106,16 +1109,17 @@ public abstract class GeckoApp
 
         // Enable Android Strict Mode for developers' local builds (the "default" channel).
         if ("default".equals(AppConstants.MOZ_UPDATE_CHANNEL)) {
             enableStrictMode();
         }
 
         if (!HardwareUtils.isSupportedSystem()) {
             // This build does not support the Android version of the device: Show an error and finish the app.
+            mIsAbortingAppLaunch = true;
             super.onCreate(savedInstanceState);
             showSDKVersionError();
             finish();
             return;
         }
 
         // The clock starts...now. Better hurry!
         mJavaUiStartupTimer = new Telemetry.UptimeTimer("FENNEC_STARTUP_TIME_JAVAUI");
@@ -1362,19 +1366,33 @@ public abstract class GeckoApp
 
         GeckoAppShell.setNotificationClient(makeNotificationClient());
         IntentHelper.init(this);
     }
 
     @Override
     public void onStart() {
         super.onStart();
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
+
         mWasFirstTabShownAfterActivityUnhidden = false; // onStart indicates we were hidden.
     }
 
+    @Override
+    protected void onStop() {
+        super.onStop();
+        // Overriding here is not necessary, but we do this so we don't
+        // forget to add the abort if we override this method later.
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
+    }
+
     /**
      * At this point, the resource system and the rest of the browser are
      * aware of the locale.
      *
      * Now we can display strings!
      *
      * You can think of this as being something like a second phase of onCreate,
      * where you can do string-related operations. Use this in place of embedding
@@ -1941,16 +1959,19 @@ public abstract class GeckoApp
     }
 
     @Override
     public void onResume()
     {
         // After an onPause, the activity is back in the foreground.
         // Undo whatever we did in onPause.
         super.onResume();
+        if (mIsAbortingAppLaunch) {
+            return;
+        }
 
         int newOrientation = getResources().getConfiguration().orientation;
         if (GeckoScreenOrientation.getInstance().update(newOrientation)) {
             refreshChrome();
         }
 
         if (!Versions.feature14Plus) {
             // Update accessibility settings in case it has been changed by the
@@ -2011,16 +2032,21 @@ public abstract class GeckoApp
             mLayerView.setFocusableInTouchMode(true);
             getWindow().setBackgroundDrawable(null);
         }
     }
 
     @Override
     public void onPause()
     {
+        if (mIsAbortingAppLaunch) {
+            super.onPause();
+            return;
+        }
+
         final HealthRecorder rec = mHealthRecorder;
         final Context context = this;
 
         // In some way it's sad that Android will trigger StrictMode warnings
         // here as the whole point is to save to disk while the activity is not
         // interacting with the user.
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
@@ -2055,32 +2081,37 @@ public abstract class GeckoApp
             }
         }
 
         super.onPause();
     }
 
     @Override
     public void onRestart() {
+        if (mIsAbortingAppLaunch) {
+            super.onRestart();
+            return;
+        }
+
         // Faster on main thread with an async apply().
         final StrictMode.ThreadPolicy savedPolicy = StrictMode.allowThreadDiskReads();
         try {
             SharedPreferences.Editor editor = GeckoApp.this.getSharedPreferences().edit();
             editor.putBoolean(GeckoApp.PREFS_WAS_STOPPED, false);
             editor.apply();
         } finally {
             StrictMode.setThreadPolicy(savedPolicy);
         }
 
         super.onRestart();
     }
 
     @Override
     public void onDestroy() {
-        if (!HardwareUtils.isSupportedSystem()) {
+        if (mIsAbortingAppLaunch) {
             // This build does not support the Android version of the device:
             // We did not initialize anything, so skip cleaning up.
             super.onDestroy();
             return;
         }
 
         EventDispatcher.getInstance().unregisterGeckoThreadListener((GeckoEventListener)this,
             "Gecko:Ready",