27946 - Incremental Reflow returns different width values than Resize Reflow

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[rods (gone)]

I discovered this bug during more testing for bug 27498. In some ways it is the 
same bug and in many ways it is a new bug.

I am assigning this to buster, although it could be a troy bug, or eventually 
even my bug

When a new item is added to the ListBox 4 reflows occur and the Incremental 
reflow returns an odd width. This causes a visual "flashing" during the reflows.

The list box has its size set to 4.

The Reflow Sequence is as follows:
Resize
Incremental
Resize
Resize

Here as the output after the first item is added:

nsListControlFrame::Reflow 3  Reason: eReflowReason_Resize
*** 1050 1050
added item
nsListControlFrame::Reflow 4  Reason: eReflowReason_Incremental
*** 1350 1050
nsListControlFrame::Reflow 5  Reason: eReflowReason_Resize
*** 1050 1050
nsListControlFrame::Reflow 6  Reason: eReflowReason_Resize
*** 1050 1050

Here is the output after the 5th item is added and vertical scrollbars have 
appeared (same as 4th):

nsListControlFrame::Reflow 15  Reason: eReflowReason_Resize
*** 1050 1050
added item
nsListControlFrame::Reflow 16  Reason: eReflowReason_Incremental
*** 1350 1050
nsListControlFrame::Reflow 17  Reason: eReflowReason_Resize
*** 1050 1050
nsListControlFrame::Reflow 18  Reason: eReflowReason_Resize
*** 1050 1050

Here is the output after the 6th item is added:

nsListControlFrame::Reflow 19  Reason: eReflowReason_Resize
*** 1050 1050
added item
nsListControlFrame::Reflow 20  Reason: eReflowReason_Incremental
*** 1110 1050
nsListControlFrame::Reflow 21  Reason: eReflowReason_Resize
*** 1050 1050
nsListControlFrame::Reflow 22  Reason: eReflowReason_Resize
*** 1050 1050

-------------------------------------
Here is what it interesting as items 1-5 are added the difference in the widths 
between the Inc and Resize reflows is 300 twips which is the width of the 
scrollbar plus the border width left and right.

For the 6th item the difference is now 60 twips which is the width of the left 
and right borders.

Another question: Why are there 4 reflows when a single item is added?

Comment 1

[rods (gone)]

Attachment: simepl test case

Comment 2

[rods (gone)]

adding troy to the cc list

Comment 3

[buster]

this is both a performance bug and a usability/correctness bug for core layout.

more data from Rod:
On line 1862 of nsBlockFrame: 
    nscoord minWidth = aState.mKidXMost + borderPadding.right; 

The variable "aState.mKidXMost" is already 30pixels larger during the Inc Reflow 
than during the  Resize Reflow. 

Comment 4

[troy]

30 pixels larger or 30 twips larger?

Comment 5

[rods (gone)]

30 Twips

Comment 6

[rods (gone)]

We get extra reflows because clicking down on a button causes a reflow of the 
document and the click up causes a reflow of the document. Troy, Kevin mentioned 
that you two had discussed a long time ago an optimization where if there is a 
resize reflow inside a reframe but the frame itself doesn't change size then 
nothing else gets reflowed. For example, when a button is pressed down the 
padding a such is adjusted via style rules to shift the content of the button 
down and to the right. This causes the page to reflow, then the same thing 
happens on the mouse up.

Comment 7

[troy]

There were two separate proposals:

1. the "nothing changed" optimization. We don't really need that optimization 
anymore, because that was working around how table cell incremental reflow used 
to be a three-step reflow process, where step one was to process the incremental 
reflow command, step two was to do an unconstrained reflow and update the maxium 
width, and step three reflowed the cell back to its cell width. Now that we can 
incrementally update the cell's maximum width it's just one step and reasonably 
efficient

2. was having the style system get the frame involved when things like padding 
changed in a zero sum way, e.g., padding added to one edge and removed from 
another edge. I think we all agreed this was necessary, but I don't remember 
where it currently stands

Pierre, do you remember?

Comment 8

[buster]

I am looking into the problem that incremental reflow is returning a different 
size than resize reflow given the same available width and height.

I have split out the bug about too many reflows being generated into bug 29605.

Comment 9

[rickg]

moving all buster m15 bugs to m16.

Comment 10

[buster]

I instrumented nsListControlFrame::Reflow, and I was unable to reproduce the 
problem Rod reports.  I see two reflows for every item added, each with output 
similar to this:

added item
01141788 ** nsLCF::Reflow 25 R: Increm AW: 6405 AH: UNC CW: UNC CH: UNC
nsLCF::Reflow, superclass reflow returned 1073741824 1530
nsLCF::Reflow, superclass reflow returned 1073741824 1530
nsLCF::Reflow, superclass reflow returned 960 1050
01141788 ** nsLCF::Reflow 26 R: Resize AW: 6405 AH: UNC CW: UNC CH: UNC

I interpret this to mean that adding an item causes two reflows, the first of 
which is incremental and causes nsListControlFrame's superclass nsScrollFrame's 
Reflow method to be called 3 times.  The second nsListControlFrame::Refow is a 
resize reflow, and it is optimized away.

Rod, can you still reproduce your problem?  If so, can you send me the 
instrumented .cpp file so I can see exactly what you are printing out?  Thanks.

Comment 11

[rods (gone)]

I think this is fixed now. verifying

Comment 12

[Chris Petersen]

Marking verified per last comments.