Bug 1169290 - Fire remote-active observer notification in component. r?maja_zf draft
authorAndreas Tolfsen <ato@sny.no>
Sat, 27 Jan 2018 18:49:03 +0000
changeset 753893 57776fa5e3a44a70d8b35586320d31d3f34769d4
parent 753892 b7bc98d4dcadb71e7b8c312fa2b20a638048d94c
child 753894 b55b7c94dd2ff47819577da324beb4b03ebfc521
push id98715
push userbmo:ato@sny.no
push dateMon, 12 Feb 2018 16:37:16 +0000
reviewersmaja_zf
bugs1169290
milestone60.0a1
Bug 1169290 - Fire remote-active observer notification in component. r?maja_zf It is more correct to fire the remote-active observer notification in the Marionette XPCOM component after the TCP listener has bound to a port, than to fire it inside the implementation of the TCP server. MozReview-Commit-ID: GXaxkQIgE6U
testing/marionette/components/marionette.js
testing/marionette/server.js
--- a/testing/marionette/components/marionette.js
+++ b/testing/marionette/components/marionette.js
@@ -20,16 +20,17 @@ XPCOMUtils.defineLazyGetter(this, "log",
 });
 
 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_LOG_LEVEL = "info";
+const NOTIFY_RUNNING = "remote-active";
 
 // Complements -marionette flag for starting the Marionette server.
 // We also set this if Marionette is running in order to start the server
 // again after a Firefox restart.
 const ENV_ENABLED = "MOZ_MARIONETTE";
 
 // Besides starting based on existing prefs in a profile and a command
 // line flag, we also support inheriting prefs out of an env var, and to
@@ -267,28 +268,31 @@ class MarionetteComponent {
             .getService().wrappedJSObject.done;
       }
       await startupRecorder;
 
       try {
         ChromeUtils.import("chrome://marionette/content/server.js");
         let listener = new server.TCPListener(prefs.port);
         listener.start();
-        log.info(`Listening on port ${listener.port}`);
         this.server = listener;
       } catch (e) {
         log.fatal("Remote protocol server failed to start", e);
         Services.startup.quit(Ci.nsIAppStartup.eForceQuit);
       }
+
+      Services.obs.notifyObservers(this, NOTIFY_RUNNING, true);
+      log.info(`Listening on port ${this.server.port}`);
     });
   }
 
   uninit() {
     if (this.running) {
       this.server.stop();
+      Services.obs.notifyObservers(this, NOTIFY_RUNNING);
     }
   }
 
   get QueryInterface() {
     return XPCOMUtils.generateQI([
       Ci.nsICommandLineHandler,
       Ci.nsIMarionette,
     ]);
--- a/testing/marionette/server.js
+++ b/testing/marionette/server.js
@@ -45,18 +45,16 @@ this.server = {};
 const PROTOCOL_VERSION = 3;
 
 const ENV_ENABLED = "MOZ_MARIONETTE";
 
 const PREF_CONTENT_LISTENER = "marionette.contentListener";
 const PREF_PORT = "marionette.port";
 const PREF_RECOMMENDED = "marionette.prefs.recommended";
 
-const NOTIFY_RUNNING = "remote-active";
-
 // Marionette sets preferences recommended for automation when it starts,
 // unless marionette.prefs.recommended has been set to false.
 // Where noted, some prefs should also be set in the profile passed to
 // Marionette to prevent them from affecting startup, since some of these
 // are checked before Marionette initialises.
 const RECOMMENDED_PREFS = new Map([
 
   // Disable automatic downloading of new releases.
@@ -347,18 +345,16 @@ server.TCPListener = class {
    * The marionette.port preference will be populated with the value
    * of {@link #port}.
    */
   start() {
     if (this.alive) {
       return;
     }
 
-    Services.obs.notifyObservers(this, NOTIFY_RUNNING, true);
-
     if (Preferences.get(PREF_RECOMMENDED)) {
       // set recommended prefs if they are not already user-defined
       for (let [k, v] of RECOMMENDED_PREFS) {
         if (!Preferences.isSet(k)) {
           logger.debug(`Setting recommended pref ${k} to ${v}`);
           Preferences.set(k, v);
           this.alteredPrefs.add(k);
         }
@@ -382,18 +378,16 @@ server.TCPListener = class {
     for (let k of this.alteredPrefs) {
       logger.debug(`Resetting recommended pref ${k}`);
       Preferences.reset(k);
     }
     this.alteredPrefs.clear();
 
     // Shutdown server socket, and no longer listen for new connections
     this.acceptConnections = false;
-
-    Services.obs.notifyObservers(this, NOTIFY_RUNNING);
     this.alive = false;
   }
 
   onSocketAccepted(serverSocket, clientSocket) {
     let input = clientSocket.openInputStream(0, 0, 0);
     let output = clientSocket.openOutputStream(0, 0, 0);
     let transport = new DebuggerTransport(input, output);