Bug 6795

Summary: Slow image load causes render problem.
Product: WebKit Reporter: Dennis Rowe <shr3kst3r>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gruber, info, mitz
Priority: P2 Keywords: HasReduction
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Bug Example
none
Reduced testcase
none
Proposed patch hyatt: review+

Description Dennis Rowe 2006-01-25 10:58:58 PST
I have tried to reduce this problem down as much as possible.  But, as you can see the test case still has a bunch of junk still in it, and when I tried to remove the rest of these tags, it caused the problem to dissappear (which was bad).  My test case uses a specialized image which is really a php script that creates a randomly colored image that changes on every load and also has a three second sleep.  This was done to cause the image to load slowly and also to stop caching of the image.  When the page is loaded the text is drawn on the page and when finally drawn to the page, the text is pushed down, but the html in the div holding "Problem Here" text is pushed down, but also another box appears at the top of the page.  The boxes are connected, because if you click on the button at the bottom of the page, it will check the button on the top instance.

These seems to affect both 417.x and 420+.
Comment 1 Dennis Rowe 2006-01-25 10:59:40 PST
Created attachment 5951 [details]
Bug Example

Here is the example of the bug.
Comment 2 Joost de Valk (AlthA) 2006-02-15 13:59:09 PST
Confirmed, this needs more reduction.
Comment 3 mitz 2006-02-16 08:02:37 PST
Created attachment 6542 [details]
Reduced testcase
Comment 4 mitz 2006-02-16 10:49:08 PST
Created attachment 6545 [details]
Proposed patch

The test is pixels-only since the render tree was correct.
Comment 5 Darin Adler 2006-02-17 18:20:39 PST
Comment on attachment 6545 [details]
Proposed patch

Looks right to me, but this is one Hyatt should review.
Comment 6 Dave Hyatt 2006-02-21 15:01:03 PST
Comment on attachment 6545 [details]
Proposed patch

I will need to think about this.  My gut says this is overkill, but my gut has been wrong before. :)
Comment 7 Dave Hyatt 2006-02-21 15:13:04 PST
The reason I'm nervous here is we had some horribly pathological behavior (as in pages with nested divs and complex float/margin combinations causing Safari to beachball) that the yPosEstimate stuff helped to prevent, and doing too much of the heavyweight marking can send you into a death spiral of relayout.
Comment 8 Dave Hyatt 2006-02-23 11:05:49 PST
I do think this is probably on the right track and maybe just needs a little refinement.  One problem I see is that won't this get triggered on the initial layout of elements, since oldChildY will be 0?
Comment 9 mitz 2006-03-12 10:05:57 PST
*** Bug 6270 has been marked as a duplicate of this bug. ***
Comment 10 mitz 2006-03-13 23:10:10 PST
As Hyatt suggested on IRC, I ran the layout tests and logged the ones where the additional marking was done:

css1/box_properties/acid_test
css1/box_properties/float
css2.1/t09-c5526c-display-00-e
css2.1/t0905-c414-flt-fit-01-d-g
css2.1/t0905-c5525-fltmult-00-d-g
fast/block/basic/011
fast/block/float/016
fast/block/float/024
fast/block/float/031
fast/block/margin-collapse/045
fast/block/margin-collapse/063
fast/css-generated-content/012

I'm not sure I see the problem with initial layout. Wouldn't all descendants be marked for layout anyway?
Comment 11 Dave Hyatt 2006-03-14 14:42:26 PST
Comment on attachment 6545 [details]
Proposed patch

r=me, make sure we have some good tests for this.