[wip] Introduce a new class for the embedder to query FedCM accounts [chromium/src : main]
Sam Goto (Gerrit)
Sam Goto added 6 comments![]( "Open in Gerrit")
if (base::FeatureList::IsEnabled(features::kFedCmNavigationCancellation)) {return early here rather than nesting another if block
std::vector<GURL> supported_idps = {GURL("https://accounts.google.com")};how can we avoid this hard-coding? what would be the most scalable way to avoid hard-coding this, architecturally speaking?
source->GetIdentityCredentialSuggestions(i'm not sure i follow the semantics of this ... why are we "GetIdentityCredential" here in a "OngetCredentialsCompleted" callback? can you fix this with a code change rather than an answer explaining?
std::move(callback).Run(std::move(result));switch the order and return early here on the condition, so that the bulk of the work is done inside of the main block
std::vector<GURL> embedder_requested_idps,why should it be the responsiblity of the caller of this method to know which are the requested idps?
: public DocumentUserData<IdentityCredentialSourceImpl>,Why does this class need to be owned by UserData?
Related details
- Yi Gu
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yi Gu (Gerrit)
Yi Gu added 6 comments![]( "Open in Gerrit")
if (base::FeatureList::IsEnabled(features::kFedCmNavigationCancellation)) {return early here rather than nesting another if block
Acknowledged
std::vector<GURL> supported_idps = {GURL("https://accounts.google.com")};how can we avoid this hard-coding? what would be the most scalable way to avoid hard-coding this, architecturally speaking?
This is just for demo purposes to show the interface. The changes in actor_login_delegate_impl will not be merged.
i'm not sure i follow the semantics of this ... why are we "GetIdentityCredential" here in a "OngetCredentialsCompleted" callback? can you fix this with a code change rather than an answer explaining?
Acknowledged
switch the order and return early here on the condition, so that the bulk of the work is done inside of the main block
Acknowledged
std::vector<GURL> embedder_requested_idps,why should it be the responsiblity of the caller of this method to know which are the requested idps?
The requester should know which IdPs are currently available on the site. It's possible that a "logged-in" IdP is not supported by the current RP. Right?
: public DocumentUserData<IdentityCredentialSourceImpl>,Why does this class need to be owned by UserData?
- Sam Goto
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |