Bug 141586

Summary: Give WebKit-owned IOSurfaces names
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Ryan Reno <rreno>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, nham, peng.liu6, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 254832    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Tim Horton 2015-02-13 18:49:57 PST
Give WebKit-owned IOSurfaces names
Comment 1 Tim Horton 2015-02-13 18:50:59 PST
Created attachment 246566 [details]
Patch
Comment 2 zalan 2015-02-13 20:15:25 PST
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.
Comment 3 Ben Nham 2022-03-02 17:35:41 PST
Created attachment 453683 [details]
Patch
Comment 4 Simon Fraser (smfr) 2022-03-02 17:45:51 PST
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?
Comment 5 Simon Fraser (smfr) 2022-03-02 17:46:28 PST
Would be useful to explicitly name canvas backing store, and WebGL/WebRTC/video-related surfaces
Comment 6 Ben Nham 2022-03-03 11:20:08 PST
Created attachment 453766 [details]
Patch
Comment 7 Ben Nham 2022-03-03 11:23:20 PST
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.
Comment 8 Radar WebKit Bug Importer 2022-03-03 19:17:00 PST
<rdar://problem/89791501>
Comment 9 Ryan Reno 2023-03-29 14:57:37 PDT
Pull request: https://github.com/WebKit/WebKit/pull/12138
Comment 10 EWS 2023-03-30 09:03:39 PDT
Committed 262331@main (e5edaea039e2): <https://commits.webkit.org/262331@main>

Reviewed commits have been landed. Closing PR #12138 and removing active labels.
Comment 11 Ryan Reno 2023-03-30 15:58:30 PDT
Reverted by https://github.com/WebKit/WebKit/pull/12208
Comment 12 Ryan Reno 2023-03-30 15:58:31 PDT
Re-opening for pull request https://github.com/WebKit/WebKit/pull/12208
Comment 13 EWS 2023-03-30 16:02:33 PDT
Committed 262370@main (3abfcfc86ce8): <https://commits.webkit.org/262370@main>

Reviewed commits have been landed. Closing PR #12208 and removing active labels.
Comment 14 Simon Fraser (smfr) 2023-03-30 16:08:29 PDT
This was reverted.
Comment 15 Ryan Reno 2023-04-05 16:40:52 PDT
Re landed in https://commits.webkit.org/262429@main