Bug 34779 – Need CA_PRINT_TREE functionality on Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chris Marrin]

CACF does not provide a useable form of this debugging tool. So we need to implement it ourselves to track down some of the layer structure bugs.

[Chris Marrin]

Created attachment 48453 [details]
Patch

[Darin Adler]

Comment on attachment 48453 [details]
Patch

> +    CFArrayRef sublayers = CACFLayerGetSublayers(layer());
> +    if (CFArrayGetCount(sublayers) <= index)
> +        return 0;

If we're checking the range of index and returning 0 rather than calling CFArrayGetValueAtIndex, then suggest we check for negative numbers too since the index is signed.

> +    return layer(reinterpret_cast<CACFLayerRef>(const_cast<void*>(CFArrayGetValueAtIndex(sublayers, index))));

This should be static_cast instead of reinterpret_cast since it's a cast from void*.

> -    bool isTransformLayer() const { return CACFLayerGetClass(layer()) == kCACFTransformLayer; }
> +    bool isTransformLayer() const;

Why this change? Is there a problem with it being inlined?

r=me

[Chris Marrin]

(In reply to comment #2)
> (From update of attachment 48453 [details])
> > +    CFArrayRef sublayers = CACFLayerGetSublayers(layer());
> > +    if (CFArrayGetCount(sublayers) <= index)
> > +        return 0;
> 
> If we're checking the range of index and returning 0 rather than calling
> CFArrayGetValueAtIndex, then suggest we check for negative numbers too since
> the index is signed.

I'll fix it.

> 
> > +    return layer(reinterpret_cast<CACFLayerRef>(const_cast<void*>(CFArrayGetValueAtIndex(sublayers, index))));
> 
> This should be static_cast instead of reinterpret_cast since it's a cast from
> void*.

Will do.

> 
> > -    bool isTransformLayer() const { return CACFLayerGetClass(layer()) == kCACFTransformLayer; }
> > +    bool isTransformLayer() const;
> 
> Why this change? Is there a problem with it being inlined?

I had to change it to use the function call version of the enum since the DLL is delay loaded. Using that function can only be done in the cpp file. I will make a note of that in the changelog

> 
> r=me

[Chris Marrin]

Created attachment 48522 [details]
Replacement patch

I realized that it's a bad idea to be calling getenv() on each render to get CA_PRINT_TREE. So I now cache it.

[Simon Fraser (smfr)]

Comment on attachment 48522 [details]
Replacement patch

>  void WKCACFLayerRenderer::renderSoon()
> Index: WebCore/platform/graphics/win/WKCACFLayerRenderer.h
> ===================================================================
> --- WebCore/platform/graphics/win/WKCACFLayerRenderer.h	(revision 54620)
> +++ WebCore/platform/graphics/win/WKCACFLayerRenderer.h	(working copy)
> @@ -97,6 +97,10 @@ private:
>      HWND m_hostWindow;
>      Timer<WKCACFLayerRenderer> m_renderTimer;
>      IntRect m_scrollFrame;
> +
> +#ifndef NDEBUG
> +    bool m_cachedPrintTreeFlag;
> +#endif

I don't think the name needs to include 'cached' or 'flag'; both are obvious.
m_printTree is fine.

This stuff is OK, but I'd still like to see a platform-independent layer dump
we can use in DRT.

[Chris Marrin]

Landed in http://trac.webkit.org/changeset/54630