Bug 79139 - CCLayerTreeHostImpl calls didDraw more frequently than willDraw
Summary: CCLayerTreeHostImpl calls didDraw more frequently than willDraw
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-21 12:20 PST by Tim Dresser
Modified: 2012-02-22 13:00 PST (History)
3 users (show)

See Also:


Attachments
Patch (2.58 KB, patch)
2012-02-21 12:43 PST, Tim Dresser
no flags Details | Formatted Diff | Diff
Patch (5.84 KB, patch)
2012-02-22 07:38 PST, Tim Dresser
no flags Details | Formatted Diff | Diff
Patch (5.83 KB, patch)
2012-02-22 11:50 PST, Tim Dresser
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Dresser 2012-02-21 12:20:25 PST
In some contexts, CCLayerTreeHostImpl doesn't call willDraw, but does call didDraw.

This results in crashes in CCVideoLayerImpl.
Comment 1 Tim Dresser 2012-02-21 12:43:22 PST
Created attachment 128024 [details]
Patch
Comment 2 Adrienne Walker 2012-02-21 14:09:48 PST
Comment on attachment 128024 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128024&action=review

Can you add a unit test for a layer with an empty visibleLayerRect to make sure there's no mismatch between didDraw/willDraw?

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:281
> +            && !CCLayerTreeHostCommon::renderSurfaceContributesToTarget(*it, it.targetRenderSurfaceLayer()->id()))

I think this conditional is redundant with it.representsItself().
Comment 3 James Robinson 2012-02-21 17:24:04 PST
Comment on attachment 128024 [details]
Patch

R- for the redundant check and lack of test. Otherwise I think this is fine.
Comment 4 Tim Dresser 2012-02-22 07:38:30 PST
Created attachment 128214 [details]
Patch
Comment 5 James Robinson 2012-02-22 11:39:01 PST
Comment on attachment 128214 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=128214&action=review

R=me

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:280
> +        if (it.representsItself()
> +            && !it->visibleLayerRect().isEmpty())

just put this on one line
Comment 6 Tim Dresser 2012-02-22 11:50:52 PST
Created attachment 128257 [details]
Patch
Comment 7 WebKit Review Bot 2012-02-22 13:00:10 PST
Comment on attachment 128257 [details]
Patch

Clearing flags on attachment: 128257

Committed r108542: <http://trac.webkit.org/changeset/108542>
Comment 8 WebKit Review Bot 2012-02-22 13:00:17 PST
All reviewed patches have been landed.  Closing bug.