Closed
Bug 381152
Opened 17 years ago
Closed 17 years ago
Hang with float and large padding and margin
Categories
(Core :: Layout: Floats, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: martijn.martijn, Assigned: dholbert)
References
Details
(Keywords: hang, regression, testcase, Whiteboard: [dbaron-1.9:RwCo])
Attachments
(2 files, 4 obsolete files)
276 bytes,
text/html
|
Details | |
1.00 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
See testcase, which hangs current trunk builds when loading. It doesn't hang on branch builds. This regressed between 2006-02-22 and 2006-02-25, this is the period where cairo was turned on on windows, so I guess that might somehow be the cause. There doesn't seem to be any layout specific check-ins that could have caused this.
Comment 1•17 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070517 Minefield/3.0a5pre It doesn't hang here.
Reporter | ||
Comment 2•17 years ago
|
||
Strange... doesn't it hang even a little bit for you?
Comment 3•17 years ago
|
||
No, it just loads a blank page. ADM Athlon XP 2000+ Processor.
Comment 4•17 years ago
|
||
On another computer it hangs indeed. Not responsive in any case while processor usage is 50-60%. I see no blank page. Processor is Intel Duo Core.
Reporter | ||
Comment 5•17 years ago
|
||
Maybe it depends on the video card? I'm using a NVidia Geforce Go7300 (with a Pentium Centrino 1.80GHz).
Comment 6•17 years ago
|
||
Yeah, maybe. The computer where Firefox gets unresponsive has also that NVidea Geforce Go 7300. The laptop without problems has an ATI Radeon.
Reporter | ||
Comment 7•17 years ago
|
||
Vlad, you have an idea what's going on here?
Comment 8•17 years ago
|
||
Dupe of bug 381074?
Comment 9•17 years ago
|
||
(In reply to comment #8) > Dupe of bug 381074? > I think so... it's hanging in the same place.
Depends on: 381074
Assignee | ||
Comment 10•17 years ago
|
||
Not a dupe of bug 381074. That bug's now patched, and the patch doesn't fix this one.
Assignee | ||
Updated•17 years ago
|
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Whiteboard: [dbaron-1.9:RwCo]
Assignee | ||
Comment 12•17 years ago
|
||
I get these warnings just before the hang: WARNING: Overflowed nscoord_MAX in conversion to nscoord: file ../../dist/include/xpcom/nsUnitConversion.h, line 100 WARNING: Overflowed nscoord_MAX in conversion to nscoord: file ../../dist/include/xpcom/nsUnitConversion.h, line 100 Area(div)(1)@0x819a2b8: UpdateBand: bad caller: height WAS 473741888(0x1c3cba40) nsBlockReflowContext: Area(div)(1)@0x819a2b8 metrics=0,1547483712!
Assignee | ||
Comment 13•17 years ago
|
||
This hack-ish patch shows (part of) the problem here. The problem starts with the "floatMargin.TopBottom()" call here in nsBlockReflowState.cpp: 762 nsSize floatSize = floatFrame->GetSize() + 763 nsSize(floatMargin.LeftRight(), floatMargin.TopBottom()); Here, floatMargin is a nsMargin with {top = 1073741824, right = 0, bottom = -599999936, left = 0} and the nsMargin::TopBottom call just returns top + bottom. So we end up doing arithmetic near nscoord_MAX, which is dangerous. This gets us into more trouble when we add the outer float's nscoord_MAX padding later on.
Assignee | ||
Comment 14•17 years ago
|
||
Assignee | ||
Updated•17 years ago
|
Attachment #287571 -
Attachment is obsolete: true
Assignee | ||
Comment 15•17 years ago
|
||
The idea of this patch is to just fix the "gets us into trouble" part of comment #13: > So we end up doing arithmetic near nscoord_MAX, which is dangerous. This gets > us into more trouble when we add the outer float's nscoord_MAX padding later > on. This patch restricts nsSpaceManager to make its bands have, at most, nscoord_MAX height. This fixes the cases (as in this bug's testcase) where rect.y ~= nscoord_MAX rect.height ~= nscoord_MAX which means rect.YMost() = 2 * nscoord_MAX Once we start working with signed-integer values close to 2 * nscoord_MAX, we run the risk of overflowing into negative numbers, which is probably why we end up hanging. BTW, these nscoord_MAX values come from the enormous padding and margins -- they're not "unconstrained_height" sentinel values. Note 1: This patch is based on the last section of the draft patch for Bug 393656. Note 2: I removed the first part of "patch v1" to create "patch v1a" because that first part wasn't necessary to fix this bug's testcases. We may still want to include that bit as well, though.
Assignee | ||
Updated•17 years ago
|
Attachment #287580 -
Flags: review?(roc)
Comment on attachment 287580 [details] [diff] [review] patch v1a This looks fine. I'm a bit worried that trying to fix all possible integer overflows this way is going to add tons of unreadable code and take forever, though.
Attachment #287580 -
Flags: superreview+
Attachment #287580 -
Flags: review?(roc)
Attachment #287580 -
Flags: review+
Assignee | ||
Comment 17•17 years ago
|
||
Assignee | ||
Comment 18•17 years ago
|
||
(added line for reftest.list)
Assignee | ||
Updated•17 years ago
|
Whiteboard: [dbaron-1.9:RwCo] → [dbaron-1.9:RwCo][needs landing]
Assignee | ||
Updated•17 years ago
|
Attachment #287610 -
Attachment is obsolete: true
Assignee | ||
Comment 19•17 years ago
|
||
patch v1a landed /cvsroot/mozilla/layout/generic/nsSpaceManager.cpp,v <-- nsSpaceManager.cpp new revision: 3.88; previous revision: 3.87
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [dbaron-1.9:RwCo][needs landing] → [dbaron-1.9:RwCo]
Reporter | ||
Comment 20•17 years ago
|
||
Again thanks for fixing this bug! Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007110705 Minefield/3.0a9pre
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 21•15 years ago
|
||
I checked in this bug's original testcase as a crashtest: http://hg.mozilla.org/mozilla-central/rev/e21014e86bad
Flags: in-testsuite? → in-testsuite+
Assignee | ||
Updated•15 years ago
|
Attachment #287613 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•