New issue
Advanced search Search tips

Issue 860290 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

CBD shows "You won't be signed out of your Google account" when I'm not signed in

Project Member Reported by dullweber@chromium.org, Jul 4

Issue description

Chrome 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.
 
Description: Show this description
Cc: sabineb@chromium.org
Labels: Hotlist-DICE-Followup
Owner: droger@chromium.org
Status: Assigned (was: Untriaged)
Addigning to David and marking it as DICE follow-up work. We will get to it later (maybe in Q4).
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.

AccountConsistencyModeManager::IsDiceEnabledForProfile() should be the right check.
If Dice is enabled, Google cookies are not cleared from this page.
> 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()).
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).
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.
Owner: dullweber@chromium.org
Cc: droger@chromium.org
Project Member

Comment 10 by bugdroid1@chromium.org, 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

Status: Fixed (was: Assigned)

Sign in to add a comment