New issue
Advanced search Search tips

Issue 919793 link

Starred by 1 user
Status: Assigned
Owner:
droger@chromium.org
Cc:
bsazonov@chromium.org
sinhak@chromium.org
blundell@chromium.org
msarda@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Enforce invariants in TokenService implementations

Project Member Reported by droger@chromium.org, Jan 8 
The token service has a conceptual invariants that are probably true on desktop (although not really enforced) but false on android.

The violations should be fixed and the invariants enforced.

The main invariants are:

- GetAccounts() should return the list of accounts for which we have refresh tokens. In particular,  RefreshTokenIsAvailable() should return true iff the account is in GetAccounts().

- When OnRefreshTokenAvailable() is called, RefreshTokenIsAvailable() should return true.

- When OnRefreshTokenRevoked() is called, RefreshTokenIsAvailable() should return false.

- GetAccounts() is empty until the tokens are fully loaded.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 8 

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

commit 4ac37e71d84ddd207dbff4fe7a6e22c94515126e
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Jan 08 14:50:38 2019

IdentityManager: Remove DCHECK on Android

IdentityManager has a sanity-check that verifies that
ProfileOAuth2TokenService::RefreshTokenIsAvailable() returns true at
various times that match the documented semantics of OAuth2TokenService:
- when an account is present in O2TS::GetAccounts().
- on a callback from O2TS::Observer::OnRefreshTokenAvailable().

However, these semantics are currently not fully enforced on Android
(see full description in crbug.com/919793). Hence, we need to eliminate
this DCHECK on that platform, as it was causing tests to fail
(https://bugs.chromium.org/p/chromium/issues/detail?id=908542#c22).

Fixing behavior on Android is tracked by crbug.com/919793.
In the meantime, IdentityManager gives clients the same view that they
previously had via interacting directly with ProfileOAuth2TokenService.

Bug: 908542, 919793
Change-Id: I402fff2b06bb1bacb96fd769f115d91a95af97a1
Reviewed-on: https://chromium-review.googlesource.com/c/1400267
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620714}
[modify] https://crrev.com/4ac37e71d84ddd207dbff4fe7a6e22c94515126e/services/identity/public/cpp/identity_manager.cc

Comment 2 by droger@chromium.org, Jan 8

Components: Services>SignIn
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 10 

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

commit 172680039455beb8321f5e98687b2c0f22fe3fb3
Author: Kush Sinha <sinhak@chromium.org>
Date: Thu Jan 10 12:53:43 2019

Fix ChromeOSOAuth2TokenServiceDelegate invariants

|ChromeOSOAuth2TokenServiceDelegate| violates
|OAuth2TokenServiceDelegate|'s invariants wrt the contract between
|GetAccounts| and |RefreshTokenIsAvailable| APIs. Please check the
attached bug for context.

Bug: 919793, 820046
Test: unit_tests --gtest_filter="*CrOSOAuthDelegateTest*"
Change-Id: I367bdc65ce3ad1796bd07d18b60d255c1916e4b8
Reviewed-on: https://chromium-review.googlesource.com/c/1400666
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621554}
[modify] https://crrev.com/172680039455beb8321f5e98687b2c0f22fe3fb3/chrome/browser/chromeos/oauth2_token_service_delegate.cc
[modify] https://crrev.com/172680039455beb8321f5e98687b2c0f22fe3fb3/chrome/browser/chromeos/oauth2_token_service_delegate_unittest.cc
[modify] https://crrev.com/172680039455beb8321f5e98687b2c0f22fe3fb3/google_apis/gaia/oauth2_token_service.h
[modify] https://crrev.com/172680039455beb8321f5e98687b2c0f22fe3fb3/google_apis/gaia/oauth2_token_service_delegate.h
►

Sign in to add a comment