Skip to content

Fix unexpected smart pointer static analysis warnings in Source/WebKit/GPUProcess#33311

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
rniwa:fix279346
Sep 9, 2024
Merged

Fix unexpected smart pointer static analysis warnings in Source/WebKit/GPUProcess#33311
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
rniwa:fix279346

Conversation

@rniwa
Copy link
Copy Markdown
Member

@rniwa rniwa commented Sep 9, 2024 •
edited by webkit-early-warning-system
Loading

cd38a1d

Fix unexpected smart pointer static analysis warnings in Source/WebKit/GPUProcess
https://bugs.webkit.org/show_bug.cgi?id=279346

Reviewed by Chris Dumez and Mike Wyrzykowski.

Deploy more smart pointers in Source/WebKit/GPUProcess to fix the static analysis warnings.

* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.cpp:
(WebKit::RemoteXRBinding::RemoteXRBinding):
(WebKit::RemoteXRBinding::protectedStreamConnection):
(WebKit::RemoteXRBinding::protectedBacking):
(WebKit::RemoteXRBinding::protectedGPU):
(WebKit::RemoteXRBinding::createProjectionLayer):
(WebKit::RemoteXRBinding::getViewSubImage):
(WebKit::RemoteXRBinding::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRProjectionLayer.cpp:
(WebKit::RemoteXRProjectionLayer::RemoteXRProjectionLayer):
(WebKit::RemoteXRProjectionLayer::protectedStreamConnection):
(WebKit::RemoteXRProjectionLayer::protectedGPU const):
(WebKit::RemoteXRProjectionLayer::connection const):
(WebKit::RemoteXRProjectionLayer::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRProjectionLayer.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRSubImage.cpp:
(WebKit::RemoteXRSubImage::RemoteXRSubImage):
(WebKit::RemoteXRSubImage::protectedStreamConnection):
(WebKit::RemoteXRSubImage::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRSubImage.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRView.cpp:
(WebKit::RemoteXRView::RemoteXRView):
(WebKit::RemoteXRView::protectedStreamConnection):
(WebKit::RemoteXRView::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRView.h:
* Source/WebKit/GPUProcess/media/RemoteLegacyCDMFactoryProxy.cpp:
(WebKit::RemoteLegacyCDMFactoryProxy::removeSession):
(WebKit::RemoteLegacyCDMFactoryProxy::logger const):

Canonical link: https://commits.webkit.org/283359@main

923e2f3

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🛠 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
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@rniwa rniwa self-assigned this Sep 9, 2024
@rniwa rniwa added the WebKit2 Bugs relating to the WebKit2 API layer label Sep 9, 2024
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Sep 9, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 0a2fcc4)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
❌ 🛠 ios-sim ❌ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
❌ 🛠 tv
❌ 🛠 tv-sim
❌ 🛠 watch
❌ 🛠 watch-sim

@rniwa rniwa requested review from basuke and cdumez September 9, 2024 08:07
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 9, 2024
@cdumez
Copy link
Copy Markdown
Contributor

cdumez commented Sep 9, 2024

EWS is unhappy.

@rniwa rniwa removed the merging-blocked Applied to prevent a change from being merged label Sep 9, 2024
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Sep 9, 2024 •
edited
Loading

EWS run on current version of this PR (hash 923e2f3)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🛠 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
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 9, 2024
@rniwa rniwa removed the merging-blocked Applied to prevent a change from being merged label Sep 9, 2024
cdumez
cdumez approved these changes Sep 9, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

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

LGTM with nits.

Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.cpp Outdated
Copy link
Copy Markdown
Contributor

@cdumez cdumez Sep 9, 2024

Choose a reason for hiding this comment

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

auto -> RefPtr ?

Copy link
Copy Markdown
Contributor

@mwyrzykowski mwyrzykowski Sep 9, 2024

Choose a reason for hiding this comment

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

Just wondering why is the call to protectedBacking() needed?

m_backing is already a Ref:

Ref<WebCore::WebGPU::XRBinding> m_backing;

Copy link
Copy Markdown
Member Author

@rniwa rniwa Sep 9, 2024

Choose a reason for hiding this comment

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

Because m_backing can be cleared.

Copy link
Copy Markdown
Member Author

@rniwa rniwa Sep 9, 2024

Choose a reason for hiding this comment

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

Oh oops, missed these review comments. Will fix.

Copy link
Copy Markdown
Member Author

@rniwa rniwa Sep 9, 2024

Choose a reason for hiding this comment

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

Fixed.

Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.cpp Outdated
Copy link
Copy Markdown
Contributor

@cdumez cdumez Sep 9, 2024

Choose a reason for hiding this comment

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

auto -> Ref?

Copy link
Copy Markdown
Member Author

@rniwa rniwa Sep 9, 2024

Choose a reason for hiding this comment

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

Fixed.

Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.cpp Outdated
Copy link
Copy Markdown
Contributor

@cdumez cdumez Sep 9, 2024

Choose a reason for hiding this comment

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

auto -> RefPtr

Copy link
Copy Markdown
Member Author

@rniwa rniwa Sep 9, 2024

Choose a reason for hiding this comment

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

Fixed.

Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.cpp Outdated
Copy link
Copy Markdown
Contributor

@cdumez cdumez Sep 9, 2024

Choose a reason for hiding this comment

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

auto -> Ref

Copy link
Copy Markdown
Member Author

@rniwa rniwa Sep 9, 2024

Choose a reason for hiding this comment

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

Fixed.

Source/WebKit/GPUProcess/media/RemoteLegacyCDMFactoryProxy.cpp Outdated
Copy link
Copy Markdown
Contributor

@cdumez cdumez Sep 9, 2024

Choose a reason for hiding this comment

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

connection ? connection->sessionID().isAlwaysOnLoggingAllowed() : false could be connection && connection->sessionID().isAlwaysOnLoggingAllowed()

Copy link
Copy Markdown
Member Author

@rniwa rniwa Sep 9, 2024

Choose a reason for hiding this comment

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

Fixed.

@rniwa rniwa added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 9, 2024
@rniwa
Fix unexpected smart pointer static analysis warnings in Source/WebKi…
cd38a1d 
…t/GPUProcess

https://bugs.webkit.org/show_bug.cgi?id=279346

Reviewed by Chris Dumez and Mike Wyrzykowski.

Deploy more smart pointers in Source/WebKit/GPUProcess to fix the static analysis warnings.

* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.cpp:
(WebKit::RemoteXRBinding::RemoteXRBinding):
(WebKit::RemoteXRBinding::protectedStreamConnection):
(WebKit::RemoteXRBinding::protectedBacking):
(WebKit::RemoteXRBinding::protectedGPU):
(WebKit::RemoteXRBinding::createProjectionLayer):
(WebKit::RemoteXRBinding::getViewSubImage):
(WebKit::RemoteXRBinding::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRBinding.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRProjectionLayer.cpp:
(WebKit::RemoteXRProjectionLayer::RemoteXRProjectionLayer):
(WebKit::RemoteXRProjectionLayer::protectedStreamConnection):
(WebKit::RemoteXRProjectionLayer::protectedGPU const):
(WebKit::RemoteXRProjectionLayer::connection const):
(WebKit::RemoteXRProjectionLayer::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRProjectionLayer.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRSubImage.cpp:
(WebKit::RemoteXRSubImage::RemoteXRSubImage):
(WebKit::RemoteXRSubImage::protectedStreamConnection):
(WebKit::RemoteXRSubImage::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRSubImage.h:
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRView.cpp:
(WebKit::RemoteXRView::RemoteXRView):
(WebKit::RemoteXRView::protectedStreamConnection):
(WebKit::RemoteXRView::stopListeningForIPC):
* Source/WebKit/GPUProcess/graphics/WebGPU/RemoteXRView.h:
* Source/WebKit/GPUProcess/media/RemoteLegacyCDMFactoryProxy.cpp:
(WebKit::RemoteLegacyCDMFactoryProxy::removeSession):
(WebKit::RemoteLegacyCDMFactoryProxy::logger const):

Canonical link: https://commits.webkit.org/283359@main
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

webkit-commit-queue commented Sep 9, 2024

Committed 283359@main (cd38a1d): https://commits.webkit.org/283359@main

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

@webkit-commit-queue webkit-commit-queue merged commit cd38a1d into WebKit:main Sep 9, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 9, 2024
@rniwa rniwa deleted the fix279346 branch September 9, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdumez cdumez cdumez approved these changes

@mwyrzykowski mwyrzykowski mwyrzykowski approved these changes

@tadeuzagallo tadeuzagallo Awaiting requested review from tadeuzagallo tadeuzagallo is a code owner

@basuke basuke Awaiting requested review from basuke

@kkinnunen-apple kkinnunen-apple Awaiting requested review from kkinnunen-apple

Assignees

@rniwa rniwa

Labels

WebKit2 Bugs relating to the WebKit2 API layer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rniwa @webkit-early-warning-system @cdumez @webkit-commit-queue @mwyrzykowski @webkit-ews-buildbot