Skip to content

Adopt more smart pointers in GPUProcess #45640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adopt more smart pointers in GPUProcess #45640

wants to merge 1 commit into from
+10 −5

Conversation

rwlbuis
Copy link
Contributor

@rwlbuis rwlbuis commented May 20, 2025 •
edited by webkit-early-warning-system
Loading

175b212

Adopt more smart pointers in GPUProcess
https://bugs.webkit.org/show_bug.cgi?id=293288

Reviewed by NOBODY (OOPS!).

Adopt more smart pointers in GPUProcess.

* Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.h:
* Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm:
(WebCore::LocalSampleBufferDisplayLayer::protectedRootLayer):
* Source/WebKit/GPUProcess/mac/GPUProcessMac.mm:
(WebKit::GPUProcess::updateProcessName):
(WebKit::GPUProcess::openDirectoryCacheInvalidated):
* Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayer.mm:
(WebKit::RemoteSampleBufferDisplayLayer::initialize):
* Source/WebKit/SaferCPPExpectations/ForwardDeclCheckerExpectations:
* Source/WebKit/SaferCPPExpectations/UnretainedCallArgsCheckerExpectations:

175b212

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ❌ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ❌ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@rwlbuis rwlbuis self-assigned this May 20, 2025
@rwlbuis rwlbuis added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label May 20, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 20, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 7dd1285)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
⏳ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ❌ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-GPUProcess branch from 7dd1285 to df20b17 Compare May 20, 2025 13:49
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 20, 2025 •
edited
Loading

EWS run on previous version of this PR (hash df20b17)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ✅ 🛠 mac ❌ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ❌ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2   🧪 api-mac ❌ 🧪 api-wpe
✅ 🧪 webkitpy ❌ 🧪 ios-wk2-wpt   🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
❌ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
⏳ 🧪 vision-wk2   🧪 mac-intel-wk2 ✅ 🛠 playstation
❌ 🛠 tv ❌ 🛠 mac-safer-cpp
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 20, 2025
@rwlbuis rwlbuis removed the merging-blocked Applied to prevent a change from being merged label May 20, 2025
@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-GPUProcess branch from df20b17 to 7f502b6 Compare May 20, 2025 14:59
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 20, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 7f502b6)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ❌ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ❌ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
❌ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
loading 🧪 services ❌ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
⏳ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
❌ 🛠 tv   🛠 mac-safer-cpp
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 20, 2025
@rwlbuis rwlbuis removed the merging-blocked Applied to prevent a change from being merged label May 20, 2025
@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-GPUProcess branch from 7f502b6 to bbd563b Compare May 20, 2025 15:39
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 20, 2025 •
edited
Loading

EWS run on previous version of this PR (hash bbd563b)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #36672 (bbd563b)

⚠️ Found 2 fixed files! Please update expectations in Source/[Project]/SaferCPPExpectations by running the following command and update your pull request:

  • Tools/Scripts/update-safer-cpp-expectations -p WebKit --UnretainedCallArgsChecker GPUProcess/mac/GPUProcessMac.mm GPUProcess/webrtc/RemoteSampleBufferDisplayLayer.mm

@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-GPUProcess branch from bbd563b to bc3c43c Compare May 21, 2025 08:53
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash bc3c43c)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ⏳ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
  🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #36812 (bc3c43c)

⚠️ Found 1 fixed file! Please update expectations in Source/[Project]/SaferCPPExpectations by running the following command and update your pull request:

  • Tools/Scripts/update-safer-cpp-expectations -p WebKit --ForwardDeclChecker GPUProcessMessages.h

@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-GPUProcess branch from bc3c43c to f35dea5 Compare May 21, 2025 13:33
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented May 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash f35dea5)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@rwlbuis rwlbuis requested a review from cdumez May 21, 2025 15:03
@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-GPUProcess branch from f35dea5 to 3dbed93 Compare June 30, 2025 11:13
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 3dbed93)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
❌ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

cdumez
cdumez reviewed Jun 30, 2025
View reviewed changes
Source/WebKit/GPUProcess/mac/GPUProcessMac.mm Outdated
@@ -67,7 +67,9 @@
void GPUProcess::updateProcessName()
{
#if !PLATFORM(MACCATALYST)
RetainPtr applicationName = adoptNS([[NSString alloc] initWithFormat:WEB_UI_NSSTRING(@"%@ Graphics and Media", "visible name of the GPU process. The argument is the application name."), m_uiProcessName.createNSString().get()]);
ALLOW_NONLITERAL_FORMAT_BEGIN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done to fix the [alpha.webkit.UnretainedCallArgsChecker] warning on this line, I am not too sure though about the NONLITERAL stuff. What do you think we should do for WEB_UI_NSSTRING, which is used a lot, and defers to WebCore::localizedNSString, which seems not a trivial function?

cdumez
cdumez reviewed Jun 30, 2025
View reviewed changes
Source/WebKit/Scripts/webkit/messages.py Outdated
@@ -504,6 +504,7 @@ def types_that_cannot_be_forward_declared():
'IPC::AsyncReplyID',
'IPC::FontReference',
'IPC::Semaphore',
'IPC::SharedBufferReference',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand the changes to this file either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added to fix the [lpha.webkit.ForwardDeclChecker] warnings in GPUProcessMessages.h, see also #45640 (comment). Do you think those warnings are too strict in this case? I'd like together those warnings out of the way, otherwise grepping for GPUProcess in SaferCPPExpectations directory makes it seems there is still work to do for GPUProcess.

@rwlbuis
Adopt more smart pointers in GPUProcess
175b212 
https://bugs.webkit.org/show_bug.cgi?id=293288

Reviewed by NOBODY (OOPS!).

Adopt more smart pointers in GPUProcess.

* Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.h:
* Source/WebCore/platform/graphics/avfoundation/objc/LocalSampleBufferDisplayLayer.mm:
(WebCore::LocalSampleBufferDisplayLayer::protectedRootLayer):
* Source/WebKit/GPUProcess/mac/GPUProcessMac.mm:
(WebKit::GPUProcess::updateProcessName):
(WebKit::GPUProcess::openDirectoryCacheInvalidated):
* Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayer.mm:
(WebKit::RemoteSampleBufferDisplayLayer::initialize):
* Source/WebKit/SaferCPPExpectations/ForwardDeclCheckerExpectations:
* Source/WebKit/SaferCPPExpectations/UnretainedCallArgsCheckerExpectations:
@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-GPUProcess branch from 3dbed93 to 175b212 Compare July 1, 2025 13:12
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 1, 2025 •
edited
Loading

EWS run on current version of this PR (hash 175b212)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ❌ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ❌ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #42224 (175b212)

❌ Found 2 failing files with 5 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez left review comments

Assignees

@rwlbuis rwlbuis

Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rwlbuis @webkit-early-warning-system @webkit-ews-buildbot @cdumez