Bug 1277333 - Keep track of a reference to GeckoApplication to eliminate "no context NPE" crashes r=sebastian
authorGrigory Kruglov <gkruglov@mozilla.com>
Tue, 26 Jul 2016 10:40:57 -0700
changeset 346763 db3f18dc78a30fec622e17da661254ba6d0335ae
parent 346737 8c361c7cc683e24771a973f2b822fd84605db0ed
child 346764 64c473c3414f8f837e5f524f4e484e562d1d405a
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1277333
milestone50.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 1277333 - Keep track of a reference to GeckoApplication to eliminate "no context NPE" crashes r=sebastian MozReview-Commit-ID: 7jzOflrIcLZ
mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
mobile/android/base/java/org/mozilla/gecko/GeckoNetworkManager.java
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java
@@ -126,19 +126,18 @@ public class GeckoApplication extends Ap
     }
 
     public void onActivityResume(GeckoActivityStatus activity) {
         if (mPausedGecko) {
             GeckoThread.onResume();
             mPausedGecko = false;
         }
 
-        final Context applicationContext = getApplicationContext();
-        GeckoBatteryManager.getInstance().start(applicationContext);
-        GeckoNetworkManager.getInstance().start();
+        GeckoBatteryManager.getInstance().start(this);
+        GeckoNetworkManager.getInstance().start(this);
 
         mInBackground = false;
     }
 
     @Override
     protected void attachBaseContext(Context base) {
         super.attachBaseContext(base);
         AppConstants.maybeInstallMultiDex(base);
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoNetworkManager.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoNetworkManager.java
@@ -44,16 +44,20 @@ import android.util.Log;
  */
 public class GeckoNetworkManager extends BroadcastReceiver implements NativeEventListener {
     private static final String LOGTAG = "GeckoNetworkManager";
 
     private static final String LINK_DATA_CHANGED = "changed";
 
     private static GeckoNetworkManager instance;
 
+    // We hackishly (yet harmlessly, in this case) keep a Context reference passed in via the start method.
+    // See context handling notes in handleManagerEvent, and Bug 1277333.
+    private Context context;
+
     public static void destroy() {
         if (instance != null) {
             instance.onDestroy();
             instance = null;
         }
     }
 
     public enum ManagerState {
@@ -115,17 +119,18 @@ public class GeckoNetworkManager extends
         };
     }
 
     @Override
     public void onReceive(Context aContext, Intent aIntent) {
         handleManagerEvent(ManagerEvent.receivedUpdate);
     }
 
-    public void start() {
+    public void start(final Context context) {
+        this.context = context;
         handleManagerEvent(ManagerEvent.start);
     }
 
     public void stop() {
         handleManagerEvent(ManagerEvent.stop);
     }
 
     public void enableNotifications() {
@@ -147,17 +152,36 @@ public class GeckoNetworkManager extends
         final ManagerState nextState = getNextState(currentState, event);
 
         Log.d(LOGTAG, "Incoming event " + event + " for state " + currentState + " -> " + nextState);
         if (nextState == null) {
             Log.w(LOGTAG, "Invalid event " + event + " for state " + currentState);
             return false;
         }
 
-        performActionsForStateEvent(currentState, event);
+        // We're being deliberately careful about handling context here; it's possible that in some
+        // rare cases and possibly related to timing of when this is called (seems to be early in the startup phase),
+        // GeckoAppShell.getApplicationContext() will be null, and .start() wasn't called yet,
+        // so we don't have a local Context reference either. If both of these are true, we have to drop the event.
+        // NB: this is hacky (and these checks attempt to isolate the hackiness), and root cause
+        // seems to be how this class fits into the larger ecosystem and general flow of events.
+        // See Bug 1277333.
+        final Context contextForAction;
+        if (context != null) {
+            contextForAction = context;
+        } else {
+            contextForAction = GeckoAppShell.getApplicationContext();
+        }
+
+        if (contextForAction == null) {
+            Log.w(LOGTAG, "Context is not available while processing event " + event + " for state " + currentState);
+            return false;
+        }
+
+        performActionsForStateEvent(contextForAction, currentState, event);
         currentState = nextState;
 
         return true;
     }
 
     /**
      * Defines a transition matrix for our state machine. For a given state/event pair, returns nextState.
      *
@@ -216,71 +240,70 @@ public class GeckoNetworkManager extends
     /**
      * For a given state/event combination, run any actions which are by-products of leaving the state
      * because of a given event. Since this is a deterministic state machine, we can easily do that
      * without any additional information.
      *
      * @param currentState State which we are leaving
      * @param event Event which is causing us to leave the state
      */
-    private void performActionsForStateEvent(ManagerState currentState, ManagerEvent event) {
+    private void performActionsForStateEvent(final Context context, final ManagerState currentState, final ManagerEvent event) {
         // NB: network state might be queried via getCurrentInformation at any time; pre-rewrite behaviour was
         // that network state was updated whenever enableNotifications was called. To avoid deviating
         // from previous behaviour and causing weird side-effects, we call updateNetworkStateAndConnectionType
         // whenever notifications are enabled.
         switch (currentState) {
             case OffNoListeners:
                 if (event == ManagerEvent.start) {
-                    updateNetworkStateAndConnectionType();
-                    registerBroadcastReceiver();
+                    updateNetworkStateAndConnectionType(context);
+                    registerBroadcastReceiver(context, this);
                 }
                 if (event == ManagerEvent.enableNotifications) {
-                    updateNetworkStateAndConnectionType();
+                    updateNetworkStateAndConnectionType(context);
                 }
                 break;
             case OnNoListeners:
                 if (event == ManagerEvent.receivedUpdate) {
-                    updateNetworkStateAndConnectionType();
-                    sendNetworkStateToListeners();
+                    updateNetworkStateAndConnectionType(context);
+                    sendNetworkStateToListeners(context);
                 }
                 if (event == ManagerEvent.enableNotifications) {
-                    updateNetworkStateAndConnectionType();
-                    registerBroadcastReceiver();
+                    updateNetworkStateAndConnectionType(context);
+                    registerBroadcastReceiver(context, this);
                 }
                 if (event == ManagerEvent.stop) {
-                    unregisterBroadcastReceiver();
+                    unregisterBroadcastReceiver(context, this);
                 }
                 break;
             case OnWithListeners:
                 if (event == ManagerEvent.receivedUpdate) {
-                    updateNetworkStateAndConnectionType();
-                    sendNetworkStateToListeners();
+                    updateNetworkStateAndConnectionType(context);
+                    sendNetworkStateToListeners(context);
                 }
                 if (event == ManagerEvent.stop) {
-                    unregisterBroadcastReceiver();
+                    unregisterBroadcastReceiver(context, this);
                 }
                 /* no-op event: ManagerEvent.disableNotifications */
                 break;
             case OffWithListeners:
                 if (event == ManagerEvent.start) {
-                    registerBroadcastReceiver();
+                    registerBroadcastReceiver(context, this);
                 }
                 /* no-op event: ManagerEvent.disableNotifications */
                 break;
             default:
                 throw new IllegalStateException("Unknown current state: " + currentState.name());
         }
     }
 
     /**
      * Update current network state and connection types.
      */
-    private void updateNetworkStateAndConnectionType() {
-        final Context applicationContext = GeckoAppShell.getApplicationContext();
-        final ConnectivityManager connectivityManager = (ConnectivityManager) applicationContext.getSystemService(
+    private void updateNetworkStateAndConnectionType(final Context context) {
+        final ConnectivityManager connectivityManager = (ConnectivityManager) context.getSystemService(
                 Context.CONNECTIVITY_SERVICE);
         // Type/status getters below all have a defined behaviour for when connectivityManager == null
         if (connectivityManager == null) {
             Log.e(LOGTAG, "ConnectivityManager does not exist.");
         }
         currentConnectionType = NetworkUtils.getConnectionType(connectivityManager);
         currentNetworkStatus = NetworkUtils.getNetworkStatus(connectivityManager);
         currentConnectionSubtype = NetworkUtils.getConnectionSubType(connectivityManager);
@@ -292,25 +315,25 @@ public class GeckoNetworkManager extends
                                                    boolean isWifi, int DHCPGateway);
 
     @WrapForJNI
     private static native void onStatusChanged(String status);
 
     /**
      * Send current network state and connection type as a GeckoEvent, to whomever is listening.
      */
-    private void sendNetworkStateToListeners() {
+    private void sendNetworkStateToListeners(final Context context) {
         if (currentConnectionType != previousConnectionType ||
                 currentConnectionSubtype != previousConnectionSubtype) {
             previousConnectionType = currentConnectionType;
             previousConnectionSubtype = currentConnectionSubtype;
 
             final boolean isWifi = currentConnectionType == ConnectionType.WIFI;
             final int gateway = !isWifi ? 0 :
-                    wifiDhcpGatewayAddress(GeckoAppShell.getApplicationContext());
+                    wifiDhcpGatewayAddress(context);
 
             if (GeckoThread.isRunning()) {
                 onConnectionChanged(currentConnectionType.value,
                                     currentConnectionSubtype.value, isWifi, gateway);
             } else {
                 GeckoThread.queueNativeCall(GeckoNetworkManager.class, "onConnectionChanged",
                                             currentConnectionType.value,
                                             String.class, currentConnectionSubtype.value,
@@ -333,29 +356,29 @@ public class GeckoNetworkManager extends
             GeckoThread.queueNativeCall(GeckoNetworkManager.class, "onStatusChanged",
                                         String.class, status);
         }
     }
 
     /**
      * Stop listening for network state updates.
      */
-    private void unregisterBroadcastReceiver() {
-        GeckoAppShell.getApplicationContext().unregisterReceiver(this);
+    private static void unregisterBroadcastReceiver(final Context context, final BroadcastReceiver receiver) {
+        context.unregisterReceiver(receiver);
     }
 
     /**
      * Start listening for network state updates.
      */
-    private void registerBroadcastReceiver() {
+    private static void registerBroadcastReceiver(final Context context, final BroadcastReceiver receiver) {
         final IntentFilter filter = new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION);
-        GeckoAppShell.getApplicationContext().registerReceiver(this, filter);
+        context.registerReceiver(receiver, filter);
     }
 
-    private static int wifiDhcpGatewayAddress(Context context) {
+    private static int wifiDhcpGatewayAddress(final Context context) {
         if (context == null) {
             return 0;
         }
 
         try {
             WifiManager mgr = (WifiManager) context.getSystemService(Context.WIFI_SERVICE);
             DhcpInfo d = mgr.getDhcpInfo();
             if (d == null) {
@@ -393,17 +416,17 @@ public class GeckoNetworkManager extends
                 }
                 break;
             case "Wifi:GetIPAddress":
                 getWifiIPAddress(callback);
                 break;
         }
     }
 
-    // This function only works for IPv4
+    // This function only works for IPv4; not part of the state machine flow.
     private void getWifiIPAddress(final EventCallback callback) {
         final WifiManager mgr = (WifiManager) GeckoAppShell.getApplicationContext().getSystemService(Context.WIFI_SERVICE);
 
         if (mgr == null) {
             callback.sendError("Cannot get WifiManager");
             return;
         }
 
@@ -448,16 +471,18 @@ public class GeckoNetworkManager extends
         return -1;
     }
 
     /**
      * These are called from JavaScript ctypes. Avoid letting ProGuard delete them.
      *
      * Note that these methods must only be called after GeckoAppShell has been
      * initialized: they depend on access to the context.
+     *
+     * Not part of the state machine flow.
      */
     @JNITarget
     public static int getMCC() {
         return getNetworkOperator(InfoType.MCC, GeckoAppShell.getApplicationContext());
     }
 
     @JNITarget
     public static int getMNC() {