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

--HG--
extra : rebase_source : 534c0608caf31c1e2cc256ad3fa8ae1972204603

testing/marionette/client/marionette_driver/geckoinstance.py

@@ -251,7 +251,7 @@ class GeckoInstance(object):
             "process_args": process_args
         }
 
-    def close(self, restart=False, clean=False):
+    def close(self, clean=False):
         """
         Close the managed Gecko process.
 
@@ -261,14 +261,15 @@ class GeckoInstance(object):
         :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.
@@ -276,11 +277,7 @@ class GeckoInstance(object):
         :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

testing/marionette/client/marionette_driver/marionette.py

@@ -1064,13 +1064,16 @@ class Marionette(object):
         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.
@@ -1103,7 +1106,7 @@ class Marionette(object):
 
         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):
@@ -1192,8 +1195,8 @@ class Marionette(object):
         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,

testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py

@@ -70,6 +70,7 @@ class TestQuitRestart(MarionetteTestCase):
         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
@@ -98,8 +99,9 @@ class TestQuitRestart(MarionetteTestCase):
             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
@@ -107,6 +109,29 @@ class TestQuitRestart(MarionetteTestCase):
         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()
 
@@ -115,12 +140,13 @@ 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:")
 
     @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)
@@ -131,6 +157,7 @@ class TestQuitRestart(MarionetteTestCase):
             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
@@ -150,6 +177,7 @@ class TestQuitRestart(MarionetteTestCase):
         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
@@ -186,6 +214,7 @@ 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:")
@@ -201,6 +230,7 @@ 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:")