LibWeb: Implement most CredentialsContainer methods
#3495
Conversation
This file contains 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
Provide the default implementation for `is_conditional_mediation_available` and `will_request_conditional_creation`.
The introduction of the abstract `CredentialInterface` class serves to implement what is mentioned in the spec as "interface objects". It is not possible to implement this perfectly to spec in Ladybird so let's make use of a custom abstraction.
This is probably a spec bug. If the property has a default value it will always be populated in the `CredentialRequestOptions` dictionary making that credential type always active.
devgianlu
force-pushed
the
credentials-cont
branch
2 times, most recently
from
c197740 to
795803e
Compare
Implement `get`, `store`, `create` on `CredentialsContainer`. Some pieces that we don't support yet, like permissions and custom user interactions, are left as TODO. The `rejects when aborted after the promise creation` tests are broken because the spec says to check the aborted signal synchronously.
devgianlu
force-pushed
the
credentials-cont
branch
from
795803e to
243df11
Compare
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.
This PR mplements
get,store,createonCredentialsContainer.Notably, it also introduces the
CredentialInterfaceabstract class to deal with credentials internal methods and metadata. It has been implemented as a singleton for each credential type. This is the design that I managed to come up with, any suggestion is appreciated.The spec isn't the best and contains some bugs:
options.signal && options.signal->aborted()check ingetandcreateis not deferred therefore fails two WPT testsinterfaces.size() < 1check increateis custom and avoids an out of bound access in the next step -> Spec issueCredentialRequestOptionsIDL dictionary has a default value forpasswordwhich renders the credential type always active. I think we are supposed to reject it when computing the relevant credential interface objects ifpasswordisfalse, but that's not specifiedI'll create some spec issues and link them here even if the repository has been inactive for the last 6 months.
Lastly, I am unsure how to check
If settings’s relevant global object has no associated Documentso I've left two TODOs.