| Summary: | Give WebKit-owned IOSurfaces names | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||||
| Component: | New Bugs | Assignee: | 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
Tim Horton
2015-02-13 18:49:57 PST
Created attachment 246566 [details]
Patch
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. Created attachment 453683 [details]
Patch
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? Would be useful to explicitly name canvas backing store, and WebGL/WebRTC/video-related surfaces Created attachment 453766 [details]
Patch
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.
Pull request: https://github.com/WebKit/WebKit/pull/12138 Committed 262331@main (e5edaea039e2): <https://commits.webkit.org/262331@main> Reviewed commits have been landed. Closing PR #12138 and removing active labels. Reverted by https://github.com/WebKit/WebKit/pull/12208 Re-opening for pull request https://github.com/WebKit/WebKit/pull/12208 Committed 262370@main (3abfcfc86ce8): <https://commits.webkit.org/262370@main> Reviewed commits have been landed. Closing PR #12208 and removing active labels. This was reverted. Re landed in https://commits.webkit.org/262429@main |