Sometimes GetPrimaryFrameFor to be called during the destruction phase of the frame manager. This causes it to return an invalid pointer to a frame. GetPrimaryFrameFor needs to check a flag, and return early if the frame manager is in the middle of it's destruction phase. This is the Bugzilla equivalent of Bugscape 12117 (http://bugscape/show_bug.cgi?id=12117), that had been reported by AOL's accessibility testers.
Status: NEW → ASSIGNED
Keywords: access, crash, sec508
Priority: -- → P1
Whiteboard: patch ready, seeking r=
Target Milestone: --- → mozilla1.0
Created attachment 73142 [details] [diff] [review] Checks flag to see if destruction phase make cause GPFF to return invalid pointer Rather than reorder the destruction, this plays it safe and uses a flag mIsDestroyingFrames. The flag is checked at the start of GetPrimaryFrameFor
Comment on attachment 73142 [details] [diff] [review] Checks flag to see if destruction phase make cause GPFF to return invalid pointer The following is unnecessary: -FrameManager::FrameManager() +FrameManager::FrameManager() : mIsDestroyingFrames(PR_FALSE) since FrameManager uses ZEROING_OPERATOR_NEW, but other than that, r=dbaron.
Attachment #73142 - Flags: review+
PropertyList* mPropertyList; nsCOMPtr<nsIContentList> mHTMLForms; nsCOMPtr<nsIContentList> mHTMLFormControls; + PRBool mIsDestroyingFrames; Indentation so the new member name starts in the same column as the existing ones doesn't hurt. /be
Created attachment 73224 [details] [diff] [review] Contains dbaron's and brendan's tweaks This should still have dbaron's r= on it.
Attachment #73142 - Attachment is obsolete: true
Comment on attachment 73224 [details] [diff] [review] Contains dbaron's and brendan's tweaks Carrying over r= from dbaron
Attachment #73224 - Flags: review+
Comment on attachment 73224 [details] [diff] [review] Contains dbaron's and brendan's tweaks sr=shaver.
Attachment #73224 - Flags: superreview+
Comment on attachment 73224 [details] [diff] [review] Contains dbaron's and brendan's tweaks a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #73224 - Flags: approval+
Changing QA contact
QA Contact: petersen → amar
QA should probaly be dsirnapalli since he was the one who originally repro'd this bug for me.
fix checked in
David Baron wrote (via email): "This seems a little risky to me for the branch at this point. I can imagine things that it could break (they're the type of things (web page X crashes) that I would think would be caught after a week of being shaken out on the trunk, especially if you put that printf / assertion in)."
fix checked into trunk, not into branch
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Spoke with dbaron on IRC. He says it should be okay to check this fix into 0.9.9 now.
-- Verified the fix on MfcEmbed of trunk build. Works fine. Verified it on MfcEmbed nightly_build also. Works fine. Marking the bug as verified.
Status: RESOLVED → VERIFIED
John, can you check this one into the trunk for me?
I didn't have a 0.9.9 branch, pulled and am building as I write. I will apply the patch and check it in asap.
Whiteboard: patch ready, seeking r= → in the trunk, preping to go in 0.9.9 branch
checked in to 0.9.9 branch I'll let edt remove the edt0.9.9+
Whiteboard: in the trunk, preping to go in 0.9.9 branch
actually, "edt0.9.9+" stays. thanks for the checkin!
-- Verified the fix in latest 0.9.9ec (03/27/02). Works fine.
You need to log in before you can comment on or make changes to this bug.