Diff
Modified: trunk/Tools/ChangeLog (111342 => 111343)
--- trunk/Tools/ChangeLog 2012-03-20 03:32:29 UTC (rev 111342)
+++ trunk/Tools/ChangeLog 2012-03-20 03:46:26 UTC (rev 111343)
@@ -1,3 +1,11 @@
+2012-03-19 Jessie Berlin <[email protected]>
+
+ Unreviewed; roll out http://trac.webkit.org/changeset/111307 and http://trac.webkit.org/changeset/111293
+ because they broke running the layout tests on mac.
+
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ (WebKitDriver._check_for_driver_crash):
+
2012-03-16 Martin Robinson <[email protected]>
[GTK] Allow running run-gtk-tests during 'make distcheck'
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py (111342 => 111343)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2012-03-20 03:32:29 UTC (rev 111342)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process.py 2012-03-20 03:46:26 UTC (rev 111343)
@@ -70,7 +70,7 @@
self._proc = None
self._output = str() # bytesarray() once we require Python 2.6
self._error = str() # bytesarray() once we require Python 2.6
- self._crashed = False
+ self.set_crashed(False)
self.timed_out = False
def process_name(self):
@@ -94,17 +94,22 @@
fl = fcntl.fcntl(fd, fcntl.F_GETFL)
fcntl.fcntl(fd, fcntl.F_SETFL, fl | os.O_NONBLOCK)
- def _handle_possible_interrupt(self):
+ def handle_interrupt(self):
"""This routine checks to see if the process crashed or exited
because of a keyboard interrupt and raises KeyboardInterrupt
accordingly."""
- # FIXME: Linux and Mac set the returncode to -signal.SIGINT if a
- # subprocess is killed with a ctrl^C. Previous comments in this
- # routine said that supposedly Windows returns 0xc000001d, but that's not what
- # -1073741510 evaluates to. Figure out what the right value is
- # for win32 and cygwin here ...
- if self._proc.returncode in (-1073741510, -signal.SIGINT):
- raise KeyboardInterrupt
+ if self.crashed:
+ # This is hex code 0xc000001d, which is used for abrupt
+ # termination. This happens if we hit ctrl+c from the prompt
+ # and we happen to be waiting on the DumpRenderTree.
+ # sdoyon: Not sure for which OS and in what circumstances the
+ # above code is valid. What works for me under Linux to detect
+ # ctrl+c is for the subprocess returncode to be negative
+ # SIGINT. And that agrees with the subprocess documentation.
+ if (-1073741510 == self._proc.returncode or
+ - signal.SIGINT == self._proc.returncode):
+ raise KeyboardInterrupt
+ return
def poll(self):
"""Check to see if the underlying process is running; returns None
@@ -123,7 +128,7 @@
except IOError, e:
self.stop()
# stop() calls _reset(), so we have to set crashed to True after calling stop().
- self._crashed = True
+ self.set_crashed(True)
def _pop_stdout_line_if_ready(self):
index_after_newline = self._output.find('\n') + 1
@@ -173,6 +178,11 @@
return self._read(deadline, retrieve_bytes_from_stdout_buffer)
+ def _check_for_crash(self, wait_for_crash_reporter=True):
+ if self.poll() != None:
+ self.set_crashed(True, wait_for_crash_reporter)
+ self.handle_interrupt()
+
def _log(self, message):
# This is a bit of a hack, but we first log a blank line to avoid
# messing up the master process's output.
@@ -196,6 +206,10 @@
self._log('Unable to sample process.')
def _handle_timeout(self):
+ self._executive.wait_newest(self._port.is_crash_reporter)
+ self._check_for_crash(wait_for_crash_reporter=False)
+ if self.crashed:
+ return
self.timed_out = True
self._sample()
@@ -224,24 +238,22 @@
# FIXME: Why do we ignore all IOErrors here?
pass
- def has_crashed(self):
- if not self._crashed and self.poll():
- self._crashed = True
- self._handle_possible_interrupt()
- return self._crashed
+ def _check_for_abort(self, deadline):
+ self._check_for_crash()
+ if time.time() > deadline:
+ self._handle_timeout()
+
+ return self.crashed or self.timed_out
+
# This read function is a bit oddly-designed, as it polls both stdout and stderr, yet
# only reads/returns from one of them (buffering both in local self._output/self._error).
# It might be cleaner to pass in the file descriptor to poll instead.
def _read(self, deadline, fetch_bytes_from_buffers_callback):
while True:
- if self.has_crashed():
+ if self._check_for_abort(deadline):
return None
- if time.time() > deadline:
- self._handle_timeout()
- return None
-
bytes = fetch_bytes_from_buffers_callback()
if bytes is not None:
return bytes
@@ -280,3 +292,9 @@
self._executive.kill_process(self._proc.pid)
_log.warning('killed')
self._reset()
+
+ def set_crashed(self, crashed, wait_for_crash_reporter=True):
+ self.crashed = crashed
+ if not self.crashed or not wait_for_crash_reporter:
+ return
+ self._executive.wait_newest(self._port.is_crash_reporter)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py (111342 => 111343)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py 2012-03-20 03:32:29 UTC (rev 111342)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/server_process_unittest.py 2012-03-20 03:46:26 UTC (rev 111343)
@@ -50,7 +50,10 @@
def check_for_leaks(self, process_name, process_pid):
pass
+ def is_crash_reporter(self, process_name):
+ return False
+
class MockFile(object):
def __init__(self, server_process):
self._server_process = server_process
@@ -88,7 +91,7 @@
def test_broken_pipe(self):
server_process = FakeServerProcess(port_obj=TrivialMockPort(), name="test", cmd=["test"])
server_process.write("should break")
- self.assertTrue(server_process.has_crashed())
+ self.assertTrue(server_process.crashed)
self.assertEquals(server_process._proc, None)
self.assertEquals(server_process.broken_pipes, [server_process.stdin])
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (111342 => 111343)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2012-03-20 03:32:29 UTC (rev 111342)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2012-03-20 03:46:26 UTC (rev 111343)
@@ -194,7 +194,7 @@
while True:
output = sp.read_stdout_line(deadline)
- if sp.timed_out or sp.has_crashed() or not output:
+ if sp.timed_out or sp.crashed or not output:
break
if output.startswith('diff'): # This is the last line ImageDiff prints.
@@ -209,7 +209,7 @@
if sp.timed_out:
_log.error("ImageDiff timed out")
- if sp.has_crashed():
+ if sp.crashed:
_log.error("ImageDiff crashed")
# FIXME: There is no need to shut down the ImageDiff server after every diff.
sp.stop()
@@ -489,20 +489,32 @@
def has_crashed(self):
if self._server_process is None:
return False
- if self._crashed_subprocess_name:
- return True
- return self._server_process.has_crashed()
+ return self._server_process.poll() is not None
def _check_for_driver_crash(self, error_line):
if error_line == "#CRASHED\n":
# This is used on Windows to report that the process has crashed
# See http://trac.webkit.org/changeset/65537.
- self._crashed_subprocess_name = self._port.driver_name()
+ self._server_process.set_crashed(True)
elif error_line == "#CRASHED - WebProcess\n":
# WebKitTestRunner uses this to report that the WebProcess subprocess crashed.
- self._crashed_subprocess_name = "WebProcess"
- return self.has_crashed()
+ self._subprocess_crashed("WebProcess")
+ return self._detected_crash()
+ def _detected_crash(self):
+ # We can't just check self._server_process.crashed because WebKitTestRunner
+ # can report subprocess crashes at any time by printing
+ # "#CRASHED - WebProcess", we want to count those as crashes as well.
+ return self._server_process.crashed or self._crashed_subprocess_name
+
+ def _subprocess_crashed(self, subprocess_name):
+ self._crashed_subprocess_name = subprocess_name
+
+ def _crashed_process_name(self):
+ if not self._detected_crash():
+ return None
+ return self._crashed_subprocess_name or self._server_process.process_name()
+
def _command_from_driver_input(self, driver_input):
if self.is_http_test(driver_input.test_name):
command = self.test_to_uri(driver_input.test_name)
@@ -551,9 +563,9 @@
self.error_from_test += self._server_process.pop_all_buffered_stderr()
return DriverOutput(text, image, actual_image_hash, audio,
- crash=self.has_crashed(), test_time=time.time() - start_time,
+ crash=self._detected_crash(), test_time=time.time() - start_time,
timeout=self._server_process.timed_out, error=self.error_from_test,
- crashed_process_name=self._crashed_subprocess_name)
+ crashed_process_name=self._crashed_process_name())
def _read_header(self, block, line, header_text, header_attr, header_filter=None):
if line.startswith(header_text) and getattr(block, header_attr) is None:
@@ -596,7 +608,7 @@
else:
out_line, err_line = self._server_process.read_either_stdout_or_stderr_line(deadline)
- if self._server_process.timed_out or self.has_crashed():
+ if self._server_process.timed_out or self._detected_crash():
break
if out_line:
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py (111342 => 111343)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2012-03-20 03:32:29 UTC (rev 111342)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2012-03-20 03:46:26 UTC (rev 111343)
@@ -240,12 +240,9 @@
class MockServerProcess(object):
def __init__(self, lines=None):
self.timed_out = False
- self.lines = lines or []
self.crashed = False
+ self.lines = lines or []
- def has_crashed(self):
- return self.crashed
-
def read_stdout_line(self, deadline):
return self.lines.pop(0) + "\n"
