Issue 639335 link

Starred by 2 users

Issue metadata

Status:	Archived
Owner:	zea@chromium.org
Closed:	Aug 2016
Cc:	mastiz@chromium.org █ tschumann@chromium.org
Components:	Services>Sync
EstimatedDays:	----
NextAction:	----
OS:	Chrome
Pri:	1
Type:	Bug
M-53 Hotlist-Merge-Approved merge-merged-2785 VerifyIn-57 VerifyIn-58 VerifyIn-59 VerifyIn-60 VerifyIn-61

Sync using wrong ID to check account equivalence

Project Member Reported by zea@chromium.org, Aug 19 2016

Issue description

The GAIA ID is not necessarily the same as account id on all platforms. As such, ProfileSyncService::OnGaiaAccountsInCookieUpdated does not properly check for account equivalence on CrOS.

Comment 1 by skym@google.com, Aug 19 2016

I don't think this is going to help this particular issue, but FYI, https://cs.chromium.org/chromium/src/components/signin/core/account_id/account_id.cc?l=67 is account equivalence logic that I believe was written for the ChromeOS use case.

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 19 2016

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

commit 5ca485b1159f942c41fa94978c3ba672928eb939
Author: zea <zea@chromium.org>
Date: Fri Aug 19 19:26:13 2016

[Sync] Fix account id comparison for cookie jar

Gaia ID != Account ID in all cases. |id| is the proper variable to use.

BUG= 639335 

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

[modify] https://crrev.com/5ca485b1159f942c41fa94978c3ba672928eb939/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
[modify] https://crrev.com/5ca485b1159f942c41fa94978c3ba672928eb939/components/browser_sync/browser/profile_sync_service.cc

Comment 3 by zea@chromium.org, Aug 19 2016

Labels: Merge-Request-53 M-53

Since it's such a small change, would be great to merge this into M53 to fix the CrOS metrics.

Comment 4 by dimu@chromium.org, Aug 20 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved

Your change meets the bar and is auto-approved for M53 (branch: 2785)

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 22 2016

Labels: -merge-approved-53 merge-merged-2785

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

commit ecb67345d631b774b949bdfce08c8a3f41bdd799
Author: Nicolas Zea <zea@chromium.org>
Date: Mon Aug 22 18:29:40 2016

[Sync] Fix account id comparison for cookie jar

Gaia ID != Account ID in all cases. |id| is the proper variable to use.

BUG= 639335 

Review-Url: https://codereview.chromium.org/2266483002
Cr-Commit-Position: refs/heads/master@{#413210}
(cherry picked from commit 5ca485b1159f942c41fa94978c3ba672928eb939)

Review URL: https://codereview.chromium.org/2261243003 .

Cr-Commit-Position: refs/branch-heads/2785@{#707}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/ecb67345d631b774b949bdfce08c8a3f41bdd799/chrome/browser/sync/test/integration/single_client_sessions_sync_test.cc
[modify] https://crrev.com/ecb67345d631b774b949bdfce08c8a3f41bdd799/components/browser_sync/browser/profile_sync_service.cc