New issue
Advanced search Search tips

Issue 729536 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Migrate Identity extensions API impl from calling AccountTracker::GetAccounts() to calling PO2TS::GetAccounts()

Project Member Reported by blundell@chromium.org, Jun 5 2017

Issue description

As detailed in  crbug.com/729533 , ProfileOAuth2TokenService::GetAccounts() is the function that the Identity Service will expose. Hence, as a preliminary step to converting the implementation of chrome.identity.getAccounts() to use the Identity Service, we should first migrate it away from using AccountTracker::GetAccounts() to using PO2TS::GetAccounts().

Note that we will have to carefully analyze the relationship between these two methods to do this correctly.
 
Labels: IdentityService
Blocking: 683124
Blocking: 729546
Blocking: 729537
Blocking: 729589
Owner: blundell@chromium.org
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 20 2017

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

commit 2e73f36bd9cc84ffe7db1b15fc12bc486b21ccbd
Author: Colin Blundell <blundell@chromium.org>
Date: Thu Jul 20 09:44:54 2017

Convert chrome.identity.getAccounts() impl away from AccountTracker

As a precursor to changing the implementation of
chrome.identity.getAccounts() to use the Identity Service, this CL
moves that implementation away from using AccountTracker::GetAccounts()
to using OAuth2TokenService::GetAccounts(). The latter is the function
that we intend to back the upcoming Identity Service API.

AccountTracker::GetAccounts() is fundamentally based on
OAuth2TokenService::GetAccounts(): they both have the basic behavior
of returning the informations of all accounts for which a refresh
token is available. However, AccountTracker::GetAccounts() layers
some extra information on top relating to the primary account (a
concept of which the OAuth2TokenService is not aware). (AccountTracker
itself gets this information via IdentityProvider; in the context of
the chrome.identity.getAccounts() impl, this is ProfileIdentityProvider,
which is backed by SigninManager).

To avoid any behavioral changes, this CL ports the particular semantics
of AccountTracker::GetAccounts() to live directly in the implementation
of chrome.identity.getAccounts():
- Returns an empty list if there is no primary account or there is no
  refresh token available for the primary account
- Otherwise, puts the primary account first in the returned list

Bug:  729536 
Change-Id: Ib4f6cff0afd2f751ec3a7409d24b797b1be0ec79
Reviewed-on: https://chromium-review.googlesource.com/574709
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488177}
[modify] https://crrev.com/2e73f36bd9cc84ffe7db1b15fc12bc486b21ccbd/chrome/browser/extensions/api/identity/identity_api.cc
[modify] https://crrev.com/2e73f36bd9cc84ffe7db1b15fc12bc486b21ccbd/chrome/browser/extensions/api/identity/identity_api.h
[modify] https://crrev.com/2e73f36bd9cc84ffe7db1b15fc12bc486b21ccbd/chrome/browser/extensions/api/identity/identity_apitest.cc
[modify] https://crrev.com/2e73f36bd9cc84ffe7db1b15fc12bc486b21ccbd/chrome/browser/extensions/api/identity/identity_get_accounts_function.cc

Status: Fixed (was: Started)
Components: Internals>Services>Identity

Sign in to add a comment