Bug 1366784 - Force quit the application if requested quit or restart doesn't happen. draft
authorHenrik Skupin <mail@hskupin.info>
Thu, 08 Jun 2017 15:59:07 +0200
changeset 591014 7506fe04eb83b6fd82a4c3612d71a2481bba8529
parent 590317 a49112c7a5765802096b3fc298069b9495436107
child 632391 ee1b4d5fb2bd95fee9f1847f7d8f1ad389a31b9c
push id62920
push userbmo:hskupin@gmail.com
push dateThu, 08 Jun 2017 13:59:35 +0000
bugs1366784
milestone55.0a1
Bug 1366784 - Force quit the application if requested quit or restart doesn't happen. In case a quit or restart is requested, but eg. the in_app callback doesn't really trigger a shutdown of the application, Marionette has to force close it after the default shutdown timeout. This is necessary because "acceptConnections" is set to false and no further connection could be made to the still running application. MozReview-Commit-ID: GwSeYyjI6M9
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -1209,17 +1209,25 @@ class Marionette(object):
                 callback()
             else:
                 self._request_in_app_shutdown()
 
             # Ensure to explicitely mark the session as deleted
             self.delete_session(send_request=False, reset_session_id=True)
 
             # Give the application some time to shutdown
-            self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
+            returncode = self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
+            if returncode is None:
+                # This will force-close the application without sending any other message.
+                self.cleanup()
+
+                message = ("Process killed because a requested application quit did not happen "
+                           "within {}s. Check gecko.log for errors.")
+                raise IOError(message.format(self.DEFAULT_SHUTDOWN_TIMEOUT))
+
         else:
             self.delete_session(reset_session_id=True)
             self.instance.close()
 
     @do_process_check
     def restart(self, clean=False, in_app=False, callback=None):
         """
         This will terminate the currently running instance, and spawn a new instance
@@ -1250,27 +1258,28 @@ class Marionette(object):
                 callback()
             else:
                 self._request_in_app_shutdown("eRestart")
 
             # Ensure to explicitely mark the session as deleted
             self.delete_session(send_request=False, reset_session_id=True)
 
             try:
-                self.raise_for_port()
+                timeout = self.DEFAULT_SHUTDOWN_TIMEOUT + self.DEFAULT_STARTUP_TIMEOUT
+                self.raise_for_port(timeout=timeout)
             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.raise_for_port(timeout=self.DEFAULT_STARTUP_TIMEOUT)
 
         self.start_session(session_id=session_id)
 
         # Restore the context as used before the restart
         self.set_context(context)
 
         if in_app and self.process_id:
             # In some cases Firefox restarts itself by spawning into a new process group.
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
@@ -156,16 +156,29 @@ class TestQuitRestart(MarionetteTestCase
         if self.marionette.session_capabilities["platformName"] == "linux":
             self.assertEqual(self.marionette.process_id, self.pid)
         else:
             self.assertNotEqual(self.marionette.process_id, self.pid)
 
         self.assertNotEqual(self.marionette.get_pref("startup.homepage_welcome_url"),
                             "about:")
 
+    def test_in_app_restart_with_callback_no_shutdown(self):
+        try:
+            timeout_startup = self.marionette.DEFAULT_STARTUP_TIMEOUT
+            timeout_shutdown = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
+            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 5
+            self.marionette.DEFAULT_STARTUP_TIMEOUT = 5
+
+            with self.assertRaisesRegexp(IOError, "the connection to Marionette server is lost"):
+                self.marionette.restart(in_app=True, callback=lambda: False)
+        finally:
+            self.marionette.DEFAULT_STARTUP_TIMEOUT = timeout_startup
+            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = timeout_shutdown
+
     @skip("Bug 1363368 - Wrong window handles after in_app restarts")
     def test_in_app_quit(self):
         if self.marionette.session_capabilities["platformName"] != "windows_nt":
             skip("Bug 1363368 - Wrong window handles after in_app restarts")
 
         self.marionette.quit(in_app=True)
 
         self.assertEqual(self.marionette.session, None)
@@ -187,16 +200,26 @@ class TestQuitRestart(MarionetteTestCase
         with self.assertRaisesRegexp(errors.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("startup.homepage_welcome_url"),
                             "about:")
 
+    def test_in_app_quit_with_callback_no_shutdown(self):
+        try:
+            timeout = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
+            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 10
+
+            with self.assertRaisesRegexp(IOError, "a requested application quit did not happen"):
+                self.marionette.quit(in_app=True, callback=lambda: False)
+        finally:
+            self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = timeout
+
     @skip("Bug 1363368 - Wrong window handles after in_app restarts")
     def test_reset_context_after_quit_by_set_context(self):
         if self.marionette.session_capabilities["platformName"] != "windows_nt":
             skip("Bug 1363368 - Wrong window handles after in_app restarts")
 
         # Check that we are in content context which is used by default in
         # Marionette
         self.assertNotIn("chrome://", self.marionette.get_url(),