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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adopt smart pointers in NetworkResourceLoader.cpp #47368

wants to merge 1 commit into from
+47 −33

Conversation

jelee53
Copy link
Contributor

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

c08fa58

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

Reviewed by NOBODY (OOPS!).

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:

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
✅ 🛠 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
Adopt smart pointers in NetworkResourceLoader.cpp
478ea0c 
https://bugs.webkit.org/show_bug.cgi?id=295194
rdar://154650870

Reviewed by NOBODY (OOPS!).

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:
@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
✅ 🛠 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
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.
5 participants
@jelee53 @webkit-early-warning-system @webkit-ews-buildbot @annevk @pvollan