Bug 141586 - Give WebKit-owned IOSurfaces names
Summary: Give WebKit-owned IOSurfaces names
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryan Reno
URL:
Keywords: InRadar
Depends on: 254832
Blocks:
  Show dependency treegraph
 
Reported: 2015-02-13 18:49 PST by Tim Horton
Modified: 2023-04-05 16:40 PDT (History)
5 users (show)

See Also:


Attachments
Patch (12.36 KB, patch)
2015-02-13 18:50 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (16.00 KB, patch)
2022-03-02 17:35 PST, Ben Nham
no flags Details | Formatted Diff | Diff
Patch (16.33 KB, patch)
2022-03-03 11:20 PST, Ben Nham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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