Bug 66577 – [chromium] touch tests failing on OSX10.5 after r93358

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Tony Chang]

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fevents%2Ftouch%2Fbasic-single-touch-events.html%2Cfast%2Fevents%2Ftouch%2Fmulti-touch-grouped-targets.html%2Cfast%2Fevents%2Ftouch%2Ftouch-gesture-scroll.html%2Cfast%2Fevents%2Ftouch%2Ftouch-target-limited.html%2Cfast%2Fevents%2Ftouch%2Ftouch-target.html

I'm going to mark them as expected to timeout for now, but rjkroege said he'd look into it.

[Robert Kroeger]

I can't yet replicate this bug locally. However, https://bugs.webkit.org/show_bug.cgi?id=66492 reverts the most likely cause introduced in r93358.

[Johnny(Jianning) Ding]

after r93358, eventSender.touchEnd  in touch tests will trigger scrollEnd gesture, which calls ScrollAnimatorChromiumMac::snapRubberBand. 

However the NSProcessInfo::systemUptime used by  snapRubberBand is not supported by Mac Leopard, which caused error "NSProcessInfo systemUptime unrecognized selector sent to instance”, then DRT was stuck in NSApplication message loop and caused timeout.

Sent patch to fix it.

[Johnny(Jianning) Ding]

Created attachment 110360 [details]
patch v1

[Johnny(Jianning) Ding]

Created attachment 110483 [details]
patch v2, fix uptime calculation

[Tony Chang]

I don't know enough about Macs/objc to review this.  cc'ing some Mac people.

[Avi Drissman]

Comment on attachment 110483 [details]
patch v2, fix uptime calculation

View in context: https://bugs.webkit.org/attachment.cgi?id=110483&action=review

Looks reasonable. LGTM with fixes, for all my decision is worth here at WK.

> Source/WebCore/platform/chromium/ScrollAnimatorChromiumMac.mm:59
> +    // Print how long system has been up. Found by looking getting "boottime" from the kernel.

Print?

> Source/WebCore/platform/chromium/ScrollAnimatorChromiumMac.mm:71
> +        NSTimeInterval ressult = uptime.tv_sec + (uptime.tv_usec / 1E+6);

s/ressult/result/ ?

[Johnny(Jianning) Ding]

Created attachment 111412 [details]
patch v2 with review comments fix

(In reply to comment #6)
Thanks for review!
> Print?
done
> s/ressult/result/ ?
done

[Avi Drissman]

For as much as it's worth, SLGTM.

[Johnny(Jianning) Ding]

(In reply to comment #8)
> For as much as it's worth, SLGTM.

Can you or someone grant a r+? :)

[Avi Drissman]

Well, you explicitly asked me for a review, so I gave you one. I'm not a webkit reviewer, though, so I can't r+ you. (That's what I meant by my comments "for as much as it's worth" and "for all my decision is worth".)

[WebKit Review Bot]

Comment on attachment 111412 [details]
patch v2 with review comments fix

Clearing flags on attachment: 111412

Committed r97831: <http://trac.webkit.org/changeset/97831>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.