393656 - Hang/assertions with <select>, float, percentage margin

Status: RESOLVED FIXED

(Core :: Layout: Floats, defect, P4)

Description

[Jesse Ruderman]

I have a bunch of related testcases that hang and/or trigger one or more of these assertions:

###!!! ASSERTION: unexpected band data count: 'aBandData.mCount > 0', file /Users/jruderman/trunk/mozilla/layout/generic/nsSpaceManager.cpp, line 397

###!!! ASSERTION: If we asked for force-fit, it should have been placed: 'placed || !forceFit', file /Users/jruderman/trunk/mozilla/layout/generic/nsBlockReflowState.cpp, line 551

###!!! ASSERTION: aBandRect should be first rect within its band: '!aBandRect || aBandRect == mBandList.Head() || aBandRect->Prev()->mBottom != aBandRect->mBottom', file /Users/jruderman/trunk/mozilla/layout/generic/nsSpaceManager.h, line 493

This bug might be related to bug 383887.

Comment 1

[Jesse Ruderman]

Attachment: testcase: hang

Comment 2

[Jesse Ruderman]

Attachment: testcase: "unexpected band data count" assertion

Comment 3

[Daniel Holbert [:dholbert]]

I posted a much-reduced version of this bug's testcases (specifically the <option style="margin: 100%;"> part) on bug 370872, because it triggers similar assertions to the second testcase in that bug, about unconstrained widths.

I'm pretty sure that bug 370872's high-percent-margin issue is (one of) the ultimate culprit(s) of this bug's problems, as well.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: patch v1 (add calls to NSCoordSaturatingAdd and NSCoordSaturatingSubtract)

GDB'd through a series of places where we need to call NSCoordSaturatingAdd / Subtract.

This patch fixes the hang (on the first testcase) and fixes all assertions mentioned in comment 1 (on both testcases).

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: patch v2 (also fixes some "metrics=xxxx,xxxx!" and "bad width" NOTREACHEDs)

Added a few more chunks to fix the following two things:

1. Some printf's like 

 nsBlockReflowContext: Block(option)(1)@0x8aebf54 metrics=xxxxx,yyyyy! 

where xxxxx is something close to, but not equal to, nscoord_MAX.


2. Some instances of this pair of messages:

###!!! ASSERTION: bad width: 'Not Reached', file /scratch/work/builds/trunk.07-10-01.09-47/mozilla/layout/generic/nsLineLayout.cpp, line 181
Block(select)(1)@0x8c7f098: Init: bad caller: width WAS 1073741584

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

For a bunch of these, it doesn't seem like either of the values being added should be nscoord_MAX.  e.g., for a computed width + a border+padding, I wouldn't expect either to be.

It might be useful to have comments that say which of the values can be nscoord_MAX when you're using these functions.  But in this case, I'm wondering whether the real problem is something else.  Or is there something I'm forgetting?

Comment 7

[Daniel Holbert [:dholbert]]

Yeah, I think you're right... We're probably working with  nscoord_MAX in some places as a result of bug 363858, when we should never have nscoord_MAX there in the first place.  (See some analysis in the duplicate bug 370872 comment 7, to see where the nscoord_MAX value originates from.)

I should probably wait to patch this until after bug 363858 is fixed, because that will probably take care of some or all of this bug's nscoord_MAX and/or hang issues.

Comment 8

[Boris Zbarsky [:bzbarsky]]

In my build with the patch for bug 363858 I don't see any hangs or asserts on the two testcases in this bug.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Is this still an issue?

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to comment #8)
> In my build with the patch for bug 363858 I don't see any hangs or asserts on
> the two testcases in this bug.

Me neither, using current trunk.
(And I was able to reproduce those issues previously.)

Resolving as fixed, by 363858.

Comment 11

[Jesse Ruderman]

Crashtests checked in.