Skip to content

Address safer cpp warnings in FormDataStreamCFNet.cpp#51228

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
cdumez:299409_FormDataStreamCFNet_safer_cpp3
Sep 24, 2025
Merged

Address safer cpp warnings in FormDataStreamCFNet.cpp#51228
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
cdumez:299409_FormDataStreamCFNet_safer_cpp3

Conversation

@cdumez
Copy link
Copy Markdown
Contributor

@cdumez cdumez commented Sep 24, 2025 •
edited by webkit-early-warning-system
Loading

01455ae

Address safer cpp warnings in FormDataStreamCFNet.cpp
https://bugs.webkit.org/show_bug.cgi?id=299409

Reviewed by Ryosuke Niwa.

* Source/WTF/wtf/cocoa/TollFreeBridging.h:
* Source/WebCore/PlatformMac.cmake:
* Source/WebCore/SaferCPPExpectations/NoUnretainedMemberCheckerExpectations:
* Source/WebCore/SourcesCocoa.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/platform/network/cf/FormDataStreamCFNet.mm: Renamed from Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp.
(WebCore::FormStreamFields::cfFormStream):
(WebCore::formCreate):
(WebCore::formFinalize):
(WebCore::formEventCallback):
* Source/WebCore/platform/network/ios/WebCoreURLResponseIOS.mm:

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

cdb4b12

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

@cdumez cdumez self-assigned this Sep 24, 2025
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Sep 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 4a3a671)

Details

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

@cdumez cdumez added the Platform Portability improvements and other general platform improvements not driven directly by site bugs. label Sep 24, 2025
@cdumez cdumez force-pushed the 299409_FormDataStreamCFNet_safer_cpp3 branch from 4a3a671 to 4a8f240 Compare September 24, 2025 01:47
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Sep 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 4a8f240)

Details

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 24, 2025
@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Sep 24, 2025
@cdumez cdumez force-pushed the 299409_FormDataStreamCFNet_safer_cpp3 branch from 4a8f240 to 77b4ebf Compare September 24, 2025 02:30
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Sep 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 77b4ebf)

Details

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

@cdumez cdumez force-pushed the 299409_FormDataStreamCFNet_safer_cpp3 branch from 77b4ebf to 744cdd5 Compare September 24, 2025 04:35
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Sep 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 744cdd5)

Details

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 24, 2025
cdumez
cdumez commented Sep 24, 2025
View reviewed changes
Comment thread Source/WebCore/platform/network/cf/FormDataStreamCFNet.mm Outdated
@@ -224,7 +231,7 @@ static void* formCreate(CFReadStreamRef stream, void* context)
static void formFinalize(CFReadStreamRef stream, void* context)
{
FormStreamFields* form = static_cast<FormStreamFields*>(context);
ASSERT_UNUSED(stream, form->formStream == stream);
ASSERT_UNUSED(stream, form->cfFormStream() == stream);
Copy link
Copy Markdown
Contributor Author

@cdumez cdumez Sep 24, 2025

Choose a reason for hiding this comment

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

Hmm, it is failing this assertion. I'll have to investigate while the pointers no longer match after casting to and from the NS type 🤷🏻

Copy link
Copy Markdown
Contributor Author

@cdumez cdumez Sep 24, 2025

Choose a reason for hiding this comment

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

Actually, my bet is that the pointer has become null now that we use a weak pointer and given that this is a finalizer function. I'll confirm.

@cdumez cdumez removed the merging-blocked Applied to prevent a change from being merged label Sep 24, 2025
@cdumez cdumez force-pushed the 299409_FormDataStreamCFNet_safer_cpp3 branch from 744cdd5 to cdb4b12 Compare September 24, 2025 09:21
@webkit-early-warning-system
Copy link
Copy Markdown
Collaborator

webkit-early-warning-system commented Sep 24, 2025 •
edited
Loading

EWS run on current version of this PR (hash cdb4b12)

Details

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 24, 2025
@cdumez cdumez added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Sep 24, 2025
@webkit-commit-queue webkit-commit-queue force-pushed the 299409_FormDataStreamCFNet_safer_cpp3 branch from cdb4b12 to 24af33e Compare September 24, 2025 11:53
@cdumez
Address safer cpp warnings in FormDataStreamCFNet.cpp
01455ae 
https://bugs.webkit.org/show_bug.cgi?id=299409

Reviewed by Ryosuke Niwa.

* Source/WTF/wtf/cocoa/TollFreeBridging.h:
* Source/WebCore/PlatformMac.cmake:
* Source/WebCore/SaferCPPExpectations/NoUnretainedMemberCheckerExpectations:
* Source/WebCore/SourcesCocoa.txt:
* Source/WebCore/WebCore.xcodeproj/project.pbxproj:
* Source/WebCore/platform/network/cf/FormDataStreamCFNet.mm: Renamed from Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp.
(WebCore::FormStreamFields::cfFormStream):
(WebCore::formCreate):
(WebCore::formFinalize):
(WebCore::formEventCallback):
* Source/WebCore/platform/network/ios/WebCoreURLResponseIOS.mm:

Canonical link: https://commits.webkit.org/300459@main
@webkit-commit-queue webkit-commit-queue force-pushed the 299409_FormDataStreamCFNet_safer_cpp3 branch from 24af33e to 01455ae Compare September 24, 2025 11:55
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

webkit-commit-queue commented Sep 24, 2025

Committed 300459@main (01455ae): https://commits.webkit.org/300459@main

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

@webkit-commit-queue webkit-commit-queue merged commit 01455ae into WebKit:main Sep 24, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rniwa rniwa rniwa approved these changes

Assignees

@cdumez cdumez

Labels

Platform Portability improvements and other general platform improvements not driven directly by site bugs.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

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