Bug 1366784 - Force quit the application if requested quit or restart doesn't happen. r=maja_zf
authorHenrik Skupin <mail@hskupin.info>
Thu, 08 Jun 2017 15:59:07 +0200
changeset 411148 b30457bfb195fd7abb1cb938cc4ca151112f6c5b
parent 411147 dcffcfdf03ad254ae04b91fe6b230cd455723f7a
child 411149 779cf4deec412fb33de2ce3765a595d02ba25002
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmaja_zf
bugs1366784
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1366784 - Force quit the application if requested quit or restart doesn't happen. r=maja_zf 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
@@ -1081,17 +1081,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
@@ -1122,27 +1130,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(),