Skip to content

Digital Credential: CredentialRequestCoordinator can remain non-Idle if AbortSignal races with picker result#56489

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
marcoscaceres:eng/Digital-Credential-CredentialRequestCoordinator-can-remain-non-Idle-if-AbortSignal-races-with-picker-result
Jan 20, 2026
Merged

Digital Credential: CredentialRequestCoordinator can remain non-Idle if AbortSignal races with picker result#56489
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
marcoscaceres:eng/Digital-Credential-CredentialRequestCoordinator-can-remain-non-Idle-if-AbortSignal-races-with-picker-result

Conversation

@marcoscaceres

@marcoscaceres marcoscaceres commented Jan 13, 2026 •
edited by webkit-early-warning-system
Loading

Copy link
Copy Markdown
Contributor

8a961b3

Digital Credential: CredentialRequestCoordinator can remain non-Idle if AbortSignal races with picker result
rdar://163295172
https://bugs.webkit.org/show_bug.cgi?id=305363

Reviewed by Pascoe.

Fix race and crash in CredentialRequestCoordinator by settling promises only after picker teardown and safely handling abort reasons.
Ensure credential requests always settle after the picker UI has fully torn down.

This change:

- Defers promise settlement until the picker dismiss callback fires
- Better handles aborts during presentation and teardown
- Avoids capturing unprotected JSValues across async boundaries
- Keeps coordinator state transitions more consistent (with better checks)

It also more closely follows the spec:
w3c-fedid/digital-credentials#420
w3c-fedid/digital-credentials#419

* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/get.tentative.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/non-fully-active.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/non-fully-active.https.html:
* Source/WebCore/Modules/identity/CredentialRequestCoordinator.cpp:
(WebCore::CredentialRequestCoordinator::PickerStateGuard::PickerStateGuard):
(WebCore::CredentialRequestCoordinator::PickerStateGuard::~PickerStateGuard):
(WebCore::CredentialRequestCoordinator::setState):
(WebCore::CredentialRequestCoordinator::prepareCredentialRequest):
(WebCore::CredentialRequestCoordinator::handleDigitalCredentialsPickerResult):
(WebCore:: const):
(WebCore::CredentialRequestCoordinator::dismissPickerAndSettle):
(WebCore::CredentialRequestCoordinator::abortPicker):
(WebCore::CredentialRequestCoordinator::contextDestroyed):
(WebCore::CredentialRequestCoordinator::~CredentialRequestCoordinator):
(): Deleted.
(WebCore::CredentialRequestCoordinator::presentPicker): Deleted.
(WebCore::CredentialRequestCoordinator::finalizeDigitalCredential): Deleted.
* Source/WebCore/Modules/identity/CredentialRequestCoordinator.h:
* Source/WebCore/Modules/identity/DigitalCredential.cpp:
(WebCore::DigitalCredential::discoverFromExternalSource):
* Source/WebCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations:

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

2d107c2

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp
✅ 🛠 watch
✅ 🛠 watch-sim

@marcoscaceres marcoscaceres self-assigned this Jan 13, 2026
@webkit-early-warning-system

webkit-early-warning-system commented Jan 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 6fb607f)

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 vision ❌ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp
✅ 🛠 watch
✅ 🛠 watch-sim

This was referenced Jan 13, 2026
Merged
Merged
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 13, 2026
@marcoscaceres marcoscaceres removed the merging-blocked Applied to prevent a change from being merged label Jan 14, 2026
@marcoscaceres marcoscaceres force-pushed the eng/Digital-Credential-CredentialRequestCoordinator-can-remain-non-Idle-if-AbortSignal-races-with-picker-result branch from 6fb607f to 10f2360 Compare January 14, 2026 05:06
@webkit-early-warning-system

webkit-early-warning-system commented Jan 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 10f2360)

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 14, 2026
@marcoscaceres marcoscaceres requested a review from Copilot January 15, 2026 02:43
Copilot AI reviewed Jan 15, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes race conditions and state management issues in the Digital Credentials CredentialRequestCoordinator. The changes ensure that credential requests always settle promises after the picker UI has fully torn down, preventing the coordinator from remaining in a non-Idle state when AbortSignal races with picker results.

Changes:

  • Defers promise settlement until after picker dismissal callback completes
  • Improves abort handling during presentation and teardown by protecting JSValues across async boundaries
  • Strengthens state transition validation and consistency checks

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Source/WebCore/Modules/identity/DigitalCredential.cpp Updates function call from presentPicker to prepareCredentialRequest
Source/WebCore/Modules/identity/CredentialRequestCoordinator.h Renames public method, adds dismissPickerAndSettle, removes move constructors/operators, adds deactivate() method
Source/WebCore/Modules/identity/CredentialRequestCoordinator.cpp Refactors state management, promise settlement timing, and abort handling to prevent race conditions
LayoutTests/imported/w3c/web-platform-tests/digital-credentials/non-fully-active.https.html Adds test for repeated abort scenarios
LayoutTests/imported/w3c/web-platform-tests/digital-credentials/non-fully-active.https-expected.txt Updates test expectations with new passing test
LayoutTests/imported/w3c/web-platform-tests/digital-credentials/get.tentative.https-expected.txt Updates test expectations showing fixed race condition test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Source/WebCore/Modules/identity/CredentialRequestCoordinator.cpp Outdated
Comment thread Source/WebCore/Modules/identity/CredentialRequestCoordinator.cpp
pascoej
pascoej reviewed Jan 15, 2026
View reviewed changes
Comment thread Source/WebCore/Modules/identity/CredentialRequestCoordinator.cpp Outdated
Comment thread Source/WebCore/Modules/identity/CredentialRequestCoordinator.cpp Outdated
@WebKit WebKit deleted a comment from Copilot AI Jan 15, 2026
@marcoscaceres marcoscaceres removed the merging-blocked Applied to prevent a change from being merged label Jan 15, 2026
@marcoscaceres marcoscaceres force-pushed the eng/Digital-Credential-CredentialRequestCoordinator-can-remain-non-Idle-if-AbortSignal-races-with-picker-result branch from 10f2360 to 061668a Compare January 15, 2026 06:16
@webkit-early-warning-system

webkit-early-warning-system commented Jan 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 061668a)

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
❌ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
⏳ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 15, 2026
@marcoscaceres marcoscaceres removed the merging-blocked Applied to prevent a change from being merged label Jan 15, 2026
@marcoscaceres marcoscaceres force-pushed the eng/Digital-Credential-CredentialRequestCoordinator-can-remain-non-Idle-if-AbortSignal-races-with-picker-result branch from 061668a to 59ee609 Compare January 15, 2026 08:02
@webkit-early-warning-system

webkit-early-warning-system commented Jan 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 59ee609)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style loading 🛠 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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
⏳ 🛠 🧪 jsc-debug-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot

webkit-ews-buildbot commented Jan 15, 2026

Copy link
Copy Markdown
Collaborator

Safer C++ Build #74741 (59ee609)

⚠️ 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 --UncountedCallArgsChecker Modules/identity/CredentialRequestCoordinator.cpp

@marcoscaceres

marcoscaceres commented Jan 19, 2026

Copy link
Copy Markdown
Contributor Author

Addressing Safer c++ issue in #56812

@marcoscaceres marcoscaceres force-pushed the eng/Digital-Credential-CredentialRequestCoordinator-can-remain-non-Idle-if-AbortSignal-races-with-picker-result branch from 59ee609 to 2d107c2 Compare January 20, 2026 04:43
@webkit-early-warning-system

webkit-early-warning-system commented Jan 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on current version of this PR (hash 2d107c2)

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp
✅ 🛠 watch
✅ 🛠 watch-sim

@marcoscaceres marcoscaceres added the merge-queue Applied to send a pull request to merge-queue label Jan 20, 2026
@marcoscaceres
Digital Credential: CredentialRequestCoordinator can remain non-Idle …
8a961b3 
…if AbortSignal races with picker result

rdar://163295172
https://bugs.webkit.org/show_bug.cgi?id=305363

Reviewed by Pascoe.

Fix race and crash in CredentialRequestCoordinator by settling promises only after picker teardown and safely handling abort reasons.
Ensure credential requests always settle after the picker UI has fully torn down.

This change:

- Defers promise settlement until the picker dismiss callback fires
- Better handles aborts during presentation and teardown
- Avoids capturing unprotected JSValues across async boundaries
- Keeps coordinator state transitions more consistent (with better checks)

It also more closely follows the spec:
w3c-fedid/digital-credentials#420
w3c-fedid/digital-credentials#419

* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/get.tentative.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/non-fully-active.https-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/digital-credentials/non-fully-active.https.html:
* Source/WebCore/Modules/identity/CredentialRequestCoordinator.cpp:
(WebCore::CredentialRequestCoordinator::PickerStateGuard::PickerStateGuard):
(WebCore::CredentialRequestCoordinator::PickerStateGuard::~PickerStateGuard):
(WebCore::CredentialRequestCoordinator::setState):
(WebCore::CredentialRequestCoordinator::prepareCredentialRequest):
(WebCore::CredentialRequestCoordinator::handleDigitalCredentialsPickerResult):
(WebCore:: const):
(WebCore::CredentialRequestCoordinator::dismissPickerAndSettle):
(WebCore::CredentialRequestCoordinator::abortPicker):
(WebCore::CredentialRequestCoordinator::contextDestroyed):
(WebCore::CredentialRequestCoordinator::~CredentialRequestCoordinator):
(): Deleted.
(WebCore::CredentialRequestCoordinator::presentPicker): Deleted.
(WebCore::CredentialRequestCoordinator::finalizeDigitalCredential): Deleted.
* Source/WebCore/Modules/identity/CredentialRequestCoordinator.h:
* Source/WebCore/Modules/identity/DigitalCredential.cpp:
(WebCore::DigitalCredential::discoverFromExternalSource):
* Source/WebCore/SaferCPPExpectations/UncountedCallArgsCheckerExpectations:

Canonical link: https://commits.webkit.org/305868@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Digital-Credential-CredentialRequestCoordinator-can-remain-non-Idle-if-AbortSignal-races-with-picker-result branch from 2d107c2 to 8a961b3 Compare January 20, 2026 09:49
@webkit-commit-queue

webkit-commit-queue commented Jan 20, 2026

Copy link
Copy Markdown
Collaborator

Committed 305868@main (8a961b3): https://commits.webkit.org/305868@main

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

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

Reviewers

Copilot code review Copilot Copilot left review comments

@pascoej pascoej pascoej approved these changes

Assignees

@marcoscaceres marcoscaceres

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@marcoscaceres @webkit-early-warning-system @webkit-ews-buildbot @webkit-commit-queue @pascoej