Bug 118493 - Deferring layer flushes can cause painting without layout being done
Summary: Deferring layer flushes can cause painting without layout being done
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-07-08 19:25 PDT by Tim Horton
Modified: 2013-07-10 14:19 PDT (History)
4 users (show)

See Also:

Attachments
preliminary patch (5.45 KB, patch)
2013-07-10 04:03 PDT, Tim Horton 		no flags Details | Formatted Diff | Diff
patch (7.88 KB, patch)
2013-07-10 13:01 PDT, Tim Horton 		simon.fraser: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2013-07-08 19:25:17 PDT 
1. We depend on a run loop observer that fires just before CA does painting to ensure that layout is up-to-date.
2. TileController manually calls setNeedsDisplay on CALayers.
3. RenderLayerCompositor can (after r147797) defer #1 and cause it to not happen in the same run loop cycle as #2.
4. This means that we can end up painting without layout being up-to-date, which can cause horrible, horrible graphical issues.

We need to not defer (#3) if TileController has setNeedsDisplay'd any layers manually (or if we've added any new dirty layers).

<rdar://problem/14286329>
Comment 1 Tim Horton 2013-07-08 19:27:11 PDT 
One of the best repro cases of actual real world badness caused by this is http://www.mercurynews.com, which frequently shows garbage (or black) upon loading.
Comment 2 Tim Horton 2013-07-10 03:53:47 PDT 
#2 is actually "TileController can parent layers which had setNeedsDisplay called on them long ago, but were unparented"
Comment 3 Tim Horton 2013-07-10 04:03:27 PDT 
Created attachment 206378 [details]
preliminary patch
Comment 4 Tim Horton 2013-07-10 13:01:40 PDT 
Created attachment 206409 [details]
patch
Comment 5 Antti Koivisto 2013-07-10 13:36:02 PDT 
Comment on attachment 206409 [details]
patch

For the benefit of future debuggers, it might be good to add a comment above platformCALayerDidCreateTiles call as it is not that obvious that it initiates an immediate layer flush.
Comment 6 Tim Horton 2013-07-10 13:38:52 PDT 
(In reply to comment #5)
> (From update of attachment 206409 [details])
> For the benefit of future debuggers, it might be good to add a comment above platformCALayerDidCreateTiles call as it is not that obvious that it initiates an immediate layer flush.

True. I'll do that.
Comment 7 Tim Horton 2013-07-10 14:19:02 PDT 
http://trac.webkit.org/changeset/152548