Bug 1277214 - Move MOZ_DISABLE_* to single MOZ_IN_AUTOMATION env var. r=grisha draft
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 01 Jun 2016 16:46:44 -0700
changeset 374146 a8274100196a42e3a3028f65109d9044fe79fbfa
parent 374145 e2a035f75078b7a66ec7417a0998746b2ef150de
child 522567 a7e15aa61663d430d3849acfbbfe3745ff57dbca
push id19946
push usermichael.l.comella@gmail.com
push dateWed, 01 Jun 2016 23:55:51 +0000
reviewersgrisha
bugs1277214, 1277390
milestone49.0a1
Bug 1277214 - Move MOZ_DISABLE_* to single MOZ_IN_AUTOMATION env var. r=grisha We hit an issue where adding a new env var, MOZ_DISABLE_TELEMETRY, added env10 and caused crashes. I suspect the issue is that there are is now a double-digit number of env vars (bug 1277390). Here, we do the quick fix by removing MOZ_DISABLE_TELEMETRY & repurposing MOZ_DISABLE_SWITCHBOARD to be generic. While we're at it, we simplify the code by making the setDisabled methods a strict getter without checking for how many times they're called. MozReview-Commit-ID: 19DDbVYRZ2
build/mobile/remoteautomation.py
mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
mobile/android/base/java/org/mozilla/gecko/util/Experiments.java
mobile/android/base/java/org/mozilla/gecko/util/IntentUtils.java
--- a/build/mobile/remoteautomation.py
+++ b/build/mobile/remoteautomation.py
@@ -81,27 +81,22 @@ class RemoteAutomation(Automation):
             env['MOZ_CRASHREPORTER_DISABLE'] = '1'
 
         # Crash on non-local network connections by default.
         # MOZ_DISABLE_NONLOCAL_CONNECTIONS can be set to "0" to temporarily
         # enable non-local connections for the purposes of local testing.
         # Don't override the user's choice here.  See bug 1049688.
         env.setdefault('MOZ_DISABLE_NONLOCAL_CONNECTIONS', '1')
 
-        # Disable Switchboard by default. This will prevent nonlocal
-        # network connections to the Switchboard server.
-        # Passing any value expect the empty string will disable it so to
-        # enable, don't pass a value.
-        env.setdefault('MOZ_DISABLE_SWITCHBOARD', '1')
-
-        # Disable Java telemetry by default to
-        # prevent network connections during testing.
-        # Passing any value expect the empty string will disable it so to
-        # enable, don't pass a value.
-        env.setdefault('MOZ_DISABLE_TELEMETRY', '1')
+        # Send an env var noting that we are in automation. Passing any
+        # value except the empty string will declare the value to exist.
+        #
+        # This may be used to disabled network connections during testing, e.g.
+        # Switchboard & telemetry uploads.
+        env.setdefault('MOZ_IN_AUTOMATION', '1')
 
         # Set WebRTC logging in case it is not set yet.
         # On Android, environment variables cannot contain ',' so the
         # standard WebRTC setting for NSPR_LOG_MODULES is not available.
         # env.setdefault('NSPR_LOG_MODULES', 'signaling:5,mtransport:5,datachannel:5,jsep:5,MediaPipelineFactory:5')
         env.setdefault('R_LOG_LEVEL', '6')
         env.setdefault('R_LOG_DESTINATION', 'stderr')
         env.setdefault('R_LOG_VERBOSE', '1')
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ -770,18 +770,19 @@ public class BrowserApp extends GeckoApp
      * extra because environment variables from our test harness aren't set
      * until Gecko is loaded, and we need to know this before then.
      *
      * This method should be called early since other initialization
      * may depend on its results.
      */
     private void configureForTestsBasedOnEnvironment(final Intent intent) {
         final HashMap<String, String> envVars = IntentUtils.getEnvVarMap(intent);
-        Experiments.setDisabledFromEnvVar(envVars);
-        TelemetryUploadService.setDisabledFromEnvVar(envVars);
+        final boolean isInAutomation = !TextUtils.isEmpty(envVars.get(IntentUtils.ENV_VAR_IN_AUTOMATION));
+        Experiments.setIsDisabled(isInAutomation);
+        TelemetryUploadService.setDisabled(isInAutomation);
     }
 
     /**
      * Initializes the default Switchboard URLs the first time.
      * @param intent
      */
     private void initSwitchboard(final Intent intent) {
         if (Experiments.isDisabled() || !AppConstants.MOZ_SWITCHBOARD) {
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/TelemetryUploadService.java
@@ -46,42 +46,24 @@ public class TelemetryUploadService exte
     private static final String LOGTAG = StringUtils.safeSubstring("Gecko" + TelemetryUploadService.class.getSimpleName(), 0, 23);
     private static final String WORKER_THREAD_NAME = LOGTAG + "Worker";
 
     private static final String ENV_VAR_NAME = "MOZ_DISABLE_TELEMETRY";
 
     public static final String ACTION_UPLOAD = "upload";
     public static final String EXTRA_STORE = "store";
 
-    /**
-     * An override for telemetry via Intents.
-     *
-     * BrowserApp.onCreate, which sets the disabled state, should run before
-     * TelemetryUploadService, so we don't have to synchronize/volatile.
-     */
-    private static Boolean isDisabledByLaunchingIntent = null;
+    // TelemetryUploadService can run in a background thread so for future proofing, we set it volatile.
+    private static volatile boolean isDisabled = false;
 
-    /**
-     * As a sanity check, this method should only be called once.
-     */
-    public static void setDisabledFromEnvVar(final HashMap<String, String> envVarMap) {
-        if (isDisabledByLaunchingIntent != null) {
-            throw new IllegalStateException("Disabled state already set");
+    public static void setDisabled(final boolean isDisabled) {
+        TelemetryUploadService.isDisabled = isDisabled;
+        if (isDisabled) {
+            Log.d(LOGTAG, "Telemetry upload disabled (env var?");
         }
-        isDisabledByLaunchingIntent = !TextUtils.isEmpty(envVarMap.get(ENV_VAR_NAME));
-        if (isDisabledByLaunchingIntent) {
-            Log.d(LOGTAG, "Telemetry disabled by environment variable: " + ENV_VAR_NAME);
-        }
-    }
-
-    private static boolean isDisabledByLaunchingIntent() {
-        if (isDisabledByLaunchingIntent == null) {
-            throw new IllegalStateException("Disabled state not yet set.");
-        }
-        return isDisabledByLaunchingIntent;
     }
 
     public TelemetryUploadService() {
         super(WORKER_THREAD_NAME);
 
         // Intent redelivery can fail hard (e.g. we OOM as we try to upload, the Intent gets redelivered, repeat)
         // so for simplicity, we avoid it. We expect the upload service to eventually get called again by the caller.
         setIntentRedelivery(false);
@@ -209,17 +191,17 @@ public class TelemetryUploadService exte
      * 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 (isDisabledByLaunchingIntent()) {
+        if (isDisabled) {
             Log.d(LOGTAG, "Telemetry upload feature is disabled by intent (in testing?)");
             return false;
         }
 
         if (!GeckoPreferences.getBooleanPref(context, GeckoPreferences.PREFS_HEALTHREPORT_UPLOAD_ENABLED, true)) {
             Log.d(LOGTAG, "Telemetry upload opt-out");
             return false;
         }
--- a/mobile/android/base/java/org/mozilla/gecko/util/Experiments.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/Experiments.java
@@ -1,37 +1,33 @@
 /* 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.util;
 
 import android.content.Context;
 
-import android.support.annotation.NonNull;
 import android.util.Log;
 import android.text.TextUtils;
 
 import com.keepsafe.switchboard.Preferences;
 import com.keepsafe.switchboard.SwitchBoard;
 import org.mozilla.gecko.GeckoSharedPrefs;
 
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 
 /**
  * This class should reflect the experiment names found in the Switchboard experiments config here:
  * https://github.com/mozilla-services/switchboard-experiments
  */
 public class Experiments {
     private static final String LOGTAG = "GeckoExperiments";
 
-    private static final String ENVVAR_DISABLED = "MOZ_DISABLE_SWITCHBOARD";
-
     // Show a system notification linking to a "What's New" page on app update.
     public static final String WHATSNEW_NOTIFICATION = "whatsnew-notification";
 
     // Subscribe to known, bookmarked sites and show a notification if new content is available.
     public static final String CONTENT_NOTIFICATIONS_12HRS = "content-notifications-12hrs";
     public static final String CONTENT_NOTIFICATIONS_8AM = "content-notifications-8am";
     public static final String CONTENT_NOTIFICATIONS_5PM = "content-notifications-5pm";
 
@@ -53,36 +49,27 @@ public class Experiments {
     public static final String TRIPLE_READERVIEW_BOOKMARK_PROMPT = "triple-readerview-bookmark-prompt";
 
     // Only show origin in URL bar instead of full URL (Bug 1236431)
     public static final String URLBAR_SHOW_ORIGIN_ONLY = "urlbar-show-origin-only";
 
     // Show name of organization (EV cert) instead of full URL in URL bar (Bug 1249594).
     public static final String URLBAR_SHOW_EV_CERT_OWNER = "urlbar-show-ev-cert-owner";
 
-    private static volatile Boolean disabled = null;
+    private static boolean isDisabled = false;
 
-    /**
-     * As a sanity check, this method may only be called once.
-     */
-    public static void setDisabledFromEnvVar(@NonNull final Map<String, String> envVarMap) {
-        if (disabled != null) {
-            throw new IllegalStateException("Disabled state already set");
-        }
-        disabled = !TextUtils.isEmpty(envVarMap.get(ENVVAR_DISABLED));
-        if (disabled) {
-            Log.d(LOGTAG, "Switchboard disabled by environment variable: " + ENVVAR_DISABLED);
+    public static void setIsDisabled(final boolean isDisabled) {
+        Experiments.isDisabled = isDisabled;
+        if (isDisabled) {
+            Log.d(LOGTAG, "Switchboard disabled (env var?)");
         }
     }
 
     public static boolean isDisabled() {
-        if (disabled == null) {
-            throw new IllegalStateException("Disabled state not yet set.");
-        }
-        return disabled;
+        return isDisabled;
     }
 
     /**
      * Returns if a user is in certain local experiment.
      * @param experiment Name of experiment to look up
      * @return returns value for experiment or false if experiment does not exist.
      */
     public static boolean isInExperimentLocal(Context context, String experiment) {
--- a/mobile/android/base/java/org/mozilla/gecko/util/IntentUtils.java
+++ b/mobile/android/base/java/org/mozilla/gecko/util/IntentUtils.java
@@ -13,16 +13,18 @@ import android.support.annotation.NonNul
 import java.util.HashMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
  * Utilities for Intents.
  */
 public class IntentUtils {
+    public static final String ENV_VAR_IN_AUTOMATION = "MOZ_IN_AUTOMATION";
+
     private static final String ENV_VAR_REGEX = "(.+)=(.*)";
 
     private IntentUtils() {}
 
     /**
      * Returns a list of environment variables and their values. These are parsed from an Intent extra
      * with the key -> value format:
      *   env# -> ENV_VAR=VALUE