Bug 141586 – Give WebKit-owned IOSurfaces names

• Component: New Bugs
• Status: RESOLVED FIXED

[Tim Horton]

Give WebKit-owned IOSurfaces names

[Tim Horton]

Created attachment 246566 [details]
Patch

[alan]

Comment on attachment 246566 [details]
Patch

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

> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:81
> +    , m_inLayerPool(false)

Do you mind replacing it with bool m_inLayerPool { false };?

> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:91
>      , m_superlayer(nullptr)
>      , m_maskLayer(nullptr)

These layers could use modern class member initialization too.

[Ben Nham]

Created attachment 453683 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 453683 [details]
Patch

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

> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:222
> +    char addr[16] { };
> +    snprintf(addr, sizeof(addr), " (%p)", reinterpret_cast<void*>(this));

This should use hex(reinterpret_cast<uintptr_t>(this), Lowercase))

> Source/WebKit/Shared/RemoteLayerTree/RemoteLayerBackingStore.mm:239
> +    if (m_frontBuffer.imageBuffer)
> +        m_frontBuffer.imageBuffer->setName(makeString("Front Buffer", addr));
> +    if (m_backBuffer.imageBuffer)
> +        m_backBuffer.imageBuffer->setName(makeString("Back Buffer", addr));
> +    if (m_secondaryBackBuffer.imageBuffer)
> +        m_secondaryBackBuffer.imageBuffer->setName(makeString("Secondary Back Buffer", addr));

PlatformCALayer has a 'name' that allows you to usually identify the HTML element the layer is associated with. Do we want that here too?

[Simon Fraser (smfr)]

Would be useful to explicitly name canvas backing store, and WebGL/WebRTC/video-related surfaces

[Ben Nham]

Created attachment 453766 [details]
Patch

[Ben Nham]

Comment on attachment 453766 [details]
Patch

I replaced logging the pointer to the backing store with logging the layer name since that seemed more useful. I didn't implement naming canvas/webrtc/etc. surfaces in this patch because that seems like it would require either propagating the rendering purpose for the surface from WebChromeClient => RemoteRenderingBackendProxy => GPUProcess or it would require implementing setName on RemoteImageBufferProxy. Not sure which way of doing it would be preferred. Maybe we should just do that in another patch.

[Radar WebKit Bug Importer]

<rdar://problem/89791501>

[Ryan Reno]

Pull request: https://github.com/WebKit/WebKit/pull/12138

[EWS]

Committed 262331@main (e5edaea039e2): <https://commits.webkit.org/262331@main>

Reviewed commits have been landed. Closing PR #12138 and removing active labels.

[Ryan Reno]

Reverted by https://github.com/WebKit/WebKit/pull/12208

[Ryan Reno]

Re-opening for pull request https://github.com/WebKit/WebKit/pull/12208

[EWS]

Committed 262370@main (3abfcfc86ce8): <https://commits.webkit.org/262370@main>

Reviewed commits have been landed. Closing PR #12208 and removing active labels.

[Simon Fraser (smfr)]

This was reverted.

[Ryan Reno]

Re landed in https://commits.webkit.org/262429@main