Bug 1269924 - Move telemetry upload to NetworkUtils.isConnected. r=grisha draft
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 19 May 2016 11:54:54 -0700
changeset 369836 4640aad21783f8e8edc568ea341a6e910a066d01
parent 369835 3ff4077f9e4bf923322c95fb7226bc0e9090f898
child 521628 25382d2505d78c8d099dc5e9a9a4b7075b3d7dd3
push id18926
push usermichael.l.comella@gmail.com
push dateMon, 23 May 2016 20:34:27 +0000
reviewersgrisha
bugs1269924
milestone49.0a1
Bug 1269924 - Move telemetry upload to NetworkUtils.isConnected. r=grisha The concept of "background data" (as it exists in the Android options menu) doesn't exist in the Android APIs - I think it should be covered under isConnected. Thus, I removed our `isBackgroundDataEnabled` method. One other network consideration, however: we may want to consider stopping uploads on roaming. In the previous implementation, we did not queue the ping for upload if the network was not connected (in order to conserve disk space). However, this doesn't allow us to see all of the days the user interacted with the device (e.g. for engagement) so in this implementation, we always queue the ping and stop in the UploadService if we're not connected. MozReview-Commit-ID: 1mjnHq3l7Jj
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
mobile/android/base/java/org/mozilla/gecko/util/NetworkUtils.java
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
@@ -144,16 +144,21 @@ public class TelemetryUploadService exte
     private static boolean isReadyToUpload(final Context context, final Intent intent) {
         // Sanity check: is upload enabled? Generally, the caller should check this before starting the service.
         // Since we don't have the profile here, we rely on the caller to check the enabled state for the profile.
         if (!isUploadEnabledByAppConfig(context)) {
             Log.w(LOGTAG, "Upload is not available by configuration; returning");
             return false;
         }
 
+        if (!NetworkUtils.isConnected(context)) {
+            Log.w(LOGTAG, "Network is not connected; returning");
+            return false;
+        }
+
         if (!isIntentValid(intent)) {
             Log.w(LOGTAG, "Received invalid Intent; returning");
             return false;
         }
 
         if (!ACTION_UPLOAD.equals(intent.getAction())) {
             Log.w(LOGTAG, "Unknown action: " + intent.getAction() + ". Returning");
             return false;
@@ -162,41 +167,40 @@ public class TelemetryUploadService exte
         return true;
     }
 
     /**
      * Determines if the telemetry upload feature is enabled via the application configuration. Prefer to use
      * {@link #isUploadEnabledByProfileConfig(Context, GeckoProfile)} if the profile is available as it takes into
      * account more information.
      *
+     * You may wish to also check if the network is connected when calling this method.
+     *
      * Note that this method logs debug statements when upload is disabled.
      */
     public static boolean isUploadEnabledByAppConfig(final Context context) {
         if (!TelemetryConstants.UPLOAD_ENABLED) {
             Log.d(LOGTAG, "Telemetry upload feature is compile-time disabled");
             return false;
         }
 
         if (!GeckoPreferences.getBooleanPref(context, GeckoPreferences.PREFS_HEALTHREPORT_UPLOAD_ENABLED, true)) {
             Log.d(LOGTAG, "Telemetry upload opt-out");
             return false;
         }
 
-        if (!NetworkUtils.isBackgroundDataEnabled(context)) {
-            Log.d(LOGTAG, "Background data is disabled");
-            return false;
-        }
-
         return true;
     }
 
     /**
      * Determines if the telemetry upload feature is enabled via profile & application level configurations. This is the
      * preferred method.
      *
+     * You may wish to also check if the network is connected when calling this method.
+     *
      * Note that this method logs debug statements when upload is disabled.
      */
     public static boolean isUploadEnabledByProfileConfig(final Context context, final GeckoProfile profile) {
         if (profile.inGuestMode()) {
             Log.d(LOGTAG, "Profile is in guest mode");
             return false;
         }
 
--- a/mobile/android/base/java/org/mozilla/gecko/util/NetworkUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/NetworkUtils.java
@@ -61,32 +61,16 @@ public class NetworkUtils {
 
         public final int value;
 
         ConnectionType(int value) {
             this.value = value;
         }
     }
 
-    public static boolean isBackgroundDataEnabled(final Context context) {
-        final NetworkInfo networkInfo = getActiveNetworkInfo(context);
-        return networkInfo != null &&
-                networkInfo.isAvailable() &&
-                networkInfo.isConnectedOrConnecting();
-    }
-
-    @Nullable
-    private static NetworkInfo getActiveNetworkInfo(final Context context) {
-        final ConnectivityManager connectivity = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
-        if (connectivity == null) {
-            return null;
-        }
-        return connectivity.getActiveNetworkInfo(); // can return null.
-    }
-
     /**
      * Indicates whether network connectivity exists and it is possible to establish connections and pass data.
      */
     public static boolean isConnected(@NonNull Context context) {
         return isConnected((ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
     }
 
     public static boolean isConnected(ConnectivityManager connectivityManager) {