Fix perf tests to be ran by run-perf-tests
Created attachment 123099 [details] fixes the bug
Comment on attachment 123099 [details] fixes the bug The change looks fine, but do we really need/want no_timeout? Isn't using a really long timeout like you've changed good enough?
Comment on attachment 123099 [details] fixes the bug No, I don't like this change. I don't like modifying the html5 spec, if we don't have to. DRT normally blocks external loads...
(In reply to comment #2) > (From update of attachment 123099 [details]) > The change looks fine, but do we really need/want no_timeout? Isn't using a really long timeout like you've changed good enough? Yes. Otherwise DRT's watchdog timer will fail and DRT times out.
Comment on attachment 123099 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=123099&action=review > Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py:291 > + self.assertEquals(driver.cmd_line(), ['MOCK output of child process/DumpRenderTree', '--pixel-tests', '--no-timeout', '-']) What ports support --no-timeout?
(In reply to comment #4) > (In reply to comment #2) > > (From update of attachment 123099 [details] [details]) > > The change looks fine, but do we really need/want no_timeout? Isn't using a really long timeout like you've changed good enough? > > Yes. Otherwise DRT's watchdog timer will fail and DRT times out. Which DRT? The Mac DRT, or the Chromium DRT? If it's the Mac DRT, that seems like a bug that should be fixed (it should be taking the passed in timeout), but that doesn't have to block this (i.e., we can land this as-is, and then file a bug to fix the Mac DRT and remove the no_timeout later).
See also my attempt at this in bug 74649.
Comment on attachment 123099 [details] fixes the bug I dont' want to change html5.html unless we have to.
(In reply to comment #3) > (From update of attachment 123099 [details]) > No, I don't like this change. I don't like modifying the html5 spec, if we don't have to. DRT normally blocks external loads... It does block and spits out a hundreds of errors about it. This patch removes those error messages. Also, isn't it better to load actual images as supposed to being blocked? e.g. if you open the spec on Safari/Chrome, those external images are loaded and affects the layout/loader behavior. My intention was to make the test's behavior on Safari/Chrome and DRT as close as possible. An alternatively we can make local copies of these images into our repository under images.whatwg.org and remove http://
Comment on attachment 123099 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=123099&action=review I think this is OK, after further discussion over IRC. > Tools/Scripts/webkitpy/layout_tests/port/webkit.py:452 > + Driver.__init__(self, port, worker_number, pixel_tests, no_timeout) I thought we got the "options" element off the port object? (I'm not saying that's necessarily a good thing,b ut I thought that was the status quo?)
(In reply to comment #10) > (From update of attachment 123099 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=123099&action=review > > I think this is OK, after further discussion over IRC. > > > Tools/Scripts/webkitpy/layout_tests/port/webkit.py:452 > > + Driver.__init__(self, port, worker_number, pixel_tests, no_timeout) > > I thought we got the "options" element off the port object? (I'm not saying that's necessarily a good thing,b ut I thought that was the status quo?) It appears that port's options object is passed down from the command line argument so I didn't think sticking a random argument there was a good idea. Of course, we could make --no-timeout a command line argument and set it by default but there's not much use in run-perf-tests if no_timeout was False since DRT ends up timing out in a number of tests (2-3 on my MacPro but probably more on slower machines or on slower DRTs). But I can change if you strongly feel about it. FWIW, it makes my patch smaller since I won't have to pass no_timeout argument everywhere.
Comment on attachment 123099 [details] fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=123099&action=review >> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:452 >> + Driver.__init__(self, port, worker_number, pixel_tests, no_timeout) > > I thought we got the "options" element off the port object? (I'm not saying that's necessarily a good thing,b ut I thought that was the status quo?) We do, normally, but unless I'm missing something there is no options.no_timeout, so this is correct. (Or we could add one, but I'd rather see us just fix DRTs to take arbitrarily long timeouts).
(In reply to comment #12) > (From update of attachment 123099 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=123099&action=review > > >> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:452 > >> + Driver.__init__(self, port, worker_number, pixel_tests, no_timeout) > > > > I thought we got the "options" element off the port object? (I'm not saying that's necessarily a good thing,b ut I thought that was the status quo?) > > We do, normally, but unless I'm missing something there is no options.no_timeout, so this is correct. (Or we could add one, but I'd rather see us just fix DRTs to take arbitrarily long timeouts). DRT DOES take arbitrarily long timeouts. It's just that some DRTs have "watchdog" timer as well, and --no-timeout will disable that.
Committed r105443: <http://trac.webkit.org/changeset/105443>
It broke NRWT on GTK and on Qt: http://build.webkit.org/results/GTK%20Linux%2064-bit%20Release/r105444%20%2816569%29/results.html Could you fix it?
Fixed the regression in http://trac.webkit.org/changeset/105450.