IdentityManager::GetPrimaryAccountInfo() and HasPrimaryAccountInfo()

[Chris Cunningham]

Random thoughts about these APIs.

HasPrimaryAccountInfo is implemted as

bool IdentityManager::HasPrimaryAccount() const {

  return !primary_account_info_.account_id.empty();

}

But AccountInfo has a first-calls IsEmpty() method. Should we use that instead?

https://cs.chromium.org/chromium/src/components/signin/core/browser/account_info.h?rcl=3f0eda935338567fd3c770fd24b44a3af46a4b3b&l=29

My other thought is, we might make this more self documenting by changing GetPrimaryAccountInfo() to return a base::Optional<AccountInfo>. All calls to HasPrimaryAccount() could be replaced with GetPrimaryAccountInfo().has_value() (or even better, you can just drop the has_value() since base::Optional implements the bool operator).

Chris

[Sylvain Defresne]

It is possible to have no information for the primary account besides account_id. This mean that we should eventually track the primary account id independently from the AccountInfo.

We discussed offline a possible refactoring were we change GetPrimaryAccountInfo() to GetPrimaryAccountId() and add a GetAccountInfoForAccountId() once we've refactored IdentityManager to track the primary account id only, and keep a map of account_id to AccountInfo for all accounts known to AccountTrackerService.

-- Sylvain

--
You received this message because you are subscribed to the Google Groups "identity-service-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to identity-service...@chromium.org.
To post to this group, send email to identity-s...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/identity-service-dev/CALG6eSqWZkYM5h%3DT_F5K--GfNbGE_28dULd5Dg6kh1q5JM%3DVqQ%40mail.gmail.com.

[Colin Blundell]

Good observations, Chris! As Sylvain mentioned, we are going to be changing how IdentityManager stores this information to clarify the relationship between having the account ID and having other elements of the AccountInfo. This will all be part of the work of extending IdentityManager to fully encompass the current functionality and use cases of AccountTrackerService.

Best,

Colin

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/identity-service-dev/CAJauWCFauUJthgP7wqG9kkSSWNKAK%3DteKyy8Fu8gF-JDr5oiBw%40mail.gmail.com.