Bug 1533704 - Don't make GeckoView's remote debugging setting control Marionette. r=snorp a=pascalc
authorNick Alexander <nalexander@mozilla.com>
Thu, 28 Mar 2019 20:28:14 +0000
changeset 525907 db23516c1a2aa62b77ff9ac18f27dee6adaac2ac
parent 525906 7cfddb82a7b9c9a09fd2db72ec20282ba8fd920f
child 525908 7c44e976056eca85a5b9945bc9c42ace1e9310ac
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp, pascalc
bugs1533704
milestone67.0
Bug 1533704 - Don't make GeckoView's remote debugging setting control Marionette. r=snorp a=pascalc We really want GeckoView's single remote debugigng setting to determine whether the engine can be remote controlled, but we're not quite there yet. The devtools use an abstract UNIX socket for this purpose, but Marionette uses a TCP socket that defaults to port 2828, and that means we see cross-App clashes for that port. Functionally this means that enabling Marionette reverts to the "old method": either pass the "--marionette" command line argument or set the `MOZ_MARIONETTE=1` environment variable to enable. Callers remain responsible for ensuring that the Marionette port is available. Differential Revision: https://phabricator.services.mozilla.com/D25138
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java
mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java
mobile/android/modules/geckoview/GeckoViewRemoteDebugger.jsm
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java
@@ -125,17 +125,16 @@ public class GeckoThread extends Thread 
     private static int uiThreadId;
 
     private static TelemetryUtils.Timer sInitTimer;
 
     // Main process parameters
     public static final int FLAG_DEBUGGING = 1 << 0; // Debugging mode.
     public static final int FLAG_PRELOAD_CHILD = 1 << 1; // Preload child during main thread start.
     public static final int FLAG_ENABLE_NATIVE_CRASHREPORTER = 1 << 2; // Enable native crash reporting.
-    public static final int FLAG_ENABLE_MARIONETTE = 1 << 3; // Enable Marionette at startup.
 
     public static final long DEFAULT_TIMEOUT = 5000;
 
     /* package */ static final String EXTRA_ARGS = "args";
     private static final String EXTRA_PREFS_FD = "prefsFd";
     private static final String EXTRA_PREF_MAP_FD = "prefMapFd";
     private static final String EXTRA_IPC_FD = "ipcFd";
     private static final String EXTRA_CRASH_FD = "crashFd";
@@ -427,22 +426,16 @@ public class GeckoThread extends Thread 
         // In Gecko, the native crash reporter is enabled by default in opt builds, and
         // disabled by default in debug builds.
         if ((mInitInfo.flags & FLAG_ENABLE_NATIVE_CRASHREPORTER) == 0 && !BuildConfig.DEBUG_BUILD) {
             env.add(0, "MOZ_CRASHREPORTER_DISABLE=1");
         } else if ((mInitInfo.flags & FLAG_ENABLE_NATIVE_CRASHREPORTER) != 0 && BuildConfig.DEBUG_BUILD) {
             env.add(0, "MOZ_CRASHREPORTER=1");
         }
 
-        if (!isChildProcess() && ((mInitInfo.flags & FLAG_ENABLE_MARIONETTE) != 0)) {
-            // The presence of this environment variable determines the initial
-            // value of `marionette.enabled`.
-            env.add(0, "MOZ_MARIONETTE=1");
-        }
-
         GeckoLoader.loadMozGlue(context);
         setState(State.MOZGLUE_READY);
 
         GeckoLoader.setupGeckoEnvironment(context, context.getFilesDir().getPath(), env, mInitInfo.prefs);
 
         initGeckoEnvironment();
 
         if ((mInitInfo.flags & FLAG_PRELOAD_CHILD) != 0) {
--- a/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoRuntime.java
@@ -181,20 +181,16 @@ public final class GeckoRuntime implemen
         if (settings.getUseContentProcessHint()) {
             flags |= GeckoThread.FLAG_PRELOAD_CHILD;
         }
 
         if (settings.getPauseForDebuggerEnabled()) {
             flags |= GeckoThread.FLAG_DEBUGGING;
         }
 
-        if (settings.getRemoteDebuggingEnabled()) {
-            flags |= GeckoThread.FLAG_ENABLE_MARIONETTE;
-        }
-
         final Class<?> crashHandler = settings.getCrashHandler();
         if (crashHandler != null) {
             try {
                 final ServiceInfo info = context.getPackageManager().getServiceInfo(new ComponentName(context, crashHandler), 0);
                 if (info.processName.equals(getProcessName(context))) {
                     throw new IllegalArgumentException("Crash handler service must run in a separate process");
                 }
 
--- a/mobile/android/modules/geckoview/GeckoViewRemoteDebugger.jsm
+++ b/mobile/android/modules/geckoview/GeckoViewRemoteDebugger.jsm
@@ -43,25 +43,16 @@ var GeckoViewRemoteDebugger = {
     }
   },
 
   onInit() {
     debug `onInit`;
     this._isEnabled = false;
     this._usbDebugger = new USBRemoteDebugger();
 
-    // For GeckoView-consuming Apps (including Fennec), we want "remote
-    // debugging" to encapsulate "Marionette" completely.  It's possible for
-    // consumers to manage the Marionette pref independently, but we don't
-    // condone or accommodate such management.
-    Services.prefs.setBoolPref("marionette.enabled", false);
-
-    // We never want Marionette to set prefs recommended for automation.
-    Services.prefs.setBoolPref("marionette.prefs.recommended", false);
-
     // This lets Marionette start listening (when it's enabled).  Both
     // GeckoView and Marionette do most of their initialization in
     // "profile-after-change", and there is no order enforced between
     // them.  Therefore we defer asking Marionette to startup until
     // after all "profile-after-change" handlers (including this one)
     // have completed.
     Services.tm.dispatchToMainThread(() => {
         Services.obs.notifyObservers(null, "marionette-startup-requested");
@@ -99,30 +90,26 @@ var GeckoViewRemoteDebugger = {
       warn `Missing env MOZ_ANDROID_PACKAGE_NAME. Unable to get package name`;
     }
 
     this._isEnabled = true;
     this._usbDebugger.stop();
 
     const portOrPath = packageName + "firefox-debugger-socket";
     this._usbDebugger.start(portOrPath);
-
-    Services.prefs.setBoolPref("marionette.enabled", true);
   },
 
   onDisable() {
     if (!this._isEnabled) {
       return;
     }
 
     debug `onDisable`;
     this._isEnabled = false;
     this._usbDebugger.stop();
-
-    Services.prefs.setBoolPref("marionette.enabled", false);
   },
 };
 
 class USBRemoteDebugger {
   start(aPortOrPath) {
     try {
       const AuthenticatorType = DebuggerServer.Authenticators.get("PROMPT");
       const authenticator = new AuthenticatorType.Server();