[webkit-reviews] review granted: [Bug 128553] Show a debug tile map indicator for UI-side compositing : [Attachment 223749] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 128553: Show a debug tile map indicator for UI-side compositing
https://bugs.webkit.org/show_bug.cgi?id=128553

Attachment 223749: Patch
https://bugs.webkit.org/attachment.cgi?id=223749&action=review

------- Additional Comments from Tim Horton <thorton at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223749&action=review


> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:188
> +	   // FIXME: get the correct scroll position;

capitals and periods

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:210
> +    m_tileMapHostLayer.get().name = @"Tile map host";

[...] instead of .get() in lots of places

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:221
> +	   const CGFloat components[] = { 1, 1, 1, 0.6 };
> +	   RetainPtr<CGColorRef> color =
adoptCF(CGColorCreate(colorSpace.get(), components));
> +	   m_tileMapHostLayer.get().backgroundColor = color.get();

I wonder if using the color cache would be neater.

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.h:51
> +    void setIsDebugLayerTreeHost(bool flag = true) { m_isDebugLayerTreeHost
= flag; }

default argument value? weird!

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.mm:133
> +	       layer = adoptNS([[CALayer alloc] init]);

maybe we should give this a background color?