New issue
Advanced search Search tips

Issue 906058 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 29
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 889902



Sign in to add a comment

Write implementation and tests for PrimaryAccountMutatorImpl::LegacyStartSigninWithRefreshTokenForPrimaryAccount

Project Member Reported by sdefresne@chromium.org, Nov 16

Issue description

The implementation will just call SigninManager::StartSignInWithRefreshToken.

The tests can be based on the tests for the SigninManager method.
 
Blocking: 889902
Cc: ma...@igalia.com
As per the discussion in  crbug.com/906063 , it's possible that the implementation of this method should be in tandem with LegacyIsPrimaryAccountAuthInProgress, LegacyCompletePendingPrimaryAccountSignin and LegacyPrimaryAccountForAuthInProgress, so that the unit tests can be written without artificially adding extra non-blocking methods to IdentityTestEnvironment.
Owner: ma...@igalia.com
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 29

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

commit 02f8a46c06b9b93537cd53fa117223879c7b0546
Author: Mario Sanchez Prada <mario@igalia.com>
Date: Thu Nov 29 12:13:03 2018

Implement legacy methods in PrimaryAccountMutator related to signin process

Implement the following methods at the same time, so that we can also
provide decent unit tests that would require of all of them to make
sure functionality works as expected:

  * LegacyStartSigninWithRefreshTokenForPrimaryAccount()
  * LegacyCompletePendingPrimaryAccountSignin()
  * LegacyIsPrimaryAccountAuthInProgress()
  * LegacyPrimaryAccountForAuthInProgress()

Also, PrimaryAccountMutatorTests unit tests for each of those methods as
part of this CL, based on those from SigninManagerTest, including a new
test for ClearPrimaryAccount (i.e. ClearPrimaryAccount_NotSignedIn) to
check that it reports FALSE when there is neither a primary account nor
an authentication process ongoing (now we can finally implement that).

Bug:  906058 ,  906059 ,  906063 ,  906064 
Change-Id: I2164c867f35020806225da4e69659364c3429a92
Reviewed-on: https://chromium-review.googlesource.com/c/1342559
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612159}
[modify] https://crrev.com/02f8a46c06b9b93537cd53fa117223879c7b0546/services/identity/public/cpp/primary_account_mutator_impl.cc
[modify] https://crrev.com/02f8a46c06b9b93537cd53fa117223879c7b0546/services/identity/public/cpp/primary_account_mutator_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment