Bug 1411197 - Quit/Restart without a callable callback shouldn't perform a shutdown. draft
authorHenrik Skupin <mail@hskupin.info>
Fri, 27 Oct 2017 15:42:31 +0200
changeset 687586 828f31ad1e1175a26473fe200baf651a31eb5b10
parent 687186 aa958b29c149a67fce772f8473e9586e71fbdb46
child 737696 7c329ed3cb546f61d01d6db5504a0111aa5dd704
push id86550
push userbmo:hskupin@gmail.com
push dateFri, 27 Oct 2017 13:43:00 +0000
bugs1411197
milestone58.0a1
Bug 1411197 - Quit/Restart without a callable callback shouldn't perform a shutdown. MozReview-Commit-ID: 9qCdmGKFocB
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
@@ -1091,17 +1091,20 @@ class Marionette(object):
                          be used to trigger the shutdown.
         """
         if not self.instance:
             raise errors.MarionetteException("quit() can only be called "
                                              "on Gecko instances launched by Marionette")
 
         cause = None
         if in_app:
-            if callable(callback):
+            if callback is not None:
+                if not callable(callback):
+                    raise ValueError("Specified callback '{}' is not callable".format(callback))
+
                 self._send_message("acceptConnections", {"value": False})
                 callback()
             else:
                 cause = self._request_in_app_shutdown()
 
             # Ensure to explicitely mark the session as deleted
             self.delete_session(send_request=False, reset_session_id=True)
 
@@ -1143,17 +1146,20 @@ class Marionette(object):
                                              "on Gecko instances launched by Marionette")
         context = self._send_message("getContext", key="value")
 
         cause = None
         if in_app:
             if clean:
                 raise ValueError("An in_app restart cannot be triggered with the clean flag set")
 
-            if callable(callback):
+            if callback is not None:
+                if not callable(callback):
+                    raise ValueError("Specified callback '{}' is not callable".format(callback))
+
                 self._send_message("acceptConnections", {"value": False})
                 callback()
             else:
                 cause = self._request_in_app_shutdown("eRestart")
 
             # Ensure to explicitely mark the session as deleted
             self.delete_session(send_request=False, reset_session_id=True)
 
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
@@ -218,17 +218,21 @@ class TestQuitRestart(MarionetteTestCase
 
         try:
             self.assertFalse(self.is_safe_mode, "Safe Mode is unexpectedly enabled")
             self.marionette.restart(in_app=True, callback=restart_in_safe_mode)
             self.assertTrue(self.is_safe_mode, "Safe Mode is not enabled")
         finally:
             self.marionette.quit(clean=True)
 
-    def test_in_app_restart_with_callback_no_shutdown(self):
+    def test_in_app_restart_with_callback_not_callable(self):
+        with self.assertRaisesRegexp(ValueError, "is not callable"):
+            self.marionette.restart(in_app=True, callback=4)
+
+    def test_in_app_restart_with_callback_missing_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)
@@ -264,26 +268,30 @@ class TestQuitRestart(MarionetteTestCase
             self.marionette.get_url()
 
         self.marionette.start_session()
         self.assertEqual(self.marionette.profile, self.profile)
         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):
+    def test_in_app_quit_with_callback_missing_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
 
+    def test_in_app_quit_with_callback_not_callable(self):
+        with self.assertRaisesRegexp(ValueError, "is not callable"):
+            self.marionette.restart(in_app=True, callback=4)
+
     @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(),