Bug 1299216 - Wait for process exit first before checking for crashes. r=automatedtester
authorHenrik Skupin <mail@hskupin.info>
Tue, 01 Nov 2016 10:48:25 +0100
changeset 321556 e7aa936d8b6e3707a29665d9eb6623d21ff5deea
parent 321555 d57d7ada1080b190a6415134f4d2dab55e382f86
child 321557 7348925c1148ebc2f925c9a64867e2f91e2db883
push id34020
push userhskupin@mozilla.com
push dateTue, 08 Nov 2016 11:11:57 +0000
treeherderautoland@e7aa936d8b6e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersautomatedtester
bugs1299216
milestone52.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 1299216 - Wait for process exit first before checking for crashes. r=automatedtester 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):