Closed
Bug 307242
Opened 19 years ago
Closed 17 years ago
ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', @ nsSpaceManager::BandRect::SplitHorizontally
Categories
(Core :: Layout: Floats, defect, P5)
Core
Layout: Floats
Tracking
()
VERIFIED
FIXED
mozilla1.9beta2
People
(Reporter: bc, Assigned: roc)
References
()
Details
(Keywords: assertion, memory-leak, testcase, Whiteboard: [dbaron-1.9:Rm])
Attachments
(3 files, 3 obsolete files)
600 bytes,
text/html
|
Details | |
4.52 KB,
patch
|
sharparrow1
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
163 bytes,
text/html
|
Details |
Found in a Firefox 1.5 build from 2005-09-05 while scanning abc.com
###!!! ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', file
c:/work/mozilla/builds/ff/1.5/mozilla/layout/generic/nsSpaceManager.cpp, line 1350
nsSpaceManager::BandRect::SplitHorizontally(int 5955) line 1350
nsSpaceManager::AddRectToBand(nsSpaceManager::BandRect * 0x03490b50,
nsSpaceManager::BandRect * 0x03491228) line 651 + 15 bytes
nsSpaceManager::InsertBandRect(nsSpaceManager::BandRect * 0x03491228) line 779
nsSpaceManager::AddRectRegion(nsIFrame * 0x03488fe0, const nsRect & {...}) line 842
nsBlockReflowState::FlowAndPlaceFloat(nsFloatCache * 0x034910d0, int *
0x0012774c, unsigned int & 0, int 0) line 989 + 22 bytes
nsBlockReflowState::AddFloat(nsLineLayout & {...}, nsPlaceholderFrame *
0x03489034, int 0, unsigned int & 0) line 626 + 24 bytes
nsLineLayout::AddFloat(nsPlaceholderFrame * 0x03489034, unsigned int & 0) line 261
nsLineLayout::ReflowFrame(nsIFrame * 0x03489034, unsigned int & 0,
nsHTMLReflowMetrics * 0x00000000, int & 0) line 1016 + 22 bytes
nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, nsIFrame * 0x03489034, unsigned char *
0x00127a0b) line 4000 + 22 bytes
nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, int * 0x00127db4, unsigned char * 0x00127aff,
int 1, int 0) line 3839 + 32 bytes
nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x00127db4, int 0, int 1) line 3712 + 46 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x00127db4, int 0) line 2681
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03486994, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 0,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 588 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x00128a00) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x00128a00, int 0) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03486834, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 588 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012964c) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012964c, int 0) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0347c96c, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 0, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 588 + 42 bytes
nsBlockFrame::ReflowFloat(nsBlockReflowState & {...}, nsPlaceholderFrame *
0x0347c9c0, nsFloatCache * 0x0348da90, unsigned int & 0) line 5899 + 54 bytes
nsBlockReflowState::FlowAndPlaceFloat(nsFloatCache * 0x0348da90, int *
0x0012a110, unsigned int & 0, int 0) line 841
nsBlockReflowState::AddFloat(nsLineLayout & {...}, nsPlaceholderFrame *
0x0347c9c0, int 0, unsigned int & 0) line 626 + 24 bytes
nsLineLayout::AddFloat(nsPlaceholderFrame * 0x0347c9c0, unsigned int & 0) line 261
nsLineLayout::ReflowFrame(nsIFrame * 0x0347c9c0, unsigned int & 0,
nsHTMLReflowMetrics * 0x00000000, int & 0) line 1016 + 22 bytes
nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, nsIFrame * 0x0347c9c0, unsigned char *
0x0012a3cf) line 4000 + 22 bytes
nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & {...}, nsLineLayout &
{...}, nsLineList_iterator {...}, int * 0x0012a778, unsigned char * 0x0012a4c3,
int 1, int 1) line 3839 + 32 bytes
nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012a778, int 1, int 1) line 3712 + 46 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012a778, int 1) line 2681
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03428264, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012b3c4) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012b3c4, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x03428074, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x03428074, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 15, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableCellFrame::Reflow(nsTableCellFrame * const 0x03428000, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 858
nsContainerFrame::ReflowChild(nsIFrame * 0x03428000, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 105, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableRowFrame::IR_TargetIsChild(nsTableRowFrame * const 0x0346e268,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, nsTableFrame & {...}, unsigned int & 0, nsIFrame * 0x03428000) line
1226 + 45 bytes
nsTableRowFrame::IncrementalReflow(nsTableRowFrame * const 0x0346e268,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, nsTableFrame & {...}, unsigned int & 0) line 1111 + 46 bytes
nsTableRowFrame::Reflow(nsTableRowFrame * const 0x0346e268, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 1409 + 35 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x0346e268, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableRowGroupFrame::IR_TargetIsChild(nsTableRowGroupFrame * const 0x033f7754,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, nsRowGroupReflowState &
{...}, unsigned int & 0, nsIFrame * 0x0346e268) line 1640 + 45 bytes
nsTableRowGroupFrame::IncrementalReflow(nsTableRowGroupFrame * const 0x033f7754,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, nsRowGroupReflowState &
{...}, unsigned int & 0) line 1324 + 42 bytes
nsTableRowGroupFrame::Reflow(nsTableRowGroupFrame * const 0x033f7754,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0) line 1230 + 31 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x033f7754, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
nsTableFrame::IR_TargetIsChild(nsTableFrame * const 0x0346df34,
nsTableReflowState & {...}, unsigned int & 0, nsIFrame * 0x033f7754) line 2932 +
50 bytes
nsTableFrame::IncrementalReflow(nsTableFrame * const 0x0346df34, const
nsHTMLReflowState & {...}, unsigned int & 0) line 2669 + 34 bytes
nsTableFrame::Reflow(nsTableFrame * const 0x0346df34, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 1925 + 23 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x0346df34, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 3, unsigned int & 0) line 904 + 31 bytes
nsTableOuterFrame::OuterReflowChild(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsIFrame * 0x0346df34, const nsHTMLReflowState &
{...}, nsHTMLReflowMetrics & {...}, int 11400, nsSize & {...}, nsMargin & {...},
nsMargin & {...}, nsMargin & {...}, nsReflowReason eReflowReason_Incremental,
unsigned int & 0, int * 0x0012c6ac) line 1314 + 47 bytes
nsTableOuterFrame::IR_InnerTableReflow(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0) line 1662 + 81 bytes
nsTableOuterFrame::IR_TargetIsInnerTableFrame(nsTableOuterFrame * const
0x0346ddd0, nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const
nsHTMLReflowState & {...}, unsigned int & 0) line 1426 + 31 bytes
nsTableOuterFrame::IR_TargetIsChild(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0, nsIFrame * 0x0346df34) line 1399 + 31 bytes
nsTableOuterFrame::IncrementalReflow(nsTableOuterFrame * const 0x0346ddd0,
nsPresContext * 0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState
& {...}, unsigned int & 0) line 1379 + 42 bytes
nsTableOuterFrame::Reflow(nsTableOuterFrame * const 0x0346ddd0, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 1925 + 31 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 0,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012d240) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012d240, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0346db74, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 0,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012de8c) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012de8c, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0346b584, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsBlockReflowContext::ReflowBlock(const nsRect & {...}, int 1,
nsCollapsingMargin & {...}, int 0, int 1, nsMargin & {...}, nsHTMLReflowState &
{...}, unsigned int & 0) line 605 + 42 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineList_iterator
{...}, int * 0x0012ead8) line 3427 + 66 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineList_iterator {...},
int * 0x0012ead8, int 1) line 2588 + 27 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}, int 1) line 2240 + 31
bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x0346b400, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 877 + 17 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x0346b400, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
CanvasFrame::Reflow(CanvasFrame * const 0x033f717c, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 522
nsContainerFrame::ReflowChild(nsIFrame * 0x033f717c, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 3, unsigned int & 0) line 904 + 31 bytes
nsHTMLScrollFrame::ReflowScrolledFrame(const ScrollReflowState & {...}, int 0,
nsHTMLReflowMetrics * 0x0012f3b0, int 1) line 512 + 54 bytes
nsHTMLScrollFrame::ReflowContents(ScrollReflowState * 0x0012f558, const
nsHTMLReflowMetrics & {...}) line 567 + 25 bytes
nsHTMLScrollFrame::Reflow(nsHTMLScrollFrame * const 0x033f72c8, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 759 + 16 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x033f72c8, nsPresContext * 0x0325c090,
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, int 0,
unsigned int 0, unsigned int & 0) line 904 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x033f70e8, nsPresContext *
0x0325c090, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...},
unsigned int & 0) line 239 + 43 bytes
IncrementalReflow::Dispatch(nsPresContext * 0x0325c090, nsHTMLReflowMetrics &
{...}, const nsSize & {...}, nsIRenderingContext & {...}) line 912
PresShell::ProcessReflowCommands(int 1) line 6874
ReflowEvent::HandleEvent() line 6700
HandlePLEvent(ReflowEvent * 0x03476778) line 6717
PL_HandleEvent(PLEvent * 0x03476778) line 688 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f39460) line 623 + 9 bytes
_md_TimerProc(HWND__ * 0x001504d0, unsigned int 275, unsigned int 0, unsigned
long 89559940) line 1013 + 9 bytes
USER32! 77d48734()
USER32! 77d49857()
USER32! 77d49791()
USER32! 77d48a10()
nsAppShell::Run(nsAppShell * const 0x00f51588) line 135
nsAppStartup::Run(nsAppStartup * const 0x00f514e8) line 145 + 26 bytes
XRE_main(int 3, char * * 0x003f6f60, const nsXREAppData * 0x0042201c kAppData)
line 2322 + 35 bytes
main(int 3, char * * 0x003f6f60) line 61 + 18 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 7c816d4f()
Reporter | ||
Comment 1•19 years ago
|
||
(In reply to comment #0)
This stack is probably entirely bogus. I will try to get a good one later today.
Summary: ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', @ nsSpaceManager::BandRect::SplitHorizontally → ASSERTION: bad argument: '(aRight > mLeft) && (aRight < mRight)', @ nsSpaceManager::BandRect::SplitHorizontally
Reporter | ||
Comment 2•19 years ago
|
||
(In reply to comment #1)
The previous stack is actually good and reproducible in Trunk as well as 1.8
branch. In this case aRight == 0x2472, mLeft == 0x2472, mRight == 0x2b89
Version: 1.8 Branch → Trunk
Assignee | ||
Comment 4•19 years ago
|
||
Is there an actual observable layout bug in the page in question? This assertion was added pretty late in 1.5.
Reporter | ||
Comment 5•19 years ago
|
||
(In reply to comment #4)
Not that I can see.
Comment 6•19 years ago
|
||
This testcase still asserts in my debug build.
Updated•19 years ago
|
Comment 7•19 years ago
|
||
I see the assertion using Martijn's testcase.
Updated•19 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
![]() |
||
Updated•19 years ago
|
Flags: blocking1.9a2?
Should this assertion have an "or equal to"?
Flags: blocking1.9a2? → blocking1.9-
Whiteboard: [wanted-1.9]
Assignee | ||
Comment 9•18 years ago
|
||
No. When equality holds, we're splitting a BandRect in a way that creates an empty BandRect, in which case we shouldn't be splitting at all.
Updated•18 years ago
|
Component: Layout → Layout: Floats
QA Contact: layout → layout.floats
Comment 10•18 years ago
|
||
FWIW, nsSpaceManager::AddRectRegion (nsSpaceManager.cpp#833) has the first indication of a failure. A breakpoint set for:
bandRect->mLeft == bandRect->mRight
immediately after bandRect is created triggers under the attached testcase.
CNN loves this assertion failure, by the way.
Comment 11•18 years ago
|
||
WFM with the URL and with Martijn's testcase.
Comment 12•17 years ago
|
||
I can trigger this assertion by visiting http://dailymotion.com, with the same situation as described in Comment #2.
XPCOM_MEM_LEAK_LOG also claims we're leaking BandRect objects on that page, but there are no remaining references.
Comment 13•17 years ago
|
||
Fwiw, it is all wfm now, using a 5 day old debug build, testing the url, the url in comment 12 and the testcase.
Comment 14•17 years ago
|
||
This patch fixes the assertions and leaks for me, but perhaps we're supposed to insert zero-width rects that have a height.
I'll also update my build to check that I'm not missing a patch.
Attachment #277773 -
Flags: review?(roc)
Comment 15•17 years ago
|
||
(In reply to comment #14)
>
> I'll also update my build to check that I'm not missing a patch.
I can still reproduce with an up-to-date build.
Comment 16•17 years ago
|
||
See Bug 387201 and the regression caused by exactly this change, bug 391412.
Comment 17•17 years ago
|
||
I just looked at this a bit more; this change specifically breaks nsSpaceManager::YMost. That looks to be fixable, though; I think all that is needed is to change it to do something more like ClearFloats (possibly just calling "ClearFloats(nscoord_MIN, NS_STYLE_CLEAR_BOTH)" would work, although I'm not sure what sort of perf impact that would have).
Comment 18•17 years ago
|
||
Comment on attachment 277773 [details] [diff] [review]
exit early for zero-width rects as well
Well, this patch is no good. It crashes on cnn.com.
Attachment #277773 -
Flags: review?(roc)
Comment 19•17 years ago
|
||
Yeah, you want something like Attachment 273676 [details] [diff], except for the fact that it breaks nsSpaceManager::YMost, and therefore bug 391412.
Reporter | ||
Comment 20•17 years ago
|
||
The asserts are not reproducible for me in winxp/linux with the url and testcase in this bug, but are still reproducible with the testcase in bug 387214 for both winxp/linux. Since this assertion was #3 in a recent top site test run with over 3000 occurrences, it is definitely not fixed. I suggest duping forward to 387214.
Comment 21•17 years ago
|
||
Here's the fix I outlined earlier. I think it fixes the BandRect leak from http://wiki.mozilla.org/LeakingPages as well.
Assignee | ||
Comment 23•17 years ago
|
||
+ autoHeight = PR_MAX(autoHeight,
+ aReflowState.mSpaceManager->ClearFloats(nscoord_MIN, NS_STYLE_CLEAR_LEFT_AND_RIGHT));
I think this can result in two calls to ClearFloats.
Comment 24•17 years ago
|
||
Yes, you're right... this is why PR_MAX sucks. I'll post a fixed version.
Comment 25•17 years ago
|
||
Attachment #279786 -
Attachment is obsolete: true
Attachment #279827 -
Flags: review?(roc)
Attachment #279786 -
Flags: review?(roc)
Assignee | ||
Comment 26•17 years ago
|
||
Comment on attachment 279827 [details] [diff] [review]
Patch v2
Approving because this fixes common assertions and also because it might fix a leak
Attachment #279827 -
Flags: superreview+
Attachment #279827 -
Flags: review?(roc)
Attachment #279827 -
Flags: review+
Updated•17 years ago
|
Attachment #279827 -
Flags: approval1.9?
Assignee | ||
Updated•17 years ago
|
Attachment #279827 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Keywords: checkin-needed
Comment 27•17 years ago
|
||
Checked-in for elif since he's awol.
Checking in layout/generic/nsBlockFrame.cpp;
/cvsroot/mozilla/layout/generic/nsBlockFrame.cpp,v <-- nsBlockFrame.cpp
new revision: 3.872; previous revision: 3.871
done
Checking in layout/generic/nsSpaceManager.cpp;
/cvsroot/mozilla/layout/generic/nsSpaceManager.cpp,v <-- nsSpaceManager.cpp
new revision: 3.86; previous revision: 3.85
done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M9
Comment 28•17 years ago
|
||
Backed out due to this patch causing a reftest failure for the test in bug 218473.
REFTEST UNEXPECTED FAIL: file:///builds/slave/trunk_osx/mozilla/layout/reftests/bugs/218473-1.html
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla1.9 M9 → ---
Comment 29•17 years ago
|
||
Reassigning to roc (as per his request) and re-requesting blocking1.9? due to the leaks this patch might fix.
Assignee: sharparrow1 → roc
Status: REOPENED → NEW
Flags: blocking1.9- → blocking1.9?
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Whiteboard: [wanted-1.9] → [wanted-1.9][dbaron-1.9:Rm]
Comment 30•17 years ago
|
||
FWIW, I don't see the assertion for the link in the URL field (on Linux).
I do see it loading this URL though:
http://www.alistapart.com/d/multicolumnlayouts/3ColLiquidWithContent.html
The "Patch v2" does NOT fix it.
Comment 31•17 years ago
|
||
Comment on attachment 279827 [details] [diff] [review]
Patch v2
Resetting approval flags on bugs that have not been checked in by Oct 22, 11:59PM PDT. Please re-request approval if needed.
Attachment #279827 -
Flags: approval1.9+
Assignee | ||
Comment 32•17 years ago
|
||
Simple problem ... mSpaceManager->ClearFloats was returning a value relative to the block's top content edge, but autoHeight is relative to the block's top border edge, so the padding wasn't being included in the computed height in the 218473-1.html testcase.
The fix is easy, just go through nsBlockReflowState::ClearFloats, which adds the padding-top in for us. We do need to pass autoHeight instead of nscoord_MIN as the default result, but that's OK, it won't affect anything either way.
I'l see if I can get Eli to review this change.
Attachment #277773 -
Attachment is obsolete: true
Attachment #279827 -
Attachment is obsolete: true
Attachment #286086 -
Flags: superreview+
Attachment #286086 -
Flags: review?(sharparrow1)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [wanted-1.9][dbaron-1.9:Rm] → [needs review][dbaron-1.9:Rm]
Comment 33•17 years ago
|
||
I get this assert @ the same function when visiting https://bugs.launchpad.net/ubuntu/+source/mozilla-thunderbird/+bug/128288 in my own debug build from 2007110101. I don't assert at all in the testcase with that build. Jesse doesn't assert either on that testcase. (he's also using a recent trunk debug)
OS: Windows XP → All
Hardware: PC → All
![]() |
||
Comment 34•17 years ago
|
||
Please attach something (not necessarily minimal) to this bug that reproduces that issue. Relying on an external URI to not go away is a recipe for disaster.
Comment 35•17 years ago
|
||
Reduced from layout/reftests/bugs/387201-3.html.
Priority: -- → P5
Comment 36•17 years ago
|
||
Comment on attachment 286086 [details] [diff] [review]
patch v3
This looks right.
Attachment #286086 -
Flags: review?(sharparrow1) → review+
Assignee | ||
Comment 37•17 years ago
|
||
Thanks Eli.
Whiteboard: [needs review][dbaron-1.9:Rm] → [needs landing][dbaron-1.9:Rm]
Comment 38•17 years ago
|
||
Checking in layout/generic/nsBlockFrame.cpp;
/cvsroot/mozilla/layout/generic/nsBlockFrame.cpp,v <-- nsBlockFrame.cpp
new revision: 3.884; previous revision: 3.883
done
Checking in layout/generic/nsSpaceManager.cpp;
/cvsroot/mozilla/layout/generic/nsSpaceManager.cpp,v <-- nsSpaceManager.cpp
new revision: 3.89; previous revision: 3.88
done
Status: NEW → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing][dbaron-1.9:Rm] → [dbaron-1.9:Rm]
Target Milestone: --- → mozilla1.9 M10
Assignee | ||
Comment 39•17 years ago
|
||
Need to make a crashtest assertion test here.
Flags: in-testsuite?
Comment 40•17 years ago
|
||
verified fixed using debug build Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120520 Minefield/3.0b2pre and the testcases from this bug
no assertions on testcases -> Verified
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•