Bug 34069 – REGRESSION: editing/selection/doubleclick-beside-cr-span.html times out on Windows

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

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

[Shinichiro Hamaji]

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]

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]

Created attachment 47828 [details]
Patch v1

[Shinichiro Hamaji]

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 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]

(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 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 on attachment 47828 [details]
Patch v1

Clearing flags on attachment: 47828

Committed r54166: <http://trac.webkit.org/changeset/54166>

[Shinichiro Hamaji]

All reviewed patches have been landed.  Closing bug.