RESOLVED FIXED 34069
REGRESSION: editing/selection/doubleclick-beside-cr-span.html times out on Windows
https://bugs.webkit.org/show_bug.cgi?id=34069
Summary REGRESSION: editing/selection/doubleclick-beside-cr-span.html times out on Wi...
Eric Seidel (no email)
Reported 2010-01-24 23:18:02 PST
REGRESSION: editing/selection/doubleclick-beside-cr-span.html times out on Windows This seems to have started today. The windows bots were broken between r53763 and r53783, and I believe it the timeouts started happening during that down time. Added 6 days ago: http://trac.webkit.org/browser/trunk/LayoutTests/editing/selection/doubleclick-beside-cr-span.html
Attachments
Patch v1 (1.21 KB, patch)
2010-02-01 03:32 PST, Shinichiro Hamaji
no flags
Shinichiro Hamaji
Comment 1 2010-01-26 04:45:54 PST
Hmm... I'm not sure if this is a regression. I ran the test with r53763 and r53783. I didn't see significant differences between the results of them. I've also noticed this test is very slow on Windows (~24 secs on my slow machine). It seems eventSender.leapForward() actually waits on Windows? So, I'm guessing somehow windows bots become slow during the downtime, but I'm not sure at all. Anyway, if we want to make the bot green, we can change from leapForward(100) to leapForward(50). But, of course, if this is indicating an actual regression, this change just hides the regression...
MORITA Hajime
Comment 2 2010-01-26 07:25:10 PST
Thanks for the investigation, Hamaji-san. I don't have windows box... > It seems eventSender.leapForward() actually waits on Windows? At a glance, the code appears to literally sleep. in WebKitTools/DumpRenderTree/win/EventSender.cpp, ---- static JSValueRef leapForwardCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception) { if (argumentCount > 0) { msgQueue[endOfQueue].delay = JSValueToNumber(context, arguments[0], exception); } ... } and void replaySavedEvents(HRESULT* oleDragAndDropReturnValue) { ... if (msgQueue[startOfQueue].delay) { ::Sleep(msgQueue[startOfQueue].delay); msgQueue[startOfQueue].delay = 0; } ... } ---- I guess it would take long time to run the test case, and the case would be around timeout border, timed out recently with small overhead added somewhere. For this reason, I think making sleep time shorter is reasonable at this time, although further investigation to speedup windows layout test is required.
Shinichiro Hamaji
Comment 3 2010-02-01 03:32:19 PST
Created attachment 47828 [details] Patch v1
Shinichiro Hamaji
Comment 4 2010-02-01 03:35:42 PST
I ran the tests with r53762 on build bot and it timed out. So, I guess the timeout happened even before r53762 (or buildbot somehow becomes slower during the windows bots outage?). http://build.webkit.org/builders/Windows%20Debug%20%28Tests%29/builds/9253
Eric Seidel (no email)
Comment 5 2010-02-01 05:17:21 PST
Comment on attachment 47828 [details] Patch v1 leapForward does not actually sleep (at least not on the mac). Instead it just queues the next event with a different time during event playback.
Shinichiro Hamaji
Comment 6 2010-02-01 05:29:29 PST
(In reply to comment #5) > (From update of attachment 47828 [details]) > leapForward does not actually sleep (at least not on the mac). Instead it just > queues the next event with a different time during event playback. Yeah, on Mac, leapForward doesn't sleep. However, on Windows, it seems leapForward sleeps as Morita-san said.
Darin Adler
Comment 7 2010-02-01 08:14:10 PST
Comment on attachment 47828 [details] Patch v1 The whole idea of leapForward was to not require sleeping, but I suppose it's OK to make this change.
Shinichiro Hamaji
Comment 8 2010-02-01 16:08:47 PST
Comment on attachment 47828 [details] Patch v1 Clearing flags on attachment: 47828 Committed r54166: <http://trac.webkit.org/changeset/54166>
Shinichiro Hamaji
Comment 9 2010-02-01 16:08:55 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.