[webkit-reviews] review granted: [Bug 36782] Add Layer Tree output capability to DRT : [Attachment 52212] Replacement patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Chris Marrin
<cmarrin at apple.com>'s request for review:
Bug 36782: Add Layer Tree output capability to DRT
https://bugs.webkit.org/show_bug.cgi?id=36782

Attachment 52212: Replacement patch
https://bugs.webkit.org/attachment.cgi?id=52212&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
> Index: WebCore/page/Frame.cpp
> ===================================================================

> +#if USE(ACCELERATED_COMPOSITING)
> +String Frame::layerTreeAsText() const

This should be inside the method, so that the method is still present in
non-accel-comp builds (which avoids breaking the exports).

The method can return an empty string if ACCELERATED_COMPOSITING is off.

> Index: WebCore/page/Frame.h
> ===================================================================

> +#if USE(ACCELERATED_COMPOSITING)
> +	   String layerTreeAsText() const;
> +#endif

This would not be wrapped here.

> Index: WebCore/platform/graphics/GraphicsLayer.cpp
> ===================================================================

> +#ifndef NDEBUG
> +void showGraphicsLayerTree(const WebCore::GraphicsLayer* layer)
> +{
> +    if (!layer)
> +	   return;
> +
> +    WebCore::String output = layer->layerTreeAsText(LayerTreeAsTextDebug);

Doesn't LayerTreeAsTextDebug needs to be WebCore::LayerTreeAsTextDebug ?

> Index: WebKit/win/WebFrame.cpp
> ===================================================================

> +#if USE(ACCELERATED_COMPOSITING)
> +    String text = frame->layerTreeAsText();
> +#else
> +    String text;
> +#endif

You wouldn't need the #ifdef here.

r=me