790630 - scrollframes are busted on armv6 builds of fennec using gcc-4.4.3, causing it to fail a svg reftest with overflow="hidden" in reftest-4 which passes on armv7 builds

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

we run plenty of successful svg tests on armv7, but this single test fails every time on armv6 builds:
REFTEST TEST-UNEXPECTED-FAIL | http://10.250.48.220:30210/tests/layout/reftests/svg/non-scaling-stroke-02.svg | image comparison (==)

Comment 1

[Ed Morley [:emorley]]

Example log:
https://tbpl.mozilla.org/php/getParsedLog.php?id=15160841&tree=Firefox

Similar to bug 790624, marking as blocking bug 784681 not for wanting to turn it off, but so releng can keep track of what is hidden vs not (after my chat with joduinn yesterday).

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Jet, can you get someone to look into these failures?

Comment 3

[Robert Longson [:longsonr]]

The images are the same except that one of them has a scrollbar. Perhaps we could set a width and height on the outer svg element (would need to be x2 for the ref as stuff is doubled there) i.e. replace overflow="hidden" with width="250" height="700" in the ref.

Alternatively we could split the test into two and have the grad1 and grad2 bits in one test (exactly as now) but move the <use> tests of the rect to another file and move them nearer the top to avoid scrollbars.

Comment 4

[Robert Longson [:longsonr]]

Dividing the test in two is probably the best way to go.

Comment 5

[Robert Longson [:longsonr]]

I looked into this some more and you do have a actual bug on armv6. The scrollbars should be suppressed by overflow="hidden" but are not. 

This test was not designed as a test for overflow="hidden", and I can fix it not to use that but that's just wallpapering over the issue.

Comment 6

[Robert Longson [:longsonr]]

Attachment: wallpaper patch

Comment 7

[Robert Longson [:longsonr]]

The wallpaper patch works on Windows, I haven't run it on Try but it ought to make armv6 pass this test.

Comment 8

[Phil Ringnalda (:philor)]

https://tbpl.mozilla.org/?tree=Try&rev=82253f9e9240&noignore=1

Comment 9

[Phil Ringnalda (:philor)]

Green, other than that it'll be another 5 hour wait for WinXP.

Comment 10

[Robert Longson [:longsonr]]

Your plan is to land this and raise a follow up to cover the underlying arm6 scrollbar suppression failure?

Comment 11

[Phil Ringnalda (:philor)]

I was considering that, yes. Now I'm thinking about letting it age, the way we let the OS X 10.7 test bugs age.

Comment 12

[Phil Ringnalda (:philor)]

Around 60 minutes a run, 5803 pushes in August, so say 1200 a week, conservatively say 800 that trigger armv6, 800 hours down the tubes.

Comment 13

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

there is little point in continuing to run this if we keep it hidden and nobody looks at it.  Do we just need to fix the test to account for the scrollbar?  Maybe add a reftest.list condition for a preference?

Comment 14

[Robert Longson [:longsonr]]

There's already a patch here that suppresses the scrollbar on this test. That could land if you want provided you're tracking the scrollbar bug somewhere.

Comment 15

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

I believe the scrollbar issue is related to different screen/pixel sizes we have for armv6 builds.  I know there are some differences, I just don't know what and why they would make a difference.

Comment 16

[Robert Longson [:longsonr]]

This test has overflow="hidden" on the outer <svg> element which should suppress scrollbars. It works on all other platforms and should be unrelated to screen/pixel sizes.

Comment 17

[Alex Keybl [:akeybl]]

Joel - to investigate this issue, do you need access to an ARMv6 phone? We'd like to resolve prior to FF17's release.

Comment 18

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

I run the armv6 builds on a tegra (armv7 chip).  Most likely this can be reproduced on other android phones, probably need android os version 2.3, not 4.0.

Comment 19

[Alex Keybl [:akeybl]]

Thanks Joel.

jwatt - would a fix for this svg test failure fall under your purview? We'd like to resolve for FF17 prior to an ARMv6 release. Let us know if you don't have access to, or can't reproduce on, an ARMv7 device.

Comment 20

[Phil Ringnalda (:philor)]

SVG already investigated it and determined that the problem is that an overflow="hidden" test has scrollbars, which is the exact opposite of overflow="hidden", and already wrote a hackaround patch that just removes the single test which shows that it is broken, but due to not trusting us to fix it rather than treat sweeping it under the rug as good enough, does not want to commit that hackaround.

Comment 21

[Phil Ringnalda (:philor)]

Oops, missed the word "correctly" before "not trusting us" :)

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Last of three, Jet passing on to you to assign to someone who can work on this in the next few weeks so we can build & ship our first ARMv6 builds with working/passing reftests.

Comment 23

[Lukas Blakk [:lsblakk] use ?needinfo]

should be addressed by bug 804641 - adding qawanted to confirm.

Comment 24

[Kevin Brosnan [Ex-Mozilla]]

Android Armv6 opt R4 does not show this failure going back through the weekend.

Comment 25

[Ed Morley [:emorley]]

No longer needs to block bug 784681, since Armv6 tests are not hidden, since we worked around it for now.

Comment 26

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

The toolchain upgrades in bug 825453 mean that this bug is no longer a problem. Resolving as fixed.

Comment 27

[Tracy Walker [:tracy]]

mass remove verifyme requests greater than 4 months old