Bug 895775 - Correctly handle lifecycle in GeckoNetworkManager. r=rnewman a=lmandel
authorMark Finkle <mfinkle@mozilla.com>
Sun, 26 Oct 2014 17:27:48 -0700
changeset 225874 ae19708887ef
parent 225873 65515de095b8
child 225877 0dd6a59ed6a5
push id4049
push usermfinkle@mozilla.com
push date2014-10-30 19:53 +0000
treeherdermozilla-beta@ae19708887ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman, lmandel
bugs895775
milestone34.0
Bug 895775 - Correctly handle lifecycle in GeckoNetworkManager. r=rnewman a=lmandel
mobile/android/base/GeckoAppShell.java
mobile/android/base/GeckoNetworkManager.java
mobile/android/base/tests/robocop.ini
mobile/android/base/tests/testNetworkManager.java
mobile/android/base/tests/testNetworkManager.js
--- a/mobile/android/base/GeckoAppShell.java
+++ b/mobile/android/base/GeckoAppShell.java
@@ -2432,22 +2432,32 @@ public class GeckoAppShell
 
     @WrapElementForJNI(stubName = "GetCurrentNetworkInformationWrapper")
     public static double[] getCurrentNetworkInformation() {
         return GeckoNetworkManager.getInstance().getCurrentInformation();
     }
 
     @WrapElementForJNI
     public static void enableNetworkNotifications() {
-        GeckoNetworkManager.getInstance().enableNotifications();
+        ThreadUtils.postToUiThread(new Runnable() {
+            @Override
+            public void run() {
+                GeckoNetworkManager.getInstance().enableNotifications();
+            }
+        });
     }
 
     @WrapElementForJNI
     public static void disableNetworkNotifications() {
-        GeckoNetworkManager.getInstance().disableNotifications();
+        ThreadUtils.postToUiThread(new Runnable() {
+            @Override
+            public void run() {
+                GeckoNetworkManager.getInstance().disableNotifications();
+            }
+        });
     }
 
     /**
      * Decodes a byte array from Base64 format.
      * No blanks or line breaks are allowed within the Base64 encoded input data.
      * @param s     A string containing the Base64 encoded data.
      * @return      An array containing the decoded data bytes.
      * @throws      IllegalArgumentException If the input is not valid Base64 encoded data.
--- a/mobile/android/base/GeckoNetworkManager.java
+++ b/mobile/android/base/GeckoNetworkManager.java
@@ -32,17 +32,17 @@ import android.util.Log;
  * Current connection is firstly obtained from Android's ConnectivityManager,
  * which is represented by the constant, and then will be mapped into the
  * connection type defined in Network Information API version 3.
  */
 
 public class GeckoNetworkManager extends BroadcastReceiver implements NativeEventListener {
     private static final String LOGTAG = "GeckoNetworkManager";
 
-    static private GeckoNetworkManager sInstance;
+    private static GeckoNetworkManager sInstance;
 
     public static void destroy() {
         if (sInstance != null) {
             sInstance.onDestroy();
             sInstance = null;
         }
     }
 
@@ -70,29 +70,30 @@ public class GeckoNetworkManager extends
     private GeckoNetworkManager() {
         EventDispatcher.getInstance().registerGeckoThreadListener(this, "Wifi:Enable");
     }
 
     private void onDestroy() {
         EventDispatcher.getInstance().unregisterGeckoThreadListener(this, "Wifi:Enable");
     }
 
-    private ConnectionType mConnectionType = ConnectionType.NONE;
+    private volatile ConnectionType mConnectionType = ConnectionType.NONE;
     private final IntentFilter mNetworkFilter = new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION);
 
     // Whether the manager should be listening to Network Information changes.
     private boolean mShouldBeListening;
 
     // Whether the manager should notify Gecko that a change in Network
     // Information happened.
     private boolean mShouldNotify;
 
     // The application context used for registering receivers, so
     // we can unregister them again later.
     private volatile Context mApplicationContext;
+    private boolean mIsListening;
 
     public static GeckoNetworkManager getInstance() {
         if (sInstance == null) {
             sInstance = new GeckoNetworkManager();
         }
 
         return sInstance;
     }
@@ -113,24 +114,33 @@ public class GeckoNetworkManager extends
         updateConnectionType();
 
         if (mShouldNotify) {
             startListening();
         }
     }
 
     private void startListening() {
+        if (mIsListening) {
+            Log.w(LOGTAG, "Already started!");
+            return;
+        }
+
         final Context appContext = mApplicationContext;
         if (appContext == null) {
             Log.w(LOGTAG, "Not registering receiver: no context!");
             return;
         }
 
-        Log.v(LOGTAG, "Registering receiver.");
-        appContext.registerReceiver(this, mNetworkFilter);
+        // registerReceiver will return null if registering fails.
+        if (appContext.registerReceiver(this, mNetworkFilter) == null) {
+            Log.e(LOGTAG, "Registering receiver failed");
+        } else {
+            mIsListening = true;
+        }
     }
 
     public void stop() {
         mShouldBeListening = false;
 
         if (mShouldNotify) {
             stopListening();
         }
@@ -153,17 +163,23 @@ public class GeckoNetworkManager extends
         }
     }
 
     private void stopListening() {
         if (null == mApplicationContext) {
             return;
         }
 
+        if (!mIsListening) {
+            Log.w(LOGTAG, "Already stopped!");
+            return;
+        }
+
         mApplicationContext.unregisterReceiver(this);
+        mIsListening = false;
     }
 
     private int wifiDhcpGatewayAddress() {
         if (mConnectionType != ConnectionType.WIFI) {
             return 0;
         }
 
         if (null == mApplicationContext) {
@@ -183,33 +199,39 @@ public class GeckoNetworkManager extends
             // getDhcpInfo() is not documented to require any permissions, but on some devices
             // requires android.permission.ACCESS_WIFI_STATE. Just catch the generic exception
             // here and returning 0. Not logging because this could be noisy.
             return 0;
         }
     }
 
     private void updateConnectionType() {
-        ConnectionType previousConnectionType = mConnectionType;
-        mConnectionType = getConnectionType();
+        final ConnectionType previousConnectionType = mConnectionType;
+        final ConnectionType newConnectionType = getConnectionType();
+        if (newConnectionType == previousConnectionType) {
+            return;
+        }
 
-        if (mConnectionType == previousConnectionType || !mShouldNotify) {
+        mConnectionType = newConnectionType;
+
+        if (!mShouldNotify) {
             return;
         }
 
         GeckoAppShell.sendEventToGecko(GeckoEvent.createNetworkEvent(
-                                       mConnectionType.value,
-                                       mConnectionType == ConnectionType.WIFI,
+                                       newConnectionType.value,
+                                       newConnectionType == ConnectionType.WIFI,
                                        wifiDhcpGatewayAddress()));
     }
 
     public double[] getCurrentInformation() {
-        return new double[] { mConnectionType.value,
-                              (mConnectionType == ConnectionType.WIFI) ? 1.0 : 0.0,
-                              wifiDhcpGatewayAddress()};
+        final ConnectionType connectionType = mConnectionType;
+        return new double[] { connectionType.value,
+                              connectionType == ConnectionType.WIFI ? 1.0 : 0.0,
+                              wifiDhcpGatewayAddress() };
     }
 
     public void enableNotifications() {
         // We set mShouldNotify *after* calling updateConnectionType() to make sure we
         // don't notify an eventual change in mConnectionType.
         mConnectionType = ConnectionType.NONE; // force a notification
         updateConnectionType();
         mShouldNotify = true;
--- a/mobile/android/base/tests/robocop.ini
+++ b/mobile/android/base/tests/robocop.ini
@@ -98,16 +98,17 @@ skip-if = android_version == "10"
 # Using JavascriptTest
 [testAccounts]
 [testAndroidLog]
 [testBrowserDiscovery]
 [testDebuggerServer]
 [testDeviceSearchEngine]
 [testJNI]
 # [testMozPay] # see bug 945675
+[testNetworkManager]
 [testOrderedBroadcast]
 [testResourceSubstitutions]
 [testRestrictedProfiles]
 [testSharedPreferences]
 [testSimpleDiscovery]
 [testUITelemetry]
 [testVideoDiscovery]
 
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/tests/testNetworkManager.java
@@ -0,0 +1,11 @@
+/* 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.tests;
+
+public class testNetworkManager extends JavascriptTest {
+    public testNetworkManager() {
+        super("testNetworkManager.js");
+    }
+}
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/tests/testNetworkManager.js
@@ -0,0 +1,28 @@
+// -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
+/* 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/. */
+
+"use strict";
+
+const { classes: Cc, interfaces: Ci, utils: Cu } = Components;
+
+Cu.import("resource://gre/modules/Services.jsm");
+
+function ok(passed, text) {
+  do_report_result(passed, text, Components.stack.caller, false);
+}
+
+add_test(function check_linktype() {
+  // Let's exercise the interface. Even if the network is not up, we can make sure nothing blows up.
+  let network = Cc["@mozilla.org/network/network-link-service;1"].getService(Ci.nsINetworkLinkService);
+  if (network.isLinkUp) {
+    ok(network.linkType != Ci.nsINetworkLinkService.LINK_TYPE_UNKNOWN, "LinkType is not UNKNOWN");
+  } else {
+    ok(network.linkType == Ci.nsINetworkLinkService.LINK_TYPE_UNKNOWN, "LinkType is UNKNOWN");
+  }
+
+  run_next_test();
+});
+
+run_next_test();