Bug 1299216 - Wait for process exit first before checking for crashes. draft
authorHenrik Skupin <mail@hskupin.info>
Tue, 01 Nov 2016 10:48:25 +0100
changeset 435305 06bf39a1b60975d418af6302ec3764b5661b5b2e
parent 435304 bc4f6e73b9d49cda6c251ba5360f984bb626daa4
child 536268 a82358e797259fb3d5bec7998c152496527a452c
push id34992
push userbmo:hskupin@gmail.com
push dateTue, 08 Nov 2016 11:06:47 +0000
bugs1299216
milestone52.0a1
Bug 1299216 - Wait for process exit first before checking for crashes. MozReview-Commit-ID: 8U48dNHoFmi
testing/marionette/client/marionette_driver/decorators.py
testing/marionette/client/marionette_driver/marionette.py
--- a/testing/marionette/client/marionette_driver/decorators.py
+++ b/testing/marionette/client/marionette_driver/decorators.py
@@ -1,50 +1,36 @@
 # 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 functools import wraps
 import socket
-import sys
-import traceback
 
 
 def _find_marionette_in_args(*args, **kwargs):
     try:
         m = [a for a in args + tuple(kwargs.values()) if hasattr(a, 'session')][0]
     except IndexError:
         print("Can only apply decorator to function using a marionette object")
         raise
     return m
 
 
 def do_process_check(func):
     """Decorator which checks the process status after the function has run."""
     @wraps(func)
     def _(*args, **kwargs):
-        m = _find_marionette_in_args(*args, **kwargs)
-
         try:
             return func(*args, **kwargs)
-        except IOError as e:
-            exc, val, tb = sys.exc_info()
-            crashed = False
-
-            try:
-                crashed = m.check_for_crash()
-            except Exception:
-                # don't want to lose the original exception
-                traceback.print_exc()
-
-            # In case of socket failures force a shutdown of the application
-            if type(e) in (socket.error, socket.timeout) or crashed:
-                m.handle_socket_failure(crashed)
-
-            raise exc, val, tb
+        except (socket.error, socket.timeout):
+            # In case of socket failures which will also include crashes of the
+            # application, make sure to handle those correctly.
+            m = _find_marionette_in_args(*args, **kwargs)
+            m.handle_socket_failure()
 
     return _
 
 
 def uses_marionette(func):
     """Decorator which creates a marionette session and deletes it
     afterwards if one doesn't already exist.
     """
--- a/testing/marionette/client/marionette_driver/marionette.py
+++ b/testing/marionette/client/marionette_driver/marionette.py
@@ -535,17 +535,16 @@ class Alert(object):
         self.marionette._send_message("sendKeysToDialog", body)
 
 
 class Marionette(object):
     """Represents a Marionette connection to a browser or device."""
 
     CONTEXT_CHROME = 'chrome'  # non-browser content: windows, dialogs, etc.
     CONTEXT_CONTENT = 'content'  # browser content: iframes, divs, etc.
-    DEFAULT_CRASH_TIMEOUT = 10
     DEFAULT_SOCKET_TIMEOUT = 60
     DEFAULT_STARTUP_TIMEOUT = 120
     DEFAULT_SHUTDOWN_TIMEOUT = 65  # Firefox will kill hanging threads after 60s
 
     def __init__(self, host='localhost', port=2828, app=None, bin=None,
                  baseurl=None, timeout=None, socket_timeout=DEFAULT_SOCKET_TIMEOUT,
                  startup_timeout=None, **instance_args):
         """
@@ -789,51 +788,48 @@ class Marionette(object):
 
         if self.instance:
             name = self.test_name or 'marionette.py'
             crash_count = self.instance.runner.check_for_crashes(test_name=name)
             self.crashed = self.crashed + crash_count
 
         return crash_count > 0
 
-    def handle_socket_failure(self, crashed=False):
-        """Handle socket failures for the currently running instance.
-
-        :param crashed: Optional flag which indicates that the process has been crashed,
-            and no further socket checks have to be performed. Defaults to False.
+    def handle_socket_failure(self):
+        """Handle socket failures for the currently running application instance.
 
         If the application crashed then clean-up internal states, or in case of a content
         crash also kill the process. If there are other reasons for a socket failure,
         wait for the process to shutdown itself, or force kill it.
 
         """
         if self.instance:
             exc, val, tb = sys.exc_info()
 
-            # If the content process crashed, Marionette forces the application to shutdown.
-            if crashed:
-                returncode = self.instance.runner.wait(timeout=self.DEFAULT_CRASH_TIMEOUT)
+            # Somehow the socket disconnected. Give the application some time to shutdown
+            # itself before killing the process.
+            returncode = self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
 
-                if returncode == 0:
-                    message = 'Content process crashed'
-                else:
-                    message = 'Process crashed (Exit code: {returncode})'
-                self.delete_session(send_request=False, reset_session_id=True)
-
+            if returncode is None:
+                message = ('Process killed because the connection to Marionette server is '
+                           'lost. Check gecko.log for errors')
+                self.quit()
             else:
-                # Somehow the socket disconnected. Give the application some time to shutdown
-                # itself before killing the process.
-                returncode = self.instance.runner.wait(timeout=self.DEFAULT_SHUTDOWN_TIMEOUT)
-                if returncode is None:
-                    self.quit()
-                    message = ('Process killed because the connection to Marionette server is '
-                               'lost. Check gecko.log for errors')
+                # If Firefox quit itself check if there was a crash
+                crash_count = self.check_for_crash()
+
+                if crash_count > 0:
+                    if returncode == 0:
+                        message = 'Content process crashed'
+                    else:
+                        message = 'Process crashed (Exit code: {returncode})'
                 else:
-                    message = 'Process has been closed (Exit code: {returncode})'
-                    self.delete_session(send_request=False, reset_session_id=True)
+                    message = 'Process has been unexpectedly closed (Exit code: {returncode})'
+
+                self.delete_session(send_request=False, reset_session_id=True)
 
             if exc:
                 message += ' (Reason: {reason})'
 
             raise IOError, message.format(returncode=returncode, reason=val), tb
 
     @staticmethod
     def convert_keys(*string):