805404 - docshell/test/test_bug590573.html fails on panda boards

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

206 INFO TEST-START | /tests/docshell/test/test_bug590573.html
207 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 1
208 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 2
209 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 3
210 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 4
211 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 5
212 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 6
213 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | Location was http://mochi.test:8888/tests/docshell/test/file_bug590573_2.html but should end with file_bug590573_2.html
214 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 7
215 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | undefined
216 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | page should have div1.
217 ERROR TEST-UNEXPECTED-FAIL | /tests/docshell/test/test_bug590573.html | test 8 - got 199, expected 200
218 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 9
219 ERROR TEST-UNEXPECTED-FAIL | /tests/docshell/test/test_bug590573.html | test 10 - got 199, expected 200
220 INFO TEST-PASS | /tests/docshell/test/test_bug590573.html | test 11
221 INFO TEST-END | /tests/docshell/test/test_bug590573.html | finished in 1298ms


We are looking to turn these tests on in production next week.  We can manifest out this test case, but that will turn it off for android on the tegras.

Comment 1

[Justin Lebar (not reading bugmail)]

I am definitely not going to be able to look at this before next week.

I'd guess that this /may/ be due to async panning and zooming, and that the failure isn't a big deal.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

any tips for debugging this?  Maybe suggestions to change the testcase to ignore this if it isn't important?

Comment 3

[Justin Lebar (not reading bugmail)]

>     is(popup.scrollY, 200, "test 8");

Why is popup.scrollY 199 instead of 200?

You could see if putting each test in a setTimeout(1000) makes the test pass.  If so, the problem is that scrollY isn't getting updated quickly enough.  I don't know if that's actually a web-compat problem or not; it kind of sounds like it is.  That is, the test as-is may be correct and exposing a bug in our async scrolling implementation.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Changing:
http://mxr.mozilla.org/mozilla-central/source/docshell/test/test_bug590573.html?force=1#167

to:
setTimeout(pageload, 1000);

seems to have solved to problem.  Remember this is on panda boards only.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

should we add this setTimeout to the test or fix it another way?

Comment 6

[Justin Lebar (not reading bugmail)]

(In reply to Joel Maher (:jmaher) from comment #5)
> should we add this setTimeout to the test or fix it another way?

Sorry, I missed comment 4.

Like I said in comment 3, if setTimeout(1000) fixes the problem, this sounds like a legitimate web-facing bug in Gecko -- likely the async pan and zoom controller.  We should probably fix that bug.

If you want to work around it, I'd be happier adding a fudge factor to the is(popup.scrollY) check (allowing say values 190 - 200) than adding a setTimeout, although neither workaround is great.

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

gbrown/kats, can you guys help us look at this bug from a mobile developer standpoint.  I would like to avoid a workaround as much as possible.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

:jmaher, did you change just that one setTimeout call or all of them?

I'm not sure that this is related to async pan/zoom because the test is setting scroll position directly via window.scroll which isn't async; it should update the scroll position instantaneously.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

I tried again with a copy of m-c from yesterday and I was not able to fix this by adjusting one (or all) of the setTimeout(pageload, 0) calls to setTimeout(pageload, 1000).  

Actually I wasn't able to fix it at all with any value in the setTimeout (up to 10000).  Any thoughts on what I could try?

Comment 10

[Justin Lebar (not reading bugmail)]

> Any thoughts on what I could try?

For the purposes of getting this test up and running, you could try a fudge factor as described in comment 6; I think that would still test what the test is checking.

Comment 11

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: allow for 199 | 200 on tests 8 and 10 (1.0)

my attempt at hacking this test case.

Comment 12

[Justin Lebar (not reading bugmail)]

Comment on attachment 692336 [details] [diff] [review]
allow for 199 | 200 on tests 8 and 10 (1.0)

Okay!

But I'd like to keep an open bug on the 199|200 issue, which means that you should link to that bug, not this one, in the testcase.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

inbound: 
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0b6350b8b24

Comment 14

[Ryan VanderMeulen [:RyanVM]]

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