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 331797 db3f18dc78a30fec622e17da661254ba6d0335ae
parent 331771 8c361c7cc683e24771a973f2b822fd84605db0ed
child 331798 64c473c3414f8f837e5f524f4e484e562d1d405a
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssebastian
bugs1277333
milestone50.0a1
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() {