Bug 53198 - [chromium] Tiled compositor crashes if compositing turned off mid-paint
Summary: [chromium] Tiled compositor crashes if compositing turned off mid-paint
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adrienne Walker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-26 15:25 PST by Adrienne Walker
Modified: 2011-01-27 16:50 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.89 KB, patch)
2011-01-26 15:26 PST, Adrienne Walker
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrienne Walker 2011-01-26 15:25:20 PST
[chromium] Tiled compositor crashes if compositing turned off mid-paint
Comment 1 Adrienne Walker 2011-01-26 15:26:58 PST
Created attachment 80247 [details]
Patch
Comment 2 Adrienne Walker 2011-01-26 15:31:25 PST
In some cases a paint operation causes layout, which causes the root layer to no longer be composited, which turns off compositing mid-composite.  This patch adds a few checks to be robust to this.

Tested by adding a layerRenderer()->setRootLayer(NULL) in the middle of the LayerTilerChromium::update after painting.  There's a flash of "compositor blue" as the page switches from the compositor back to software, but it behaves correctly.  Future changes to separate out the compositor into a separate thread won't have this behavior.

See: http://crbug.com/69161
Comment 3 James Robinson 2011-01-26 15:43:50 PST
Comment on attachment 80247 [details]
Patch

Good catch! R=me.

We (in the general sense of "we") should experiment with sublayers as well to make sure they handle this case.
Comment 4 Adrienne Walker 2011-01-26 16:10:35 PST
(In reply to comment #3)
> (From update of attachment 80247 [details])
> Good catch! R=me.
> 
> We (in the general sense of "we") should experiment with sublayers as well to make sure they handle this case.

Hmm.  By code inspection, LayerRendererChromium::updateLayersRecursive appears to insert naked pointers to LayerChromium objects into a Vector that it uses over the course of update/draw.  If a paint call ends up deleting a child layer (maybe by deleting the owning GraphicsLayer?), then the draw pass could traverse into bogus memory.  That looks like the only possibility for bad behavior, but I don't know enough about the lifetime of GraphicsLayer or LayerChromium objects to know if that's possible during a paint call.
Comment 5 WebKit Commit Bot 2011-01-27 16:50:42 PST
Comment on attachment 80247 [details]
Patch

Clearing flags on attachment: 80247

Committed r76864: <http://trac.webkit.org/changeset/76864>
Comment 6 WebKit Commit Bot 2011-01-27 16:50:45 PST
All reviewed patches have been landed.  Closing bug.