New issue
Advanced search Search tips

Issue 890810 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 1
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Task

Blocking:
issue 883330



Sign in to add a comment

Convert chrome/browser/ui/webui/settings/people_handler_unittest.cc to IdentityManager

Project Member Reported by sdefresne@chromium.org, Oct 1

Issue description

API used:
- SigninManager::SignOut()

 
Owner: sdoyon@chromium.org
Status: Started (was: Available)
Owner: toniki...@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31

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

commit 947d8086bef4a6e39f3e0fd260eefe784871ef97
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Oct 31 21:20:09 2018

[s13n] Convert c/b/ui/webui/settings/people_handler_unittest.cc to IdentityManager

This CL converts the PeopleHandler unittests away from SignManager and
ProfileOAuth2TokenService, in favor of the new IdentityManager.

Production code has been converted in [1]. This is a follow up steps.

[1] https://crrev.com/c/1261637

BUG= 890810 

Change-Id: I405e46d2126180a2da61393bf32263ae43877b6b
Reviewed-on: https://chromium-review.googlesource.com/c/1311093
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#604377}
[modify] https://crrev.com/947d8086bef4a6e39f3e0fd260eefe784871ef97/chrome/browser/ui/webui/settings/people_handler_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 1

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

commit 50d29054e4bb8faba79ebb8a60d1fcd55e22ea56
Author: Adam Rice <ricea@chromium.org>
Date: Thu Nov 01 10:01:03 2018

Revert "[s13n] Convert c/b/ui/webui/settings/people_handler_unittest.cc to IdentityManager"

This reverts commit 947d8086bef4a6e39f3e0fd260eefe784871ef97.

Reason for revert: Caused failure on memory bot:

https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20CFI/builds/11050

Original change's description:
> [s13n] Convert c/b/ui/webui/settings/people_handler_unittest.cc to IdentityManager
> 
> This CL converts the PeopleHandler unittests away from SignManager and
> ProfileOAuth2TokenService, in favor of the new IdentityManager.
> 
> Production code has been converted in [1]. This is a follow up steps.
> 
> [1] https://crrev.com/c/1261637
> 
> BUG= 890810 
> 
> Change-Id: I405e46d2126180a2da61393bf32263ae43877b6b
> Reviewed-on: https://chromium-review.googlesource.com/c/1311093
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#604377}

TBR=xiyuan@chromium.org,tonikitoo@igalia.com

Change-Id: Idbab8bee31fd8a61ca66bb60cf2a680506866d39
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  890810 
Reviewed-on: https://chromium-review.googlesource.com/c/1312152
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604547}
[modify] https://crrev.com/50d29054e4bb8faba79ebb8a60d1fcd55e22ea56/chrome/browser/ui/webui/settings/people_handler_unittest.cc

Status: Started (was: Fixed)
Lets fix it, and reland.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 1

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

commit 3feabd44bc8456ed5725c6b208dcc622b3b48399
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Thu Nov 01 15:21:27 2018

Reland "[s13n] Convert c/b/ui/webui/settings/people_handler_unittest.cc to IdentityManager"

This is a reland of 947d8086bef4a6e39f3e0fd260eefe784871ef97

Compared to the reverted CL, this CL changes the way profiles are created, making use of
the helper IdentityTestEnvironmentProfileAdaptor::CreateProfileForIdentityTestEnvironment,
by overriding CreateBrowserContext.
It is worth saying that the original memory failure was reproduced locally with the original
CL and is now fixed.

Original change's description:
> [s13n] Convert c/b/ui/webui/settings/people_handler_unittest.cc to IdentityManager
>
> This CL converts the PeopleHandler unittests away from SignManager and
> ProfileOAuth2TokenService, in favor of the new IdentityManager.
>
> Production code has been converted in [1]. This is a follow up steps.
>
> [1] https://crrev.com/c/1261637
>
> BUG= 890810 
>
> Change-Id: I405e46d2126180a2da61393bf32263ae43877b6b
> Reviewed-on: https://chromium-review.googlesource.com/c/1311093
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#604377}

Bug:  890810 
Change-Id: I0aa039575603765bf4cec86cbb87bc86fd0b301f
Reviewed-on: https://chromium-review.googlesource.com/c/1312909
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604588}
[modify] https://crrev.com/3feabd44bc8456ed5725c6b208dcc622b3b48399/chrome/browser/ui/webui/settings/people_handler_unittest.cc

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 7

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

commit 1aa22c07329b208217461e442f1cdcf1332dbb87
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Nov 07 19:25:24 2018

[s13n] Replace direct AccountTrackerService use from PeopleHandlerDice unittests

CL is a follow up of blundell's comment in [1].

Basically, it replaces the direct calls to AccountTrackerService::SeedAccountInfo
(and the sibling IdentityTestEnv::SetRefreshTokenForAccount calls),
with an existing IdentityTestEnv equivalent method: MakeAccountAvailable.

[1] https://crrev.com/c/1311093/2/chrome/browser/ui/webui/settings/people_handler_unittest.cc#1039

BUG= 890810 

Change-Id: I15753453a61e435d59ed3c1a879f89b9cb027dcf
Reviewed-on: https://chromium-review.googlesource.com/c/1324049
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#606122}
[modify] https://crrev.com/1aa22c07329b208217461e442f1cdcf1332dbb87/chrome/browser/ui/webui/settings/people_handler_unittest.cc

Sign in to add a comment