586628 - ASSERTION: We should have a margin here! (out of memory?)

Status: NEW

(Core :: XUL, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

I see this assertion in current trunk build, with the attached testcase:
###!!! ASSERTION: We should have a margin here! (out of memory?): 'hasMargin', f
ile c:/mozilla-build/mozilla-central/layout/generic/nsFrame.cpp, line 616

Comment 1

[Mats Palmgren (inactive)]

The problem occurs when a XUL box participates in reflow:

#0  nsBox::BeginLayout at layout/xul/base/src/nsBox.cpp:159
#1  nsIFrame::Layout at layout/xul/base/src/nsBox.cpp:566
#2  nsBoxFrame::Reflow at layout/xul/base/src/nsBoxFrame.cpp:745
#3  nsLineLayout::ReflowFrame at layout/generic/nsLineLayout.cpp:850
#4  nsBlockFrame::ReflowInlineFrame layout/generic/nsBlockFrame.cpp:3794
...

nsLineLayout::ReflowFrame sets up a nsHTMLReflowState correctly for
the XUL frame (adds a UsedMarginProperty for the percentage margin)
but then nsBox::BeginLayout unconditionally removes these properties:
http://hg.mozilla.org/mozilla-central/annotate/6eef4ea21123/layout/xul/base/src/nsBox.cpp#l183
This block was added to cover the case where DidSetStyleContext
detects a change and stores the old value until we reach Reflow/Layout.
See bug 542361 comment 28 - 31.

Comment 2

[Mats Palmgren (inactive)]

Attachment: wip2 (wdiff)

Detecting if we're in Reflow might be one alternative...

Comment 3

[Mats Palmgren (inactive)]

Attachment: wip3 (wdiff)

... or, only storing the old value for the XUL-in-HTML case.
I don't think we're using the GetUsed* methods during Layout yet.

Comment 4

[Jesse Ruderman]

After the fixes for bug 1269901 and bug 1272983, this now triggers:

###!!! ASSERTION: Returning bogus 0-sized margin, because this margin depends on layout & isn't cached!: 'Error', file layout/generic/nsFrame.cpp, line 975

The crashtest for bug 1272983 is annotated as asserting because of this bug (bug 586628).

For more background, see https://bugzilla.mozilla.org/show_bug.cgi?id=1272983#c5

Comment 5

[Jesse Ruderman]

I have plenty of testcase that trigger a similar assertion for padding rather than margin, i.e.

###!!! ASSERTION: Returning bogus 0-sized padding, because this padding depends on layout & isn't cached!: 'Error', file layout/generic/nsFrame.cpp, line 1053