Convert ProfileDownloader to talk to Identity Service client library |
|||||||||
Issue descriptionProfileDownloader is an O2TS observer and consumer and an AccountTrackerService observer. It fetches access tokens for the primary account. It also fetches user info for a given account ID via the AccountFetcherService, getting the account info from the AccountTrackerService. It observes PO2TS to know when a refresh token is available to kick off its flow. It observes AccountTrackerService to know when a given account’s info is available. This task is blocked on our solution for getting account information by account ID in the Identity Service client library. Once that is in place, the conversion should be straightforward.
,
Jan 15 2018
,
Aug 3
Hi Jochen, You should be able to make the ProfileOAuth2TokenService-related changes described in this bug now. The AccountTrackerService changes will need to wait.
,
Aug 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0bb406071cc9a1cd0c7bb17e3a764e0a9edf6ed1 commit 0bb406071cc9a1cd0c7bb17e3a764e0a9edf6ed1 Author: Jochen Eisinger <jochen@chromium.org> Date: Wed Aug 08 09:50:33 2018 Migrate ProfileDownloader to PrimaryAccountAccessTokenFetcher BUG=801969 R=msarda@chromium.org Change-Id: Ibfc22ab735f62622286def651ae5df065e59ebc7 Reviewed-on: https://chromium-review.googlesource.com/1163620 Commit-Queue: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Mihai Sardarescu <msarda@chromium.org> Cr-Commit-Position: refs/heads/master@{#581508} [modify] https://crrev.com/0bb406071cc9a1cd0c7bb17e3a764e0a9edf6ed1/chrome/browser/profiles/profile_downloader.cc [modify] https://crrev.com/0bb406071cc9a1cd0c7bb17e3a764e0a9edf6ed1/chrome/browser/profiles/profile_downloader.h
,
Aug 8
Assigning back to Colin for account tracker
,
Sep 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/941095ea9c05aeac476989da24eec36008bd97f8 commit 941095ea9c05aeac476989da24eec36008bd97f8 Author: Colin Blundell <blundell@chromium.org> Date: Mon Sep 17 11:09:38 2018 Fix ProfileDownloader access token fetches after move to IdentityManager https://chromium-review.googlesource.com/c/chromium/src/+/1163620 changed ProfileDownloader to fetch access tokens via IdentityManager and PrimaryAccountAccessTokenFetcher. However, ProfileDownloader is sometimes used in contexts other than the primary account: - ProfileDownloader::StartForAccount() takes in an account ID. - This method is invoked from Java on Android via ProfileDownloaderAndroid. The Java side passes an |is_pre_signin| bool that indicates that this flow can be used in contexts where the primary account is not set in IdentityManager/SigninManager. This CL changes ProfileDownloader to use an AccessTokenFetcher, mirroring its fetching of access tokens via |account_id_| from before the migration. This CL also adds a unittest that fails without this change (it verifies that when a refresh token is present for the account for which ProfileDownloader is asked to fetch info, an access token request is made for *that account*). Bug: 801969 Change-Id: Ibe3946448530ea22312baefa5f9079c5f6e221eb Reviewed-on: https://chromium-review.googlesource.com/1225805 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/heads/master@{#591638} [modify] https://crrev.com/941095ea9c05aeac476989da24eec36008bd97f8/chrome/browser/profiles/profile_downloader.cc [modify] https://crrev.com/941095ea9c05aeac476989da24eec36008bd97f8/chrome/browser/profiles/profile_downloader.h [modify] https://crrev.com/941095ea9c05aeac476989da24eec36008bd97f8/chrome/browser/profiles/profile_downloader_unittest.cc
,
Sep 20
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision aed7b40366e09cbc546ee6ec03323ea690fba6b1 was merged to refs/branch-heads/3538 branch with no merge approval from a TPM! Please explain why this change was merged to the branch! - AcknowledgeMerge: Notification Required --
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aed7b40366e09cbc546ee6ec03323ea690fba6b1 Commit: aed7b40366e09cbc546ee6ec03323ea690fba6b1 Author: blundell@chromium.org Commiter: blundell@chromium.org Date: 2018-09-20 08:42:31 +0000 UTC Fix ProfileDownloader access token fetches after move to IdentityManager https://chromium-review.googlesource.com/c/chromium/src/+/1163620 changed ProfileDownloader to fetch access tokens via IdentityManager and PrimaryAccountAccessTokenFetcher. However, ProfileDownloader is sometimes used in contexts other than the primary account: - ProfileDownloader::StartForAccount() takes in an account ID. - This method is invoked from Java on Android via ProfileDownloaderAndroid. The Java side passes an |is_pre_signin| bool that indicates that this flow can be used in contexts where the primary account is not set in IdentityManager/SigninManager. This CL changes ProfileDownloader to use an AccessTokenFetcher, mirroring its fetching of access tokens via |account_id_| from before the migration. This CL also adds a unittest that fails without this change (it verifies that when a refresh token is present for the account for which ProfileDownloader is asked to fetch info, an access token request is made for *that account*). Bug: 801969 Change-Id: Ibe3946448530ea22312baefa5f9079c5f6e221eb Reviewed-on: https://chromium-review.googlesource.com/1225805 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#591638}(cherry picked from commit 941095ea9c05aeac476989da24eec36008bd97f8) Reviewed-on: https://chromium-review.googlesource.com/1235576 Reviewed-by: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#539} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aed7b40366e09cbc546ee6ec03323ea690fba6b1 commit aed7b40366e09cbc546ee6ec03323ea690fba6b1 Author: Colin Blundell <blundell@chromium.org> Date: Thu Sep 20 08:42:31 2018 Fix ProfileDownloader access token fetches after move to IdentityManager https://chromium-review.googlesource.com/c/chromium/src/+/1163620 changed ProfileDownloader to fetch access tokens via IdentityManager and PrimaryAccountAccessTokenFetcher. However, ProfileDownloader is sometimes used in contexts other than the primary account: - ProfileDownloader::StartForAccount() takes in an account ID. - This method is invoked from Java on Android via ProfileDownloaderAndroid. The Java side passes an |is_pre_signin| bool that indicates that this flow can be used in contexts where the primary account is not set in IdentityManager/SigninManager. This CL changes ProfileDownloader to use an AccessTokenFetcher, mirroring its fetching of access tokens via |account_id_| from before the migration. This CL also adds a unittest that fails without this change (it verifies that when a refresh token is present for the account for which ProfileDownloader is asked to fetch info, an access token request is made for *that account*). Bug: 801969 Change-Id: Ibe3946448530ea22312baefa5f9079c5f6e221eb Reviewed-on: https://chromium-review.googlesource.com/1225805 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: Colin Blundell <blundell@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#591638}(cherry picked from commit 941095ea9c05aeac476989da24eec36008bd97f8) Reviewed-on: https://chromium-review.googlesource.com/1235576 Reviewed-by: Colin Blundell <blundell@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#539} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/aed7b40366e09cbc546ee6ec03323ea690fba6b1/chrome/browser/profiles/profile_downloader.cc [modify] https://crrev.com/aed7b40366e09cbc546ee6ec03323ea690fba6b1/chrome/browser/profiles/profile_downloader.h [modify] https://crrev.com/aed7b40366e09cbc546ee6ec03323ea690fba6b1/chrome/browser/profiles/profile_downloader_unittest.cc
,
Sep 20
Note for auditor(s): The request/approval for merge for this CL happened on crbug.com/885072 . I'll leave the labels here and let the auditors determine whether they want to remove them.
,
Sep 27
Ben please take a look at the merge violation on this bug
,
Sep 28
Yes, approval was granted on crbug.com/885072 |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by blundell@chromium.org
, Jan 15 2018Status: Available (was: Untriaged)