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
--- 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