New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 653546 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

GaiaCookieManagerService::ListAccounts returns stale data

Project Member Reported by bzanotti@chromium.org, Oct 6 2016

Issue description

Preconditions:
1. Sign In to Chrome, with few accounts in SSO (A and B).

Steps to reproduce:
1. Cold start Chrome.
2. Go to Settings > Accounts > Add Account.
3. Add Account C to Chrome.
4. Navigate to https://accounts.google.com/SignOutOptions and observe A, B, and C are present.
5. Go to Settings > Accounts > Account C > Remove Account.
6. Navigate to https://accounts.google.com/SignOutOptions again and wait few seconds.

Observed results:
7. No accounts are available.

Expected results:
7. Accounts A and B are present, and only them. 

Additional comments:
GaiaCookieManagerService::ListAccounts returns its cached accounts unless they are marked as stale (via |list_accounts_stale_|).

Marking the listed accounts cache as stale is done:
* At the start of the GCMS.
* On cookie change notifications.

The later doesn't happen on iOS since Chrome is using WKWebView which doesn't have the necessary cookie APIs (we force it in very specific cases though).

The result is that adding an account or logging out all the accounts won't mark the listed accounts cache as stale, and GCMS::ListAccounts will keep returning the same accounts over and over.

The fix is to mark the cache as stale in GMCS::OnMergeSessionSuccess and GCMS::OnLogOutSuccess (the two requests that can modify the accounts available in the cookies). This won't have any effect on non-iOS platform, as the cache is already marked as stale when the cookie changes.
 
Summary: GaiaCookieManagerService::ListAccounts returns stale data (was: GaiaCookieManagerServer::ListAccounts returns stale data)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 6 2016

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

commit 987cf5a621eae6c77edf3774ba08b3d55bb3bd49
Author: bzanotti <bzanotti@chromium.org>
Date: Thu Oct 06 17:38:22 2016

Fix GCMS::ListAccounts returning stale data on iOS.

Marking the listed accounts cache as stale in GCMS is done by listening
to cookie changes (as this will catch any changes).
However, cookie changes notifications are not available on iOS, causing
the cache to be considered valid almost all the time.

This CL correctly marks the cache as stale for the most ususal actions
that will modify the listed accounts (AddAccount and LogOut).

BUG= 653546 

Review-Url: https://codereview.chromium.org/2394203002
Cr-Commit-Position: refs/heads/master@{#423576}

[modify] https://crrev.com/987cf5a621eae6c77edf3774ba08b3d55bb3bd49/components/signin/core/browser/gaia_cookie_manager_service.cc

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/987cf5a621eae6c77edf3774ba08b3d55bb3bd49

commit 987cf5a621eae6c77edf3774ba08b3d55bb3bd49
Author: bzanotti <bzanotti@chromium.org>
Date: Thu Oct 06 17:38:22 2016

Fix GCMS::ListAccounts returning stale data on iOS.

Marking the listed accounts cache as stale in GCMS is done by listening
to cookie changes (as this will catch any changes).
However, cookie changes notifications are not available on iOS, causing
the cache to be considered valid almost all the time.

This CL correctly marks the cache as stale for the most ususal actions
that will modify the listed accounts (AddAccount and LogOut).

BUG= 653546 

Review-Url: https://codereview.chromium.org/2394203002
Cr-Commit-Position: refs/heads/master@{#423576}

[modify] https://crrev.com/987cf5a621eae6c77edf3774ba08b3d55bb3bd49/components/signin/core/browser/gaia_cookie_manager_service.cc

Comment 5 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840

Sign in to add a comment