Skip to content

[Safe CPP] Address Warnings in RenderFrame and HTMLFrameElement#46235

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
stwrt:eng/render_frame
Jun 4, 2025
Merged

[Safe CPP] Address Warnings in RenderFrame and HTMLFrameElement#46235
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
stwrt:eng/render_frame

Conversation

@stwrt
Copy link
Copy Markdown
Contributor

@stwrt stwrt commented Jun 3, 2025

4af4465

[Safe CPP] Address Warnings in RenderFrame and HTMLFrameElement
https://bugs.webkit.org/show_bug.cgi?id=293931
rdar://problem/152464519

Reviewed by Chris Dumez.

Address safe cpp warnings in RenderFrame and HTMLFrameElement.

* Source/WebCore/html/HTMLFrameElement.cpp:
(WebCore::HTMLFrameElement::attributeChanged):
* Source/WebCore/html/HTMLFrameElement.h:
* Source/WebCore/rendering/RenderFrame.cpp:
(WebCore::RenderFrame::frameElement const):
(WebCore::RenderFrame::edgeInfo const):
* Source/WebCore/rendering/RenderFrame.h:
(WebCore::HTMLFrameElement::renderer const):

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

d1e96c3

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
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@stwrt stwrt requested review from cdumez and rniwa as code owners June 3, 2025 00:28
@stwrt stwrt self-assigned this Jun 3, 2025
@stwrt stwrt added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Jun 3, 2025
@stwrt stwrt force-pushed the eng/render_frame branch from 69fe000 to 65a9cca Compare June 3, 2025 00:31
Comment thread Source/WebCore/html/HTMLFrameElement.h Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is against our style. Our getters do not return smart pointers. Sometimes, we do have a second getter which is named protectedFoo() or checkedFoo() which return a Ref or a CheckedRef. I'm not sure this is helpful here since this is nullable and the caller will likely have to store the result in a local variable (which can be a CheckedPtr) in order to null check it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed the requested style changes.

Comment thread Source/WebCore/rendering/RenderFrame.h Outdated
Comment thread Source/WebCore/rendering/RenderFrame.h Outdated
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #38446 (65a9cca)

⚠️ 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 WebCore --UncheckedLocalVarsChecker html/HTMLFrameElement.cpp --UncountedLocalVarsChecker html/HTMLFrameElement.cpp

@stwrt stwrt force-pushed the eng/render_frame branch from 65a9cca to b910232 Compare June 3, 2025 04:12
@stwrt stwrt changed the title [Safe CPP] Address Warnings in RenderFrame [Safe CPP] Address Warnings in RenderFrame and HTMLFrameElement Jun 3, 2025
@stwrt stwrt force-pushed the eng/render_frame branch from b910232 to 8eacea0 Compare June 3, 2025 04:16
@stwrt stwrt requested a review from cdumez June 3, 2025 04:16
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 3, 2025
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #38462 (b910232)

⚠️ 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 WebCore --UncheckedLocalVarsChecker html/HTMLFrameElement.cpp --UncountedLocalVarsChecker html/HTMLFrameElement.cpp

Comment thread Source/WebCore/rendering/RenderFrame.cpp Outdated
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #38464 (8eacea0)

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

@stwrt stwrt removed the merging-blocked Applied to prevent a change from being merged label Jun 3, 2025
@stwrt stwrt force-pushed the eng/render_frame branch from 8eacea0 to 45da766 Compare June 3, 2025 13:37
@stwrt stwrt requested a review from nt1m June 3, 2025 13:37
@stwrt stwrt added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 3, 2025
Comment thread Source/WebCore/html/HTMLFrameElement.cpp Outdated
Comment thread Source/WebCore/rendering/RenderFrame.cpp Outdated
@stwrt stwrt removed the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 3, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 3, 2025
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #38506 (45da766)

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

@cdumez cdumez self-requested a review June 3, 2025 15:48
@stwrt stwrt removed the merging-blocked Applied to prevent a change from being merged label Jun 3, 2025
@stwrt stwrt force-pushed the eng/render_frame branch from 45da766 to 3f791e3 Compare June 3, 2025 16:36
@stwrt stwrt requested a review from nt1m June 3, 2025 16:36
@stwrt stwrt force-pushed the eng/render_frame branch from 3f791e3 to 3da04c9 Compare June 3, 2025 16:39
@stwrt stwrt force-pushed the eng/render_frame branch from 3da04c9 to d1e96c3 Compare June 3, 2025 18:15
@stwrt stwrt added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 3, 2025
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Failed api-wpe checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #46235 from merge queue.

@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Jun 3, 2025
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safe-Merge-Queue: Build #59268.

@stwrt stwrt 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 Jun 4, 2025
https://bugs.webkit.org/show_bug.cgi?id=293931
rdar://problem/152464519

Reviewed by Chris Dumez.

Address safe cpp warnings in RenderFrame and HTMLFrameElement.

* Source/WebCore/html/HTMLFrameElement.cpp:
(WebCore::HTMLFrameElement::attributeChanged):
* Source/WebCore/html/HTMLFrameElement.h:
* Source/WebCore/rendering/RenderFrame.cpp:
(WebCore::RenderFrame::frameElement const):
(WebCore::RenderFrame::edgeInfo const):
* Source/WebCore/rendering/RenderFrame.h:
(WebCore::HTMLFrameElement::renderer const):

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

Committed 295787@main (4af4465): https://commits.webkit.org/295787@main

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

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

Labels

WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants