Define "prepare credential requests" algorithm#420
Open
marcoscaceres wants to merge 3 commits intomainfrom
Open
Define "prepare credential requests" algorithm#420marcoscaceres wants to merge 3 commits intomainfrom
marcoscaceres wants to merge 3 commits intomainfrom
Conversation
9 tasks
9 tasks
d02002d to
bb35ade
Compare
bb35ade to
0222f65
Compare
Collaborator
Author
|
Blocked on tests. |
webkit-commit-queue
pushed a commit
to marcoscaceres/WebKit
that referenced
this pull request
Jan 20, 2026
…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
jschanck
reviewed
Jan 21, 2026
9 tasks
mohamedamir
reviewed
Jan 21, 2026
mohamedamir
reviewed
Jan 21, 2026
|
Discussed in 21 January 2026 Series B call |
d11d30f to
14054cb
Compare
35336be to
0b7552c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Defines the “prepare credential requests” algorithm for the credential request coordinator and wires it into the DigitalCredential get/create integration flow.
Changes:
- Adds a detailed “Prepare credential requests” algorithm, including request filtering, protocol checks, validation, error propagation, and abort integration.
- Updates
[[DiscoverFromExternalSource]]and[[Create]]to create/return a promise and delegate to the new preparation algorithm. - Updates WebIDL around
DigitalCredential.protocolby introducing aDigitalCredentialProtocoltypedef and changing the attribute type accordingly.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5 tasks
Contributor
|
@marcoscaceres I've opened a new pull request, #459, to work on those changes. Once the pull request is ready, I'll request review from you. |
876e8fb to
e0bf221
Compare
e0bf221 to
5f800a9
Compare
60bfb2f to
8c172e7
Compare
8c172e7 to
f520d19
Compare
marcoscaceres
commented
Feb 26, 2026
marcoscaceres
commented
Feb 26, 2026
| <li>If the [=user agent=] does not allow |protocol|, | ||
| [=iteration/continue=]. | ||
| </li> | ||
| <li>Let |validatedRequest| be the result of validating |request|'s |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| <li>Let |validatedRequest| be the result of validating |request|'s | |
| <li>Let |validatedRequestOrException| be the result of validating |request|'s |
| </p> | ||
| </aside> | ||
| </li> | ||
| <li>If |validatedRequest| is an [=exception=]: |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| <li>If |validatedRequest| is an [=exception=]: | |
| <li>If |validatedRequestOrException| is an [=exception=]: |
| <li>If |validatedRequest| is an [=exception=]: | ||
| <ol> | ||
| <li>[=credential request coordinator/Complete credential request | ||
| with=] |promise| and |validatedRequest|. |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| with=] |promise| and |validatedRequest|. | |
| with error=] passing |promise| and |validatedRequestOrException|. |
| </li> | ||
| </ol> | ||
| </li> | ||
| <li>[=list/Append=] |validatedRequest| to |validatedRequests|. |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| <li>[=list/Append=] |validatedRequest| to |validatedRequests|. | |
| <li>[=list/Append=] |validatedRequestOrException| to |validatedRequests|. |
Comment on lines
919
to
922
| To <dfn data-dfn-for="credential request coordinator" data-local-lt= | ||
| "complete credential request with">complete credential request with | ||
| error</dfn> given a {{Promise}} |promise:Promise| and (a JavaScript | ||
| Value) |error|: |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| To <dfn data-dfn-for="credential request coordinator">reject request with</dfn> a (JavaScript Value) |error| and a {{Promise}} |promise:Promise|: |
| </li> | ||
| </ol> | ||
| <h3> | ||
| Complete credential request with error |
Collaborator
Author
There was a problem hiding this comment.
Suggested change
| Complete credential request with error | |
| Reject request with error |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added the "prepare credential requests" algorithm, detailing validation of each request, handling of unsupported protocols, error propagation, and abort signal integration.
Closes #432
Closes #249
Closes #220
Closes #207
Closes #159
The following tasks have been completed:
Implementation commitment:
Documentation and checks
Preview | Diff