Bug 1296597 - Allow Marionette to quit a running application instance. draft
authorHenrik Skupin <mail@hskupin.info>
Mon, 22 Aug 2016 13:20:08 +0200
changeset 406870 071e9635213a17304af1d26adc39cd29290bd429
parent 406838 acfb2c3ac6ae0a704e2756184815296ac1314f89
child 529773 60436d6af42c98379b07ec5b72040f4eae7eb179
push id27854
push userbmo:hskupin@gmail.com
push dateMon, 29 Aug 2016 19:29:32 +0000
bugs1296597
milestone51.0a1
Bug 1296597 - Allow Marionette to quit a running application instance. MozReview-Commit-ID: 4RJmGQ1IqHw
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/harness/marionette/marionette_test.py
testing/marionette/harness/marionette/tests/unit/test_profile_management.py
testing/marionette/harness/marionette/tests/unit/test_quit_restart.py
testing/marionette/harness/marionette/tests/unit/unit-tests.ini
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -752,17 +752,17 @@ class Marionette(object):
         If we've launched the binary we are connected to, wait for it to shut down.
         In the case when it doesn't happen, force its shut down.
 
         """
         if self.instance:
             exc, val, tb = sys.exc_info()
 
             # Give the application some time to shutdown
-            returncode = self.instance.runner.wait(timeout=self.DEFAULT_STARTUP_TIMEOUT)
+            returncode = self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
             if returncode is None:
                 self.cleanup()
                 message = ('Process killed because the connection to Marionette server is lost.'
                            ' Check gecko.log for errors')
             else:
                 message = 'Process has been closed (Exit code: {returncode})'
 
             if exc:
@@ -1027,74 +1027,102 @@ class Marionette(object):
         self.set_context(self.CONTEXT_CONTENT)
         if not pref_exists:
             self.delete_session()
             self.instance.restart(prefs)
             self.raise_for_port(self.wait_for_port())
             self.start_session()
             self.reset_timeouts()
 
+    def _request_in_app_shutdown(self, shutdown_flags=None):
+        """Terminate the currently running instance from inside the application.
+
+        :param shutdown_flags: If specified use additional flags for the shutdown
+                               of the application. Possible values here correspond
+                               to constants in nsIAppStartup: http://mzl.la/1X0JZsC.
+        """
+        flags = set(["eForceQuit"])
+        if shutdown_flags:
+            flags.add(shutdown_flags)
+        self._send_message("quitApplication", {"flags": list(flags)})
+
+        self.delete_session(in_app=True)
+
     @do_process_check
-    def quit_in_app(self):
-        """
-        This will terminate the currently running instance.
+    def quit(self, in_app=False):
+        """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.
+
+        :param in_app: If True, marionette will cause a quit from within the
+                       browser. Otherwise the browser will be quit immediately
+                       by killing the process.
         """
         if not self.instance:
-            raise errors.MarionetteException("quit_in_app() can only be called "
+            raise errors.MarionetteException("quit() can only be called "
                                              "on Gecko instances launched by Marionette")
-        # Values here correspond to constants in nsIAppStartup.
-        # See http://mzl.la/1X0JZsC
-        restart_flags = [
-            "eForceQuit",
-            "eRestart",
-        ]
-        self._send_message("quitApplication", {"flags": restart_flags})
-        self.client.close()
+
+        self.reset_timeouts()
+
+        if in_app:
+            self._request_in_app_shutdown()
 
-        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
+            # 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
     def restart(self, clean=False, in_app=False):
         """
         This will terminate the currently running instance, and spawn a new instance
         with the same profile and then reuse the session id when creating a session again.
 
-        : param clean: If False the same profile will be used after the restart. Note
-                       that the in app initiated restart always maintains the same
-                       profile.
-        : param in_app: If True, marionette will cause a restart from within the
-                        browser. Otherwise the browser will be restarted immediately
-                        by killing the process.
+        :param clean: If False the same profile will be used after the restart. Note
+                      that the in app initiated restart always maintains the same
+                      profile.
+        :param in_app: If True, marionette will cause a restart from within the
+                       browser. Otherwise the browser will be restarted immediately
+                       by killing the process.
         """
         if not self.instance:
             raise errors.MarionetteException("restart() can only be called "
                                              "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")
-            self.quit_in_app()
+
+            self._request_in_app_shutdown("eRestart")
+
+            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
+
         else:
             self.delete_session()
             self.instance.restart(clean=clean)
             self.raise_for_port(self.wait_for_port())
 
-        self.start_session(session_id=self.session_id)
+        self.start_session(session_id=session_id)
         self.reset_timeouts()
 
-        if in_app and self.session.get('processId'):
+        if in_app and self.session.get("processId"):
             # In some cases Firefox restarts itself by spawning into a new process group.
             # As long as mozprocess cannot track that behavior (bug 1284864) we assist by
             # informing about the new process id.
-            self.instance.runner.process_handler.check_for_detached(self.session['processId'])
+            self.instance.runner.process_handler.check_for_detached(self.session["processId"])
 
     def absolute_url(self, relative_url):
         '''
         Returns an absolute url for files served from Marionette's www directory.
 
         :param relative_url: The url of a static file, relative to Marionette's www directory.
         '''
         return "%s%s" % (self.baseurl, relative_url)
@@ -1108,17 +1136,17 @@ class Marionette(object):
         :param desired_capabilities: An optional dict of desired
             capabilities.  This is currently ignored.
         :param timeout: Timeout in seconds for the server to be ready.
         :param session_id: unique identifier for the session. If no session id is
             passed in then one will be generated by the marionette server.
 
         :returns: A dict of the capabilities offered."""
         if self.instance:
-            returncode = self.instance.runner.process_handler.proc.returncode
+            returncode = self.instance.runner.returncode
             if returncode is not None:
                 # We're managing a binary which has terminated, so restart it.
                 self.instance.restart()
 
         self.client = transport.TcpTransport(
             self.host,
             self.port,
             self.socket_timeout)
@@ -1140,19 +1168,25 @@ class Marionette(object):
     def test_name(self):
         return self._test_name
 
     @test_name.setter
     def test_name(self, test_name):
         self._send_message("setTestName", {"value": test_name})
         self._test_name = test_name
 
-    def delete_session(self):
-        """Close the current session and disconnect from the server."""
-        self._send_message("deleteSession")
+    def delete_session(self, in_app=False):
+        """Close the current session and disconnect from the server.
+
+        :param in_app: False, if the session should be closed from the client.
+                       Otherwise a request to quit or restart the instance from
+                       within the application itself is used.
+        """
+        if not in_app:
+            self._send_message("deleteSession")
         self.session_id = None
         self.session = None
         self.window = None
         self.client.close()
 
     @property
     def session_capabilities(self):
         '''
--- a/testing/marionette/harness/marionette/marionette_test.py
+++ b/testing/marionette/harness/marionette/marionette_test.py
@@ -661,16 +661,21 @@ class MarionetteTestCase(CommonTestCase)
     def setUp(self):
         CommonTestCase.setUp(self)
         self.marionette.test_name = self.test_name
         self.marionette.execute_script("log('TEST-START: %s:%s')" %
                                        (self.filepath.replace('\\', '\\\\'), self.methodName),
                                        sandbox="simpletest")
 
     def tearDown(self):
+        # In the case no session is active (eg. the application was quit), start
+        # a new session for clean-up steps.
+        if not self.marionette.session:
+            self.marionette.start_session()
+
         if not self.marionette.check_for_crash():
             try:
                 self.marionette.clear_imported_scripts()
                 self.marionette.execute_script("log('TEST-END: %s:%s')" %
                                                (self.filepath.replace('\\', '\\\\'),
                                                 self.methodName),
                                                sandbox="simpletest")
                 self.marionette.test_name = None
--- a/testing/marionette/harness/marionette/tests/unit/test_profile_management.py
+++ b/testing/marionette/harness/marionette/tests/unit/test_profile_management.py
@@ -1,66 +1,36 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-import os
-from marionette_driver.errors import JavascriptException
 from marionette import MarionetteTestCase
 
-class TestLog(MarionetteTestCase):
+
+class TestProfileManagement(MarionetteTestCase):
+
     def setUp(self):
         MarionetteTestCase.setUp(self)
-        self.marionette.enforce_gecko_prefs({"marionette.test.bool": True, "marionette.test.string": "testing", "marionette.test.int": 3})
-        self.marionette.set_context('chrome')
+        self.marionette.enforce_gecko_prefs(
+            {"marionette.test.bool": True,
+             "marionette.test.string": "testing",
+             "marionette.test.int": 3
+             })
+        self.marionette.set_context("chrome")
 
     def test_preferences_are_set(self):
-        bool_value = self.marionette.execute_script("return Services.prefs.getBoolPref('marionette.test.bool');")
-        string_value = self.marionette.execute_script("return Services.prefs.getCharPref('marionette.test.string');")
-        int_value = self.marionette.execute_script("return Services.prefs.getIntPref('marionette.test.int');")
-        self.assertTrue(bool_value)
-        self.assertEqual(string_value, "testing")
-        self.assertEqual(int_value, 3)
+        self.assertTrue(self.marionette.get_pref("marionette.test.bool"))
+        self.assertEqual(self.marionette.get_pref("marionette.test.string"), "testing")
+        self.assertEqual(self.marionette.get_pref("marionette.test.int"), 3)
 
-    def test_change_preset(self):
-        bool_value = self.marionette.execute_script("return Services.prefs.getBoolPref('marionette.test.bool');")
-        self.assertTrue(bool_value)
+    def test_change_preference(self):
+        self.assertTrue(self.marionette.get_pref("marionette.test.bool"))
+
         self.marionette.enforce_gecko_prefs({"marionette.test.bool": False})
         self.marionette.set_context('chrome')
-        bool_value = self.marionette.execute_script("return Services.prefs.getBoolPref('marionette.test.bool');")
-        self.assertFalse(bool_value)
+
+        self.assertFalse(self.marionette.get_pref("marionette.test.bool"))
 
     def test_clean_profile(self):
         self.marionette.restart(clean=True)
         self.marionette.set_context('chrome')
-        with self.assertRaisesRegexp(JavascriptException, "NS_ERROR_UNEXPECTED"):
-            bool_value = self.marionette.execute_script("return Services.prefs.getBoolPref('marionette.test.bool');")
 
-    def test_can_restart_the_browser(self):
-        self.marionette.enforce_gecko_prefs({"marionette.test.restart": True})
-        self.marionette.restart()
-        self.marionette.set_context('chrome')
-        bool_value = self.marionette.execute_script("return Services.prefs.getBoolPref('marionette.test.restart');")
-        self.assertTrue(bool_value)
-
-    def test_in_app_restart_the_browser(self):
-        self.marionette.execute_script("Services.prefs.setBoolPref('marionette.test.restart', true);")
-
-        # A "soft" restart initiated inside the application should keep track of this pref.
-        self.marionette.restart(in_app=True)
-        self.marionette.set_context('chrome')
-        bool_value = self.marionette.execute_script("""
-          return Services.prefs.getBoolPref('marionette.test.restart');
-        """)
-        self.assertTrue(bool_value)
-
-        bool_value = self.marionette.execute_script("""
-          Services.prefs.setBoolPref('marionette.test.restart', false);
-          return Services.prefs.getBoolPref('marionette.test.restart');
-        """)
-        self.assertFalse(bool_value)
-
-        # A "hard" restart is still possible (i.e., our instance is still able
-        # to kill the browser).
-        # Note we need to clean the profile at this point so the old browser
-        # process doesn't interfere with the new one on Windows when it attempts
-        # to access the profile on startup.
-        self.marionette.restart(clean=True)
+        self.assertEqual(self.marionette.get_pref("marionette.test.bool"), None)
new file mode 100644
--- /dev/null
+++ b/testing/marionette/harness/marionette/tests/unit/test_quit_restart.py
@@ -0,0 +1,76 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from marionette import MarionetteTestCase
+from marionette_driver.errors import MarionetteException
+
+
+class TestQuitRestart(MarionetteTestCase):
+
+    def setUp(self):
+        MarionetteTestCase.setUp(self)
+
+        self.pid = self.marionette.session["processId"]
+        self.session_id = self.marionette.session_id
+
+        self.assertNotEqual(self.marionette.get_pref("browser.startup.page"), 3)
+        self.marionette.set_pref("browser.startup.page", 3)
+
+    def tearDown(self):
+        # Ensure to restart a session if none exist for clean-up
+        if not self.marionette.session:
+            self.marionette.start_session()
+
+        self.marionette.clear_pref("browser.startup.page")
+
+        MarionetteTestCase.tearDown(self)
+
+    def test_force_restart(self):
+        self.marionette.restart()
+        self.assertEqual(self.marionette.session_id, self.session_id)
+
+        # A forced restart will cause a new process id
+        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(self):
+        self.marionette.restart(in_app=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)
+
+        # 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_clean_restart(self):
+        with self.assertRaises(ValueError):
+            self.marionette.restart(in_app=True, clean=True)
+
+    def test_force_quit(self):
+        self.marionette.quit()
+
+        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 test_in_app_quit(self):
+        self.marionette.quit(in_app=True)
+
+        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)
--- a/testing/marionette/harness/marionette/tests/unit/unit-tests.ini
+++ b/testing/marionette/harness/marionette/tests/unit/unit-tests.ini
@@ -107,16 +107,18 @@ skip-if = true # buildapp == 'b2g' -- Bu
 [test_chrome_async_finish.js]
 [test_screen_orientation.py]
 [test_errors.py]
 
 [test_execute_isolate.py]
 [test_click_scrolling.py]
 [test_profile_management.py]
 skip-if = buildapp == 'b2g'
+[test_quit_restart.py]
+skip-if = buildapp == 'b2g' || appname == 'fennec' # Bug 1298921
 [test_set_window_size.py]
 skip-if = buildapp == 'b2g' || os == "linux" || appname == 'fennec' # Bug 1085717
 [test_with_using_context.py]
 
 [test_modal_dialogs.py]
 skip-if = buildapp == 'b2g' || appname == 'fennec'
 [test_key_actions.py]
 [test_mouse_action.py]