New issue
Advanced search Search tips

Issue 885072 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Sep 18
Cc:
jochen@chromium.org
benmason@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Fix ProfileDownloader's fetching of access tokens after port to IdentityManager

Project Member Reported by blundell@chromium.org, Sep 18 
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.

In these contexts, ProfileDownloader will fetch access tokens for the wrong account. The fix was made in https://chromium-review.googlesource.com/c/chromium/src/+/1225805.

The breaking CL landed in M70, so I've split this bug out of crbug.com/801969 in order to request cherrypick for the fix. It's not clear exactly what the impact of the bug is, but it's a straightforward fix.
Project Member

Comment 1 by sheriffbot@chromium.org, Sep 18

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 2 by blundell@chromium.org, Sep 18 

I didn't immediately realize that we've already promoted to beta. I think that it would also be viable to just let beta bake and see if any concrete issues come up. This shouldn't result in any severe usability errors such as crashes; in the worst case, the user would be missing information about their profile such as the profile picture. If we decide to go that way, we can of course remove the RBS.

Comment 3 by benmason@chromium.org, Sep 19

Labels: -Hotlist-Merge-Review -Merge-Review-70 Merge-Approved-70
Approved for merge to 70, branch 3538.

Comment 4 by blundell@chromium.org, Sep 20

Labels: merge-merged-3538
The merge happened on https://chromium-review.googlesource.com/c/chromium/src/+/1235576.
Project Member

Comment 5 by sheriffbot@chromium.org, Sep 24

Cc: benmason@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by blundell@chromium.org, Sep 25

Labels: -Merge-Approved-70
►

Sign in to add a comment