[Webkit-unassigned] [Bug 81659] [GTK] crash log reports support

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=81659





--- Comment #8 from Dirk Pranke <dpranke at chromium.org>  2012-03-27 19:12:36 PST ---
(From update of attachment 134094)
The patch generally looks pretty good to me, just a few nits ...

>> Tools/Scripts/new-run-webkit-tests:61
>> +        cmd.insert(1, jhbuild)
> 
> This can be one line.

Ew, this feels like a hack; then again, so is much of this file :(

>> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:156
>> +        pid_str = str(pid or '<unknown>')
> 
> Okay, just a nit but you should use full words, unless the abbreviation would be more canonical. So IMO pid is good and str is bad.
> 
> In what cases is pid == None? If pid can be None and name can be None shouldn't you handle that case explicitly and do an early return?

pid is None if WebProcess crashes but we're not told the pid of the WebProcess (which I think is currently always on the GTk, and will be until you add support for that). name is probably never None now.

> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:165
> +        dumps = self._filesystem.files_under(log_directory)

note that you can pass match_filename as a filter to files_under() directly.

>> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:170
>> +        stderr_lines = errors + (stderr or '<empty>').decode('utf8').splitlines()
> 
> What guarantee do we have that stderr is in UTF-8 format?

I'm not actually sure that it's always (or even ever) in UTF-8; I saw a crash earlier today on the chromium mac 10.6 bot when we failed to decode something as utf-8, so we might want to trap a codec error here and try it the other way as well.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.