Skip to content
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

Fix use-after-move on navigationActionData #8822

Merged
merged 1 commit into from Jan 21, 2023
Merged

Fix use-after-move on navigationActionData #8822

merged 1 commit into from Jan 21, 2023

Conversation

charliewolfe
Copy link
Member

@charliewolfe charliewolfe commented Jan 19, 2023 •
edited by webkit-early-warning-system

2f177bc

Fix use-after-move on navigationActionData
https://bugs.webkit.org/show_bug.cgi?id=250826
rdar://104420066

Reviewed by Youenn Fablet and David Kilzer.

This patch fixes a use-after-move on `navigationActionData`. This is
benign, but we should still change this to fix the clang-tidy warning.

* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction):

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

db28602

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@charliewolfe charliewolfe self-assigned this Jan 19, 2023
@charliewolfe charliewolfe added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Jan 19, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

youennf
youennf approved these changes Jan 19, 2023
View reviewed changes
Source/WebKit/UIProcess/WebPageProxy.cpp Outdated Show resolved Hide resolved
@charliewolfe charliewolfe force-pushed the eng/Fix-use-after-move-on-navigationActionData branch from 4fe3368 to e2fe6f4 Compare January 19, 2023 21:25
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash e2fe6f4)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ❌ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 watch-sim

ddkilzer
ddkilzer requested changes Jan 20, 2023
View reviewed changes
Copy link
Contributor

@ddkilzer ddkilzer left a comment

Choose a reason for hiding this comment

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

I think we should switch to using navigationAction.data() to access navigationActionData values after creating the navigationAction object.

Source/WebKit/UIProcess/WebPageProxy.cpp Outdated Show resolved Hide resolved
@charliewolfe charliewolfe force-pushed the eng/Fix-use-after-move-on-navigationActionData branch from e2fe6f4 to db28602 Compare January 20, 2023 22:43
@webkit-early-warning-system
Copy link
Collaborator

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

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 21, 2023
ddkilzer
ddkilzer approved these changes Jan 21, 2023
View reviewed changes
Copy link
Contributor

@ddkilzer ddkilzer 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

@ddkilzer ddkilzer added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged merge-queue Applied to send a pull request to merge-queue labels Jan 21, 2023
@charliewolfe @ddkilzer
Fix use-after-move on navigationActionData
2f177bc 
https://bugs.webkit.org/show_bug.cgi?id=250826
rdar://104420066

Reviewed by Youenn Fablet and David Kilzer.

This patch fixes a use-after-move on `navigationActionData`. This is
benign, but we should still change this to fix the clang-tidy warning.

* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::decidePolicyForNavigationAction):

Canonical link: https://commits.webkit.org/259164@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Fix-use-after-move-on-navigationActionData branch from db28602 to 2f177bc Compare January 21, 2023 05:31
@webkit-commit-queue
Copy link
Collaborator

Committed 259164@main (2f177bc): https://commits.webkit.org/259164@main

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

@webkit-early-warning-system webkit-early-warning-system merged commit 2f177bc into WebKit:main Jan 21, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ddkilzer ddkilzer ddkilzer approved these changes

@youennf youennf youennf approved these changes

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

Assignees

@charliewolfe charliewolfe

Labels
WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).
Projects
None yet
Milestone
No milestone
6 participants
@charliewolfe @webkit-early-warning-system @webkit-commit-queue @ddkilzer @youennf @webkit-ews-buildbot