New issue
Advanced search Search tips

Issue 893504 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 26
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 883318
issue 883330
issue 894075



Sign in to add a comment

Convert UserCloudPolicyTokenForwarder to interact with IdentityManager and PrimaryAccountAccessTokenFetcher

Project Member Reported by blundell@chromium.org, Oct 9

Issue description

UserCloudPolicyTokenForwarder fetches access tokens for the primary account and observes ProfileOAuth2TokenService. This can be changed in a straightforward way to interact with IdentityManager and PrimaryAccountAccessTokenFetcher. Note: In the conversion let's just do a straight-up replacement pass and use PAATF in kImmediate mode. As a future cleanup it might be possible to streamline this code to use PAATF in kWaitUntilAvailable mode, but that's not obvious.

Changing this production code might cause unit or browsertests to break in non-obvious ways. If you detect that, reach out early and we can investigate what changes need to happen together.

As part of this work, the includes in user_cloud_policy_token_forwarder_factory.cc should naturally go away.
 
Blocking: 894075
Owner: toniki...@chromium.org
Status: Started (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 24

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c881935813d304562386beee6e75d4824c427e2a

commit c881935813d304562386beee6e75d4824c427e2a
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Oct 24 02:57:49 2018

[s13n] Convert UserCloudPolicyTokenForwarder to IdentityManager

In a follow up step UserCloudPolicyTokenForwarder will be migrated away
from using ProfileOAuth2TokenService, in favor of PrimaryAccountAccessTokenFetcher.

BUG= 893504 

Change-Id: I3b1f96a46e5ef61a8f6b796af37f344deef3023f
Reviewed-on: https://chromium-review.googlesource.com/c/1296873
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#602235}
[modify] https://crrev.com/c881935813d304562386beee6e75d4824c427e2a/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/c881935813d304562386beee6e75d4824c427e2a/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc
[modify] https://crrev.com/c881935813d304562386beee6e75d4824c427e2a/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.h
[modify] https://crrev.com/c881935813d304562386beee6e75d4824c427e2a/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder_factory.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ceb16ba4f2b87b99b43f4b9fc0e14bd024daff7

commit 2ceb16ba4f2b87b99b43f4b9fc0e14bd024daff7
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Thu Oct 25 14:45:12 2018

[s13n] Convert UserCloudPolicyTokenForwarder to IdentityManager::Observer

This is a 2/n CL that migrates UserCloudPolicyTokenForwarder to IdentityManager.
1/n was [1].

In a follow up step UserCloudPolicyTokenForwarder will be migrated away
from using ProfileOAuth2TokenService, in favor of PrimaryAccountAccessTokenFetcher.

[1] https://crrev.com/c/1296873

BUG= 893504 

Change-Id: I46c34bf299d1a7165151c671642959220ddbbb3b
Reviewed-on: https://chromium-review.googlesource.com/c/1298775
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#602704}
[modify] https://crrev.com/2ceb16ba4f2b87b99b43f4b9fc0e14bd024daff7/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc
[modify] https://crrev.com/2ceb16ba4f2b87b99b43f4b9fc0e14bd024daff7/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.h

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 25

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c06645d4b069d122224d78a9be7b47fff1cce7b2

commit c06645d4b069d122224d78a9be7b47fff1cce7b2
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Thu Oct 25 19:31:46 2018

[s13n] Convert UserCloudPolicyTokenForwarder away from OAuth2TokenService::Consumer

This is a 3/n CL that migrates UserCloudPolicyTokenForwarder to
IdentityManager. Previous two CLs were [1] and [2].

In a follow up step UserCloudPolicyManagerChromeOSTest will be migrated away
from using SigninManager and ProfileOAuth2TokenService altogether.

[1] https://crrev.com/c/1296873
[2] https://crrev.com/c/1298775

BUG= 893504 

Change-Id: I25d66d1ebc7b4c5dbd20b9e281d0cbf788db80e8
Reviewed-on: https://chromium-review.googlesource.com/c/1299313
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602827}
[modify] https://crrev.com/c06645d4b069d122224d78a9be7b47fff1cce7b2/chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos_unittest.cc
[modify] https://crrev.com/c06645d4b069d122224d78a9be7b47fff1cce7b2/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc
[modify] https://crrev.com/c06645d4b069d122224d78a9be7b47fff1cce7b2/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.h
[modify] https://crrev.com/c06645d4b069d122224d78a9be7b47fff1cce7b2/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder_factory.cc

Status: Fixed (was: Started)
Fixed, leaving the remaining work to  issue 894075 .
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b7db98a72d96f4b9107881d330fc3fa7e7acba3

commit 2b7db98a72d96f4b9107881d330fc3fa7e7acba3
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Nov 07 15:36:17 2018

[s13n] Convert UserCloudPolicyTokenForwarder to PrimaryAccountAccessTokenFetcher

This is a follow up of blundell@ request in [0]. It brings no functionality, but
eliminates the need of UserCloudPolicyTokenForwarder to directly inherit from
IdentityManager::Observer.

[0] https://crrev.com/c/1299313/3/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc#78

For completeness, this CL is a 4/n CL that migrates UserCloudPolicyTokenForwarder to
IdentityManager. Previous two CLs were [1], [2] and [3].

[1] https://crrev.com/c/1296873
[2] https://crrev.com/c/1298775
[3] https://crrev.com/c/1299313.

It also takes the opportunity to convert the use of OAuth2TokenService::ScopeSet
to Identity::ScopeSet (driven-by).

BUG= 893504 , 899175 

Change-Id: I907520ed211debe3fbd1a4c21b341da4b0a81dd8
Reviewed-on: https://chromium-review.googlesource.com/c/1319770
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#606050}
[modify] https://crrev.com/2b7db98a72d96f4b9107881d330fc3fa7e7acba3/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc
[modify] https://crrev.com/2b7db98a72d96f4b9107881d330fc3fa7e7acba3/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.h

Sign in to add a comment