New issue
Advanced search Search tips

Issue 911675 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 883318
issue 908840



Sign in to add a comment

Determine whether AccountFetcherService belongs inside IdentityManager

Project Member Reported by blundell@chromium.org, Dec 4

Issue description

Technically, it could remain outside, as nothing currently slotted for the IdentityManager implementation depends on it. However, it is very closely tied in functionality to AccountTrackerService: AccountFetcherService asynchronously fetches metadata for accounts at various times (e.g., when the accounts are first added), which AccountTrackerService then stores.

It would bring in some extra dependencies.

We need to determine the answer to this question as AccountFetcherService depends on ProfileOAuth2TokenService and AccountTrackerService, so it either needs to be brought in or converted to use IdentityManager.
 
Blocking: 908840
Cc: sdefresne@chromium.org
This also of course will determine whether FakeAccountFetcherService is inside; like the production one, it directly depends on both AccountTrackerService and ProfileOAuth2TokenService.
Cc: ma...@igalia.com
Mario pointed out on https://docs.google.com/document/d/1kLjnxPnBAX0G6W-3u6OF_VguuJc0pXdgCRVR0z3yn-g/edit# that this question also impacts the flow of account removal: currently, AccountFetcherService observes token revocation and removes the account from AccountTrackerService. If we leave AccountFetcherService outside, then nothing inside would guarantee that the account's metadata is removed when its refresh token is revoked. This intuitively seems wrong.
AccountFetcherService cannot be untangled from AccountTrackerService. They are conceptually a single service. AFS directly calls ATS private methods, ATS invariants are enforced by AFS, ATS state is updated by AFS.

ATS could be simply a std::vector<AccountInfo> owned by AFS, and its methods moved to different objects and/or free functions.
Owner: sdefresne@chromium.org
Status: Fixed (was: Available)
This makes complete sense, Sylvain! So there's a clear resolution here that AccountFetcherService belongs inside IdentityManager.

Sign in to add a comment