Bugzilla@Mozilla – Bug 453406
[FIX]"ASSERTION: Child not in controls" and other badness
Last modified: 2009-03-05 13:48:21 PST
Summon comment box
Created attachment 336620 [details] testcase (can crash Firefox when closed or later) Steps to reproduce: 1. Load the testcase. 2. Close the window (or for better reproducibility, Quit) Result: Crash [@ nsDefaultComparator] during js_GC, *or* ###!!! ASSERTION: Child not in controls: 'index != controls.NoIndex', file nsHTMLFormElement.cpp, line 1464 ###!!! ASSERTION: Invalid start index: 'count == 0 || start < Length()', file nsTArray.h, line 597 often followed by various other assertions, malloc complaints such as "incorrect checksum for freed object", crashes, etc.
Should that nsTArray.h assertion be an NS_ABORT_IF_FALSE?
I'm not super opinionated on if we should bounds check in various nsTArray methods or not. As long as we are consistent. Though we should assert as there is for sure a bug somewhere if you try to remove elements out of bounds.
Created attachment 336683 [details] Testcase that asserts immediately instead of sometime later
Created attachment 336685 [details] [review] Attempt at a mochitest not using the designMode stuff Sadly, I couldn't get this to work because I couldn't get the form to become GCed.... I guess we should use the other testcase for this, eh? :(
Created attachment 336687 [details] [review] Fix So the key part is that we have an input that is the kid of a form. Someone's holding a ref to the input. Then the form's destructor fires (in this case because designMode does some DOM rearrangement which includes removing all the forms from the document). This calls SetForm() on the input, but tells it to NOT remove itself from the form, which means it doesn't reset its ADDED_TO_FORM flag. Then the input is added to a new form, but since ADDED_TO_FORM is set, doesn't add itself to the form. Then when we go to remove it from the form later, we get the assertions, etc. This patch fixes the flag-munging. I'll file a followup bug on improving the API to make this sort of thing less likely to happen. This patch applies as-is to 1.9.0, and we want it there.
Pushed changeset 8b309b9a6ba7.
Does this happen on the 1.8 branch?
Comment on attachment 336687 [details] [review] Fix Approved for 1.9.0.4, a=dveditz for release-drivers
I would assume this isn't an issue on branch, since this code was changed pretty heavily from 1.8 to 1.9....
Assertion does not happen on the 1.8 branch
Fixed on 1.9 branch.
verified fixed on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102401 Firefox/3.0.4pre i see not the assertion and not the crash on this build, when using the testcase. The only warning i see is WARNING: recurring into frame construction: 'mPresContext->mLayoutPhaseCount[eLayoutPhase_FrameC] == 0', file ../../dist/include/layout/nsPresContext.h, line 970 --> Verified 1.9.0.4
This was actually fixed in http://hg.mozilla.org/mozilla-central/rev/10dfd117b6c9 Which landed before 1.9.1 branched
Verified fix against first testcase in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3) Gecko/20090304 Firefox/3.1b3 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090305 Minefield/3.2a1pre