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

Issue 2872033007: [Payments] Do not show error on initial load of address screen (Closed)

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

Description

[Payments] Do not show error on initial load of address screen Previously would show an error if no shipping option was selected. Also no longer having an error if no entries are selected but some are available, which is a valid state to be in. BUG=721127 TEST=browser_tests, components_unittests Review-Url: https://codereview.chromium.org/2872033007 Cr-Commit-Position: refs/heads/master@{#470962} Committed: https://chromium.googlesource.com/chromium/src/+/b77b873b4678bafcab1c97ea39882ec9b43ee14d

Patch Set 1 #

Patch Set 2 : tests, slightly different logic #

Patch Set 3 : fix #

Patch Set 4 : deps check #

Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -19 lines) Patch
M chrome/browser/ui/views/payments/shipping_option_view_controller_browsertest.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M components/payments/content/BUILD.gn View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M components/payments/content/payment_request_spec.h View 1 2 1 chunk +6 lines, -3 lines 0 comments Download
M components/payments/content/payment_request_spec.cc View 1 4 chunks +22 lines, -13 lines 0 comments Download
M components/payments/content/payment_request_spec_unittest.cc View 1 2 chunks +45 lines, -3 lines 0 comments Download

Messages

Total messages: 14 (8 generated)
Mathieu
Anthony, PTAL!
3 years, 7 months ago (2017-05-11 01:04:17 UTC) #3
anthonyvd
lgtm
3 years, 7 months ago (2017-05-11 14:03:04 UTC) #4
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/2872033007/40001
3 years, 7 months ago (2017-05-11 14:06:46 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_compile_dbg/builds/267265) android_cronet on master.tryserver.chromium.android (JOB_FAILED, ...
3 years, 7 months ago (2017-05-11 14:12:00 UTC) #8
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/2872033007/60001
3 years, 7 months ago (2017-05-11 14:15:31 UTC) #11
commit-bot: I haz the power
3 years, 7 months ago (2017-05-11 15:27:01 UTC) #14
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b77b873b4678bafcab1c97ea3988...

Powered by Google App Engine
This is Rietveld 408576698