Bug 1244293 - Don't upload telemetry if user opts out. r=mfinkle draft
authorMichael Comella <michael.l.comella@gmail.com>
Fri, 29 Jan 2016 15:21:50 -0800
changeset 327252 6cf1befef3aba37031e7fc3306b446e87114b601
parent 327241 75c6a6c1fec29d30999b7b0f6251ada5d0efc0c2
child 513674 193aa06fe84f8c185a924568fab2a9eae5bd6906
push id10213
push usermichael.l.comella@gmail.com
push dateFri, 29 Jan 2016 23:22:21 +0000
reviewersmfinkle
bugs1244293
milestone47.0a1
Bug 1244293 - Don't upload telemetry if user opts out. r=mfinkle I'm slightly concerned we're providing too much configuration information in the debugging statements.
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
mobile/android/services/src/main/java/org/mozilla/gecko/background/BackgroundService.java
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -3923,17 +3923,17 @@ public class BrowserApp extends GeckoApp
         mActionBarFlipper.showPrevious();
 
         // Only slide the urlbar out if it was hidden when the action mode started
         // Don't animate hiding it so that there's no flash as we switch back to url mode
         mDynamicToolbar.setTemporarilyVisible(false, VisibilityTransition.IMMEDIATE);
     }
 
     private void uploadTelemetry(final GeckoProfile profile) {
-        if (!TelemetryConstants.UPLOAD_ENABLED || profile.inGuestMode()) {
+        if (!TelemetryUploadService.isUploadEnabledByProfileConfig(this, profile)) {
             return;
         }
 
         final SharedPreferences sharedPrefs = GeckoSharedPrefs.forProfileName(this, profile.getName());
         final int seq = sharedPrefs.getInt(TelemetryConstants.PREF_SEQ_COUNT, 1);
 
         final Intent i = new Intent(TelemetryConstants.ACTION_UPLOAD_CORE);
         i.setClass(this, TelemetryUploadService.class);
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
@@ -1,23 +1,25 @@
 /* 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.telemetry;
 
+import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
 import android.support.annotation.NonNull;
 import android.util.Log;
 import ch.boye.httpclientandroidlib.HttpResponse;
 import ch.boye.httpclientandroidlib.client.ClientProtocolException;
 import org.mozilla.gecko.GeckoProfile;
 import org.mozilla.gecko.GeckoSharedPrefs;
 import org.mozilla.gecko.background.BackgroundService;
+import org.mozilla.gecko.preferences.GeckoPreferences;
 import org.mozilla.gecko.sync.net.BaseResource;
 import org.mozilla.gecko.sync.net.BaseResourceDelegate;
 import org.mozilla.gecko.sync.net.Resource;
 import org.mozilla.gecko.util.StringUtils;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
 import java.security.GeneralSecurityException;
@@ -54,22 +56,25 @@ public class TelemetryUploadService exte
      * de-duplicate documents. In order to maintain this consistency, we receive the doc ID and seq from the Intent and
      * rely on the caller to update the values. The Service can be killed at any time so we can't ensure seq could be
      * incremented properly if we tried to do so in the Service.
      */
     @Override
     public void onHandleIntent(final Intent intent) {
         Log.d(LOGTAG, "Service started");
 
-        if (!TelemetryConstants.UPLOAD_ENABLED) {
-            Log.d(LOGTAG, "Telemetry upload feature is compile-time disabled; not handling upload 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(this)) {
+            Log.w(LOGTAG, "Upload is not available by configuration; returning");
             return;
         }
 
-        if (!isReadyToUpload(intent)) {
+        if (!isIntentValid(intent)) {
+            Log.w(LOGTAG, "Received invalid Intent; returning");
             return;
         }
 
         if (!TelemetryConstants.ACTION_UPLOAD_CORE.equals(intent.getAction())) {
             Log.w(LOGTAG, "Unknown action: " + intent.getAction() + ". Returning");
             return;
         }
 
@@ -77,48 +82,83 @@ public class TelemetryUploadService exte
         final int seq = intent.getIntExtra(TelemetryConstants.EXTRA_SEQ, -1);
 
         final String profileName = intent.getStringExtra(TelemetryConstants.EXTRA_PROFILE_NAME);
         final String profilePath = intent.getStringExtra(TelemetryConstants.EXTRA_PROFILE_PATH);
 
         uploadCorePing(docId, seq, profileName, profilePath);
     }
 
-    private boolean isReadyToUpload(final Intent intent) {
-        // Intent can be null. Bug 1025937.
-        if (intent == null) {
-            Log.d(LOGTAG, "Received null intent. Returning.");
+    /**
+     * 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.
+     *
+     * 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 (!backgroundDataIsEnabled(context)) {
+            Log.d(LOGTAG, "Background data is disabled");
             return false;
         }
 
-        // Don't do anything if the device can't talk to the server.
-        if (!backgroundDataIsEnabled()) {
-            Log.d(LOGTAG, "Background data is not enabled; skipping.");
+        return true;
+    }
+
+    /**
+     * Determines if the telemetry upload feature is enabled via profile & application level configurations. This is the
+     * preferred 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;
+        }
+
+        return isUploadEnabledByAppConfig(context);
+    }
+
+    private boolean isIntentValid(final Intent intent) {
+        // Intent can be null. Bug 1025937.
+        if (intent == null) {
+            Log.d(LOGTAG, "Received null intent");
             return false;
         }
 
         if (intent.getStringExtra(TelemetryConstants.EXTRA_DOC_ID) == null) {
-            Log.w(LOGTAG, "Received invalid doc ID in Intent. Returning");
+            Log.d(LOGTAG, "Received invalid doc ID in Intent");
             return false;
         }
 
         if (!intent.hasExtra(TelemetryConstants.EXTRA_SEQ)) {
-            Log.w(LOGTAG, "Received Intent without sequence number. Returning");
+            Log.d(LOGTAG, "Received Intent without sequence number");
             return false;
         }
 
         if (intent.getStringExtra(TelemetryConstants.EXTRA_PROFILE_NAME) == null) {
-            Log.w(LOGTAG, "Received invalid profile name in Intent. Returning");
+            Log.d(LOGTAG, "Received invalid profile name in Intent");
             return false;
         }
 
         // GeckoProfile can use the name to get the path so this isn't strictly necessary.
         // However, getting the path requires parsing an ini file so we optimize by including it here.
         if (intent.getStringExtra(TelemetryConstants.EXTRA_PROFILE_PATH) == null) {
-            Log.w(LOGTAG, "Received invalid profile path in Intent. Returning");
+            Log.d(LOGTAG, "Received invalid profile path in Intent");
             return false;
         }
 
         return true;
     }
 
     private void uploadCorePing(@NonNull final String docId, final int seq, @NonNull final String profileName,
                 @NonNull final String profilePath) {
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/background/BackgroundService.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/background/BackgroundService.java
@@ -36,18 +36,18 @@ public abstract class BackgroundService 
     context.startService(service);
   }
 
   /**
    * Returns true if the OS will allow us to perform background
    * data operations. This logic varies by OS version.
    */
   @SuppressWarnings("deprecation")
-  protected boolean backgroundDataIsEnabled() {
-    ConnectivityManager connectivity = (ConnectivityManager) this.getSystemService(Context.CONNECTIVITY_SERVICE);
+  protected static boolean backgroundDataIsEnabled(final Context context) {
+    final ConnectivityManager connectivity = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
     if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH) {
       return connectivity.getBackgroundDataSetting();
     }
     NetworkInfo networkInfo = connectivity.getActiveNetworkInfo();
     if (networkInfo == null) {
       return false;
     }
     return networkInfo.isAvailable();