Skip to content

Adopt smart pointers in NetworkResourceLoader.cpp #47368

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
Merged
merged 1 commit into from
Jul 1, 2025
Merged

Adopt smart pointers in NetworkResourceLoader.cpp #47368

merged 1 commit into from
Jul 1, 2025
+47 −33

Conversation

jelee53
Copy link
Contributor

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

5c33c7f

Adopt smart pointers in NetworkResourceLoader.cpp
https://bugs.webkit.org/show_bug.cgi?id=295194
rdar://154650870

Reviewed by Per Arne Vollan and Anne van Kesteren.

Smart pointer adoption as per static analyzer.

* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::startContentFiltering):
(WebKit::NetworkResourceLoader::convertToDownload):
(WebKit::NetworkResourceLoader::doCrossOriginOpenerHandlingOfResponse):
(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::didFinishLoading):
(WebKit::NetworkResourceLoader::willSendRedirectedRequestInternal):
(WebKit::NetworkResourceLoader::bufferingTimerFired):
(WebKit::NetworkResourceLoader::sendBuffer):
(WebKit::NetworkResourceLoader::didRetrieveCacheEntry):
(WebKit::NetworkResourceLoader::sendResultForCacheEntry):
(WebKit::NetworkResourceLoader::continueAfterServiceWorkerReceivedData):
(WebKit::NetworkResourceLoader::continueAfterServiceWorkerReceivedResponse):
(WebKit::NetworkResourceLoader::serviceWorkerDidFinish):
(WebKit::NetworkResourceLoader::contentFilterDidBlock):
(WebKit::NetworkResourceLoader::checkedContentFilter):
* Source/WebKit/NetworkProcess/NetworkResourceLoader.h:
* Source/WebKit/SaferCPPExpectations/UncheckedCallArgsCheckerExpectations:

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

478ea0c

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

@jelee53 jelee53 self-assigned this Jun 30, 2025
@jelee53 jelee53 added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Jun 30, 2025
@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 ddb42b5)

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

@jelee53 jelee53 requested review from pvollan, rniwa and RupinMittal June 30, 2025 08:28
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 30, 2025
@webkit-ews-buildbot
Copy link
Collaborator

Safer C++ Build #42042 (ddb42b5)

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

annevk
annevk requested changes Jun 30, 2025
View reviewed changes
Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h Outdated

IPC::Connection& connection() { return m_connection.get(); }
Ref<IPC::Connection> protectedConnection() { return connection(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be needed. m_connection is const.

Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.h Outdated
NetworkProcess& networkProcess() { return m_networkProcess.get(); }
Ref<NetworkProcess> protectedNetworkProcess() { return networkProcess(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be needed. m_networkProcess is const.

@jelee53 jelee53 removed the merging-blocked Applied to prevent a change from being merged label Jun 30, 2025
@jelee53 jelee53 force-pushed the eng/Adopt-smart-pointers-in-NetworkResourceLoader-cpp branch from ddb42b5 to dadd611 Compare June 30, 2025 20:52
@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 dadd611)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe   🛠 win
  🛠 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 webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 30, 2025
@jelee53 jelee53 removed the merging-blocked Applied to prevent a change from being merged label Jun 30, 2025
@jelee53 jelee53 force-pushed the eng/Adopt-smart-pointers-in-NetworkResourceLoader-cpp branch from dadd611 to c08fa58 Compare June 30, 2025 21: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 c08fa58)

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

@pvollan pvollan requested a review from annevk June 30, 2025 21:23
pvollan
pvollan approved these changes Jun 30, 2025
View reviewed changes
Copy link
Contributor

@pvollan pvollan left a comment

Choose a reason for hiding this comment

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

R=me.

@jelee53 jelee53 force-pushed the eng/Adopt-smart-pointers-in-NetworkResourceLoader-cpp branch from c08fa58 to 478ea0c Compare June 30, 2025 23:17
@webkit-early-warning-system
Copy link
Collaborator

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

EWS run on current version of this PR (hash 478ea0c)

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

@jelee53 jelee53 marked this pull request as ready for review July 1, 2025 09:24
@jelee53 jelee53 requested a review from cdumez as a code owner July 1, 2025 09:24
@pvollan pvollan added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jul 1, 2025
@jelee53 @pvollan
Adopt smart pointers in NetworkResourceLoader.cpp
5c33c7f 
https://bugs.webkit.org/show_bug.cgi?id=295194
rdar://154650870

Reviewed by Per Arne Vollan and Anne van Kesteren.

Smart pointer adoption as per static analyzer.

* Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::startContentFiltering):
(WebKit::NetworkResourceLoader::convertToDownload):
(WebKit::NetworkResourceLoader::doCrossOriginOpenerHandlingOfResponse):
(WebKit::NetworkResourceLoader::didReceiveResponse):
(WebKit::NetworkResourceLoader::didFinishLoading):
(WebKit::NetworkResourceLoader::willSendRedirectedRequestInternal):
(WebKit::NetworkResourceLoader::bufferingTimerFired):
(WebKit::NetworkResourceLoader::sendBuffer):
(WebKit::NetworkResourceLoader::didRetrieveCacheEntry):
(WebKit::NetworkResourceLoader::sendResultForCacheEntry):
(WebKit::NetworkResourceLoader::continueAfterServiceWorkerReceivedData):
(WebKit::NetworkResourceLoader::continueAfterServiceWorkerReceivedResponse):
(WebKit::NetworkResourceLoader::serviceWorkerDidFinish):
(WebKit::NetworkResourceLoader::contentFilterDidBlock):
(WebKit::NetworkResourceLoader::checkedContentFilter):
* Source/WebKit/NetworkProcess/NetworkResourceLoader.h:
* Source/WebKit/SaferCPPExpectations/UncheckedCallArgsCheckerExpectations:

Canonical link: https://commits.webkit.org/296860@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Adopt-smart-pointers-in-NetworkResourceLoader-cpp branch from 478ea0c to 5c33c7f Compare July 1, 2025 11:57
@webkit-commit-queue
Copy link
Collaborator

Committed 296860@main (5c33c7f): https://commits.webkit.org/296860@main

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

@webkit-commit-queue webkit-commit-queue merged commit 5c33c7f into WebKit:main Jul 1, 2025
@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 Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annevk annevk annevk approved these changes

@pvollan pvollan pvollan approved these changes

@rniwa rniwa Awaiting requested review from rniwa

@RupinMittal RupinMittal Awaiting requested review from RupinMittal

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

Assignees

@jelee53 jelee53

Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jelee53 @webkit-early-warning-system @webkit-ews-buildbot @webkit-commit-queue @annevk @pvollan