1366783 - devtools responsive design mode tests are too strict on 300ms touch event measurements

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, enhancement)

Description

[Ben Kelly [:bkelly, not reviewing]]

Currently devtools tests do a very strict 300ms check in:

https://dxr.mozilla.org/mozilla-central/source/devtools/client/responsive.html/test/browser/touch.html

This starts failing with my patches in bug 1363829 because some setTimeout(f, 0) calls run faster.  As a result we end up measuring 299.99ms instead of 300ms.  I think this is reasonable given some of the actual delay is eaten up by firing events, etc.

I'd like to loosen this check a little bit.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Loosen responsive design mode test check for 300ms touch delay. r=jryans

See comment 0 for description of the problem.

I went back and forth on best way to do this.  I had `Math.round() >= 300` before, but settled on `>= 299.5` in the end.  I can adjust to whatever you think is most readable.

Thanks!

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8870053 [details] [diff] [review]
Loosen responsive design mode test check for 300ms touch delay. r=jryans

Review of attachment 8870053 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, seems fine to me!

Could you also update the version for old RDM at http://searchfox.org/mozilla-central/source/devtools/client/responsivedesign/test/touch.html#73?  The corresponding test that uses that version is currently disabled for intermittents...  but it seems nice to keep the files in sync.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Loosen responsive design mode test check for 300ms touch delay. r=jryans

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfa1a3fba07d
Loosen responsive design mode test check for 300ms touch delay. r=jryans

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dfa1a3fba07d

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Is there any chance the platform change here could break web content that tries something like this?  

I assume most people just call `setTimeout` without measuring, but the `setTimeout` API suggests you should always get a delay at least as long as the argument you supply, and `performance.now` is (I assume) the highest precision way to check, so perhaps other pages could be confused, like this test was?

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #6)
> I assume most people just call `setTimeout` without measuring, but the
> `setTimeout` API suggests you should always get a delay at least as long as
> the argument you supply, and `performance.now` is (I assume) the highest
> precision way to check, so perhaps other pages could be confused, like this
> test was?

AFAIK early timers have been possible gecko for a long time.  Consider this test site that triggers a bunch of one shot timeouts and measures their accuracy:

https://people-mozilla.org/~bkelly/timeout-precision?mode=delta&delay=10&count=100

You can see that we do actually fire early (negative delta number) on today's nightly and release.

You do make a good point, though.  The latest version of my patches have a positive mean delta, but a slightly negative median delta.  That suggests we are firing early more often than before which could be somewhat breaking.

I will adjust my patches to fix this.  I'm still inclined to leave these patches landed, though, since we've always had the possibility of slightly early timeout firing.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Okay, just wanted to make we've considered the possible web observable impact.  For these tests, I am fine with tweaking them however we need.