Closed
Bug 373611
Opened 17 years ago
Closed 17 years ago
Hang with table, "clear: right", and large height
Categories
(Core :: Layout: Tables, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: bzbarsky)
References
Details
(Keywords: hang, testcase)
Attachments
(2 files, 1 obsolete file)
295 bytes,
text/html
|
Details | |
957 bytes,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
Here's a sample from the hang, where 300 = 100% of the main thread: 300 BasicTableLayoutStrategy::ComputeColumnWidths(nsHTMLReflowState const&) 256 nsTableFrame::DidResizeColumns() 107 nsTableFrame::DidResizeColumns() 96 nsSplittableFrame::GetNextInFlow() const 67 nsSplittableFrame::GetNextInFlow() const 29 nsIFrame::GetStateBits() const 29 nsIFrame::GetStateBits() const 30 0x19e72e47 30 0x19e72e47 23 nsIFrame::GetStateBits() const 23 nsIFrame::GetStateBits() const 44 nsSplittableFrame::GetNextInFlow() const 44 nsSplittableFrame::GetNextInFlow() const
Reporter | ||
Comment 1•17 years ago
|
||
This testcase hangs Firefox 2007-02-08 but not 2007-02-06. So it might be a regression from bug 177805 "Fix the use of units in Gecko", or it might be that before bug 177805, a different height could have triggered the same hang.
Assignee | ||
Comment 2•17 years ago
|
||
This is actually a reflow branch regression; bug 177805 just changed the height it'll happen with. roc's changes to not split things if the avail height is unconstrained will help here for screen, of course. But the underlying bug remains for print...
Blocks: reflow-refactor
Assignee | ||
Comment 3•17 years ago
|
||
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #258297 -
Flags: superreview?(dbaron)
Attachment #258297 -
Flags: review?(dbaron)
Assignee | ||
Comment 4•17 years ago
|
||
Attachment #258297 -
Attachment is obsolete: true
Attachment #258298 -
Flags: superreview?(dbaron)
Attachment #258298 -
Flags: review?(dbaron)
Attachment #258297 -
Flags: superreview?(dbaron)
Attachment #258297 -
Flags: review?(dbaron)
Comment on attachment 258298 [details] [diff] [review] Whoops, need the right diff Oops. r+sr=dbaron
Attachment #258298 -
Flags: superreview?(dbaron)
Attachment #258298 -
Flags: superreview+
Attachment #258298 -
Flags: review?(dbaron)
Attachment #258298 -
Flags: review+
Assignee | ||
Comment 6•17 years ago
|
||
Fixed. Not sure we have a good test suite to put hangs into...
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Comment 7•17 years ago
|
||
(In reply to comment #6) > Fixed. Not sure we have a good test suite to put hangs into... > why not put it in reftest? It probably highlights a tricky area that we should have rendering tests for anyway.
Assignee | ||
Comment 8•17 years ago
|
||
> why not put it in reftest?
Will it actually make reftest fail in a sane way?
Comment 9•17 years ago
|
||
(In reply to comment #8) > > why not put it in reftest? > > Will it actually make reftest fail in a sane way? > If reftest doesn't catch hangs, that's a bug in reftest, but I don't think it blocks landing this test.
Assignee | ||
Comment 10•17 years ago
|
||
The other issue, of course, is it's not clear what the reference should be...
Comment 11•17 years ago
|
||
(In reply to comment #8) > Will it actually make reftest fail in a sane way? It'll make it not report PASS, so I consider that a "sane" way. There are IT people on call to deal with it if an errant checkin were to cause a tinderbox to hang. :-) Actually, I have an idea for making reftest handle most hangs without hanging -- see bug 373836. > The other issue, of course, is it's not clear what the reference should be... Why not the same page a second time?
Maybe we should have a separate directory of hang/crash testcases? They'll run faster if we don't spend time doing image comparison.
Yes, let's do that.
Assignee | ||
Comment 14•17 years ago
|
||
That sounds better to me. The comparison here would just be misleading anyway.
You need to log in
before you can comment on or make changes to this bug.
Description
•