New issue
Advanced search Search tips

Issue 899175 link

Starred by 2 users
Status: Fixed
Owner:
ma...@igalia.com
Closed: Oct 29
Cc:
blundell@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Migrate IdentityManager APIs to use identity::ScopeSet

Project Member Reported by ma...@igalia.com, Oct 26 
As referenced from a review comment for CL 1291314[1], OAuth2TokenService::ScopeSet (defined in [2]) is being used at the moment in the API of the IdentityManager everywhere [3], which hints clients of the IdentityManager API into including oauth2_token_service.h just to use that typedef in order to define an ScopeSet and pass it to the relevant functions.

Since we're migrating many parts of the code base away from O2TS and into the Identity service, it would be nicer if we didn't have to rely on that definition when using the IdentityManager APIs and use identity::ScopeSet, which is equivalent to O2TS:ScopeSet and is defined in //services/identity/public/cpp/scope_set.h [4].

This ticket would take care of updating the IdentityManager's API and relevant callers.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 29 

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

commit c56d53c422b314d564c4127b08069fd8a7d6a7bd
Author: Mario Sanchez Prada <mario@igalia.com>
Date: Mon Oct 29 12:31:16 2018

Migrate IdentityManager APIs to use identity::ScopeSet

Move all public methods from IdentityManager to using identity::ScopeSet
instead of the equivalent type definition OAuth2TokenService::ScopeSet.
This will make it clearer when migrating from SigninManager[Base] and
[Profile]OAuth2TokenService to the IdentityManager, as code will be
explicitly using a type definition from the identity service.

As a side effect, this patch changes the signature of the constructors
for AccessTokenFetcher and PrimaryAccountAccessTokenFetcher, and update
clients of these APIs wherever needed.

TBR=sky@chromium.org,dvadym@chromium.org,jkrcal@chromium.org,fgorski@chromium.org,peter@chromium.org,sdefresne@chromium.org,dcheng@chromium.org,fgorski@chromium.org,jdonnelly@chromium.org,mathp@chromium.org,hashimoto@chromium.org

Bug:  899175 
Change-Id: I4bda23f4e928cf94654d7c4ce2eb7961ff91dd9e
Reviewed-on: https://chromium-review.googlesource.com/c/1301520
Reviewed-by: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Cr-Commit-Position: refs/heads/master@{#603468}
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/chrome/browser/feedback/feedback_uploader_chrome.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/chrome/browser/printing/cloud_print/gcd_api_flow_impl.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/chrome/browser/profiles/profile_downloader.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/chrome/browser/safe_browsing/advanced_protection_status_manager.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/chrome/browser/search/background/ntp_background_service.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/autofill/core/browser/payments/payments_client.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/browser_sync/sync_auth_manager.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/feed/core/feed_networking_host.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/gcm_driver/account_tracker.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/gcm_driver/gcm_account_tracker.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/history/core/browser/web_history_service.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/invalidation/impl/profile_identity_provider.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/ntp_snippets/breaking_news/subscription_manager_impl.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/ntp_snippets/remote/remote_suggestions_fetcher_impl.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/omnibox/browser/contextual_suggestions_service.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/omnibox/browser/document_suggestions_service.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/components/suggestions/suggestions_service_impl.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/google_apis/drive/auth_service.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/BUILD.gn
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/access_token_fetcher.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/access_token_fetcher.h
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/identity_manager.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/identity_manager.h
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/identity_manager_unittest.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/identity_test_environment.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/identity_test_environment.h
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/primary_account_access_token_fetcher.cc
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/primary_account_access_token_fetcher.h
[modify] https://crrev.com/c56d53c422b314d564c4127b08069fd8a7d6a7bd/services/identity/public/cpp/scope_set.h

Comment 3 by ma...@igalia.com, Oct 29

Status: Fixed (was: Started)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 7 

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

commit 2b7db98a72d96f4b9107881d330fc3fa7e7acba3
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Nov 07 15:36:17 2018

[s13n] Convert UserCloudPolicyTokenForwarder to PrimaryAccountAccessTokenFetcher

This is a follow up of blundell@ request in [0]. It brings no functionality, but
eliminates the need of UserCloudPolicyTokenForwarder to directly inherit from
IdentityManager::Observer.

[0] https://crrev.com/c/1299313/3/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc#78

For completeness, this CL is a 4/n CL that migrates UserCloudPolicyTokenForwarder to
IdentityManager. Previous two CLs were [1], [2] and [3].

[1] https://crrev.com/c/1296873
[2] https://crrev.com/c/1298775
[3] https://crrev.com/c/1299313.

It also takes the opportunity to convert the use of OAuth2TokenService::ScopeSet
to Identity::ScopeSet (driven-by).

BUG= 893504 , 899175 

Change-Id: I907520ed211debe3fbd1a4c21b341da4b0a81dd8
Reviewed-on: https://chromium-review.googlesource.com/c/1319770
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#606050}
[modify] https://crrev.com/2b7db98a72d96f4b9107881d330fc3fa7e7acba3/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.cc
[modify] https://crrev.com/2b7db98a72d96f4b9107881d330fc3fa7e7acba3/chrome/browser/chromeos/policy/user_cloud_policy_token_forwarder.h
►

Sign in to add a comment