Closed Bug 1226751 Opened 9 years ago Closed 8 years ago

Enabling gfx/tests/crashtests/358732-1.xhtml causes intermittent or permanent 408754-1.html | assertion count 1 is more than expected (ASSERTION: didn't subtract all that we added)

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla46
Tracking Flags:
Tracking Status
firefox45 --- fixed

People

(Reporter: KWierso, Assigned: philor)

References

Details

(Keywords: intermittent-failure, Whiteboard: gfx-noted)

Whiteboard: gfx-noted
Isn't this a permanent failure now?
This is too high at this point. Can we please look into it?

For reference: https://dxr.mozilla.org/mozilla-central/source/gfx/tests/crashtests/408754-1.html

21:56:24     INFO -  REFTEST INFO | Loading a blank page
21:56:24     INFO -  ++DOMWINDOW == 63 (0x7fe138360400) [pid = 1041] [serial = 2094] [outer = 0x7fe16b137c00]
21:56:24     INFO -  REFTEST TEST-UNEXPECTED-FAIL | file:///home/worker/workspace/build/tests/reftest/tests/gfx/tests/crashtests/408754-1.html | assertion count 2 is more than expected 0 assertions
21:56:24     INFO -  REFTEST TEST-END | file:///home/worker/workspace/build/tests/reftest/tests/gfx/tests/crashtests/408754-1.html
21:56:24     INFO -  REFTEST TEST-START | file:///home/worker/workspace/build/tests/reftest/tests/gfx/tests/crashtests/410728-1.xml
21:56:24     INFO -  REFTEST TEST-LOAD | file:///home/worker/workspace/build/tests/reftest/tests/gfx/tests/crashtests/410728-1.xml | 869 / 2963 (29%)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #10)
> From
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-
> searchStr=Android%204.
> 3%20API11%2B%20debug%20Reftest%20Crashtest%20R%28C2%29&tochange=afb8ed11f936&
> fromchange=c13228f84d85&selectedJob=17958840 the regression is pretty
> obviously from one of tbsaunde's changes: bug 1218762 or bug 1225943.

I'm pretty sure its from1225943, all the other one does on !windows is add dead code.  I believe 1225943 was basically reenabling an old crash test, and I don't know much more than that about what's going on with these tests.  So I'm not sure I can really help more than disabling the test again or saying annotate the assertion as expected :(
Flags: needinfo?(tbsaunde+mozbugs)
I guess standard procedure is to back out the offending change unless there's a better option. Do you want to do that or should I?
Nobody got the assertion out of the logcat? I feel like we have other bugs on file for this assert. Assuming that to be true, I'd vote for annotating.

###!!! ASSERTION: didn't subtract all that we added: '(space == 0 || space == nscoord_MAX) && ((l2t == FLEX_PCT_LARGE) ? (-0.001f < basis.f && basis.f < 0.001f) : (basis.c == 0 || basis.c == nscoord_MAX))', file /builds/slave/m-in-and-api-11-d-000000000000/build/src/layout/tables/BasicTableLayoutStrategy.cpp, line 1073
Component: Graphics → Layout: Tables
Summary: Intermittent 408754-1.html | assertion count 1 is more than expected 0 assertions → Intermittent 408754-1.html | assertion count 1 is more than expected (ASSERTION: didn't subtract all that we added)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #12)
> I guess standard procedure is to back out the offending change unless
> there's a better option. Do you want to do that or should I?

I expect there is a better way of disabling the test that bug reenabled than adding bogus code that throws, so I'd prefer to not do a pure backout, but personally I'm fine with disabling it.  Of course annotating seems somewhat better than disabling.
I guess my point is that the patch should have been backed out on landing since it causes a permafail. I'm not sure why that didn't happen. However my involvement here is limited to my being irritated by the permafail, since I've been doing a lot of Android try pushes lately. Backing out the offending patch is the maximum amount of effort I'm willing to put into this. If you want to do something less heavy-handed, go for it.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #15)
> I guess my point is that the patch should have been backed out on landing
> since it causes a permafail. I'm not sure why that didn't happen. However my

I'm not sure, I have the impression it only fails on a variety  of android that isn't tier 1?

> involvement here is limited to my being irritated by the permafail, since
> I've been doing a lot of Android try pushes lately. Backing out the
> offending patch is the maximum amount of effort I'm willing to put into
> this. If you want to do something less heavy-handed, go for it.

It looks like that test has been annotating as asserting on android now so I'm not sure why you still see failures.

I expect you have more reason to care about that test than I do, so if annotating it somehow hasn't worked, and your happy to disable that test in a weird not obvious way go ahead.
*sigh*

Apparently this was already discussed on bug 1218762 (comments 9 and onwards) and a few days ago philor landed a patch to add the annotation. So really this is already fixed.

https://hg.mozilla.org/mozilla-central/rev/1a8bc262190e
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Well, not really, on several fronts:

* it also intermittently asserts twice on Linux

* saying it is fixed and having no open bug about it says that we fully expect that 358732-1.xhtml should cause an assertion failure in a test forty tests later, that doing so is entirely right and proper and shouldn't ever be fixed

* the hell? a non-table-using crashtest causes table layout assertions forty tests later?

* given the timing (the first was actually filed off the same cset), this is probably also the cause of the unexamined bug 1226750 and bug 1227392, which extends the period that this test continues to run and have an effect on other tests from forty tests to several hundred tests
Blocks: 1226750, 1227392
Status: RESOLVED → REOPENED
Component: Layout: Tables → Graphics
Resolution: FIXED → ---
Summary: Intermittent 408754-1.html | assertion count 1 is more than expected (ASSERTION: didn't subtract all that we added) → Enabling gfx/tests/crashtests/358732-1.xhtml causes intermittent or permanent 408754-1.html | assertion count 1 is more than expected (ASSERTION: didn't subtract all that we added)
Version: 45 Branch → Trunk
Keywords: leave-open
Whiteboard: gfx-noted → gfx-noted [test disabled]
358732-1.xhtml appears to be a race between not testing anything and accidentally setting font zoom (breaking later tests). So do -2 and -3 for the same bug.

Mats, do you want to turn these into proper crashtests (with reftest-wait) or just back out https://hg.mozilla.org/integration/mozilla-inbound/rev/8822a6b8b08a?
Flags: needinfo?(mats)
Just back them out I guess, unless you have time to fix them? (I don't, sorry)
Flags: needinfo?(mats)
Can we please back it out?
It isn't in any way blocking you from anything at all other than pushing mozilla-central as it was in late November to try, and nothing can change the fact that for a couple of days in late November there are revisions of mozilla-central which will fail.
No longer blocks: tc-linux64-debug
Assignee: nobody → philringnalda
Status: REOPENED → RESOLVED
Closed: 9 years ago → 8 years ago
Keywords: leave-open
Resolution: --- → FIXED
Whiteboard: gfx-noted [test disabled] → gfx-noted
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.