Closed Bug 473042 Opened 11 years ago Closed 11 years ago

Crash [@ nsStyleContext::Destroy] with display:none root

Categories

(Core :: Layout, defect, P2, critical)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: jruderman, Assigned: dbaron)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [sg:critical] regression from 283686)

Crash Data

Attachments

(4 files, 2 obsolete files)

Crashes calling 0xdddddddd.  Regression from bug 283686?
Flags: blocking1.9.1?
Whiteboard: [sg:critical]
Priority: -- → P2
Hardware: x86 → All
Assignee: nobody → dbaron
Attached patch patch (obsolete) — Splinter Review
Attachment #356626 - Flags: superreview?(bzbarsky)
Attachment #356626 - Flags: review?(bzbarsky)
Oops, that assertion should be asserting about undisplayedParent, not localContent.  I'll fix that locally.
Comment on attachment 356626 [details] [diff] [review]
patch

With the assert fixed, looks good.
Attachment #356626 - Flags: superreview?(bzbarsky)
Attachment #356626 - Flags: superreview+
Attachment #356626 - Flags: review?(bzbarsky)
Attachment #356626 - Flags: review+
Note that the bug that caused this behavior, bug 283686, is currnetly marked as "[needs 1.9.1 landing]".  If/when that lands, we should make sure to land this as well.
Fixed: http://hg.mozilla.org/mozilla-central/rev/046f5da25280
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical] → [sg:critical][needs 1.9.1 landing]
Target Milestone: --- → mozilla1.9.2a1
Backed out due to crash in layout/forms/test/ mochitests.
http://hg.mozilla.org/mozilla-central/rev/dc598a5eafe1
http://hg.mozilla.org/mozilla-central/rev/1d21f44cf01c
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [sg:critical][needs 1.9.1 landing] → [sg:critical][needs new patch]
Specifically, it's layout/forms/test/test_bug402198.html , which is actually toggling display:none on the root.
Attached patch patch (obsolete) — Splinter Review
This has two additional changes from the previous patch.

The first is the fix for the crash:  the problem was that the primary frame for the root element was ending up being the canvas frame because the code to reset the primary frames for the scrollbars could end up, when no scrollbars were present, setting the primary frame for the root to be the canvas.  This meant that, when the root was undisplayed and we reframed it, we'd delete the frame constructor's mDocElementContainingBlock without clearing the mDocElementContainingBlock pointer.

The second change is the added call to ClearUndisplayedContentIn, which I think is also important.  (I wrote that part first, actually.)
Attachment #356626 - Attachment is obsolete: true
Attached patch patchSplinter Review
Here's a version that also has an assertion in SetPrimaryFrameFor.
Attachment #357018 - Attachment is obsolete: true
And I checked that the assertion does fire without the ConstructDocElementFrame part of the patch.
Comment on attachment 357027 [details] [diff] [review]
patch

Ran full reftests and mochitests without crashing and without hitting my new assertion.  (I think the failures I saw in mochitest are unrelated.)
Attachment #357027 - Flags: superreview?(bzbarsky)
Attachment #357027 - Flags: review?(bzbarsky)
Whiteboard: [sg:critical][needs new patch] → [sg:critical][needs review]
Comment on attachment 357027 [details] [diff] [review]
patch

r+sr=bzbarsky
Attachment #357027 - Flags: superreview?(bzbarsky)
Attachment #357027 - Flags: superreview+
Attachment #357027 - Flags: review?(bzbarsky)
Attachment #357027 - Flags: review+
Fixed: http://hg.mozilla.org/mozilla-central/rev/867b1cf06252
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical][needs review] → [sg:critical][needs 1.9.1 landing]
Flags: in-testsuite?
Flags: blocking1.9.1? → blocking1.9.1+
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d3e2e7c4578f
Keywords: fixed1.9.1
Whiteboard: [sg:critical][needs 1.9.1 landing] → [sg:critical]
Target Milestone: mozilla1.9.2a1 → mozilla1.9.1b3
Verified fixed on trunk and 1.9.1 with builds on OS X and Windows:

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2a1pre) Gecko/20090204 Minefield/3.2a1pre ID:20090204032912

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090204 Shiretoko/3.1b3pre ID:20090204034421
Status: RESOLVED → VERIFIED
Target Milestone: mozilla1.9.1b3 → mozilla1.9.2a1
I don't crash on 1.9.0.11pre, I assume we aren't going to take correctness bug 283686 on the 1.9.0 branch and therefore don't need this fix either.
Flags: wanted1.9.0.x-
Whiteboard: [sg:critical] → [sg:critical] regression from 283686
Incidentally, is this the right testcase? I get an XML error in both 1.9.0 and 1.9.1 -- my previous comment was based on adding the missing </mrow> to make the parser happy, but maybe that wasn't the point.
Crash Signature: [@ nsStyleContext::Destroy]
Group: core-security
Landed the crashtest:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fa55f7a3d14
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.