New issue
Advanced search Search tips

Issue 809435 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 796544
issue 809927



Sign in to add a comment

Convert Autofill to talk to Identity Service client library

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

Issue description

PaymentsClient makes access token requests for the active account via IdentityProvider. I verified that the IdentityProvider being use here is a ProfileIdentityProvider, as it is supplied either by AutofillClient or by CvcUnmaskViewController, both of which supply a ProfileIdentityProvider. Hence it should be feasible to instead use a PrimaryAccountAccessTokenFetcher.

PersonalDataManager gets the authenticated account info (via SigninManager/AccountTrackerService) to merge in data that came from the server. This will be straightforward to convert to talking to IdentityManager.

 
Blocking: 796544
Components: Internals>Services>Identity
Status: Available (was: Untriaged)
Status: Started (was: Available)
As part of this work, Autofill's usage of IdentityProvider should be eliminated and AutofillClient::GetIdentityProvider() removed.
Owner: blundell@chromium.org
Project Member

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

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

commit 3520fb171ac7e64b2ca8ea53c093679aceabcf96
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Feb 20 16:18:29 2018

[Autofill] Change PaymentsClient to use IdentityManager

This CL change's Autofill's PaymentsClient class to interact with the
user's Google identity via IdentityManager rather than IdentityProvider.
This change should have no behavioral effect: the IdentityProvider
being supplied is always a ProfileIdentityProvider, which is backed
by the same SigninManager and ProfileOAuth2TokenService instances as
the IdentityManager instance now being supplied (see  crbug.com/809435 
for the details of exactly where the IdentityProvider instance comes
from in all cases).

I tested this change manually on Linux with a fake credit card following
the instructions given in
https://chromium-review.googlesource.com/c/chromium/src/+/904992#message-244dec285c2ea73ecb01d86550a067454739ba46.
One addendum to the above-given instructions: Per
https://chromium-review.googlesource.com/c/chromium/src/+/904992#message-83d6d1c236e3791c5fb2ec0c129aae701730e5ad,
step 5 of the top set of instructions is not relevant on Linux and should
be ignored.

TBR=jochen@chromium.org

Bug:  809435 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7d6bea8690e3546f8832e26cadea17394425c0ba
Reviewed-on: https://chromium-review.googlesource.com/904992
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Reviewed-by: Jared Saul <jsaul@google.com>
Cr-Commit-Position: refs/heads/master@{#537790}
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/android_webview/browser/aw_autofill_client.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/android_webview/browser/aw_autofill_client.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/chrome/browser/ui/autofill/chrome_autofill_client.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/chrome/browser/ui/autofill/chrome_autofill_client.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/chrome/browser/ui/views/payments/cvc_unmask_view_controller.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/chrome/browser/ui/views/payments/cvc_unmask_view_controller.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/BUILD.gn
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/DEPS
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/autofill_client.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/credit_card_save_manager_unittest.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/payments/full_card_request_unittest.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/payments/payments_client.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/payments/payments_client.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/payments/payments_client_unittest.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/payments/test_payments_client.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/payments/test_payments_client.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/test_autofill_client.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/test_autofill_client.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/components/autofill/core/browser/test_autofill_manager.cc
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/ios/chrome/browser/ui/autofill/BUILD.gn
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.mm
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/ios/web_view/internal/autofill/cwv_autofill_controller.mm
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/ios/web_view/internal/autofill/web_view_autofill_client_ios.h
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/ios/web_view/internal/autofill/web_view_autofill_client_ios.mm
[modify] https://crrev.com/3520fb171ac7e64b2ca8ea53c093679aceabcf96/services/identity/public/cpp/identity_test_utils.cc

Project Member

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

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

commit db232ebc844b27a78197430b214d577e549a8f46
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Feb 21 11:21:01 2018

[Autofill] Remove unneeded signin code from tests

I discovered while converting Autofill to use
//services/identity/public/cpp that the presence or absence of the code
deleted in this CL has no effect on any tests passing or failing.

Bug:  809435 
Change-Id: I7fcac665c2ae964af82d42325b3e5f6dc9113370
Reviewed-on: https://chromium-review.googlesource.com/913610
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538063}
[modify] https://crrev.com/db232ebc844b27a78197430b214d577e549a8f46/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/db232ebc844b27a78197430b214d577e549a8f46/components/autofill/core/browser/form_data_importer_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Feb 27 2018

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

commit fbb98d83616ecd331d1d31d6637a07156917103e
Author: Colin Blundell <blundell@chromium.org>
Date: Tue Feb 27 16:39:40 2018

[Autofill] Convert PersonalDataManager to use IdentityManager

The conversion is straightforward, getting the email address of the
primary account from IdentityManager rather than SigninManager and
AccountTrackerService.

Straightforward unittest conversions are also made.

NOTE: This CL was not tested manually as I don't know how to exercise
this functionality.

TBR=sdefresne@chromium.org

Bug:  809435 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7e145538b9e8634326a409c812b03a04c0254b05
Reviewed-on: https://chromium-review.googlesource.com/913390
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539456}
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/chrome/browser/autofill/personal_data_manager_factory.cc
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/components/autofill/core/browser/autofill_metrics_unittest.cc
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/components/autofill/core/browser/form_data_importer_unittest.cc
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/components/autofill/core/browser/personal_data_manager.cc
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/components/autofill/core/browser/personal_data_manager.h
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/components/autofill/core/browser/personal_data_manager_unittest.cc
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/components/browser_sync/profile_sync_service_autofill_unittest.cc
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/ios/chrome/browser/autofill/personal_data_manager_factory.cc
[modify] https://crrev.com/fbb98d83616ecd331d1d31d6637a07156917103e/ios/web_view/internal/autofill/web_view_personal_data_manager_factory.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 28 2018

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

commit c1f615f31605760aad574209e858dfd5e9bc31dd
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Feb 28 08:05:44 2018

[Autofill] Move CreditCardSaveManager away from using IdentityProvider

CreditCardSaveManager uses IdentityProvider to get the "active username."
In practice it is always a ProfileIdentityProvider instance that is
passed in (cf.  crbug.com/809435 ), and the active username in that
context is the email address of the primary (authenticated) account.

As we are looking to eliminate ProfileIdentityProvider, this CL relpaces
CreditCardSaveManager's usage with equivalent usage of IdentityManager.

Bug:  809435 , 809927
Change-Id: I12fe0523c7297495b362d4360d056b7208b0077f
Reviewed-on: https://chromium-review.googlesource.com/915945
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539750}
[modify] https://crrev.com/c1f615f31605760aad574209e858dfd5e9bc31dd/components/autofill/core/browser/credit_card_save_manager.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 28 2018

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

commit c9d3b816ea51d7fd06fe179046ff15b41c996c45
Author: Colin Blundell <blundell@chromium.org>
Date: Wed Feb 28 09:40:35 2018

[Autofill] Remove unneeded deps on //components/signin

After recent refactorings, //components/autofill no longer uses
//components/signin. This CL rips out dead includes, APIs, and
dependencies.

TBR=jam@chromium.org

Bug:  809435 , 809927
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If5289bd88003f9e58f1ce404870fd47b0fffa2e3
Reviewed-on: https://chromium-review.googlesource.com/916197
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539770}
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/android_webview/browser/aw_autofill_client.cc
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/android_webview/browser/aw_autofill_client.h
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/chrome/browser/ui/autofill/chrome_autofill_client.cc
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/chrome/browser/ui/autofill/chrome_autofill_client.h
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/components/autofill/core/browser/BUILD.gn
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/components/autofill/core/browser/DEPS
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/components/autofill/core/browser/autofill_client.h
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/components/autofill/core/browser/autofill_manager.cc
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/components/autofill/core/browser/autofill_test_utils.cc
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/components/autofill/core/browser/test_autofill_client.cc
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/components/autofill/core/browser/test_autofill_client.h
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/ios/chrome/browser/autofill/autofill_controller.mm
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.h
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/ios/chrome/browser/ui/autofill/chrome_autofill_client_ios.mm
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/ios/web_view/internal/autofill/cwv_autofill_controller.mm
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/ios/web_view/internal/autofill/web_view_autofill_client_ios.h
[modify] https://crrev.com/c9d3b816ea51d7fd06fe179046ff15b41c996c45/ios/web_view/internal/autofill/web_view_autofill_client_ios.mm

Status: Fixed (was: Started)

Sign in to add a comment