New issue
Advanced search Search tips

Issue 809452 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 729549
issue 796544
issue 809927
issue 840703



Sign in to add a comment

Convert //components/invalidation to talk to the Identity Service client library

Project Member Reported by blundell@chromium.org, Feb 6 2018

Issue description

GCMInvalidationBridge fetches access tokens via the IdentityProvider’s TokenService for the IdentityProvider’s active account ID. This IdentityProvider is the instance supplied to TiclInvalidationService (below).

TiclInvalidationService is an O2TS Observer *and* an IdentityProvider Observer and has synchronous usage of getting the active account ID via the IdentityProvider. This IdentityProvider instance can either be a ProfileIdentityProvider or a DeviceIdentityProvider:
-https://cs.chromium.org/chromium/src/chrome/browser/invalidation/profile_invalidation_provider_factory.cc?type=cs&q=%22TiclInvalidationService(%22&l=128 (look at the OS_CHROMEOS block above) 
-https://cs.chromium.org/chromium/src/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl.cc?type=cs&q=%22TiclInvalidationService(%22&l=305

Undertaking this project is thus blocked on determining the relationship between device identity and the Identity Service (see blocking bug).
 
Blocking: 796544
Components: Internals>Services>Identity
Status: Available (was: Untriaged)
Blocking: 809927
Finally, a sync integration test also supplies a ProfileIdentityProvider as the IdentityProvider to use for invalidations:

https://cs.chromium.org/chromium/src/chrome/browser/sync/test/integration/sync_test.cc?q=integration/sync_test.cc&dr&l=198
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 21 2018

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

commit a012b4cba417640093ea85e6c878395b5b28d5b9
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Feb 21 06:49:49 2018

[Invalidations] Remove InvalidationService::GetIdentityProvider()

This accessor is never used in the codebase. Removing it clarifies
the work necessary to convert //components/invalidation to use
the Identity Service client library.

Bug:  809452 
Change-Id: I419d3b5fe05d761785996dd6499fb4526092118d
Reviewed-on: https://chromium-review.googlesource.com/925704
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538040}
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/chrome/browser/sync/test/integration/fake_server_invalidation_service.cc
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/chrome/browser/sync/test/integration/fake_server_invalidation_service.h
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/chrome/browser/sync/test/integration/sync_test.cc
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/fake_invalidation_service.cc
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/fake_invalidation_service.h
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/invalidation_service_android.cc
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/invalidation_service_android.h
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/p2p_invalidation_service.cc
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/p2p_invalidation_service.h
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/ticl_invalidation_service.cc
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/impl/ticl_invalidation_service.h
[modify] https://crrev.com/a012b4cba417640093ea85e6c878395b5b28d5b9/components/invalidation/public/invalidation_service.h

Comment 6 by treib@chromium.org, Apr 18 2018

Cc: treib@chromium.org

Comment 7 by treib@chromium.org, May 8 2018

Blocking: 840703
Owner: blundell@chromium.org
Status: Started (was: Available)
I have removed all usage of IdentityProvider outside of //components/invalidation from the codebase. Notably, //components/invalidation is the only user of DeviceOAuth2TokenService in the codebase. At this time, we are not going to put device identity inside the Identity Service, meaning that we still need some abstraction layer for //components/identity to talk to either device identity or user identity. Here is my plan for converting //components/invalidation to talk to IdentityManager for user identity while preserving the above constraint:

- Move IdentityProvider into //components/invalidation (it is not intended to survive as an abstraction for general-purpose usage).
- Move ProfileIdentityProvider into //components/invalidation (similar to the above, this is not intended for general-purpose usage going forward).
- Tighten the IdentityProvider interface to exactly match //components/invalidation's needs and make it as simple as possible to refactor ProfileIdentityProvider to use IdentityManager rather than //components/signin. Most notably, eliminate the IdentityProvider::GetTokenService() accessor.
- Refactor ProfileIdentityProvider to be backed by IdentityManager rather than //components/signin.
Cc: melandory@chromium.org pav...@chromium.org
pavely@: Can you comment on the brief design outlined in c#8 from an owners POV?
melandory@: Can you confirm whether the work outlined in c#8 conflicts in any way with the refactoring that you're currently doing of invalidations?

If you folks are both OK with this, I'll get started ASAP.

(BTW, I missed a cosmetic step: after all the dust clears, I'll likely rename IdentityProvider and ProfileIdentityProvider to more precisely match their use case for invalidations).
The plan sounds good to me. The only reason GCMInvalidationBridge needs IdentityProvider is to request access token for active account and to invalidate the token if it gets rejected by server.
Looks good to me
Project Member

Comment 12 by bugdroid1@chromium.org, Jun 4 2018

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

commit bdd903dc9580e8b2ae137b79e06fe338f2afc873
Author: Colin Blundell <blundell@chromium.org>
Date: Mon Jun 04 15:00:53 2018

[Invalidation] Remove unused includes/deps on //components/signin

We will shortly be refactoring //components/invalidation to use the
next-generation C++ APIs for interacting with the user's Google
identities. To clarify the analysis of what is required to port this
code, this CL eliminates existing includes and deps that are not used
and muddy the waters.

Bug:  809452 
Change-Id: I7db38fc1673dbcfc8abaff949fddf26b4866be1b
Reviewed-on: https://chromium-review.googlesource.com/1082354
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564093}
[modify] https://crrev.com/bdd903dc9580e8b2ae137b79e06fe338f2afc873/components/invalidation/impl/BUILD.gn
[modify] https://crrev.com/bdd903dc9580e8b2ae137b79e06fe338f2afc873/components/invalidation/impl/gcm_invalidation_bridge.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 5 2018

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

commit 3e0efae59949e758ac634c7f91bf671d0343f1a0
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Jun 05 07:11:49 2018

Move ProfileIdentityProvider to //components/invalidation

IdentityProvider is a bespoke abstraction for interacting with the
"active account". ProfileIdentityProvider is an implementation of
IdentityProvider that defines the "active account" as the user's primary
account (i.e., the one blessed for sync).

These interfaces and implementations are now only used by
//components/invalidation. Neither of them are part of the long-term
vision for general-purpose APIs for accessing the user's Google
identities. However, they are still needed by //components/invalidation,
as the invalidation code also interacts with "device identity",
represented by DeviceIdentityProvider. Thus, we are narrowing their
scope to be invalidation-specific. This CL begins that narrowing of
scope by moving ProfileIdentityProvider into //components/invalidation.
An immediate followup will move IdentityProvider into
//components/invalidation. Note that we do the moves in this order as
to do them in the opposite order would introduce a (temporary)
circular dependency between //components/invalidation/impl and
//components/signin/core/browser.

Bug:  809452 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I59ebba6c9397b324bd075642eb625aa91c6296d1
Reviewed-on: https://chromium-review.googlesource.com/1084600
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564390}
[modify] https://crrev.com/3e0efae59949e758ac634c7f91bf671d0343f1a0/chrome/browser/invalidation/profile_invalidation_provider_factory.cc
[modify] https://crrev.com/3e0efae59949e758ac634c7f91bf671d0343f1a0/components/invalidation/impl/BUILD.gn
[rename] https://crrev.com/3e0efae59949e758ac634c7f91bf671d0343f1a0/components/invalidation/impl/profile_identity_provider.cc
[rename] https://crrev.com/3e0efae59949e758ac634c7f91bf671d0343f1a0/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/3e0efae59949e758ac634c7f91bf671d0343f1a0/components/signin/core/browser/BUILD.gn
[modify] https://crrev.com/3e0efae59949e758ac634c7f91bf671d0343f1a0/ios/chrome/browser/invalidation/ios_chrome_profile_invalidation_provider_factory.mm

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 5 2018

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

commit 81bc16278f4ae5b28bb200a36a53e32a6089f445
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Jun 05 10:04:37 2018

Move IdentityProvider to //components/invalidation

IdentityProvider is a bespoke abstraction for interacting with the
"active account". This interface is now only used by
//components/invalidation. It is not part of the long-term vision for
general-purpose APIs for accessing the user's Google identities.
However, is still needed by //components/invalidation, as the
invalidation code interacts with "device identity" and "user identity".
Thus, we are narrowing its scope to be invalidation-specific. This CL
moves IdentityProvider into //components/invalidation. A followup CL
will tighten the API of IdentityProvider to precisely match
invalidation's use case.

TBR=atwilson@

Bug:  809452 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I4298cea4f19f078089e861a3b37fd544148e4d33
Reviewed-on: https://chromium-review.googlesource.com/1085048
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564429}
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl.cc
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/chrome/browser/chromeos/settings/device_identity_provider.h
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/DEPS
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/BUILD.gn
[rename] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/fake_identity_provider.cc
[rename] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/fake_identity_provider.h
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/gcm_invalidation_bridge.cc
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/gcm_invalidation_bridge.h
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/gcm_invalidation_bridge_unittest.cc
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/ticl_invalidation_service.h
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/ticl_invalidation_service_unittest.cc
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/impl/ticl_profile_settings_provider_unittest.cc
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/public/BUILD.gn
[rename] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/public/identity_provider.cc
[rename] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/components/invalidation/public/identity_provider.h
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/google_apis/BUILD.gn
[modify] https://crrev.com/81bc16278f4ae5b28bb200a36a53e32a6089f445/ios/chrome/browser/invalidation/ios_chrome_profile_invalidation_provider_factory.mm

Project Member

Comment 15 by bugdroid1@chromium.org, Jun 5 2018

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

commit 349bb65c3023784f0126ce582457bb41718b5086
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Jun 05 12:11:24 2018

IdentityProvider: Remove dead API

This CL removes the now-unused IdentityProvider::GetActiveUsername().

TBR=bartfab@chromium.org

Bug:  809452 
Change-Id: I0c428b1833cb99a10fb035c30f6efabf55dbcefa
Reviewed-on: https://chromium-review.googlesource.com/1085054
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564460}
[modify] https://crrev.com/349bb65c3023784f0126ce582457bb41718b5086/chrome/browser/chromeos/settings/device_identity_provider.cc
[modify] https://crrev.com/349bb65c3023784f0126ce582457bb41718b5086/chrome/browser/chromeos/settings/device_identity_provider.h
[modify] https://crrev.com/349bb65c3023784f0126ce582457bb41718b5086/components/invalidation/impl/fake_identity_provider.cc
[modify] https://crrev.com/349bb65c3023784f0126ce582457bb41718b5086/components/invalidation/impl/fake_identity_provider.h
[modify] https://crrev.com/349bb65c3023784f0126ce582457bb41718b5086/components/invalidation/impl/profile_identity_provider.cc
[modify] https://crrev.com/349bb65c3023784f0126ce582457bb41718b5086/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/349bb65c3023784f0126ce582457bb41718b5086/components/invalidation/public/identity_provider.h

Project Member

Comment 16 by bugdroid1@chromium.org, Jun 5 2018

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

commit a212f4fac694ad2be20c680a1da600e76a124b49
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Jun 05 14:13:11 2018

IdentityProvider: Add and use IsActiveAccountAvailable() API

We are in the process of refactoring the API surface of IdentityProvider
to allow for changing ProfileIdentityProvider to be backed by
IdentityManager. In particular, we must remove all references to
OAuth2TokenService from the IdentityProvider API.

This CL takes a step toward that goal by introducang an
IdentityProvider::IsActiveAccountAvailable() API. This API allows for
eliminating client usage of IdentityProvider::GetTokenService() for
the purpose of querying whether a refresh token is available.

Note that the implementation of this API is currently essentially
identical across the three IdentityProvider implementations. That will
of course change when we refactor ProfileIdentityProvider to be
backed by IdentityManager.

TBR=bartfab@chromium.org

Bug:  809452 
Change-Id: Ia1cbd222ddbef6c3d4489a9364ee54fd772b99b5
Reviewed-on: https://chromium-review.googlesource.com/1085059
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564480}
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/chrome/browser/chromeos/settings/device_identity_provider.cc
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/chrome/browser/chromeos/settings/device_identity_provider.h
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/components/invalidation/impl/fake_identity_provider.cc
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/components/invalidation/impl/fake_identity_provider.h
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/components/invalidation/impl/profile_identity_provider.cc
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/components/invalidation/impl/ticl_invalidation_service.cc
[modify] https://crrev.com/a212f4fac694ad2be20c680a1da600e76a124b49/components/invalidation/public/identity_provider.h

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 6 2018

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

commit 217735e0fbf2f2032b6a0eab44dcca2f40492d9f
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Jun 06 10:10:28 2018

IdentityProvider: Merge its two observer APIs

IdentityProvider currently provides two observer APIs:
1. IdentityProvider::Observer, which forwards login/logout events
2. An "active account refresh token observer" API, which is a filtered
   version of OAuth2TokenService::Observer that forwards its events
   on only for the active account

This CL merges the second into the first, thus eliminating clients'
having to be OAuth2TokenService::Observers in order to observe active
account refresh token events via IdentityProvider. This CL is a step
on the path to eliminating visibility of OAuth2TokenService in the
IdentityProvider interface and allowing for an IdentityManager-backed
implementation of IdentityProvider.

Bug:  809452 
Change-Id: I002a3b2bd112a12ffcfa576089012461d8e06d11
Reviewed-on: https://chromium-review.googlesource.com/1085447
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564844}
[modify] https://crrev.com/217735e0fbf2f2032b6a0eab44dcca2f40492d9f/components/invalidation/impl/ticl_invalidation_service.cc
[modify] https://crrev.com/217735e0fbf2f2032b6a0eab44dcca2f40492d9f/components/invalidation/impl/ticl_invalidation_service.h
[modify] https://crrev.com/217735e0fbf2f2032b6a0eab44dcca2f40492d9f/components/invalidation/public/identity_provider.cc
[modify] https://crrev.com/217735e0fbf2f2032b6a0eab44dcca2f40492d9f/components/invalidation/public/identity_provider.h

Project Member

Comment 18 by bugdroid1@chromium.org, Jun 6 2018

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

commit 3b37c4e4913639d346efc02cb8dfeed80c3b1b27
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Jun 06 11:35:43 2018

Eliminate FakeIdentityProvider

As a convenience step in the porting of the IdentityProvider interface
and implementation away from OAuth2TokenService, this CL eliminates
FakeIdentityProvider. This change reduces the number of IdentityProvider
implementations to two and will avoid duplication of code between
FakeIdentityProvider and DeviceIdentityProvider when
IdentityProvider and ProfileIdentityProvider are later ported away from
knowledge of/dependence on OAuth2TokenService.

The change is straightforward: The remaining tests that use
FakeIdentityProvider don't use any of its custom hooks for testing. All
that was necessary was to let ProfileIdentityProvider have a test
constructor that allows for its SigninManager instance to be null. This
avoids the need for these tests to construct a FakeSigninManager(Base)
instance, which is a pain to do. Once ProfileIdentityProvider is
converted to take in IdentityManager, this test constructor can go
away, as it is a breeze to construct IdentityManager in a testing
context via IdentityTestEnvironment.

Bug:  809452 
Change-Id: I2f699d6776542626800e59bda731579338076077
Reviewed-on: https://chromium-review.googlesource.com/1087054
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564856}
[modify] https://crrev.com/3b37c4e4913639d346efc02cb8dfeed80c3b1b27/components/invalidation/impl/BUILD.gn
[delete] https://crrev.com/f0bd58341f6ae77974df0d01cf7c8e456c570101/components/invalidation/impl/fake_identity_provider.cc
[delete] https://crrev.com/f0bd58341f6ae77974df0d01cf7c8e456c570101/components/invalidation/impl/fake_identity_provider.h
[modify] https://crrev.com/3b37c4e4913639d346efc02cb8dfeed80c3b1b27/components/invalidation/impl/gcm_invalidation_bridge_unittest.cc
[modify] https://crrev.com/3b37c4e4913639d346efc02cb8dfeed80c3b1b27/components/invalidation/impl/profile_identity_provider.cc
[modify] https://crrev.com/3b37c4e4913639d346efc02cb8dfeed80c3b1b27/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/3b37c4e4913639d346efc02cb8dfeed80c3b1b27/components/invalidation/impl/ticl_invalidation_service_unittest.cc
[modify] https://crrev.com/3b37c4e4913639d346efc02cb8dfeed80c3b1b27/components/invalidation/impl/ticl_profile_settings_provider_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 8 2018

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

commit 646f29b494be0543268ae0edb2fa625275511f63
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Jun 08 08:10:05 2018

IdentityProvider: Provide explicit interface to fetch access tokens

One of IdentityProvider's clients' use cases is to fetch access tokens
for the active account. IdentityProvider currently provides no explicit
mechanism for this, instead letting clients directly interact with the
underlying OAuth2TokenService instance via GetTokenService(). However,
it's necessary to decouple IdentityProvider from OAuth2TokenService at
an interface level in order to allow for porting ProfileIdentityProvider
to be backed by IdentityManager.

This CL adds an explicit interface for clients of IdentityProvider to
request access tokens for the active account. This interface is
currently implemented directly in IdentityProvider itself, as it is
common to ProfileIdentityProvider and DeviceIdentityProvider. Before
porting ProfileIdentityProvider to be backed by IdentityManager we
will move this implementation out into those two derived classes,
temporarily duplicating it. However, we will only do that when we are
just at the point of porting ProfileIdentityProvider away from using
ProfileOAuth2TokenService.

Bug:  809452 
Change-Id: I402af0cb19b5e0057edc3870ca2a8f564f4f8e10
Reviewed-on: https://chromium-review.googlesource.com/1089051
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565584}
[modify] https://crrev.com/646f29b494be0543268ae0edb2fa625275511f63/components/invalidation/impl/gcm_invalidation_bridge.cc
[modify] https://crrev.com/646f29b494be0543268ae0edb2fa625275511f63/components/invalidation/impl/gcm_invalidation_bridge.h
[modify] https://crrev.com/646f29b494be0543268ae0edb2fa625275511f63/components/invalidation/impl/ticl_invalidation_service.cc
[modify] https://crrev.com/646f29b494be0543268ae0edb2fa625275511f63/components/invalidation/impl/ticl_invalidation_service.h
[modify] https://crrev.com/646f29b494be0543268ae0edb2fa625275511f63/components/invalidation/public/identity_provider.cc
[modify] https://crrev.com/646f29b494be0543268ae0edb2fa625275511f63/components/invalidation/public/identity_provider.h

Project Member

Comment 20 by bugdroid1@chromium.org, Jun 8 2018

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

commit 0dba1124f15341162a6cf9b646d9ee41c6b71ae0
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Jun 08 13:06:03 2018

Introduce IdentityProvider interface for invalidating access tokens

One of IdentityProvider's clients' use cases is to invalidate access
tokens for the active account. IdentityProvider currently provides no
explicit mechanism for this, instead letting clients directly interact
with the underlying OAuth2TokenService instance via GetTokenService().
However, it's necessary to decouple IdentityProvider from
OAuth2TokenService at an interface level in order to allow for porting
ProfileIdentityProvider to be backed by IdentityManager.

This CL adds an explicit interface for clients of IdentityProvider to
invalidate access tokens for the active account. This interface is
currently implemented directly in IdentityProvider itself, as it is
common to ProfileIdentityProvider and DeviceIdentityProvider. Before
porting ProfileIdentityProvider to be backed by IdentityManager we
will move this implementation out into those two derived classes,
temporarily duplicating it. However, we will only do that when we are
just at the point of porting ProfileIdentityProvider away from using
ProfileOAuth2TokenService.

This CL finishes eliminating the exposure of OAuth2TokenService to
IdentityProvider clients. The next step is to decouple the
IdentityProvider implementation itself from the token service.

Bug:  809452 
Change-Id: Idf3d2b35fa88b5b96dfb7204268e298222032d3d
Reviewed-on: https://chromium-review.googlesource.com/1089056
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565615}
[modify] https://crrev.com/0dba1124f15341162a6cf9b646d9ee41c6b71ae0/components/invalidation/impl/gcm_invalidation_bridge.cc
[modify] https://crrev.com/0dba1124f15341162a6cf9b646d9ee41c6b71ae0/components/invalidation/impl/ticl_invalidation_service.cc
[modify] https://crrev.com/0dba1124f15341162a6cf9b646d9ee41c6b71ae0/components/invalidation/impl/ticl_invalidation_service.h
[modify] https://crrev.com/0dba1124f15341162a6cf9b646d9ee41c6b71ae0/components/invalidation/public/identity_provider.cc
[modify] https://crrev.com/0dba1124f15341162a6cf9b646d9ee41c6b71ae0/components/invalidation/public/identity_provider.h

Project Member

Comment 21 by bugdroid1@chromium.org, Jun 13 2018

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

commit 51f1f5499ffceb81362520449f3918d4de8ec164
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Jun 13 16:12:57 2018

Add DISALLOW_COPY_AND_ASSIGN to ActiveAccountAccessTokenFetcher

Tiny followup to https://chromium-review.googlesource.com/1089051.

Bug:  809452 
Change-Id: Ibbc31c786b5a88fdc90aaeca38740a7d95c5d275
Reviewed-on: https://chromium-review.googlesource.com/1098961
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566856}
[modify] https://crrev.com/51f1f5499ffceb81362520449f3918d4de8ec164/components/invalidation/public/identity_provider.h

Project Member

Comment 22 by bugdroid1@chromium.org, Jun 28 2018

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

commit e54e5b9a2092fc38b288401bd0aefb3a858ad940
Author: Colin Blundell <blundell@chromium.org>
Date: Thu Jun 28 10:51:37 2018

Move ActiveAccountAccessTokenFetcherImpl out of IdentityProvider

IdentityProvider currently has baked-in knowledge of OAuth2TokenService
for functionality that is shared between ProfileIdentityProvider and
DeviceIdentityProvider. However, as we will be converting
ProfileIdentityProvider to talk to IdentityManager rather than
OAuth2TokenService, this baked-in knowledge needs to be moved out of
IdentityProvider; ultimately, it will end up only in
DeviceIdentityProvider.

This CL takes the first step along that path by moving
ActiveAccountAccessTokenFetcherImpl, which fetches access tokens via
OAuth2TokenService, out of identity_provider.cc into its own file. The
next CL will have ProfileIdentityProvider and DeviceIdentityProvider
each implement FetchAccessToken() rather than having IdentityProvider do
so directly.

Bug:  809452 
Change-Id: I664c4da998cf90d0a398dc2c9e309425f95515a3
Reviewed-on: https://chromium-review.googlesource.com/1114604
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571070}
[modify] https://crrev.com/e54e5b9a2092fc38b288401bd0aefb3a858ad940/components/invalidation/public/BUILD.gn
[add] https://crrev.com/e54e5b9a2092fc38b288401bd0aefb3a858ad940/components/invalidation/public/active_account_access_token_fetcher_impl.cc
[add] https://crrev.com/e54e5b9a2092fc38b288401bd0aefb3a858ad940/components/invalidation/public/active_account_access_token_fetcher_impl.h
[modify] https://crrev.com/e54e5b9a2092fc38b288401bd0aefb3a858ad940/components/invalidation/public/identity_provider.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Jun 28 2018

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

commit 7b50d3321a7c7a2074959462e86e25d9bd4ab495
Author: Colin Blundell <blundell@chromium.org>
Date: Thu Jun 28 14:42:52 2018

Move IdentityProvider interaction with access tokens into subclasses

IdentityProvider currently has baked-in knowledge of OAuth2TokenService
for functionality that is shared between ProfileIdentityProvider and
DeviceIdentityProvider. However, as we will be converting
ProfileIdentityProvider to talk to IdentityManager rather than
OAuth2TokenService, this baked-in knowledge needs to be moved out of
IdentityProvider; ultimately, it will end up only in
DeviceIdentityProvider.

This CL takes a step along that path by moving the IdentityProvider
method implementations that interact with access tokens into the two
derived classes, making fetching and invalidating access tokens virtual
methods of IdentityProvider.

TBR=pastaramovj@chromium.org

Bug:  809452 
Change-Id: I7b80f54d783dfa4de3236aec46c663bbfcb92422
Reviewed-on: https://chromium-review.googlesource.com/1114611
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571120}
[modify] https://crrev.com/7b50d3321a7c7a2074959462e86e25d9bd4ab495/chrome/browser/chromeos/settings/device_identity_provider.cc
[modify] https://crrev.com/7b50d3321a7c7a2074959462e86e25d9bd4ab495/chrome/browser/chromeos/settings/device_identity_provider.h
[modify] https://crrev.com/7b50d3321a7c7a2074959462e86e25d9bd4ab495/components/invalidation/impl/profile_identity_provider.cc
[modify] https://crrev.com/7b50d3321a7c7a2074959462e86e25d9bd4ab495/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/7b50d3321a7c7a2074959462e86e25d9bd4ab495/components/invalidation/public/identity_provider.cc
[modify] https://crrev.com/7b50d3321a7c7a2074959462e86e25d9bd4ab495/components/invalidation/public/identity_provider.h

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 29 2018

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

commit d0b89cd89acb3076c52bd2fff38bab1128ed2790
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Jun 29 13:39:00 2018

Move OAuth2TokenService observing out of IdentityProvider

IdentityProvider currently has baked-in knowledge of OAuth2TokenService
for functionality that is shared between ProfileIdentityProvider and
DeviceIdentityProvider. However, as we will be converting
ProfileIdentityProvider to talk to IdentityManager rather than
OAuth2TokenService, this baked-in knowledge needs to be moved out of
IdentityProvider; ultimately, it will end up only in
DeviceIdentityProvider.

As the last step of removing this knowledge from IdentityProvider, this
CL moves IdentityProvider's observing of OAuth2TokenService into its
two derived classes. ProfileIdentityProvider is now ready to be
converted to use IdentityManager; that work will shortly follow.

Bug:  809452 
Change-Id: I9beaae509b2aae22308c0bd47f962a42cba93f8d
Reviewed-on: https://chromium-review.googlesource.com/1114855
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571458}
[modify] https://crrev.com/d0b89cd89acb3076c52bd2fff38bab1128ed2790/chrome/browser/chromeos/settings/device_identity_provider.cc
[modify] https://crrev.com/d0b89cd89acb3076c52bd2fff38bab1128ed2790/chrome/browser/chromeos/settings/device_identity_provider.h
[modify] https://crrev.com/d0b89cd89acb3076c52bd2fff38bab1128ed2790/components/invalidation/impl/profile_identity_provider.cc
[modify] https://crrev.com/d0b89cd89acb3076c52bd2fff38bab1128ed2790/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/d0b89cd89acb3076c52bd2fff38bab1128ed2790/components/invalidation/public/identity_provider.cc
[modify] https://crrev.com/d0b89cd89acb3076c52bd2fff38bab1128ed2790/components/invalidation/public/identity_provider.h

Project Member

Comment 25 by bugdroid1@chromium.org, Jul 13

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

commit 2a96a6919d313c720fd841ab67e6c1587779fd90
Author: Colin Blundell <blundell@chromium.org>
Date: Fri Jul 13 08:57:51 2018

[invalidation] Port ProfileIdentityProvider to use IdentityManager

This CL ports ProfileIdentityProvider to talk to IdentityManager rather
than SigninManager and ProfileOAuth2TokenService. The conversion is
straightforward after a long line of preparatory work.

Bug:  809452 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ifa638f1b7a9c77089bb87fd7215e60041c762d3a
Reviewed-on: https://chromium-review.googlesource.com/1118163
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574865}
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/chrome/browser/invalidation/profile_invalidation_provider_factory.cc
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/components/invalidation/impl/BUILD.gn
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/components/invalidation/impl/DEPS
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/components/invalidation/impl/gcm_invalidation_bridge_unittest.cc
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/components/invalidation/impl/profile_identity_provider.cc
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/components/invalidation/impl/profile_identity_provider.h
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/components/invalidation/impl/ticl_invalidation_service_unittest.cc
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/components/invalidation/impl/ticl_profile_settings_provider_unittest.cc
[modify] https://crrev.com/2a96a6919d313c720fd841ab67e6c1587779fd90/ios/chrome/browser/invalidation/ios_chrome_profile_invalidation_provider_factory.mm

Status: Fixed (was: Started)

Sign in to add a comment