Bug 1373635 - Application profile has only be reset if explicitly requested. r=ato
authorHenrik Skupin <mail@hskupin.info>
Fri, 16 Jun 2017 15:31:45 +0200
changeset 364413 274e6b6d1399b1d1e042b4150b6704d537bc1a60
parent 364412 943dc0fef2638e9fe70b8328e2b67e057210358f
child 364414 1f216a97ac0633e2ffd5f9af2893e4610f7206f7
push id44912
push userhskupin@mozilla.com
push dateFri, 16 Jun 2017 22:21:58 +0000
treeherderautoland@274e6b6d1399 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1373635
milestone56.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 1373635 - Application profile has only be reset if explicitly requested. r=ato For both quit() and restart() methods the profile should not be reset, unless it has been requested. The current behavior breaks various tests which make use of quit() and session_start() and which assume that previously set preferences are still set, eg. sessionrestore tests. MozReview-Commit-ID: 4BxSSJPrTYF
testing/marionette/client/marionette_driver/geckoinstance.py
testing/marionette/client/marionette_driver/marionette.py
testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
--- a/testing/marionette/client/marionette_driver/geckoinstance.py
+++ b/testing/marionette/client/marionette_driver/geckoinstance.py
@@ -246,46 +246,43 @@ class GeckoInstance(object):
             "binary": self.binary,
             "profile": self.profile,
             "cmdargs": ["-no-remote", "-marionette"] + self.app_args,
             "env": env,
             "symbols_path": self.symbols_path,
             "process_args": process_args
         }
 
-    def close(self, restart=False, clean=False):
+    def close(self, clean=False):
         """
         Close the managed Gecko process.
 
         Depending on self.runner_class, setting `clean` to True may also kill
         the emulator process in which this instance is running.
 
         :param restart: If True, assume this is being called by restart method.
         :param clean: If True, also perform runner cleanup.
         """
-        if not restart:
-            self.profile = None
-
         if self.runner:
             self.runner.stop()
             if clean:
                 self.runner.cleanup()
 
+        if clean and self.profile:
+            self.profile.cleanup()
+            self.profile = None
+
     def restart(self, prefs=None, clean=True):
         """
         Close then start the managed Gecko process.
 
         :param prefs: Dictionary of preference names and values.
         :param clean: If True, reset the profile before starting.
         """
-        self.close(restart=True)
-
-        if clean and self.profile:
-            self.profile.cleanup()
-            self.profile = None
+        self.close(clean=clean)
 
         if prefs:
             self.prefs = prefs
         else:
             self.prefs = None
         self.start()
 
 
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -1059,23 +1059,26 @@ class Marionette(object):
         if len(flags) > 0:
             body = {"flags": list(flags)}
 
         # quitApplication was renamed quit in bug 1337743,
         # and this can safely be renamed when Firefox 56 becomes stable
         self._send_message("quitApplication", body)
 
     @do_process_check
-    def quit(self, in_app=False, callback=None):
+    def quit(self, clean=False, 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, :func:`start_session` has to be called.
 
+        :param clean: If False the same profile will be used after the next start of
+                      the application. Note that the in app initiated restart always
+                      maintains the same profile.
         :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.
         :param callback: If provided and `in_app` is True, the callback will
                          be used to trigger the shutdown.
         """
         if not self.instance:
             raise errors.MarionetteException("quit() can only be called "
@@ -1098,17 +1101,17 @@ class Marionette(object):
                 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()
+            self.instance.close(clean=clean)
 
     @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
         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
@@ -1187,18 +1190,18 @@ class Marionette(object):
         :returns: A dict of the capabilities offered.
 
         """
         self.crashed = 0
 
         if self.instance:
             returncode = self.instance.runner.returncode
             if returncode is not None:
-                # We're managing a binary which has terminated, so restart it.
-                self.instance.restart()
+                # We're managing a binary which has terminated, so start it again.
+                self.instance.start()
 
         self.client = transport.TcpTransport(
             self.host,
             self.port,
             self.socket_timeout)
 
         # Call wait_for_port() before attempting to connect in
         # the event gecko hasn't started yet.
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
@@ -65,16 +65,17 @@ class TestServerQuitApplication(Marionet
 
 
 class TestQuitRestart(MarionetteTestCase):
 
     def setUp(self):
         MarionetteTestCase.setUp(self)
 
         self.pid = self.marionette.process_id
+        self.profile = self.marionette.profile
         self.session_id = self.marionette.session_id
 
         # Use a preference to check that the restart was successful. If its
         # value has not been forced, a restart will cause a reset of it.
         self.assertNotEqual(self.marionette.get_pref("startup.homepage_welcome_url"),
                             "about:")
         self.marionette.set_pref("startup.homepage_welcome_url", "about:")
 
@@ -93,49 +94,75 @@ class TestQuitRestart(MarionetteTestCase
             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,))
 
-    def test_force_restart(self):
-        self.marionette.restart()
+    def test_force_clean_restart(self):
+        self.marionette.restart(clean=True)
+        self.assertNotEqual(self.marionette.profile, self.profile)
         self.assertEqual(self.marionette.session_id, self.session_id)
 
         # A forced restart will cause a new process id
         self.assertNotEqual(self.marionette.process_id, self.pid)
         self.assertNotEqual(self.marionette.get_pref("startup.homepage_welcome_url"),
                             "about:")
 
+    def test_force_restart(self):
+        self.marionette.restart()
+        self.assertEqual(self.marionette.profile, self.profile)
+        self.assertEqual(self.marionette.session_id, self.session_id)
+
+        # A forced restart will cause a new process id
+        self.assertNotEqual(self.marionette.process_id, self.pid)
+        self.assertNotEqual(self.marionette.get_pref("startup.homepage_welcome_url"),
+                            "about:")
+
+    def test_force_clean_quit(self):
+        self.marionette.quit(clean=True)
+
+        self.assertEqual(self.marionette.session, None)
+        with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"):
+            self.marionette.get_url()
+
+        self.marionette.start_session()
+        self.assertNotEqual(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_force_quit(self):
         self.marionette.quit()
 
         self.assertEqual(self.marionette.session, None)
         with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"):
             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:")
 
     @skip("Bug 1363368 - Wrong window handles after in_app restarts")
-    def test_in_app_clean_restart(self):
+    def test_no_in_app_clean_restart(self):
         # Test that in_app and clean cannot be used in combination
         with self.assertRaises(ValueError):
             self.marionette.restart(in_app=True, clean=True)
 
     @skip("Bug 1363368 - Wrong window handles after in_app restarts")
     def test_in_app_restart(self):
         if self.marionette.session_capabilities["platformName"] != "windows_nt":
             skip("Bug 1363368 - Wrong window handles after in_app restarts")
 
         self.marionette.restart(in_app=True)
+        self.assertEqual(self.marionette.profile, self.profile)
         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.process_id, self.pid)
         else:
             self.assertNotEqual(self.marionette.process_id, self.pid)
 
@@ -145,16 +172,17 @@ class TestQuitRestart(MarionetteTestCase
     @skip("Bug 1363368 - Wrong window handles after in_app restarts")
     def test_in_app_restart_with_callback(self):
         if self.marionette.session_capabilities["platformName"] != "windows_nt":
             skip("Bug 1363368 - Wrong window handles after in_app restarts")
 
         self.marionette.restart(in_app=True,
                                 callback=lambda: self.shutdown(restart=True))
 
+        self.assertEqual(self.marionette.profile, self.profile)
         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.process_id, self.pid)
         else:
             self.assertNotEqual(self.marionette.process_id, self.pid)
 
@@ -181,31 +209,33 @@ class TestQuitRestart(MarionetteTestCase
 
         self.marionette.quit(in_app=True)
 
         self.assertEqual(self.marionette.session, None)
         with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"):
             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:")
 
     @skip("Bug 1363368 - Wrong window handles after in_app restarts")
     def test_in_app_quit_with_callback(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, callback=self.shutdown)
         self.assertEqual(self.marionette.session, None)
         with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"):
             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):
         try:
             timeout = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
             self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 10