-
Notifications
You must be signed in to change notification settings - Fork 181
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.
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
Updates based on DOM's abort reason #1706
Conversation
I'm worried that because the algorithm doesn't make a distinction between successful and failure return values you could spoof successful return values as abort reason can be anything. It would be better for this algorithm to either throw or return something I think. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your work here! I think this might require a little more work to avoid breaking the Credential Management integration. Happy to answer any questions you might have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Removed spam link]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Once you update [[Store]] on the credential management spec, would you mind changing the language on webauthn's implementation of that method to throw as well? Thank you!
Done :) |
Still LGTM. From my side this looks good to land once w3c/webappsec-credential-management#197 lands. @annevk, would you mind taking another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally this looks good to me, but there is some existing language that might be worth cleaning up in a follow-up PR:
- 'DOMException whose name is "X"' -> '"X" DOMException'
- Throwing implies termination so need to state that the algorithm is terminated. See https://infra.spec.whatwg.org/#algorithm-control-flow.
- There are some cases where it looks like a for each loop is followed by a throw, but it could be read as if the throw was run after a single iteration.
I've added a patch to this PR addressing these, so hopefully that looks better.
@nsatragno: Is it okay if I ask you to do this in a follow-up PR instead? |
Friendly ping to @nsatragno @akshayku for final review?/landing if all looks good. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks! And sorry for the delay.
AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef
AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef
AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef
AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Reviewed-by: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1039866}
AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Reviewed-by: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1039866}
AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Reviewed-by: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1039866}
This reverts commit 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Reason for revert: crbug.com/1357180 Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357180 Change-Id: Ida573df85179b8575b3cba725109a45a09a30507
This reverts commit 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Reason for revert: crbug.com/1357180 Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357180 Change-Id: Ida573df85179b8575b3cba725109a45a09a30507
This reverts commit 626cf94. Reason for revert: crbug.com/1357180 Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357180 Change-Id: Ida573df85179b8575b3cba725109a45a09a30507 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861492 Auto-Submit: Francois Pierre Doray <fdoray@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Owners-Override: Francois Pierre Doray <fdoray@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/main@{#1040433}
This reverts commit 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Reason for revert: crbug.com/1357180 Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357180 Change-Id: Ida573df85179b8575b3cba725109a45a09a30507 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861492 Auto-Submit: Francois Pierre Doray <fdoray@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Owners-Override: Francois Pierre Doray <fdoray@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/main@{#1040433}
This reverts commit 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Reason for revert: crbug.com/1357180 Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357180 Change-Id: Ida573df85179b8575b3cba725109a45a09a30507 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861492 Auto-Submit: Francois Pierre Doray <fdoray@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Owners-Override: Francois Pierre Doray <fdoray@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/main@{#1040433}
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357621 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357621 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba
This is a reland of 626cf94. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357621 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861890 Reviewed-by: Nina Satragno <nsatragno@chromium.org> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1041601}
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357621 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861890 Reviewed-by: Nina Satragno <nsatragno@chromium.org> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1041601}
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357621 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861890 Reviewed-by: Nina Satragno <nsatragno@chromium.org> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1041601}
…an abort reason, a=testonly Automatic update from web-platform-tests webauthn: Update abort handling to take an abort reason AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Reviewed-by: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1039866} -- wpt-commits: fb1c37a30d96d7c8e2f88703f8d98ae1da9fe6f5 wpt-pr: 35579
…to take an abort reason", a=testonly Automatic update from web-platform-tests Revert "webauthn: Update abort handling to take an abort reason" This reverts commit 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Reason for revert: crbug.com/1357180 Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357180 Change-Id: Ida573df85179b8575b3cba725109a45a09a30507 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861492 Auto-Submit: Francois Pierre Doray <fdoray@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Owners-Override: Francois Pierre Doray <fdoray@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/main@{#1040433} -- wpt-commits: a7b3cd4017c97861de7712df98a7aa9de4b659ed wpt-pr: 35692
…to take an abort reason", a=testonly Automatic update from web-platform-tests Reland "webauthn: Update abort handling to take an abort reason" This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357621 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861890 Reviewed-by: Nina Satragno <nsatragno@chromium.org> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1041601} -- wpt-commits: fa5194921814a92ad4226843fba386f19612e1b6 wpt-pr: 35695
AbortController now has a `reason` [1] parameter which has been added to Web Authentication [2], Credential Management [3], and Web OTP [4]. We should update CredentialsContainer to take this `reason` to the promise then aborting a request. [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason [2] w3c/webauthn#1706 [3] w3c/webappsec-credential-management#196 [4] WICG/web-otp#57 Bug: 1272541, 1329938 Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Reviewed-by: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1039866} NOKEYCHECK=True GitOrigin-RevId: 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b
This reverts commit 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Reason for revert: crbug.com/1357180 Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357180 Change-Id: Ida573df85179b8575b3cba725109a45a09a30507 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861492 Auto-Submit: Francois Pierre Doray <fdoray@chromium.org> Commit-Queue: Stephen McGruer <smcgruer@chromium.org> Owners-Override: Francois Pierre Doray <fdoray@chromium.org> Reviewed-by: Stephen McGruer <smcgruer@chromium.org> Cr-Commit-Position: refs/heads/main@{#1040433} NOKEYCHECK=True GitOrigin-RevId: 678e86ea4027f8350e2c8ae2502e7bb1ef0178d9
This is a reland of 626cf945c1cb51b2b51cf145d5be7cdaf6e2879b. Fix test failures in linux-bfcache-rel to pass isUserConsenting false to fix a race between resolving it and aborting. Original change's description: > webauthn: Update abort handling to take an abort reason > > AbortController now has a `reason` [1] parameter which > has been added to Web Authentication [2], Credential Management [3], > and Web OTP [4]. We should update CredentialsContainer to take > this `reason` to the promise then aborting a request. > > [1] https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/reason > [2] w3c/webauthn#1706 > [3] w3c/webappsec-credential-management#196 > [4] WICG/web-otp#57 > > Bug: 1272541, 1329938 > Change-Id: Idb9ef40d6a97ae240ae3a28892a426e4c0ffbfef > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3715820 > Commit-Queue: Euisang Lim <eui-sang.lim@samsung.com> > Commit-Queue: Nina Satragno <nsatragno@chromium.org> > Reviewed-by: Nina Satragno <nsatragno@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1039866} Bug: 1272541, 1329938, 1357621 Change-Id: I3861d5c2440519592e0438e8f4df774372a7c0ba Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861890 Reviewed-by: Nina Satragno <nsatragno@chromium.org> Commit-Queue: Nina Satragno <nsatragno@chromium.org> Cr-Commit-Position: refs/heads/main@{#1041601} NOKEYCHECK=True GitOrigin-RevId: ebe81ac00cfe27c2aca8bee59b490ac6980e32e1
After whatwg/dom#1027, the "aborted flag" no longer exists which broke this spec.
This change replaces checking the signal's "aborted flag" with checking the aborted predicate instead. This change also includes returning the signal's abort reason, instead of returning a new "AbortError" DOMException if the signal is aborted.
Fixes #1682
@annevk, @nsatragno: Is it okay if you could take a look at this?
Preview | Diff