Bug 1493796 - [mozprocess] Revert poll() behavior on Windows due to regression. r=gbrown
authorHenrik Skupin <mail@hskupin.info>
Thu, 18 Oct 2018 15:32:28 +0000
changeset 490330 9782e0b1657da7b2742e693222f8ac912920b521
parent 490329 740c3623a44aed64203d830a03682e14f90917f9
child 490331 b5b389d5788ed89f7e666c4de6e80e6a3cb99a19
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersgbrown
bugs1493796, 1433905
milestone64.0a1
Bug 1493796 - [mozprocess] Revert poll() behavior on Windows due to regression. r=gbrown Originally landed as changeset 8793e332890e via bug 1433905 the patch caused a regression because GetExitCodeProcess() returns 0 for an inside of Firefox restarted process. It can be relanded once the process id of the job object can successfully be tracked. Differential Revision: https://phabricator.services.mozilla.com/D9020
testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
testing/mozbase/mozprocess/mozprocess/processhandler.py
testing/mozbase/mozprocess/tests/test_poll.py
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py
@@ -1,14 +1,16 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 from __future__ import absolute_import, print_function
 
+import sys
+import unittest
 import urllib
 
 from unittest import skip
 
 from marionette_driver import errors
 from marionette_driver.by import By
 from marionette_harness import MarionetteTestCase
 
@@ -191,27 +193,29 @@ class TestQuitRestart(MarionetteTestCase
 
         self.assertNotEqual(self.marionette.get_pref("startup.homepage_welcome_url"),
                             "about:about")
 
     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)
 
+    @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_in_app_restart_with_callback_but_process_quit(self):
         timeout_shutdown = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
         # Wait at least 70s for the hang monitor in case of a shutdown hang
         self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 70
 
         try:
             with self.assertRaisesRegexp(IOError, "Process unexpectedly quit without restarting"):
                 self.marionette.restart(in_app=True, callback=lambda: self.shutdown(restart=False))
         finally:
             self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = timeout_shutdown
 
+    @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_in_app_restart_with_callback_missing_shutdown(self):
         try:
             timeout_shutdown = self.marionette.DEFAULT_SHUTDOWN_TIMEOUT
             self.marionette.DEFAULT_SHUTDOWN_TIMEOUT = 5
 
             with self.assertRaisesRegexp(IOError, "the connection to Marionette server is lost"):
                 self.marionette.restart(in_app=True, callback=lambda: False)
         finally:
--- a/testing/mozbase/mozprocess/mozprocess/processhandler.py
+++ b/testing/mozbase/mozprocess/mozprocess/processhandler.py
@@ -200,37 +200,24 @@ class ProcessHandlerMixin(object):
                     # a signal was explicitly set or not posix
                     send_sig(sig or signal.SIGKILL)
 
             self.returncode = self.wait()
             self._cleanup()
             return self.returncode
 
         def poll(self):
-            """Check if child process has terminated, and set returncode attribute.
-
-            This method overrides the Popen.poll implementation for our custom
-            process handling for Windows.
+            """ Popen.poll
+                Check if child process has terminated. Set and return returncode attribute.
             """
-            if isWin:
-                # If we have a handle, check that the process is still alive.
-                if self._handle:
-                    returncode = winprocess.GetExitCodeProcess(self._handle)
+            # If we have a handle, the process is alive
+            if isWin and getattr(self, '_handle', None):
+                return None
 
-                    # If the process doesn't exist anymore run cleanup steps
-                    if returncode != winprocess.STILL_ACTIVE:
-                        self.returncode = returncode
-                        self._cleanup()
-                    else:
-                        self.returncode = None
-
-            else:
-                self.returncode = subprocess.Popen.poll(self)
-
-            return self.returncode
+            return subprocess.Popen.poll(self)
 
         def wait(self):
             """ Popen.wait
                 Called to wait for a running process to shut down and return
                 its exit code
                 Returns the main process's exit code
             """
             # This call will be different for each OS
--- a/testing/mozbase/mozprocess/tests/test_poll.py
+++ b/testing/mozbase/mozprocess/tests/test_poll.py
@@ -1,15 +1,17 @@
 #!/usr/bin/env python
 
 from __future__ import absolute_import
 
 import os
 import signal
+import sys
 import time
+import unittest
 
 import mozinfo
 import mozunit
 
 from mozprocess import processhandler
 
 import proctest
 
@@ -98,16 +100,17 @@ class ProcTestPoll(proctest.ProcTest):
         else:
             self.assertLess(returncode, 0,
                             'Negative returncode expected, got "%s"' % returncode)
 
         self.assertEqual(returncode, p.poll())
 
         self.determine_status(p)
 
+    @unittest.skipIf(sys.platform.startswith("win"), "Bug 1493796")
     def test_poll_after_external_kill(self):
         """Process is killed externally, and poll() is called."""
         p = processhandler.ProcessHandler([self.python, self.proclaunch,
                                            "process_normal_finish.ini"],
                                           cwd=here)
         p.run()
 
         os.kill(p.pid, signal.SIGTERM)