Bug 1309556 - Ensure to correctly shutdown the application for quit/restart when callbacks are used.
By using a callback the usual shutdown logic from quitApplication() is not executed and as such
will create a race-condition for the client when trying to re-connect to the server. To fix that
we have to stop the server from accepting new connections until the application has been completely
shutdown.
Also delete_session() has to be called for the default in_app shutdown logic and when using a callback.
MozReview-Commit-ID: GmIM2GGwQ2P
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -1056,17 +1056,16 @@ class Marionette(object):
createInstance(Components.interfaces.nsISupportsPRBool);
Services.obs.notifyObservers(cancelQuit, "quit-application-requested", null);
return cancelQuit.data;
""")
if canceled:
raise errors.MarionetteException("Something canceled the quit application request")
self._send_message("quitApplication", {"flags": list(flags)})
- self.delete_session(in_app=True)
@do_process_check
def quit(self, in_app=False, callback=None):
"""Terminate the currently running instance.
This command will delete the active marionette session. It also allows
manipulation of eg. the profile data while the application is not running.
To start the application again, start_session() has to be called.
@@ -1080,19 +1079,21 @@ class Marionette(object):
if not self.instance:
raise errors.MarionetteException("quit() can only be called "
"on Gecko instances launched by Marionette")
self.reset_timeouts()
if in_app:
if callable(callback):
+ self._send_message("acceptConnections", {"value": False})
callback()
else:
self._request_in_app_shutdown()
+ self.delete_session(in_app=True)
# Give the application some time to shutdown
self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
else:
self.delete_session()
self.instance.close()
@do_process_check
@@ -1115,19 +1116,21 @@ class Marionette(object):
"on Gecko instances launched by Marionette")
session_id = self.session_id
if in_app:
if clean:
raise ValueError("An in_app restart cannot be triggered with the clean flag set")
if callable(callback):
+ self._send_message("acceptConnections", {"value": False})
callback()
else:
self._request_in_app_shutdown("eRestart")
+ self.delete_session(in_app=True)
try:
self.raise_for_port(self.wait_for_port())
except socket.timeout:
if self.instance.runner.returncode is not None:
exc, val, tb = sys.exc_info()
self.cleanup()
raise exc, "Requested restart of the application was aborted", tb
--- a/testing/marionette/driver.js
+++ b/testing/marionette/driver.js
@@ -86,22 +86,22 @@ this.Context.fromString = function(s) {
* browsing context's content frame message listener via ListenerProxy.
*
* Throughout this prototype, functions with the argument {@code cmd}'s
* documentation refers to the contents of the {@code cmd.parameters}
* object.
*
* @param {string} appName
* Description of the product, for example "B2G" or "Firefox".
- * @param {function()} stopSignal
- * Signal to stop the Marionette server.
+ * @param {MarionetteServer} server
+ * The instance of Marionette server.
*/
-this.GeckoDriver = function(appName, stopSignal) {
+this.GeckoDriver = function(appName, server) {
this.appName = appName;
- this.stopSignal_ = stopSignal;
+ this._server = server;
this.sessionId = null;
this.wins = new browser.Windows();
this.browsers = {};
// points to current browser
this.curBrowser = null;
this.context = Context.CONTENT;
this.scriptTimeout = 30000; // 30 seconds
@@ -2555,30 +2555,54 @@ GeckoDriver.prototype.sendKeysToDialog =
GeckoDriver.prototype._checkIfAlertIsPresent = function() {
if (!this.dialog || !this.dialog.ui) {
throw new NoAlertOpenError(
"No tab modal was open when attempting to get the dialog text");
}
};
/**
+ * Enables or disables accepting new socket connections.
+ *
+ * By calling this method with `false` the server will not accept any further
+ * connections, but existing connections will not be forcible closed. Use `true`
+ * to re-enable accepting connections.
+ *
+ * Please note that when closing the connection via the client you can end-up in
+ * a non-recoverable state if it hasn't been enabled before.
+ *
+ * This method is used for custom in application shutdowns via marionette.quit()
+ * or marionette.restart(), like File -> Quit.
+ *
+ * @param {boolean} state
+ * True if the server should accept new socket connections.
+ */
+GeckoDriver.prototype.acceptConnections = function(cmd, resp) {
+ if (typeof cmd.parameters.value != "boolean") {
+ throw InvalidArgumentError("Value has to be of type 'boolean'");
+ }
+
+ this._server.acceptConnections = cmd.parameters.value;
+}
+
+/**
* Quits Firefox with the provided flags and tears down the current
* session.
*/
GeckoDriver.prototype.quitApplication = function(cmd, resp) {
if (this.appName != "Firefox") {
throw new WebDriverError("In app initiated quit only supported in Firefox");
}
let flags = Ci.nsIAppStartup.eAttemptQuit;
for (let k of cmd.parameters.flags || []) {
flags |= Ci.nsIAppStartup[k];
}
- this.stopSignal_();
+ this._server.acceptConnections = false;
resp.send();
this.sessionTearDown();
Services.startup.quit(flags);
};
/**
* Helper function to convert an outerWindowID into a UID that Marionette
@@ -2771,10 +2795,11 @@ GeckoDriver.prototype.commands = {
"setScreenOrientation": GeckoDriver.prototype.setScreenOrientation,
"getWindowSize": GeckoDriver.prototype.getWindowSize,
"setWindowSize": GeckoDriver.prototype.setWindowSize,
"maximizeWindow": GeckoDriver.prototype.maximizeWindow,
"dismissDialog": GeckoDriver.prototype.dismissDialog,
"acceptDialog": GeckoDriver.prototype.acceptDialog,
"getTextFromDialog": GeckoDriver.prototype.getTextFromDialog,
"sendKeysToDialog": GeckoDriver.prototype.sendKeysToDialog,
+ "acceptConnections": GeckoDriver.prototype.acceptConnections,
"quitApplication": GeckoDriver.prototype.quitApplication,
};
--- a/testing/marionette/harness/marionette/tests/unit/test_quit_restart.py
+++ b/testing/marionette/harness/marionette/tests/unit/test_quit_restart.py
@@ -60,19 +60,18 @@ class TestQuitRestart(MarionetteTestCase
self.assertEqual(self.marionette.session["processId"], self.pid)
else:
self.assertNotEqual(self.marionette.session["processId"], self.pid)
# If a preference value is not forced, a restart will cause a reset
self.assertNotEqual(self.marionette.get_pref("browser.startup.page"), 3)
def test_in_app_restart_with_callback(self):
- def callback():
- self.marionette._request_in_app_shutdown(shutdown_flags='eRestart')
- self.marionette.restart(in_app=True, callback=callback)
+ self.marionette.restart(in_app=True,
+ callback=lambda: self.shutdown(restart=True))
self.assertEqual(self.marionette.session_id, self.session_id)
# An in-app restart will keep the same process id only on Linux
if self.marionette.session_capabilities['platformName'] == 'linux':
self.assertEqual(self.marionette.session["processId"], self.pid)
else:
self.assertNotEqual(self.marionette.session["processId"], self.pid)
@@ -87,17 +86,27 @@ class TestQuitRestart(MarionetteTestCase
with self.assertRaisesRegexp(MarionetteException, "Please start a session"):
self.marionette.get_url()
self.marionette.start_session()
self.assertNotEqual(self.marionette.session_id, self.session_id)
self.assertNotEqual(self.marionette.get_pref("browser.startup.page"), 3)
def test_in_app_quit_with_callback(self):
- self.marionette.quit(in_app=True,
- callback=self.marionette._request_in_app_shutdown)
+ self.marionette.quit(in_app=True, callback=self.shutdown)
self.assertEqual(self.marionette.session, None)
with self.assertRaisesRegexp(MarionetteException, "Please start a session"):
self.marionette.get_url()
self.marionette.start_session()
self.assertNotEqual(self.marionette.session_id, self.session_id)
self.assertNotEqual(self.marionette.get_pref("browser.startup.page"), 3)
+
+ def shutdown(self, restart=False):
+ self.marionette.set_context("chrome")
+ self.marionette.execute_script("""
+ Components.utils.import("resource://gre/modules/Services.jsm");
+ let flags = Ci.nsIAppStartup.eAttemptQuit
+ if(arguments[0]) {
+ flags |= Ci.nsIAppStartup.eRestart;
+ }
+ Services.startup.quit(flags);
+ """, script_args=[restart])
--- a/testing/marionette/server.js
+++ b/testing/marionette/server.js
@@ -42,16 +42,17 @@ const MANAGE_OFFLINE_STATUS_PREF = "netw
* accept all connections.
*/
this.MarionetteServer = function(port, forceLocal) {
this.port = port;
this.forceLocal = forceLocal;
this.conns = {};
this.nextConnId = 0;
this.alive = false;
+ this._acceptConnections = false;
};
/**
* Function produces a GeckoDriver.
*
* Determines application nameto initialise the driver with.
*
* @return {GeckoDriver}
@@ -65,48 +66,64 @@ MarionetteServer.prototype.driverFactory
if (bypassOffline) {
logger.debug("Bypassing offline status");
Preferences.set(MANAGE_OFFLINE_STATUS_PREF, false);
Services.io.manageOfflineStatus = false;
Services.io.offline = false;
}
- let stopSignal = () => this.stop();
- return new GeckoDriver(appName, stopSignal);
+ return new GeckoDriver(appName, this);
};
+MarionetteServer.prototype.__defineSetter__("acceptConnections", function(value) {
+ if (!value) {
+ logger.info("New connections will no longer be accepted");
+ } else {
+ logger.info("New connections are accepted again");
+ }
+
+ this._acceptConnections = value;
+});
+
MarionetteServer.prototype.start = function() {
if (this.alive) {
return;
}
let flags = Ci.nsIServerSocket.KeepWhenOffline;
if (this.forceLocal) {
flags |= Ci.nsIServerSocket.LoopbackOnly;
}
this.listener = new ServerSocket(this.port, flags, 1);
this.listener.asyncListen(this);
this.alive = true;
+ this._acceptConnections = true;
};
MarionetteServer.prototype.stop = function() {
if (!this.alive) {
return;
}
this.closeListener();
this.alive = false;
+ this._acceptConnections = false;
};
MarionetteServer.prototype.closeListener = function() {
this.listener.close();
this.listener = null;
};
MarionetteServer.prototype.onSocketAccepted = function(
serverSocket, clientSocket) {
+ if (!this._acceptConnections) {
+ logger.warn("New connections are currently not accepted");
+ return;
+ }
+
let input = clientSocket.openInputStream(0, 0, 0);
let output = clientSocket.openOutputStream(0, 0, 0);
let transport = new DebuggerTransport(input, output);
let connId = "conn" + this.nextConnId++;
let dispatcher = new Dispatcher(connId, transport, this.driverFactory.bind(this));
dispatcher.onclose = this.onConnectionClosed.bind(this);
this.conns[connId] = dispatcher;