Commit 9fbd2462 authored by Julie Jeongeun Kim's avatar Julie Jeongeun Kim Committed by Commit Bot

Clean up OAuth2TokenService from code comments

This CL is a part of moving access token management to
OAuth2AccessTokenManager.

It is specifically a step toward folding O2TS into PO2TS
now that O2TS is only one subclass of PO2TS.

OAuth2TokenService has been folded completely into
ProfileOAuth2TokenService on https://crrev.com/c/1712560.
So, it's not valid to comment OAuth2TokenService on the
current codebase.

This CL updates comments for OAuth2TOkenService with
IdentityManager, the access token fetcher or
OAuth2AccessTokenManager.

Bug: 967598
Change-Id: I1aba6c57d8cc65063f12be512cece3404d602fcf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725222
Commit-Queue: Julie Jeongeun Kim <jkim@igalia.com>
Reviewed-by: default avatarSam McNally <sammc@chromium.org>
Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Reviewed-by: default avatarColin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682542}
parent 0566b94c
......@@ -196,8 +196,6 @@ class OAuth2LoginManager : public KeyedService,
static void RecordCookiesCheckOutcome(bool is_pre_merge,
MergeVerificationOutcome outcome);
// Keeps the track if we have already reported OAuth2 token being loaded
// by OAuth2TokenService.
Profile* user_profile_;
SessionRestoreStrategy restore_strategy_;
SessionRestoreState state_;
......
......@@ -35,7 +35,7 @@ class GoogleServiceAuthError;
namespace policy {
// Interacts with the device management service and determines whether Android
// management is enabled for the user or not. Uses the OAuth2TokenService to
// management is enabled for the user or not. Uses the IdentityManager to
// acquire access tokens for the device management.
class AndroidManagementClient {
public:
......
......@@ -18,12 +18,12 @@ class DataSegment;
// UploadJob can be used to upload screenshots and logfiles to the cloud.
// Data is uploaded via a POST request of type "multipart/form-data". The class
// relies on OAuth2TokenService to acquire an access token with a sufficient
// scope. Data segments can be added to the upload queue using AddDataSegment()
// and the upload is started by calling Start(). Calls to AddDataSegment() are
// only allowed prior to the first call to Start().
// An Upload instance may be destroyed at any point in time, the pending
// operations are guaranteed to be canceled and the Delegate::OnSuccess() and
// relies on OAuth2AccessTokenManager to acquire an access token with a
// sufficient scope. Data segments can be added to the upload queue using
// AddDataSegment() and the upload is started by calling Start(). Calls to
// AddDataSegment() are only allowed prior to the first call to Start(). An
// Upload instance may be destroyed at any point in time, the pending operations
// are guaranteed to be canceled and the Delegate::OnSuccess() and
// Delegate::OnFailure() methods will not be invoked.
class UploadJob {
public:
......
......@@ -1040,8 +1040,8 @@ TEST_P(UserCloudPolicyManagerChromeOSTest, ReregistrationFails) {
class UserCloudPolicyManagerChromeOSChildTest
: public UserCloudPolicyManagerChromeOSTest {
public:
// Issues OAuthToken for device management scopes using OAuth2TokenService.
void IssueOAuthTokenWithTokenService(base::TimeDelta token_lifetime) {
// Issues OAuthToken for device management scopes.
void IssueOAuth2AccessToken(base::TimeDelta token_lifetime) {
identity::ScopeSet scopes;
scopes.insert(GaiaConstants::kDeviceManagementServiceOAuth);
scopes.insert(GaiaConstants::kOAuthWrapBridgeUserInfoScope);
......@@ -1100,7 +1100,7 @@ TEST_P(UserCloudPolicyManagerChromeOSChildTest, RefreshSchedulerStart) {
LoadStoreWithCachedData();
EXPECT_FALSE(manager_->core()->refresh_scheduler());
IssueOAuthTokenWithTokenService(base::TimeDelta::FromSeconds(3600));
IssueOAuth2AccessToken(base::TimeDelta::FromSeconds(3600));
EXPECT_TRUE(manager_->core()->refresh_scheduler());
}
......@@ -1113,7 +1113,7 @@ TEST_P(UserCloudPolicyManagerChromeOSChildTest, RefreshScheduler) {
// This starts refresh scheduler.
const base::TimeDelta token_lifetime = base::TimeDelta::FromMinutes(50);
IssueOAuthTokenWithTokenService(token_lifetime);
IssueOAuth2AccessToken(token_lifetime);
// First refresh is scheduled with delay of 0s - let it execute.
FetchPolicy(
......@@ -1133,7 +1133,7 @@ TEST_P(UserCloudPolicyManagerChromeOSChildTest, RefreshScheduler) {
for (int i = 0; i < iterations; ++i) {
task_runner_->FastForwardBy(token_lifetime);
refresh_delay -= token_lifetime;
IssueOAuthTokenWithTokenService(token_lifetime);
IssueOAuth2AccessToken(token_lifetime);
}
// Advance the clock by the remaining time to get scheduled policy refresh.
......
......@@ -366,7 +366,8 @@ std::string DeviceOAuth2TokenService::GetRefreshToken() const {
// This shouldn't happen: GetRefreshToken() is only called for actual
// token minting operations. In above states, requests are either queued
// or short-circuited to signal error immediately, so no actual token
// minting via OAuth2TokenService::FetchOAuth2Token should be triggered.
// minting via OAuth2AccessTokenManager::FetchOAuth2Token should be
// triggered.
NOTREACHED();
return std::string();
case STATE_VALIDATION_PENDING:
......
......@@ -121,12 +121,12 @@ class SyncAuthTest : public SyncTest {
void DisableTokenFetchRetries() {
// If ProfileSyncService observes a transient error like SERVICE_UNAVAILABLE
// or CONNECTION_FAILED, this means the OAuth2TokenService has given up
// trying to reach Gaia. In practice, OA2TS retries a fixed number of times,
// but the count is transparent to PSS.
// or CONNECTION_FAILED, this means the access token fetcher has given
// up trying to reach Gaia. In practice, the access token fetching code
// retries a fixed number of times, but the count is transparent to PSS.
// Disable retries so that we instantly trigger the case where
// ProfileSyncService must pick up where OAuth2TokenService left off (in
// terms of retries).
// ProfileSyncService must pick up where the access token fetcher left off
// (in terms of retries).
signin::DisableAccessTokenFetchRetries(
IdentityManagerFactory::GetForProfile(GetProfile(0)));
}
......@@ -153,7 +153,7 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, Sanity) {
}
// Verify that ProfileSyncService continues trying to fetch access tokens
// when OAuth2TokenService has encountered more than a fixed number of
// when the access token fetcher has encountered more than a fixed number of
// HTTP_INTERNAL_SERVER_ERROR (500) errors.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnInternalServerError500) {
ASSERT_TRUE(SetupSync());
......@@ -168,7 +168,7 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnInternalServerError500) {
}
// Verify that ProfileSyncService continues trying to fetch access tokens
// when OAuth2TokenService has encountered more than a fixed number of
// when the access token fetcher has encountered more than a fixed number of
// HTTP_FORBIDDEN (403) errors.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnHttpForbidden403) {
ASSERT_TRUE(SetupSync());
......@@ -183,7 +183,7 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnHttpForbidden403) {
}
// Verify that ProfileSyncService continues trying to fetch access tokens
// when OAuth2TokenService has encountered a URLRequestStatus of FAILED.
// when the access token fetcher has encountered a URLRequestStatus of FAILED.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnRequestFailed) {
ASSERT_TRUE(SetupSync());
ASSERT_FALSE(AttemptToTriggerAuthError());
......@@ -197,7 +197,7 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnRequestFailed) {
}
// Verify that ProfileSyncService continues trying to fetch access tokens
// when OAuth2TokenService receives a malformed token.
// when the access token fetcher receives a malformed token.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnMalformedToken) {
ASSERT_TRUE(SetupSync());
ASSERT_FALSE(AttemptToTriggerAuthError());
......@@ -211,8 +211,8 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryOnMalformedToken) {
}
// Verify that ProfileSyncService ends up with an INVALID_GAIA_CREDENTIALS auth
// error when an invalid_grant error is returned by OAuth2TokenService with an
// HTTP_BAD_REQUEST (400) response code.
// error when an invalid_grant error is returned by the access token fetcher
// with an HTTP_BAD_REQUEST (400) response code.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, InvalidGrant) {
ASSERT_TRUE(SetupSync());
ASSERT_FALSE(AttemptToTriggerAuthError());
......@@ -227,7 +227,7 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, InvalidGrant) {
}
// Verify that ProfileSyncService retries after SERVICE_ERROR auth error when
// an invalid_client error is returned by OAuth2TokenService with an
// an invalid_client error is returned by the access token fetcher with an
// HTTP_BAD_REQUEST (400) response code.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryInvalidClient) {
ASSERT_TRUE(SetupSync());
......@@ -242,7 +242,8 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryInvalidClient) {
}
// Verify that ProfileSyncService retries after REQUEST_CANCELED auth error
// when OAuth2TokenService has encountered a URLRequestStatus of CANCELED.
// when the access token fetcher has encountered a URLRequestStatus of
// CANCELED.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryRequestCanceled) {
ASSERT_TRUE(SetupSync());
ASSERT_FALSE(AttemptToTriggerAuthError());
......@@ -257,7 +258,7 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryRequestCanceled) {
// Verify that ProfileSyncService fails initial sync setup during backend
// initialization and ends up with an INVALID_GAIA_CREDENTIALS auth error when
// an invalid_grant error is returned by OAuth2TokenService with an
// an invalid_grant error is returned by the access token fetcher with an
// HTTP_BAD_REQUEST (400) response code.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, FailInitialSetupWithPersistentError) {
ASSERT_TRUE(SetupClients());
......@@ -274,8 +275,8 @@ IN_PROC_BROWSER_TEST_F(SyncAuthTest, FailInitialSetupWithPersistentError) {
// Verify that ProfileSyncService fails initial sync setup during backend
// initialization, but continues trying to fetch access tokens when
// OAuth2TokenService receives an HTTP_INTERNAL_SERVER_ERROR (500) response
// code.
// the access token fetcher receives an HTTP_INTERNAL_SERVER_ERROR (500)
// response code.
IN_PROC_BROWSER_TEST_F(SyncAuthTest, RetryInitialSetupWithTransientError) {
ASSERT_TRUE(SetupClients());
GetFakeServer()->SetHttpError(net::HTTP_UNAUTHORIZED);
......
......@@ -29,7 +29,7 @@ class SharedURLLoaderFactory;
namespace gcm {
struct AccountIds {
std::string account_key; // The account ID used by OAuth2TokenService.
std::string account_key; // The account ID used by IdentityManager.
std::string gaia;
std::string email;
};
......
......@@ -97,7 +97,7 @@ class GCMAccountTracker : public AccountTracker::Observer,
friend class GCMAccountTrackerTest;
// Maps account keys to account states. Keyed by account_ids as used by
// OAuth2TokenService.
// IdentityManager.
typedef std::map<std::string, AccountInfo> AccountInfos;
// AccountTracker::Observer overrides.
......
......@@ -115,8 +115,8 @@ class GCMInvalidationBridgeTest : public ::testing::Test {
TEST_F(GCMInvalidationBridgeTest, RequestToken) {
base::RunLoop run_loop;
// Make sure that call to RequestToken reaches OAuth2TokenService and gets
// back to callback.
// Make sure that call to RequestToken reaches the access token fetcher and
// gets back to callback.
delegate_->RequestToken(
base::Bind(&GCMInvalidationBridgeTest::RequestTokenFinished,
base::Unretained(this), run_loop.QuitClosure()));
......
......@@ -154,9 +154,9 @@ void FakeProfileOAuth2TokenServiceDelegate::UpdateAuthError(
const CoreAccountId& account_id,
const GoogleServiceAuthError& error) {
backoff_entry_.InformOfRequest(!error.IsTransientError());
// Drop transient errors to match OAuth2TokenService's stated contract for
// GetAuthError() and to allow clients to test proper behavior in the case of
// transient errors.
// Drop transient errors to match ProfileOAuth2TokenService's stated contract
// for GetAuthError() and to allow clients to test proper behavior in the case
// of transient errors.
if (error.IsTransientError())
return;
......
......@@ -68,8 +68,8 @@ class OAuth2TokenServiceDelegateAndroid
const std::vector<CoreAccountId>& prev_ids,
const std::vector<CoreAccountId>& curr_ids);
// Overridden from OAuth2TokenService to complete signout of all
// OA2TService aware accounts.
// Overridden from ProfileOAuth2TokenService to complete signout of all
// POA2TService aware accounts.
void RevokeAllCredentials() override;
void LoadCredentials(const CoreAccountId& primary_account_id) override;
......
......@@ -106,7 +106,7 @@ ProfileOAuth2TokenServiceDelegateChromeOS::CreateAccessTokenFetcher(
if (it != errors_.end() && it->second.last_auth_error.IsPersistentError()) {
VLOG(1) << "Request for token has been rejected due to persistent error #"
<< it->second.last_auth_error.state();
// |OAuth2TokenService| will manage the lifetime of this pointer.
// |ProfileOAuth2TokenService| will manage the lifetime of this pointer.
return std::make_unique<OAuth2AccessTokenFetcherImmediateError>(
consumer, it->second.last_auth_error);
}
......@@ -114,7 +114,7 @@ ProfileOAuth2TokenServiceDelegateChromeOS::CreateAccessTokenFetcher(
if (backoff_entry_.ShouldRejectRequest()) {
VLOG(1) << "Request for token has been rejected due to backoff rules from"
<< " previous error #" << backoff_error_.state();
// |OAuth2TokenService| will manage the lifetime of this pointer.
// |ProfileOAuth2TokenService| will manage the lifetime of this pointer.
return std::make_unique<OAuth2AccessTokenFetcherImmediateError>(
consumer, backoff_error_);
}
......
......@@ -25,7 +25,7 @@ class ProfileOAuth2TokenServiceObserver {
// Called whenever the login-scoped refresh token becomes unavailable for
// account |account_id|.
virtual void OnRefreshTokenRevoked(const CoreAccountId& account_id) {}
// Called after all refresh tokens are loaded during OAuth2TokenService
// Called after all refresh tokens are loaded during ProfileOAuth2TokenService
// startup.
virtual void OnRefreshTokensLoaded() {}
// Sent after a batch of refresh token changes is done.
......
......@@ -101,7 +101,7 @@ const char kSigninAllowed[] = "signin.allowed";
// True if the token service has been prepared for Dice migration.
const char kTokenServiceDiceCompatible[] = "token_service.dice_compatible";
// Boolean which stores if the OAuth2TokenService should ignore secondary
// Boolean which stores if the ProfileOAuth2TokenService should ignore secondary
// accounts.
const char kTokenServiceExcludeAllSecondaryAccounts[] =
"token_service.exclude_all_secondary_accounts";
......
......@@ -230,7 +230,7 @@ class ProfileSyncService : public SyncService,
// Used by MigrationWatcher. May return null.
BackendMigrator* GetBackendMigratorForTest();
// Used by tests to inspect interaction with OAuth2TokenService.
// Used by tests to inspect interaction with the access token fetcher.
bool IsRetryingAccessTokenFetchForTest() const;
// Used by tests to inspect the OAuth2 access tokens used by PSS.
......
......@@ -42,11 +42,11 @@ blundell@chromium.org.
## Obtaining an Access Token
Where you would have called OAuth2TokenService::StartRequest(), you should
Where you would have called ProfileOAuth2TokenService::StartRequest(), you should
instead call IdentityManager::GetAccessToken(). This interface is a 1:1
correspondence that should transparently work in all use cases, although there
are currently still some input parameters that need to be added to its API to
match esoteric variants of the OAuth2TokenService API. Here is an [example
match esoteric variants of the ProfileOAuth2TokenService API. Here is an [example
CL](https://chromium-review.googlesource.com/c/514047/) that illustrates
conversion of a client.
......@@ -58,7 +58,7 @@ to work this class into the Identity Service interface.
## Being Notified on Signin of the Primary Account
If you were previously listening to PrimaryAccountManager::GoogleSigninSucceeded()
or OAuth2TokenService::OnRefreshTokenIsAvailable() to determine when the primary
or ProfileOAuth2TokenService::OnRefreshTokenIsAvailable() to determine when the primary
account is available, you should call
IdentityManager::GetPrimaryAccountWhenAvailable(). This method will fire when
the authenticated account is signed in, has a refresh token available, and the
......@@ -73,7 +73,7 @@ pattern.
## Determining if an Account Has a Refresh Token Available
There is no way to determine this information synchronously, i.e., there is no
literal equivalent to OAuth2TokenService::IsRefreshTokenAvailable(). To date,
literal equivalent to ProfileOAuth2TokenService::IsRefreshTokenAvailable(). To date,
all use cases of this method that we have seen have been part of a larger
asynchronous flow (e.g., determining whether the user needs to sign in before we
can obtain an access token in response to an invocation of the chrome.identity
......@@ -89,7 +89,7 @@ blundell@chromium.org.
## Being Notified When An Account Has a Refresh Token Available
All of the use cases of OAuth2TokenService::OnRefreshTokenAvailable() that we
All of the use cases of ProfileOAuth2TokenService::OnRefreshTokenAvailable() that we
have seen to date have been for the purpose of knowing when the authenticated
account is available (i.e., signed in with a refresh token available). For this
purpose, you should call IdentityManager::GetPrimaryAccountWhenAvailable(). Here
......@@ -101,7 +101,7 @@ blundell@chromium.org.
## Observing Signin-Related Events
There are plans to build a unified Observer interface that will supersede the
various current Observer interfaces (AccountTracker, OAuth2TokenService,
various current Observer interfaces (AccountTracker, ProfileOAuth2TokenService,
PrimaryAccountManager, AccountTrackerService). However, this functionality has
not yet been built. Contact blundell@chromium.org with your use case, which can
help drive the bringup of this interface.
......
......@@ -40,7 +40,7 @@ interface IdentityAccessor {
// |error| will give information detailing the reason for the failure, and
// |expiration_time| is undefined. |consumer_id| is an arbitrary string used
// to identity the consumer (e.g., in about:://signin-internals).
// NOTE: |account_id| corresponds to that used by OAuth2TokenService.
// NOTE: |account_id| corresponds to that used by IdentityManager.
GetAccessToken(CoreAccountId account_id, ScopeSet scopes, string consumer_id)
=> (string? token,
mojo_base.mojom.Time expiration_time,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment