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

Crash at WebExtensionDynamicScripts::executeScript. #29982

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Crash at WebExtensionDynamicScripts::executeScript. #29982

merged 1 commit into from
Jun 20, 2024
+18 −21

Conversation

xeenon
Copy link
Contributor

@xeenon xeenon commented Jun 19, 2024 •
edited by webkit-early-warning-system

d794fd4

Crash at WebExtensionDynamicScripts::executeScript.
https://webkit.org/b/275668
rdar://130154054

Reviewed by Jeff Miller.

The crash was happening because an `optional` was being accessed while not being checked for a value.
We don't need to use `optional` here, so this change removes the optional uses and uses `WTF::compactMap`
for the one remaining `optional` return to filter the `SourcePair` results into a `SourcePairs` `Vector`.

* Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPITabsCocoa.mm:
(WebKit::WebExtensionContext::tabsExecuteScript):
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionDynamicScriptsCocoa.mm:
(WebKit::WebExtensionDynamicScripts::getSourcePairsForParameters):
(WebKit::WebExtensionDynamicScripts::executeScript):
(WebKit::WebExtensionDynamicScripts::injectStyleSheets):
(WebKit::WebExtensionDynamicScripts::removeStyleSheets):
* Source/WebKit/UIProcess/Extensions/WebExtensionDynamicScripts.h:

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

9a112d2

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

@xeenon xeenon requested a review from b-weinstein as a code owner June 19, 2024 18:19
@xeenon xeenon self-assigned this Jun 19, 2024
@xeenon xeenon added the WebKit Extensions Bugs related to extension support. label Jun 19, 2024
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 9a112d2)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv
✅ 🛠 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 19, 2024
@xeenon xeenon removed the merging-blocked Applied to prevent a change from being merged label Jun 20, 2024
@xeenon xeenon requested a review from jeffmapple June 20, 2024 18:18
@xeenon xeenon added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 20, 2024
@xeenon
Crash at WebExtensionDynamicScripts::executeScript.
d794fd4 
https://webkit.org/b/275668
rdar://130154054

Reviewed by Jeff Miller.

The crash was happening because an `optional` was being accessed while not being checked for a value.
We don't need to use `optional` here, so this change removes the optional uses and uses `WTF::compactMap`
for the one remaining `optional` return to filter the `SourcePair` results into a `SourcePairs` `Vector`.

* Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPITabsCocoa.mm:
(WebKit::WebExtensionContext::tabsExecuteScript):
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionDynamicScriptsCocoa.mm:
(WebKit::WebExtensionDynamicScripts::getSourcePairsForParameters):
(WebKit::WebExtensionDynamicScripts::executeScript):
(WebKit::WebExtensionDynamicScripts::injectStyleSheets):
(WebKit::WebExtensionDynamicScripts::removeStyleSheets):
* Source/WebKit/UIProcess/Extensions/WebExtensionDynamicScripts.h:

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

Committed 280212@main (d794fd4): https://commits.webkit.org/280212@main

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

@webkit-commit-queue webkit-commit-queue merged commit d794fd4 into WebKit:main Jun 20, 2024
@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 Jun 20, 2024
@xeenon xeenon deleted the bug/275668 branch June 21, 2024 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffmapple jeffmapple jeffmapple approved these changes

@kiaraarose kiaraarose kiaraarose approved these changes

@b-weinstein b-weinstein Awaiting requested review from b-weinstein b-weinstein is a code owner

@davidjohnson91 davidjohnson91 Awaiting requested review from davidjohnson91

@ddkilzer ddkilzer Awaiting requested review from ddkilzer

@elijahsawyers elijahsawyers Awaiting requested review from elijahsawyers

Assignees

@xeenon xeenon

Labels
WebKit Extensions Bugs related to extension support.
Projects
None yet
Milestone
No milestone
6 participants
@xeenon @webkit-early-warning-system @webkit-commit-queue @jeffmapple @kiaraarose @webkit-ews-buildbot