Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(191)

Issue 2842463002: [Payments] Normalize billing address for response on Desktop. (Closed)

Created:
3 years, 8 months ago by sebsg
Modified:
3 years, 8 months ago
Reviewers:
Mathieu, anthonyvd, Moe
CC:
chromium-reviews, gogerald+paymentswatch_chromium.org, mahmadi+paymentswatch_chromium.org, rouslan+payments_chromium.org, sebsg+paymentswatch_chromium.org, tfarina
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Payments] Normalize billing address for response on Desktop. BUG=714646 Review-Url: https://codereview.chromium.org/2842463002 Cr-Commit-Position: refs/heads/master@{#467416} Committed: https://chromium.googlesource.com/chromium/src/+/4540f48b0b6a2511b938b09986be4baee13a1520

Patch Set 1 #

Patch Set 2 : Keep the old version for iOS #

Total comments: 8

Patch Set 3 : Added tests and addresses comments #

Patch Set 4 : Rebase + Rebase fixes #

Patch Set 5 : iOS fix #

Total comments: 8

Patch Set 6 : Addressed comments #

Total comments: 4

Patch Set 7 : Addressed moe's comments #

Messages

Total messages: 62 (49 generated)
sebsg
Hi Anthony, PTAL?
3 years, 8 months ago (2017-04-24 15:42:08 UTC) #5
anthonyvd
lgtm
3 years, 8 months ago (2017-04-24 15:47:28 UTC) #6
sebsg
Hi Math, PTAL?
3 years, 8 months ago (2017-04-24 15:47:58 UTC) #8
Mathieu
can we test the new behavior in autofill_payment_instrument? https://codereview.chromium.org/2842463002/diff/60001/components/payments/core/autofill_payment_instrument.cc File components/payments/core/autofill_payment_instrument.cc (right): https://codereview.chromium.org/2842463002/diff/60001/components/payments/core/autofill_payment_instrument.cc#newcode56 components/payments/core/autofill_payment_instrument.cc:56: // ...
3 years, 8 months ago (2017-04-24 19:42:09 UTC) #16
sebsg
Thanks Math, another look? https://codereview.chromium.org/2842463002/diff/60001/components/payments/core/autofill_payment_instrument.cc File components/payments/core/autofill_payment_instrument.cc (right): https://codereview.chromium.org/2842463002/diff/60001/components/payments/core/autofill_payment_instrument.cc#newcode56 components/payments/core/autofill_payment_instrument.cc:56: // TODO(crbug.com/602666): Ensure we reach ...
3 years, 8 months ago (2017-04-25 17:36:44 UTC) #36
Mathieu
lgtm https://codereview.chromium.org/2842463002/diff/220001/components/payments/core/autofill_payment_instrument.cc File components/payments/core/autofill_payment_instrument.cc (right): https://codereview.chromium.org/2842463002/diff/220001/components/payments/core/autofill_payment_instrument.cc#newcode111 components/payments/core/autofill_payment_instrument.cc:111: if (!is_waiting_for_billing_address_normalization_) { no curlies https://codereview.chromium.org/2842463002/diff/220001/components/payments/core/autofill_payment_instrument.cc#newcode128 components/payments/core/autofill_payment_instrument.cc:128: if ...
3 years, 8 months ago (2017-04-25 19:27:43 UTC) #45
sebsg
eugenebut@chromium.org: Could you please take a look at payment_request_coordinator.mm ? Thanks! https://codereview.chromium.org/2842463002/diff/220001/components/payments/core/autofill_payment_instrument.cc File components/payments/core/autofill_payment_instrument.cc (right): ...
3 years, 8 months ago (2017-04-25 19:35:23 UTC) #47
sebsg
lpromero@chromium.org: Could you please take a look at payment_request_coordinator.mm? Thanks!
3 years, 8 months ago (2017-04-25 19:36:06 UTC) #51
Moe
On 2017/04/25 19:36:06, sebsg wrote: > mailto:lpromero@chromium.org: Could you please take a look at > ...
3 years, 8 months ago (2017-04-26 18:03:37 UTC) #54
Moe
https://codereview.chromium.org/2842463002/diff/240001/ios/chrome/browser/ui/payments/payment_request_coordinator.mm File ios/chrome/browser/ui/payments/payment_request_coordinator.mm (right): https://codereview.chromium.org/2842463002/diff/240001/ios/chrome/browser/ui/payments/payment_request_coordinator.mm#newcode224 ios/chrome/browser/ui/payments/payment_request_coordinator.mm:224: autofill::AutofillProfile billing_address; s/billing_address/billingAddress https://codereview.chromium.org/2842463002/diff/240001/ios/chrome/browser/ui/payments/payment_request_coordinator.mm#newcode232 ios/chrome/browser/ui/payments/payment_request_coordinator.mm:232: autofill::PersonalDataManager::GetProfileFromProfilesByGUID( s/billing_address_ptr/billingAddressPtr
3 years, 8 months ago (2017-04-26 18:03:49 UTC) #55
sebsg
Thanks, sending to CQ https://codereview.chromium.org/2842463002/diff/240001/ios/chrome/browser/ui/payments/payment_request_coordinator.mm File ios/chrome/browser/ui/payments/payment_request_coordinator.mm (right): https://codereview.chromium.org/2842463002/diff/240001/ios/chrome/browser/ui/payments/payment_request_coordinator.mm#newcode224 ios/chrome/browser/ui/payments/payment_request_coordinator.mm:224: autofill::AutofillProfile billing_address; On 2017/04/26 18:03:49, ...
3 years, 8 months ago (2017-04-26 18:28:26 UTC) #56
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2842463002/260001
3 years, 8 months ago (2017-04-26 18:29:09 UTC) #59
commit-bot: I haz the power
3 years, 8 months ago (2017-04-26 19:41:18 UTC) #62
Message was sent while issue was closed.
Committed patchset #7 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/4540f48b0b6a2511b938b09986be...

Powered by Google App Engine
This is Rietveld 408576698