Bug 84812 – [chromium] Prevent CCLayerImpl::willDraw/didDraw mismatches

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adrienne Walker]

[chromium] Prevent CCLayerImpl::willDraw/didDraw mismatches

[Adrienne Walker]

Created attachment 138707 [details]
Patch

[Adrienne Walker]

Created attachment 138709 [details]
Patch

[Adrienne Walker]

(Ack.  webkit-patch upload failed.  Ignore that patch.)

[Adrienne Walker]

Created attachment 138822 [details]
Patch

[Adrienne Walker]

I'm not totally happy with "didDrawAllLayers" as a function name here and will happily take suggestions, since it might be confusing that you call it even if you don't call drawLayers.

[Dana Jansens]

Comment on attachment 138822 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h:103
>      virtual bool prepareToDraw(FrameData&);
>      virtual void drawLayers(const FrameData&);
> +    // Must be called if and only if prepareToDraw was called.
> +    void didDrawAllLayers(const FrameData&);

The naming here is getting funny.. or do you think this is more correct/descriptive? We have:
1. draw
2. drawLayers
3. drawAllLayers

[James Robinson]

Comment on attachment 138822 [details]
Patch

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

R=me.

> Source/WebCore/platform/graphics/chromium/cc/CCLayerImpl.h:354
> +    bool m_betweenWillDrawAndDidDraw;

guard in #ifndef NDEBUG? i want to make it clear what bits of state are "real" and what are for asserts - i think it'd be pretty weird if we started basing "real" logic on this

[Adrienne Walker]

Committed r115278: <http://trac.webkit.org/changeset/115278>

[Adrienne Walker]

(In reply to comment #8)
> Committed r115278: <http://trac.webkit.org/changeset/115278>

^ Guarded things was #ifndef NDEBUG.

danakj: I don't know that the name difference is that confusing, even if it's inconsistent since there's only one type of thing we draw.  

Sorry for leaving the bikeshed painted an ugly color, but I wanted to land this because there's a video crash fix that depends on it.