CBD shows "You won't be signed out of your Google account" when I'm not signed in |
|||||
Issue descriptionChrome Version: 69.0.3472.3 (with #account-consistency=DICE) OS: Linux What steps will reproduce the problem? (1) chrome://settings/clearBrowserData while being signed out What is the expected result? The cookie checkbox should just tell me which cookies get deleted What happens instead? The dialog tells me that I won't get signed out of an account that I'm not even signed in to.
,
Jul 12
Addigning to David and marking it as DICE follow-up work. We will get to it later (maybe in Q4).
,
Jul 12
I can fix this as well. "maybe Q4" sounds quite far in the future and it shouldn't be hard to fix as we have different history labels depending on signin and sync state already. Could you give me a hint at what is the right check for "is google cookie deletion disabled"? It looks like SigninManagerBase::IsAuthenticated() only returns true if sync is enabled but cookies already don't get deleted if I'm signed in but not syncing.
,
Jul 12
AccountConsistencyModeManager::IsDiceEnabledForProfile() should be the right check. If Dice is enabled, Google cookies are not cleared from this page.
,
Jul 12
> Could you give me a hint at what is the right check for "is google cookie deletion disabled"? Actually I don't think that's what you need, because we already only show that message when cookie deletion is disabled. I think what you need is whether the user currently has Google accounts in their cookies, that we would be rebuilt. There is no way to actually check that precisely (because we may fail to rebuild the cookies after deleting them). I think the closest approximation would be to check if there are accounts in the token service ProfileOAuth2TokenService::GetAccounts() that are not in error (RefreshTokenHasError()).
,
Jul 12
Also that "cookie deletion" is not disabled. We do delete the cookies, but rebuild them after. This distinction is somewhat important because we may fail to rebuild them in some cases (if our token has been revoked or expired).
,
Jul 12
Thanks, ProfileOAuth2TokenService::GetAccounts() sounds good. We currently always say that cookies don't get deleted even if someone isn't signed in. So it's still an improvement if we check for existing accounts even if there is a rare case where we are wrong.
,
Jul 12
,
Jul 12
,
Jul 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f285088be08c8844611c70577c85c8212e0f6fab commit f285088be08c8844611c70577c85c8212e0f6fab Author: Christian Dullweber <dullweber@chromium.org> Date: Mon Jul 16 09:18:12 2018 Only show cookie exception when signed in We should only show the exception about not getting signed out when the user is actually signed in and cookies will be recreated on deletion. Bug: 860290 Cq-Include-Trybots: luci.chromium.try:closure_compilation Change-Id: I03577189ec114513c2a341b4cf05eab47a8339fb Reviewed-on: https://chromium-review.googlesource.com/1136293 Reviewed-by: Martin Šrámek <msramek@chromium.org> Reviewed-by: Dave Schuyler <dschuyler@chromium.org> Reviewed-by: David Roger <droger@chromium.org> Commit-Queue: Christian Dullweber <dullweber@chromium.org> Cr-Commit-Position: refs/heads/master@{#575209} [modify] https://crrev.com/f285088be08c8844611c70577c85c8212e0f6fab/chrome/browser/browsing_data/counters/browsing_data_counter_utils.cc [modify] https://crrev.com/f285088be08c8844611c70577c85c8212e0f6fab/chrome/browser/browsing_data/counters/browsing_data_counter_utils.h [modify] https://crrev.com/f285088be08c8844611c70577c85c8212e0f6fab/chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.html [modify] https://crrev.com/f285088be08c8844611c70577c85c8212e0f6fab/chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.js [modify] https://crrev.com/f285088be08c8844611c70577c85c8212e0f6fab/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc [modify] https://crrev.com/f285088be08c8844611c70577c85c8212e0f6fab/chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
,
Jul 16
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dullweber@chromium.org
, Jul 4