New issue
Advanced search Search tips

Issue 910146 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 908855



Sign in to add a comment

Refactor SignOutAndClearAccounts to use PrimaryAccountMutator::ClearPrimaryAccount

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

Issue description

Currently SignOutAndClearAccounts in ios/chrome/test/app/signin_test_util.mm uses AuthenticationService::SignOut() to clear the primary account (this invoke identity_manager_->ClearPrimaryAccount with ClearAccountTokensAction::kDefault) and then iterate over AccountTrackerService accounts to remove them.

Instead, it should be refactored to pass ClearAccountTokensAction::kRemoveAll.

This will allow removing the following code that uses AccountTrackerService::RemoveAccount and AccountTrackerService::GetAccounts:

  // Clear the tracked accounts.
  AccountTrackerService* account_tracker =
      ios::AccountTrackerServiceFactory::GetForBrowserState(browser_state);
  for (const AccountInfo& info : account_tracker->GetAccounts()) {
    account_tracker->RemoveAccount(info.account_id);
  }

 
Sylvain, could this be a vendor bug?
Labels: -Pri-1 Pri-2
Moving to P2 to reflect the fact that the current milestone is for converting PO2TS/SigninManager.
Owner: toniki...@chromium.org
Status: Started (was: Available)
Took a quick look at this, and uploaded a CL: https://chromium-review.googlesource.com/c/chromium/src/+/1393367

Please let me know if it is too off the original idea.
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 8

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

commit ecff01c4f3c559efdbe00f79a124b28990815037
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Tue Jan 08 19:10:28 2019

[ios] Remove AccountTrackerService references from SignOutAndClearAccounts

SigninManager::SignOutandRemoveAllAccounts() always pass kRemoveAllAccounts
for the action, while SigninManager::SignOut() checks accounts_consistency_.
If the consistency method is kDice, then it passes kRemoveAuthenticatedAccountIfInError,
otherwise it uses kRemoveAllAccounts. Given that in iOS the account consistency
is always set to kMirror, kRemoveAllAccounts is always used and
SigninManager::SignOutandRemoveAllAccounts and SigninManager::SignOut() become
equivalent.

This CL removes the AccountTrackerService::RemoveAccount calls in
chrome_test_util::SignOutAndClearAccounts [1], which are not needed.

[1] //ios/chrome/test/app/signin_test_util.mm.

BUG= 910146 

Change-Id: Ia598d4eb2a1aa790a868e19b7163a38b1f03acf1
Reviewed-on: https://chromium-review.googlesource.com/c/1393367
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620825}
[modify] https://crrev.com/ecff01c4f3c559efdbe00f79a124b28990815037/ios/chrome/test/app/signin_test_util.mm

Status: Fixed (was: Started)
Follow on bug https://bugs.chromium.org/p/chromium/issues/detail?id=919841

Sign in to add a comment