Bug 11720

Summary:

REGRESSION: large amounts of CPU consumed viewing this site

Product:

WebKit

Reporter:

Rachael Worthington (cheers) <rachael>

Component:

Layout and Rendering

Assignee:

mitz

Status:

RESOLVED FIXED

Severity:

Normal

CC:

hyatt, mitz, mrowe

Priority:

Keywords:

HasReduction, InRadar, Regression

Version:

420+

Hardware:

Mac

OS:

OS X 10.4

URL:

http://www.wowhead.com/?talent-bc=ihe0oEgsVzVoMZV0x

Bug Depends on:

Bug Blocks:

5399, 5821

Attachments:

Description	Flags
Sample of Webkit Misbehaving	none
Invalidate only the area where the updated image paints. Also fixes bug 5399.	hyatt: review-
Binaries part of the patch	none
Invalidate only the area where the updated image paints. Also fixes bug 5399 and bug 5821.	hyatt: review+
Binaries part of the patch	none

Description Rachael Worthington (cheers) 2006-11-29 17:45:10 PST

go to the above link in OmniWeb or the lates WebKit nightly, and watch your CPU consumption. It rockets up, and stays high, until you hide the page by opening a new tab over it. (being in a background tab it doesn't consume silly amounts of processing time, but hidden and in a front-most tab it does. I'm attaching a sample as well.

Comment 1 Rachael Worthington (cheers) 2006-11-29 17:46:01 PST

Created attachment 11675 [details]
Sample of Webkit Misbehaving

Comment 2 mitz 2006-11-30 11:58:38 PST

A huge part of the page is constantly repainting at 10fps due to one or two elements having a small animated gif as their non-repeating background image. Something like:
<div style="height: 500px; background: url(http://www.wowhead.com/images/loading2.gif) no-repeat"></div>
causes the entire div to repaint on each animation frame. Note that the background is non-repeating, so there's no reason to repaint the whole area. Shipping Safari repaints only the 16x16 area that animates.

Comment 3 Dave Hyatt 2006-11-30 12:09:52 PST

This is fallout from having a real animation observer architecture.  That the old way worked more efficiently was largely coincidental.  RenderObjects will have to study their background geometry and be smarter about what they repaint when they get told that a new animation frame is available.

Comment 4 Mark Rowe (bdash) 2007-01-28 19:12:57 PST

<rdar://problem/4960272>

Comment 5 mitz 2007-02-01 14:50:33 PST

Created attachment 12861 [details]
Invalidate only the area where the updated image paints. Also fixes bug 5399.

Comment 6 mitz 2007-02-01 14:59:06 PST

Created attachment 12862 [details]
Binaries part of the patch

Comment 7 Dave Hyatt 2007-02-01 16:10:57 PST

Comment on attachment 12861 [details]
Invalidate only the area where the updated image paints. Also fixes bug 5399.

(1) Inline flows need to be handled (if they're broken up across multiple lines).

(2) Would it be better to put the table checks into imageChanged  overrides in the table code itself?

(3) Change the htmlTag/bodyTag stuff to isRoot()/isBody().

Comment 8 mitz 2007-02-02 01:21:12 PST

Created attachment 12868 [details]
Invalidate only the area where the updated image paints. Also fixes bug 5399 and bug 5821.

Addressed Hyatt's comments: inline flows do a full repaint; table parts handled by overrides, also changed <col> to repaint the whole table instead of nothing, fixing bug 5821; changed to use isBody() and isRoot().

Comment 9 mitz 2007-02-02 01:23:37 PST

Created attachment 12869 [details]
Binaries part of the patch

Comment 10 Dave Hyatt 2007-02-07 02:18:59 PST

Comment on attachment 12868 [details]
Invalidate only the area where the updated image paints. Also fixes bug 5399 and bug 5821.

r=me

Comment 11 Mark Rowe (bdash) 2007-02-07 21:06:01 PST

Landed in r19490.

Comment 12 Mark Rowe (bdash) 2007-02-11 23:17:35 PST

Binary bits were in r19570.