Last Comment Bug 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
: scrollframes are busted on armv6 builds of fennec using gcc-4.4.3, causing it...
: qawanted
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: ARM Android
-- normal (vote)
: ---
Assigned To: Jet Villegas (:jet)
: Jet Villegas (:jet)
Depends on: 825453
Blocks: 790624 791103
  Show dependency treegraph
Reported: 2012-09-12 07:32 PDT by Joel Maher ( :jmaher)
Modified: 2014-01-10 10:40 PST (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

wallpaper patch (1.25 KB, patch)
2012-09-13 02:09 PDT, Robert Longson
no flags Details | Diff | Splinter Review

Description User image Joel Maher ( :jmaher) 2012-09-12 07:32:50 PDT
we run plenty of successful svg tests on armv7, but this single test fails every time on armv6 builds:
REFTEST TEST-UNEXPECTED-FAIL | | image comparison (==)
Comment 1 User image Ed Morley [:emorley] 2012-09-12 07:37:38 PDT
Example log:

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 User image Brad Lassey [:blassey] (use needinfo?) 2012-09-12 10:21:52 PDT
Jet, can you get someone to look into these failures?
Comment 3 User image Robert Longson 2012-09-12 10:38:07 PDT
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 User image Robert Longson 2012-09-12 11:58:56 PDT
Dividing the test in two is probably the best way to go.
Comment 5 User image Robert Longson 2012-09-13 02:08:31 PDT
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 User image Robert Longson 2012-09-13 02:09:05 PDT
Created attachment 660755 [details] [diff] [review]
wallpaper patch
Comment 7 User image Robert Longson 2012-09-13 02:10:36 PDT
The wallpaper patch works on Windows, I haven't run it on Try but it ought to make armv6 pass this test.
Comment 8 User image Phil Ringnalda (:philor) 2012-09-18 20:20:23 PDT
Comment 9 User image Phil Ringnalda (:philor) 2012-09-18 23:46:48 PDT
Green, other than that it'll be another 5 hour wait for WinXP.
Comment 10 User image Robert Longson 2012-09-19 01:17:48 PDT
Your plan is to land this and raise a follow up to cover the underlying arm6 scrollbar suppression failure?
Comment 11 User image Phil Ringnalda (:philor) 2012-09-19 20:49:49 PDT
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 User image Phil Ringnalda (:philor) 2012-09-25 22:07:45 PDT
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 User image Joel Maher ( :jmaher) 2012-09-26 06:24:39 PDT
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 User image Robert Longson 2012-09-26 06:45:17 PDT
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 User image Joel Maher ( :jmaher) 2012-09-26 06:49:54 PDT
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 User image Robert Longson 2012-09-26 06:58:42 PDT
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 User image Alex Keybl [:akeybl] 2012-10-04 09:56:01 PDT
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 User image Joel Maher ( :jmaher) 2012-10-04 09:57:53 PDT
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 User image Alex Keybl [:akeybl] 2012-10-04 13:10:39 PDT
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 User image Phil Ringnalda (:philor) 2012-10-04 14:30:45 PDT
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 User image Phil Ringnalda (:philor) 2012-10-04 14:31:45 PDT
Oops, missed the word "correctly" before "not trusting us" :)
Comment 22 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-10-16 14:52:40 PDT
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 User image Lukas Blakk [:lsblakk] use ?needinfo 2012-10-30 14:49:56 PDT
should be addressed by bug 804641 - adding qawanted to confirm.
Comment 24 User image Kevin Brosnan [:kbrosnan] 2012-10-31 14:57:09 PDT
Android Armv6 opt R4 does not show this failure going back through the weekend.
Comment 25 User image Ed Morley [:emorley] 2013-01-02 04:02:48 PST
No longer needs to block bug 784681, since Armv6 tests are not hidden, since we worked around it for now.
Comment 26 User image Kartikaya Gupta ( 2013-01-03 12:08:56 PST
The toolchain upgrades in bug 825453 mean that this bug is no longer a problem. Resolving as fixed.
Comment 27 User image Tracy Walker [:tracy] 2014-01-10 10:40:38 PST
mass remove verifyme requests greater than 4 months old

Note You need to log in before you can comment on or make changes to this bug.