Bug 1344748 - Document and make MarionetteComponent easier to read; r?whimboo draft
authorAndreas Tolfsen <ato@mozilla.com>
Thu, 09 Mar 2017 17:57:26 +0000
changeset 504648 0a05278baa47e54188126c03589a211cee9f01c3
parent 504647 2e0848d29849f88b594fb08df7a8dc062a19bf50
child 504649 1ed994b38f4ad1e0ccb0ea90cd0bc5c730187648
push id50832
push userbmo:ato@mozilla.com
push dateFri, 24 Mar 2017 14:10:32 +0000
reviewerswhimboo
bugs1344748
milestone55.0a1
Bug 1344748 - Document and make MarionetteComponent easier to read; r?whimboo MozReview-Commit-ID: 4VTCOsTYftG
testing/marionette/components/marionette.js
--- a/testing/marionette/components/marionette.js
+++ b/testing/marionette/components/marionette.js
@@ -110,46 +110,61 @@ const prefs = {
           Preferences.set("marionette." + prefName, prefs[prefName]);
         }
       }
     }
   },
 };
 
 function MarionetteComponent() {
+  // keeps track of whether Marionette is available,
+  // either as a result of the marionette.enabled pref
+  // or by use of the --marionette flag
   this.enabled = prefs.enabled;
-  this.loaded = false;
+
+  // guards against this component
+  // being initialised multiple times
+  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);
-  this.server = null;
-  this.gfxWindow = null;
-  this.finalUIStartup = false;
 }
 
 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}
+    {category: "profile-after-change", service: true},
   ],
 };
 
 MarionetteComponent.prototype.onSocketAccepted = function (socket, transport) {
   this.logger.info("onSocketAccepted for Marionette dummy socket");
 };
 
 MarionetteComponent.prototype.onStopListening = function (socket, status) {
   this.logger.info(`onStopListening for Marionette dummy socket, code ${status}`);
   socket.close();
 };
 
-/** Checks |cmdLine| for the --marionette flag. */
+// Handle --marionette flag
 MarionetteComponent.prototype.handle = function (cmdLine) {
   if (cmdLine.handleFlag("marionette", false)) {
     this.enabled = true;
     this.logger.debug("Marionette enabled via command-line flag");
     this.init();
   }
 };
 
@@ -158,18 +173,16 @@ MarionetteComponent.prototype.observe = 
     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", false);
 
       prefs.readFromEnvironment(ENV_PREF_VAR);
 
       if (this.enabled) {
-        this.logger.debug("Marionette is enabled");
-
         // We want to suppress the modal dialog that's shown
         // when starting up in safe-mode to enable testing.
         if (Services.appinfo.inSafeMode) {
           Services.obs.addObserver(this, "domwindowopened", false);
         }
       }
       break;
 
@@ -222,63 +235,62 @@ MarionetteComponent.prototype.observe = 
 
 MarionetteComponent.prototype.setupLogger = function (level) {
   let logger = Log.repository.getLogger("Marionette");
   logger.level = level;
   logger.addAppender(new Log.DumpAppender());
   return logger;
 };
 
-/** Wait for the modal dialogue to finish loading. */
 MarionetteComponent.prototype.suppressSafeModeDialog = function (win) {
-  win.addEventListener("load", function() {
+  win.addEventListener("load", () => {
     if (win.document.getElementById("safeModeDialog")) {
-      // Accept the dialog to start in safe-mode
+      // accept the dialog to start in safe-mode
       win.setTimeout(() => {
         win.document.documentElement.getButton("accept").click();
       });
     }
   }, {once: true});
 };
 
 MarionetteComponent.prototype.init = function () {
-  if (this.loaded || !this.enabled || !this.finalUIStartup) {
+  if (this.running || !this.enabled || !this.finalUIStartup) {
     return;
   }
 
-  this.loaded = true;
+  this.running = true;
 
   if (!prefs.forceLocal) {
     // See bug 800138.  Because the first socket that opens with
     // force-local=false fails, we open a dummy socket that will fail.
     // keepWhenOffline=true so that it still work when offline (local).
     // This allows the following attempt by Marionette to open a socket
     // to succeed.
     let insaneSacrificialGoat =
         new ServerSocket(666, Ci.nsIServerSocket.KeepWhenOffline, 4);
     insaneSacrificialGoat.asyncListen(this);
   }
 
-  let so;
+  let s;
   try {
     Cu.import("chrome://marionette/content/server.js");
-    so = new server.TCPListener(prefs.port, prefs.forceLocal);
-    so.start();
-    this.logger.info(`Listening on port ${so.port}`);
+    s = new server.TCPListener(prefs.port, prefs.forceLocal);
+    s.start();
+    this.logger.info(`Listening on port ${s.port}`);
   } catch (e) {
     this.logger.error(`Error on starting server: ${e}`);
-    dump(e.toString() + "\n" + e.stack + "\n");
+    dump(`${e.toString()}\n${e.stack}\n`);
   } finally {
-    if (so) {
-      this.server = so;
+    if (s) {
+      this.server = s;
     }
   }
 };
 
 MarionetteComponent.prototype.uninit = function () {
-  if (!this.loaded) {
+  if (!this.running) {
     return;
   }
   this.server.stop();
-  this.loaded = false;
+  this.running = false;
 };
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([MarionetteComponent]);