1355821 - Fix and re-enable test_bug842853.html, test_bug842853-2.html, test_bug851445.html, and test_bug851485.html on Android

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Ryan VanderMeulen [:RyanVM]]

As part of getting layout/base/tests running on Android again, it was determined that test_bug842853.html, test_bug842853-2.html, test_bug851445.html, and test_bug851485.html are currently permafailing with very similar failure modes. In the interests of getting the directory running again, the test has been disabled for now until it can be fixed and re-enabled.

317 INFO TEST-START | layout/base/tests/test_bug842853.html
318 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/test_bug842853.html | undefined assertion name - got 499.79998779296875, expected 500
    SimpleTest.is@SimpleTest/SimpleTest.js:310:5
    runTest@layout/base/tests/test_bug842853.html:20:5
    onload@layout/base/tests/test_bug842853.html:1:1
319 INFO TEST-OK | layout/base/tests/test_bug842853.html | took 3360ms

317 INFO TEST-START | layout/base/tests/test_bug842853-2.html
318 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/test_bug842853-2.html | undefined assertion name - got 499.79998779296875, expected 500
    SimpleTest.is@SimpleTest/SimpleTest.js:310:5
    verifyAfterLoad@layout/base/tests/test_bug842853-2.html:21:5
    onload@layout/base/tests/test_bug842853-2.html:1:1
319 INFO TEST-OK | layout/base/tests/test_bug842853-2.html | took 4882ms

319 INFO TEST-START | layout/base/tests/test_bug851445.html
320 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/test_bug851445.html | undefined assertion name - got 100.44999694824219, expected 100
    SimpleTest.is@SimpleTest/SimpleTest.js:310:5
    handleLoad2@layout/base/tests/test_bug851445.html:23:5
321 INFO TEST-OK | layout/base/tests/test_bug851445.html | took 5358ms

319 INFO TEST-START | layout/base/tests/test_bug851485.html
320 INFO TEST-PASS | layout/base/tests/test_bug851485.html | A valid string reason is expected
321 INFO TEST-PASS | layout/base/tests/test_bug851485.html | Reason cannot be empty
322 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/test_bug851485.html | undefined assertion name - got 499.79998779296875, expected 500
    SimpleTest.is@SimpleTest/SimpleTest.js:310:5
    verifyAfterLoad@layout/base/tests/test_bug851485.html:41:5
    onload@layout/base/tests/test_bug851485.html:1:1
323 INFO TEST-FAIL | layout/base/tests/test_bug851485.html | The author of the test has indicated that flaky timeouts are expected.  Reason: untriaged
324 INFO TEST-PASS | layout/base/tests/test_bug851485.html | undefined assertion name
325 INFO TEST-OK | layout/base/tests/test_bug851485.html | took 5624ms

Comment 1

[Ryan VanderMeulen [:RyanVM]]

NI myself to see if we can re-enable these now.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Looks like these are still failing with the exact same errors as before. Mats, any thoughts on what's going on with these?

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Given Mats' departure, maybe you have an idea Hiro?

Comment 4

[Hiroyuki Ikezoe (:hiro)]

This is due to this ClampAndAlignWithLayerPixels that the purpose is to align scroll positions to screen pixels on the device to avoid repeatedly invalidation (TBH, I don't fully understand the alignment stuff, but I can imagine it). In this test case, the test_bug851485.html which is an iframe inside the top level document opened by mochitest harnes, and the top level document gets scaled to some extent, probably it's due to lacking of "viewport meta tag" (though I don't know where the top level document is defined).

A solution is to allow some tolerance (which should depend on the top level document scale). Another solution is specifying "width=device-width,initial-scale=1" meta viewport tag in the mochitest harness. The latter will affect all our mochitests, so it will break some other tests, so I don't recommend it for now.

I am going to push the fix allowing the tolerance once after this try run finished; https://treeherder.mozilla.org/jobs?repo=try&revision=56c6e2818199d7e96eba0a00bce519885786321b

Comment 5

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #4)

Another solution is specifying "width=device-width,initial-scale=1" meta viewport tag in the mochitest harness. The latter will affect all our mochitests, so it will break some other tests, so I don't recommend it for now.

Filed bug 1745050.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1355821 - Allow a half pixel difference if the top level document gets scaled under 1.0. r?tnikkel

Our mochitest harness doesn't specify meta viewport tags, so the test document
basically gets scaled down on mobile platforms, thus scroll position is aligned
with screen pixels (by ClampAndAlignWithLayerPixels in nsGfxScroll.cpp) rather
than CSS pixels, we should allow a half pixel difference on such platforms.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4f982fc49e6e
Allow a half pixel difference if the top level document gets scaled under 1.0. r=tnikkel

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/4f982fc49e6e