New issue
Advanced search Search tips

Issue 890801 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Dec 4
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Task

Blocked on:
issue 889902
issue 906064

Blocking:
issue 883330



Sign in to add a comment

Convert chrome/browser/ui/startup/startup_tab_provider.cc to IdentityManager

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

Issue description

API used:
- SigninManager::IsAuthenticated()
- SigninManager::AuthInProgress()

 
Labels: Proj-Servicification-VendorBug
Cc: blundell@chromium.org
 Issue 903892  has been merged into this issue.
Owner: svil...@igalia.com
Status: Started (was: Available)
I'll take this
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 22

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

commit ec18ec703263af9db944f39477ba654d2f995ca3
Author: Sergio Villar Senin <svillar@igalia.com>
Date: Thu Nov 22 12:40:15 2018

Convert startup_tab_provider.cc to IdentityManager

It was using the SigninManager API to check whether the primary
account was authenticated and an authentication was in progress. We
can use the IdentityManager and PrimaryAccountMutator APIs to perform
the same operations.

Bug:  890801 
Change-Id: Ia2567a567481a4d67b68eee6b714aab72c537b5a
Reviewed-on: https://chromium-review.googlesource.com/c/1346467
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#610389}
[modify] https://crrev.com/ec18ec703263af9db944f39477ba654d2f995ca3/chrome/browser/ui/startup/startup_tab_provider.cc

Status: Fixed (was: Started)
Fixed now.
Blockedon: 906064
Status: Available (was: Fixed)
The CL invokes method that have not been implemented yet (implementation is still in progress at https://chromium-review.googlesource.com/c/chromium/src/+/1342559). I've created a CL to revert the conversion.

This is the root cause for https://bugs.chromium.org/p/chromium/issues/detail?id=909712. It also indicate that chrome/browser/ui/startup/startup_tab_provider.cc is not throughly tested.

Pending revert: https://chromium-review.googlesource.com/c/chromium/src/+/1353980
Owner: ----
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 28

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

commit c14e542837671788da94baf7d9430599f6f1ae50
Author: Sylvain Defresne <sdefresne@chromium.org>
Date: Wed Nov 28 17:59:59 2018

Revert "Convert startup_tab_provider.cc to IdentityManager"

This reverts commit ec18ec703263af9db944f39477ba654d2f995ca3.

Reason for revert: the conversion was incorrect as the new API
is not yet implemented. This probably causes regressions (sad
that no tests caught this) and spam logs (see  crbug.com/909712 ).

Original change's description:
> Convert startup_tab_provider.cc to IdentityManager
> 
> It was using the SigninManager API to check whether the primary
> account was authenticated and an authentication was in progress. We
> can use the IdentityManager and PrimaryAccountMutator APIs to perform
> the same operations.
> 
> Bug:  890801 
> Change-Id: Ia2567a567481a4d67b68eee6b714aab72c537b5a
> Reviewed-on: https://chromium-review.googlesource.com/c/1346467
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Commit-Queue: Sergio Villar <svillar@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#610389}

TBR=ellyjones@chromium.org,blundell@chromium.org,svillar@igalia.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  890801 
Change-Id: I8df5a79a40c503abd6dd74974a84eded8f69472a
Reviewed-on: https://chromium-review.googlesource.com/c/1353980
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611769}
[modify] https://crrev.com/c14e542837671788da94baf7d9430599f6f1ae50/chrome/browser/ui/startup/startup_tab_provider.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 4

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

commit 3817e4c7836f2157384e167c35da91b6faa96e9c
Author: Sergio Villar Senin <svillar@igalia.com>
Date: Tue Dec 04 15:21:13 2018

Reland "Convert startup_tab_provider.cc to IdentityManager"

This is a reland of ec18ec703263af9db944f39477ba654d2f995ca3

Original change's description:
> Convert startup_tab_provider.cc to IdentityManager
> 
> It was using the SigninManager API to check whether the primary
> account was authenticated and an authentication was in progress. We
> can use the IdentityManager and PrimaryAccountMutator APIs to perform
> the same operations.
> 
> Bug:  890801 
> Change-Id: Ia2567a567481a4d67b68eee6b714aab72c537b5a
> Reviewed-on: https://chromium-review.googlesource.com/c/1346467
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Commit-Queue: Sergio Villar <svillar@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#610389}

Bug:  890801 
Change-Id: I6f2f762d6861ba9feddd7867919c0e39660dbd34
Reviewed-on: https://chromium-review.googlesource.com/c/1356584
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#613555}
[modify] https://crrev.com/3817e4c7836f2157384e167c35da91b6faa96e9c/chrome/browser/ui/startup/startup_tab_provider.cc

Status: Fixed (was: Available)
We can close this now that the patch has relanded

Sign in to add a comment