New issue
Advanced search Search tips

Issue 900590 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Dec 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 749535



Sign in to add a comment

DCHECK failure for ChromeOSOAuth2TokenServiceDelegate::GetAccounts

Project Member Reported by sinhak@chromium.org, Oct 31

Issue description

Chrome Version: 72
OS: Chrome OS

What steps will reproduce the problem?
(1) Enable crOS Account Manager via flags
(2) Deploy a DCHECK / Debug build to your Chromebook
(3) Try logging in to your Chromebook

What is the expected result?
Login should succeed

What happens instead?
DCHECK at https://cs.chromium.org/chromium/src/chrome/browser/chromeos/oauth2_token_service_delegate.cc?l=195&rcl=1e26651d4dbdd6ec4d0b9c8637764468a8d2a228 is triggered.

Root Cause:

This DCHECK basically asserts that ChromeOSOAuth2TokenServiceDelegate::GetAccounts should not be called unless ChromeOSOAuth2TokenServiceDelegate::LoadCredentials has been called at least once, so that we can be sure that we are not returning incorrect account information (an empty list) at any point.

This expectation is not followed by AccountReconcilor here : https://cs.chromium.org/chromium/src/components/signin/core/browser/account_reconcilor.cc?l=792&rcl=27aab564a92387ff9d74fbfc4a83f18e14f1fba9 where it attempts to fetch accounts even when |token_service_->AreAllCredentialsLoaded()| returns false.
 
Why is LoadCredentials() not being called in this flow? Does it fall under one of the cases listed in the OP of  crbug.com/749535 , or is it something else?
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 5

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

commit 9b6cbd537b9a6de76d626f1d4e1956ef2eef0b65
Author: Kush Sinha <sinhak@chromium.org>
Date: Wed Dec 05 21:33:54 2018

Remove load credentials DCHECK from ChromeOSOAuth2TokenServiceDelegate

This DCHECK is impossible to guarantee right now. Please check the
attached bugs for context.

This has feature parity with the current token service delegate on
Chrome OS, which does not check this either : https://goo.gl/scLHhc

Bug:  900590 
Bug:  749535 
Change-Id: I02dd4ef9403f2f78863218ce2aacb647754e3854
Reviewed-on: https://chromium-review.googlesource.com/c/1363200
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614106}
[modify] https://crrev.com/9b6cbd537b9a6de76d626f1d4e1956ef2eef0b65/chrome/browser/chromeos/oauth2_token_service_delegate.cc

Status: WontFix (was: Assigned)
Sorry for the late reply. blundell@: It does get called but "later". Also it gets triggered in browsers tests which have the Chrome OS Account Manager flag enabled (the fake identity test environment gets setup "later" too). I am basically giving up on this DCHECK guarantee for now which is probably fine because MutablePO2TSDelegate (the current OAuth delegate on Chrome OS) does not guarantee it either.

Closing as WontFix.

Sign in to add a comment