Convert chrome/browser/ui/ash/multi_user/multi_user_util_chromeos_unittest.cc to IdentityManager |
||
Issue descriptionAPI used: - SigninManagerBase::GetAuthenticatedAccountInfo()
,
Oct 31
,
Nov 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd05804c617938acd5c47da11f5a869eaf28d53a commit cd05804c617938acd5c47da11f5a869eaf28d53a Author: Mario Sanchez Prada <mario@igalia.com> Date: Mon Nov 05 17:01:02 2018 Migrate MultiUserUtilTest for ChromeOS to the IdentityManager Remove dependencies on AccountTrackerService and SigninManagerBase and rely instead on IdentityManager, IdentityTestEnvironment and IdentityTestEnvironmentProfileAdaptor for the unit tests. Note that this CL also changes the implementation of the tests slightly, but this shouldn't change the logic that the test is trying to check (that a valid account ID is always returned for a valid profile even after revoking the refresh token). The reasoning behind this modification (which has been bigger than in other migrations) was based on several factors: - Since we're relying on IdentityTestEnvironment now, there's no need to keep using the fake SigninManagerBase nor the AccountTrackerService directly anymore. - With the AccountTrackerService (ATS) out of the way, we no longer simulate the token being revoked by directly removing the account from the ATS, meaning that further calls to the MultiUserTestingProfile::GetProfileUserName() method would never return an empty string. This is because that method would be now implemented in terms of IdentityManager, which would still be returning a valid email after the revoking since calling IdentityTestEnvironment::RemoveRefreshTokenForAccount() won't cause the account to be removed, just invalidated. - As a result, the way the ReturnValidAccountIdIfTokenRevoked test used to check whether the token is revoked, based on checking whether GetUserProfileName() returned an empty string or not, is no longer valid. Fortunately, we can simply use another API that is more accurate: IdentityManager::HasAccountWithRefreshToken(). - Finally, since we're not using GetUserProfileName() at all anymore, there's also no need at all to subclass TestingProfile since overriding that method in terms of SigninManagerBase was its sole purpose for existing in the first place. Thus, we can drop that class as well. In summary, quite some changes to the tests but the essence of it should remain the same, relying in IdentityManager::HasAccountWithRefreshToken() now instead of in the override for TestingProfile::GetUserProfileName(), which is probably more precise and accurate. Bug: 890799 Change-Id: I8210678c4486c226abbe920c1945ae473d8e28cf Reviewed-on: https://chromium-review.googlesource.com/c/1307442 Reviewed-by: Stefan Kuhne <skuhne@chromium.org> Reviewed-by: Colin Blundell <blundell@chromium.org> Commit-Queue: Mario Sanchez Prada <mario@igalia.com> Cr-Commit-Position: refs/heads/master@{#605364} [modify] https://crrev.com/cd05804c617938acd5c47da11f5a869eaf28d53a/chrome/browser/ui/ash/multi_user/multi_user_util_chromeos_unittest.cc
,
Nov 5
|
||
►
Sign in to add a comment |
||
Comment 1 by ma...@igalia.com
, Oct 30Status: Started (was: Available)