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...
Status: RESOLVED FIXED
[armv6]
: qawanted
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: ---
Assigned To: Jet Villegas (:jet)
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


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

Description 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 | http://10.250.48.220:30210/tests/layout/reftests/svg/non-scaling-stroke-02.svg | image comparison (==)
Comment 1 Ed Morley [:emorley] 2012-09-12 07:37:38 PDT
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?) 2012-09-12 10:21:52 PDT
Jet, can you get someone to look into these failures?
Comment 3 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 Robert Longson 2012-09-12 11:58:56 PDT
Dividing the test in two is probably the best way to go.
Comment 5 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 Robert Longson 2012-09-13 02:09:05 PDT
Created attachment 660755 [details] [diff] [review]
wallpaper patch
Comment 7 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 Phil Ringnalda (:philor) 2012-09-18 20:20:23 PDT
https://tbpl.mozilla.org/?tree=Try&rev=82253f9e9240&noignore=1
Comment 9 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 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 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 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 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 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 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 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 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 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 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 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 Phil Ringnalda (:philor) 2012-10-04 14:31:45 PDT
Oops, missed the word "correctly" before "not trusting us" :)
Comment 22 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 Lukas Blakk [:lsblakk] use ?needinfo 2012-10-30 14:49:56 PDT
should be addressed by bug 804641 - adding qawanted to confirm.
Comment 24 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 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 Kartikaya Gupta (email:kats@mozilla.com) 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 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.