Closed Bug 373611 Opened 17 years ago Closed 17 years ago

Hang with table, "clear: right", and large height

Categories

(Core :: Layout: Tables, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: bzbarsky)

References

Details

(Keywords: hang, testcase)

Attachments

(2 files, 1 obsolete file)

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
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.
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...
Attached patch Fix (obsolete) — Splinter Review
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #258297 - Flags: superreview?(dbaron)
Attachment #258297 - Flags: review?(dbaron)
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+
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
(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.
> why not put it in reftest?

Will it actually make reftest fail in a sane way?
(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.
The other issue, of course, is it's not clear what the reference should be...
(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.
That sounds better to me.  The comparison here would just be misleading anyway.
Blocks: 375980
Crashtest checked in.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: