New issue
Advanced search Search tips

Issue 906023 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 16
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 883318
issue 905243
issue 906025



Sign in to add a comment

Implement identity::UbertokenFetcher to enable porting consumers of existing UbertokenFetcher class

Project Member Reported by blundell@chromium.org, Nov 16

Issue description

UbertokenFetcher takes in an OAuth2TokenService instance, which in practice is usually a ProfileOAuth2TokenService instance. Hence, we need a solution here to fully migrate usage of PO2TS.
 
Blocking: 906025
Blocking: 905243
Owner: blundell@chromium.org
Status: Started (was: Available)
Labels: Proj-Servicification-VendorBug
Owner: ----
Status: Available (was: Started)
Summary: Implement identity::UbertokenFetcher to enable porting consumers of existing UbertokenFetcher class (was: Determine design for porting usage of UbertokenFetcher to IdentityManager)
Design doc is now finalized. Conclusion:

- UbertokenFetcher will be inside IdentityManager.
- It will be exposed to consumers via a new identity::UbertokenFetcher class whose design should be as closely parallel to that of identity::AccessTokenFetcher as possible.

For ideas on developing the unittest of identity::UbertokenFetcher, see the existing ubertoken_fetcher_unittest.cc file; hopefully the ways that the underlying class is driven there can be reused/adapted for testing the new layer.
Owner: toniki...@chromium.org
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 14

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

commit a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Mon Jan 14 13:17:57 2019

Refine UbertokenFetcher API surface to work similarly to identity::AccessTokenFetcher

This CL is the 1 out of n CLs to incorporate UbertokenFetcher into the
IdentityManager API.
It mainly changes the existing UbertokenFetcher API surface [1], as following,
to comply with identity::AccessTokenFetcher:

1) StartFetchingToken and StartFetchingTokenWithToken APIs are removed.

Similarly to identity::AccessTokenFetcher, by the time an instance of UbertokenFetcher
is created the fetch procedure gets triggered.

2) UbertokenFetcher::set_is_bound_to_channel_id API is removed, in favor of
a parameter in the UbertokenFetcher ctors.

In order to be able to start its fetching procedure right at its creation,
identity::UbertokenFetcher needs get configure when it is constructed.
Because of this, the previous UbertokenFetcher::set_is_bound_to_channel_id
API is replaced by a parameter in its UbertokenFetcher ctors.

3) Removal of UbertokenConsumer API

Similarly, to identity::AccessTokenFetcher, UbertokenFetcher now takes on
its ctor a completion callback, to be called when the fetch procedure completes,
either successfully or not.
This allows the removal of UbertokenConsumer class, and existing specializations
of it do not need to override OnUbertokenSuccess and OnUbertokenFailure methods
anymore.

[1] //google_apis/gaia/ubertoken_fetcher.cc|h

TBR=khmel@chromium.org (chrome/browser/chromeos/arc/auth/)

BUG= 906023 

Change-Id: Iee4532ee3eb85af64574462d1337711332da9865
Reviewed-on: https://chromium-review.googlesource.com/c/1407009
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622438}
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/chrome/browser/chromeos/arc/auth/arc_auth_context.cc
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/chrome/browser/chromeos/arc/auth/arc_auth_context.h
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/chrome/browser/extensions/api/identity/gaia_web_auth_flow.cc
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/chrome/browser/extensions/api/identity/gaia_web_auth_flow.h
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/chrome/browser/extensions/api/identity/gaia_web_auth_flow_unittest.cc
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/components/signin/core/browser/gaia_cookie_manager_service.cc
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/components/signin/core/browser/gaia_cookie_manager_service.h
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/components/signin/core/browser/gaia_cookie_manager_service_unittest.cc
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/google_apis/gaia/ubertoken_fetcher.cc
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/google_apis/gaia/ubertoken_fetcher.h
[modify] https://crrev.com/a24d192a5feaedcacee4bf8c4d3fe697b33ad2dc/google_apis/gaia/ubertoken_fetcher_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 14

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

commit f9825b542939184ba74f1049f174ef40ac330ea1
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Mon Jan 14 14:37:14 2019

Move UbertokenFetcher into //components/signin/core/browser (:internals target)

This CL is the 2nd out n CLs to incorporate UbertokenFetcher into the
IdentityManager API.
It basically moves the implementation of UbertokenFetcher from
//google_apis/gaia to //components/signin/core/browser
(::signin_internals target).

Additionally, it also puts UbertokenFetcher under signin:: namespace.

TBR=khmel@chromium.org (for chrome/browser/chromeos/arc/auth/).

BUG= 906023 

Change-Id: If4277cb5094c39a53ae863062a1bf4db84d33539
Reviewed-on: https://chromium-review.googlesource.com/c/1407010
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622448}
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/chrome/browser/chromeos/arc/auth/arc_auth_context.cc
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/chrome/browser/chromeos/arc/auth/arc_auth_context.h
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/chrome/browser/extensions/api/identity/gaia_web_auth_flow.cc
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/chrome/browser/extensions/api/identity/gaia_web_auth_flow.h
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/components/signin/core/browser/BUILD.gn
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/components/signin/core/browser/gaia_cookie_manager_service.cc
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/components/signin/core/browser/gaia_cookie_manager_service.h
[rename] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/components/signin/core/browser/ubertoken_fetcher.cc
[rename] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/components/signin/core/browser/ubertoken_fetcher.h
[rename] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/components/signin/core/browser/ubertoken_fetcher_unittest.cc
[modify] https://crrev.com/f9825b542939184ba74f1049f174ef40ac330ea1/google_apis/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 16

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

commit 7266f8d4a3797203b45ddb918af3d041a0f6482a
Author: Antonio Gomes <tonikitoo@igalia.com>
Date: Wed Jan 16 01:46:58 2019

Prepare UbertokenFetcher to have a base (public) parent class

This CL is the 3rd out of 4 CLs in order to make UbertokenFetcher creation
methods available through IdentityManager APIs.

Basically, it is a preparation CL that renames the current
UbertokenFetcher class and file to UbertokenFetcherImpl and
ubertoken_fetcher_impl.cc|h, in order to more easily accommodate
the introduction of a base class UbertokenFetcher.

Note that strictly speaking, this is is not a requirement (ie
the introduction of the base class and the rename of this existing
implementation class and files could happen on the same CL).
However, for the same of an easier diff to review, this step was
split in 2.

BUG= 906023 

TBR=khmel@chromium.org (for chrome/browser/chromeos/arc/auth/).

Change-Id: Ic9e35444efa5dd913d37de598e0ec334cc29b05e
Reviewed-on: https://chromium-review.googlesource.com/c/1409642
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622998}
[modify] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/chrome/browser/chromeos/arc/auth/arc_auth_context.cc
[modify] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/chrome/browser/chromeos/arc/auth/arc_auth_context.h
[modify] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/chrome/browser/extensions/api/identity/gaia_web_auth_flow.cc
[modify] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/chrome/browser/extensions/api/identity/gaia_web_auth_flow.h
[modify] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/components/signin/core/browser/BUILD.gn
[modify] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/components/signin/core/browser/gaia_cookie_manager_service.cc
[modify] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/components/signin/core/browser/gaia_cookie_manager_service.h
[rename] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/components/signin/core/browser/ubertoken_fetcher_impl.cc
[rename] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/components/signin/core/browser/ubertoken_fetcher_impl.h
[rename] https://crrev.com/7266f8d4a3797203b45ddb918af3d041a0f6482a/components/signin/core/browser/ubertoken_fetcher_impl_unittest.cc

Status: Fixed (was: Started)
Fixed by https://chromium-review.googlesource.com/c/chromium/src/+/1407012.

Sign in to add a comment