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

Issue 2829903004: Reland: Normalize shipping address for merchant on Desktop. (Closed)

Created:
3 years, 8 months ago by sebsg
Modified:
3 years, 8 months ago
CC:
chromium-reviews, jam, rouslan+payments_chromium.org, rouslan+autofill_chromium.org, tfarina, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, gogerald+paymentswatch_chromium.org, mahmadi+paymentswatch_chromium.org, mathp+autofillwatch_chromium.org, darin-cc_chromium.org, rogerm+autofillwatch_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org, sebsg+paymentswatch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland: Normalize shipping address for merchant on Desktop. The CL was causing some tests to fail, because the FakeAddressNormalizer in tests initialized the AddressValidator with a null storage, which triggered an assertion failure in some cases. By creating an AddressNormalizer interface, the FakeAddressNormalizer doesn't need to create an AddressValidator object and can be used in test more easily. BUG=707765 Original CL: https://codereview.chromium.org/2829503002/ Review-Url: https://codereview.chromium.org/2829903004 Cr-Commit-Position: refs/heads/master@{#466369} Committed: https://chromium.googlesource.com/chromium/src/+/8a9c23403a7f9840cf4d9cbc517535ad6b8de953

Patch Set 1 : Original CL (crrev.com/2829503002) #

Total comments: 2

Patch Set 2 : Created an interface for the AddressNormalizer for better testing #

Total comments: 4

Patch Set 3 : Addressed comments #

Patch Set 4 : Rebase #

Patch Set 5 : Make android code use the new impl #

Unified diffs Side-by-side diffs Delta from patch set Stats (+317 lines, -547 lines) Patch
M chrome/browser/autofill/android/personal_data_manager_android.h View 1 2 3 4 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/payments/chrome_payment_request_delegate.h View 1 4 chunks +7 lines, -2 lines 0 comments Download
M chrome/browser/payments/chrome_payment_request_delegate.cc View 3 chunks +10 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/payments/payment_request_payment_response_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/payments/shipping_address_editor_view_controller_browsertest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.h View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/views/payments/test_chrome_payment_request_delegate.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/autofill/core/browser/address.cc View 3 chunks +5 lines, -3 lines 0 comments Download
M components/autofill/core/browser/autofill_data_util.h View 1 chunk +4 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_data_util.cc View 2 chunks +12 lines, -0 lines 0 comments Download
M components/autofill/core/browser/autofill_data_util_unittest.cc View 1 chunk +41 lines, -0 lines 0 comments Download
M components/payments/content/payment_request_state.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M components/payments/content/payment_response_helper.h View 3 chunks +23 lines, -6 lines 0 comments Download
M components/payments/content/payment_response_helper.cc View 3 chunks +70 lines, -29 lines 0 comments Download
M components/payments/content/payment_response_helper_unittest.cc View 1 11 chunks +44 lines, -7 lines 0 comments Download
M components/payments/core/BUILD.gn View 1 2 chunks +3 lines, -2 lines 0 comments Download
M components/payments/core/address_normalizer.h View 1 3 chunks +7 lines, -36 lines 0 comments Download
D components/payments/core/address_normalizer.cc View 1 1 chunk +0 lines, -189 lines 0 comments Download
A components/payments/core/address_normalizer_impl.h View 1 2 1 chunk +62 lines, -0 lines 0 comments Download
A + components/payments/core/address_normalizer_impl.cc View 1 3 chunks +8 lines, -8 lines 0 comments Download
A + components/payments/core/address_normalizer_impl_unittest.cc View 1 3 chunks +6 lines, -6 lines 0 comments Download
D components/payments/core/address_normalizer_unittest.cc View 1 1 chunk +0 lines, -247 lines 0 comments Download
M components/payments/core/payment_request_delegate.h View 2 chunks +6 lines, -1 line 0 comments Download

Messages

Total messages: 23 (14 generated)
sebsg
Hi Rouslan, Could you please take a look? I'll need to change one line on ...
3 years, 8 months ago (2017-04-21 15:02:39 UTC) #6
please use gerrit instead
How does this fix the issue that caused the revert?
3 years, 8 months ago (2017-04-21 15:04:27 UTC) #9
please use gerrit instead
https://codereview.chromium.org/2829903004/diff/1/components/payments/core/address_normalizer.h File components/payments/core/address_normalizer.h (right): https://codereview.chromium.org/2829903004/diff/1/components/payments/core/address_normalizer.h#newcode21 components/payments/core/address_normalizer.h:21: namespace libadderssinput { This was misspelled btw.
3 years, 8 months ago (2017-04-21 15:07:02 UTC) #10
please use gerrit instead
https://codereview.chromium.org/2829903004/diff/60001/components/payments/core/address_normalizer_impl.h File components/payments/core/address_normalizer_impl.h (right): https://codereview.chromium.org/2829903004/diff/60001/components/payments/core/address_normalizer_impl.h#newcode20 components/payments/core/address_normalizer_impl.h:20: namespace libadderssinput { Still misspelled here.
3 years, 8 months ago (2017-04-21 15:07:34 UTC) #11
please use gerrit instead
https://codereview.chromium.org/2829903004/diff/60001/components/payments/core/address_normalizer_impl.h File components/payments/core/address_normalizer_impl.h (right): https://codereview.chromium.org/2829903004/diff/60001/components/payments/core/address_normalizer_impl.h#newcode10 components/payments/core/address_normalizer_impl.h:10: #include <vector> #include <string>
3 years, 8 months ago (2017-04-21 15:09:32 UTC) #12
sebsg
Hi Rouslan, updated the description to detail how this patch fixes the issue. Thanks! https://codereview.chromium.org/2829903004/diff/1/components/payments/core/address_normalizer.h ...
3 years, 8 months ago (2017-04-21 15:17:49 UTC) #14
please use gerrit instead
lgtm
3 years, 8 months ago (2017-04-21 15:23:45 UTC) #16
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/2829903004/140001
3 years, 8 months ago (2017-04-21 15:54:55 UTC) #20
commit-bot: I haz the power
3 years, 8 months ago (2017-04-21 17:05:40 UTC) #23
Message was sent while issue was closed.
Committed patchset #5 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/8a9c23403a7f9840cf4d9cbc5175...

Powered by Google App Engine
This is Rietveld 408576698