Bug 1355888 - Remove marionette.enabled pref; r?whimboo,maja_zf,ted draft
authorAndreas Tolfsen <ato@mozilla.com>
Thu, 13 Apr 2017 15:08:14 +0100
changeset 567000 7eac3b4af6a194a93b0f5c05bfd1d557637c07d7
parent 566999 371726ecd09013c2f0583d11fbfb235f671a9456
child 567001 92125174608ec1919e908349fe0c3a5ae884da0a
push id55406
push userbmo:ato@mozilla.com
push dateMon, 24 Apr 2017 09:34:42 +0000
reviewerswhimboo, maja_zf, ted
bugs1355888
milestone55.0a1
Bug 1355888 - Remove marionette.enabled pref; r?whimboo,maja_zf,ted There are no current use cases for starting and stopping the Marionette server at runtime through a preference. Since it is possible for arbitrary addons to modify any preference, we are removing it to reduce the potential attack surface for Marionette. This effectively leaves only two ways of starting Marionette: By passing the -marionette flag to the Firefox binary at startup, and by calling server.TCPListener#start(), which is an internal chrome API. MozReview-Commit-ID: 9zKsV8ufySU
testing/marionette/README
testing/marionette/components/marionette.js
testing/marionette/prefs/marionette.js
--- a/testing/marionette/README
+++ b/testing/marionette/README
@@ -9,21 +9,18 @@ DESCRIPTION
   It can remotely control either the UI or the internal JavaScript of
   the Gecko platform, such as Firefox.  It can control both the chrome
   and the content document, giving a high level of control and ability to
   replicate user interaction.  In addition to performing actions on the
   browser, Marionette can also read properties and attributes of the DOM.
 
 USAGE
 
-  It is included in Firefox, but not enabled by default unless the
-  --marionette flag is passed or the marionette.enabled preference is
-  set to true.
-
-  To start Firefox with the remote protocol turned on:
+  Marionette can be activated by passing the --marionette flag.  To start
+  Firefox with the remote protocol turned on:
 
   	% firefox --marionette

   	1491228343089	Marionette	INFO	Listening on port 2828
 
   This binds to a TCP socket, over which CLIENTS can communicate with
   Marionette using the PROTOCOL.
 
--- a/testing/marionette/components/marionette.js
+++ b/testing/marionette/components/marionette.js
@@ -12,18 +12,16 @@ Cu.import("resource://gre/modules/Servic
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(
     this, "env", "@mozilla.org/process/environment;1", "nsIEnvironment");
 
 const MARIONETTE_CONTRACT_ID = "@mozilla.org/marionette;1";
 const MARIONETTE_CID = Components.ID("{786a1369-dca5-4adc-8486-33d23c88010a}");
 
-const PREF_ENABLED = "marionette.enabled";
-const PREF_ENABLED_FALLBACK = "marionette.defaultPrefs.enabled";
 const PREF_PORT = "marionette.port";
 const PREF_PORT_FALLBACK = "marionette.defaultPrefs.port";
 const PREF_LOG_LEVEL = "marionette.log.level";
 const PREF_LOG_LEVEL_FALLBACK = "marionette.logging";
 
 const DEFAULT_PORT = 2828;
 const DEFAULT_LOG_LEVEL = "info";
 const LOG_LEVELS = new class extends Map {
@@ -57,17 +55,17 @@ const ENV_ENABLED = "MARIONETTE";
 // line flag, we also support inheriting prefs out of an env var, and to
 // start Marionette that way.
 //
 // This allows marionette prefs to persist when we do a restart into
 // a different profile in order to test things like Firefox refresh.
 // The environment variable itself, if present, is interpreted as a
 // JSON structure, with the keys mapping to preference names in the
 // "marionette." branch, and the values to the values of those prefs. So
-// something like {"enabled": true} would result in the marionette.enabled
+// something like {"port": 4444} would result in the marionette.enabled
 // pref being set to true, thus triggering marionette being enabled for
 // that startup.
 const ENV_PRESERVE_PREFS = "MOZ_MARIONETTE_PREF_STATE_ACROSS_RESTARTS";
 
 const ServerSocket = CC("@mozilla.org/network/server-socket;1",
     "nsIServerSocket",
     "initSpecialConnection");
 
@@ -117,45 +115,35 @@ const prefs = {
       }
     }
   },
 };
 
 function MarionetteComponent() {
   this.enabled = env.exists(ENV_ENABLED);
   this.running = false;
-
-  // holds a reference to server.TCPListener
   this.server = null;
 
   // holds reference to ChromeWindow
   // used to run GFX sanity tests on Windows
   this.gfxWindow = null;
 
   // indicates that all pending window checks have been completed
   // and that we are ready to start the Marionette server
   this.finalUIStartup = false;
 
   this.logger = this.setupLogger(prefs.logLevel);
-  Services.prefs.addObserver(PREF_ENABLED, this);
-
-  if (Preferences.isSet(PREF_ENABLED_FALLBACK)) {
-    this.logger.warn(`Deprecated preference ${PREF_ENABLED_FALLBACK} detected, ` +
-        `please use ${PREF_ENABLED}`);
-    Preferences.set(PREF_ENABLED, Preferences.get(PREF_ENABLED_FALLBACK));
-  }
 }
 
 MarionetteComponent.prototype = {
   classDescription: "Marionette component",
   classID: MARIONETTE_CID,
   contractID: MARIONETTE_CONTRACT_ID,
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsICommandLineHandler,
-    Ci.nsIObserver,
   ]),
   _xpcom_categories: [
     {category: "command-line-handler", entry: "b-marionette"},
     {category: "profile-after-change", service: true},
   ],
   helpInfo: "  --marionette       Enable remote control server.\n",
 };
 
@@ -170,36 +158,18 @@ MarionetteComponent.prototype.onStopList
 
 // Handle --marionette flag
 MarionetteComponent.prototype.handle = function (cmdLine) {
   if (cmdLine.handleFlag("marionette", false)) {
     this.enabled = true;
   }
 };
 
-Object.defineProperty(MarionetteComponent.prototype, "enabled", {
-  set (value) {
-    Preferences.set(PREF_ENABLED, value);
-  },
-
-  get () {
-    return Preferences.get(PREF_ENABLED);
-  },
-});
-
 MarionetteComponent.prototype.observe = function (subject, topic, data) {
   switch (topic) {
-    case "nsPref:changed":
-      if (Preferences.get(PREF_ENABLED)) {
-        this.init();
-      } else {
-        this.uninit();
-      }
-      break;
-
     case "profile-after-change":
       // Using sessionstore-windows-restored as the xpcom category doesn't
       // seem to work, so we wait for that by adding an observer here.
       Services.obs.addObserver(this, "sessionstore-windows-restored");
 
       prefs.readFromEnvironment(ENV_PRESERVE_PREFS);
 
       if (this.enabled) {
@@ -295,13 +265,12 @@ MarionetteComponent.prototype.init = fun
   }
 };
 
 MarionetteComponent.prototype.uninit = function () {
   if (!this.running) {
     return;
   }
   this.server.stop();
-  this.logger.info("Ceased listening");
   this.running = false;
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([MarionetteComponent]);
--- a/testing/marionette/prefs/marionette.js
+++ b/testing/marionette/prefs/marionette.js
@@ -4,19 +4,16 @@
 
 // Marionette is the remote protocol that lets OOP programs communicate
 // with, instrument, and control Gecko.
 //
 // It is included in Firefox, but not enabled by default unless the
 // --marionette flag is passed or the marionette.enabled preference is
 // set to true.
 
-// Whether or not Marionette is enabled.
-pref("marionette.enabled", false);
-
 // Port to start Marionette server on.
 pref("marionette.port", 2828);
 
 // Marionette logging verbosity.  Allowed values are "fatal", "error",
 // "warn", "info", "config", "debug", and "trace".
 pref("marionette.log.level", "info");
 
 // Sets preferences recommended when using Firefox in automation with