New issue
Advanced search Search tips

Issue 915628 link

Starred by 3 users
Status: Assigned
Owner:
sinhak@chromium.org
Cc:
blundell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

browser tests generally don't work with account manager enabled

Project Member Reported by jochen@chromium.org, Dec 17 
in browser tests, profiles are either not signed in at all, or signed in with a fake account (via --login-user). account manager runs into DCHECKS in these situations, as it assumes that a profile is either fully signed in (with a valid gaia refresh token), or a guest profile / incognito.

Comment 1 by jochen@chromium.org, Dec 17 

same with running the profile in guest mode..
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 19 

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

commit bdf260ef72ba767a32b772aa34cac082381b2ec6
Author: Jochen Eisinger <jochen@chromium.org>
Date: Wed Dec 19 18:55:20 2018

Have SigninManagerBase::Initialize guarantee to load refresh tokens

This will guarantee that we always fire OnRefreshTokensLoaded during profile
startup.

Previously, on Chrome OS, this wasn't always the case, e.g., for guest mode, or
managed users, this was not invoked.

BUG= 749535 ,740117,915628
R=blundell@chromium.org

Change-Id: Ibecad59e6eb40db85a2090af4d59940c3d79b54b
Reviewed-on: https://chromium-review.googlesource.com/c/1374985
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617896}
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/chromeos/login/signin/oauth2_login_manager.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/download/notification/download_notification_interactive_uitest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/signin/fake_signin_manager_builder.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/browser/signin/signin_manager_factory.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/chrome/test/base/chrome_render_view_host_test_harness.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/search_provider_logos/logo_service_impl_unittest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/account_reconcilor_unittest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/fake_signin_manager.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/fake_signin_manager.h
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/signin_manager.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/signin_manager.h
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/signin_manager_base.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/signin_manager_base.h
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/components/signin/core/browser/signin_tracker_unittest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/services/identity/identity_manager_impl_unittest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/services/identity/public/cpp/accounts_mutator.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/services/identity/public/cpp/accounts_mutator.h
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/services/identity/public/cpp/accounts_mutator_unittest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/services/identity/public/cpp/identity_manager.h
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/services/identity/public/cpp/identity_manager_unittest.cc
[modify] https://crrev.com/bdf260ef72ba767a32b772aa34cac082381b2ec6/services/identity/public/cpp/identity_test_environment.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 19 

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

commit 11a9a05dbff5010b373489ed176d0992841c53eb
Author: CJ DiMeglio <lethalantidote@chromium.org>
Date: Wed Dec 19 21:24:43 2018

Revert "Have SigninManagerBase::Initialize guarantee to load refresh tokens"

This reverts commit bdf260ef72ba767a32b772aa34cac082381b2ec6.

Reason for revert: SigninInteractionControllerTestCase fails on chromium.mac/ios-slimnav (see  https://crbug.com/916731  )

Original change's description:
> Have SigninManagerBase::Initialize guarantee to load refresh tokens
> 
> This will guarantee that we always fire OnRefreshTokensLoaded during profile
> startup.
> 
> Previously, on Chrome OS, this wasn't always the case, e.g., for guest mode, or
> managed users, this was not invoked.
> 
> BUG= 749535 ,740117,915628
> R=​blundell@chromium.org
> 
> Change-Id: Ibecad59e6eb40db85a2090af4d59940c3d79b54b
> Reviewed-on: https://chromium-review.googlesource.com/c/1374985
> Commit-Queue: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
> Reviewed-by: Colin Blundell <blundell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#617896}

TBR=blundell@chromium.org,msarda@chromium.org,jochen@chromium.org

Change-Id: I32e10a4181cafd97b2f53f17e755b870ecdfc302
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  749535 , 740117, 915628
Reviewed-on: https://chromium-review.googlesource.com/c/1384640
Reviewed-by: CJ DiMeglio <lethalantidote@chromium.org>
Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617951}
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/chromeos/login/signin/oauth2_login_manager.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/download/notification/download_notification_interactive_uitest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/signin/fake_signin_manager_builder.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/browser/signin/signin_manager_factory.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/chrome/test/base/chrome_render_view_host_test_harness.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/account_reconcilor_unittest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/fake_signin_manager.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/fake_signin_manager.h
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/signin_manager.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/signin_manager.h
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/signin_manager_base.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/signin_manager_base.h
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/components/signin/core/browser/signin_tracker_unittest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/services/identity/identity_manager_impl_unittest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/services/identity/public/cpp/accounts_mutator.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/services/identity/public/cpp/accounts_mutator.h
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/services/identity/public/cpp/accounts_mutator_unittest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/services/identity/public/cpp/identity_manager.h
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/services/identity/public/cpp/identity_manager_unittest.cc
[modify] https://crrev.com/11a9a05dbff5010b373489ed176d0992841c53eb/services/identity/public/cpp/identity_test_environment.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 20 

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

commit 19ee90e7fcaf8c82d346506e71133b7071df1ef6
Author: Jochen Eisinger <jochen@chromium.org>
Date: Thu Dec 20 14:22:46 2018

Reland "Have SigninManagerBase::Initialize guarantee to load refresh tokens"

This reverts commit 11a9a05dbff5010b373489ed176d0992841c53eb.

Reason for revert: Unclear how the test failure should be related to this CL

Original change's description:
> Revert "Have SigninManagerBase::Initialize guarantee to load refresh tokens"
> 
> This reverts commit bdf260ef72ba767a32b772aa34cac082381b2ec6.
> 
> Reason for revert: SigninInteractionControllerTestCase fails on chromium.mac/ios-slimnav (see  https://crbug.com/916731  )
> 
> Original change's description:
> > Have SigninManagerBase::Initialize guarantee to load refresh tokens
> > 
> > This will guarantee that we always fire OnRefreshTokensLoaded during profile
> > startup.
> > 
> > Previously, on Chrome OS, this wasn't always the case, e.g., for guest mode, or
> > managed users, this was not invoked.
> > 
> > BUG= 749535 ,740117,915628
> > R=​blundell@chromium.org
> > 
> > Change-Id: Ibecad59e6eb40db85a2090af4d59940c3d79b54b
> > Reviewed-on: https://chromium-review.googlesource.com/c/1374985
> > Commit-Queue: Jochen Eisinger <jochen@chromium.org>
> > Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
> > Reviewed-by: Colin Blundell <blundell@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#617896}
> 
> TBR=blundell@chromium.org,msarda@chromium.org,jochen@chromium.org
> 
> Change-Id: I32e10a4181cafd97b2f53f17e755b870ecdfc302
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  749535 , 740117, 915628
> Reviewed-on: https://chromium-review.googlesource.com/c/1384640
> Reviewed-by: CJ DiMeglio <lethalantidote@chromium.org>
> Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#617951}

TBR=blundell@chromium.org,msarda@chromium.org,jochen@chromium.org,lethalantidote@chromium.org

Change-Id: Ie84f18559a192f516d11960d2c3047291d3687e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  749535 , 740117, 915628
Reviewed-on: https://chromium-review.googlesource.com/c/1386427
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618205}
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/chromeos/authpolicy/auth_policy_credentials_manager.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/chromeos/login/session/user_session_manager.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/chromeos/login/signin/oauth2_login_manager.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/download/notification/download_notification_interactive_uitest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/signin/fake_signin_manager_builder.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/browser/signin/signin_manager_factory.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/chrome/test/base/chrome_render_view_host_test_harness.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/account_reconcilor_unittest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/fake_signin_manager.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/fake_signin_manager.h
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/signin_manager.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/signin_manager.h
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/signin_manager_base.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/signin_manager_base.h
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/components/signin/core/browser/signin_tracker_unittest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/services/identity/identity_manager_impl_unittest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/services/identity/public/cpp/accounts_mutator.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/services/identity/public/cpp/accounts_mutator.h
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/services/identity/public/cpp/accounts_mutator_unittest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/services/identity/public/cpp/identity_manager.h
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/services/identity/public/cpp/identity_manager_unittest.cc
[modify] https://crrev.com/19ee90e7fcaf8c82d346506e71133b7071df1ef6/services/identity/public/cpp/identity_test_environment.cc

Comment 5 by benhenry@google.com, Jan 11

Status: Assigned (was: Untriaged)
This issue has an owner, a component and a priority, but is still listed as untriaged or unconfirmed. By definition, this bug is triaged. Changing status to "assigned". Please reach out to me if you disagree with how I've done this.
►

Sign in to add a comment