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

Issue 2066483010: Phone and email selection UI for PaymentRequest (Closed)

Created:
4 years, 6 months ago by please use gerrit instead
Modified:
4 years, 6 months ago
CC:
Aaron Boodman, abarth-chromium, ben+mojo_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, darin (slow to review), dglazkov+blink, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Phone and email selection UI for PaymentRequest This patch adds the user interface for the user to select their preferred contact phone number and email address. An addition, this patch passes the request for these fields from Blink to the browser. BUG=617195 Committed: https://crrev.com/cf584c715d7973f4c206bcacee348d64c0ea34e6 Cr-Commit-Position: refs/heads/master@{#400603}

Patch Set 1 #

Patch Set 2 : Fix webkit tests #

Total comments: 2

Patch Set 3 : Add tests #

Patch Set 4 : Follow mocks more #

Patch Set 5 : Rebase #

Total comments: 2

Patch Set 6 : Utility functions #

Patch Set 7 : Rebase again #

Patch Set 8 : Rebase #X #

Patch Set 9 : Fix integration test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+311 lines, -78 lines) Patch
A chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillContact.java View 1 2 3 1 chunk +49 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java View 1 2 3 4 5 6 7 8 9 chunks +86 lines, -32 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentInformation.java View 2 chunks +17 lines, -4 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java View 1 2 3 4 5 16 chunks +63 lines, -25 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/payments/ui/SectionInformation.java View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/android/java/strings/android_chrome_strings.grd View 1 2 3 4 5 2 chunks +9 lines, -0 lines 0 comments Download
M chrome/android/java_sources.gni View 1 2 3 4 5 6 3 chunks +3 lines, -0 lines 0 comments Download
A + chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestContactDetailsTest.java View 1 2 3 chunks +7 lines, -8 lines 0 comments Download
A chrome/android/junit/src/org/chromium/chrome/browser/payments/AutofillContactTest.java View 1 2 3 1 chunk +60 lines, -0 lines 0 comments Download
A + chrome/test/data/android/payments/contact_details.js View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -3 lines 0 comments Download
M chrome/test/data/android/payments/dynamic_shipping.js View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/android/payments/free_shipping.js View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/android/payments/no_shipping.js View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
A + chrome/test/data/android/payments/payment_request_contact_details_test.html View 1 2 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/payments/PaymentRequest.cpp View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/public/platform/modules/payments/payment_request.mojom View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 32 (16 generated)
please use gerrit instead
dfalcantara@, ptal android. This is functional, but I'd like to add some more tests. Also, ...
4 years, 6 months ago (2016-06-16 04:29:49 UTC) #2
gone
l-g-t-m, pending rebasing https://chromiumcodereview.appspot.com/2066483010/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://chromiumcodereview.appspot.com/2066483010/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java#newcode228 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:228: * @param requestContact Whether the UI ...
4 years, 6 months ago (2016-06-16 18:40:02 UTC) #3
please use gerrit instead
Also added tests. No rebase yet. https://chromiumcodereview.appspot.com/2066483010/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java File chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java (right): https://chromiumcodereview.appspot.com/2066483010/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java#newcode228 chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:228: * @param requestContact ...
4 years, 6 months ago (2016-06-16 21:12:45 UTC) #4
please use gerrit instead
(mek@ is ooo ? haraken@ : mek@), ptal PaymentRequest.cpp. I'm passing request_payer_email/phone from Blink to ...
4 years, 6 months ago (2016-06-16 23:21:56 UTC) #10
haraken
Source/modules/ LGTM
4 years, 6 months ago (2016-06-16 23:58:28 UTC) #11
Marijn Kruisselbrink
lgtm
4 years, 6 months ago (2016-06-17 07:16:09 UTC) #12
gone
lgtm
4 years, 6 months ago (2016-06-17 19:51:25 UTC) #13
palmer
LGTM with optional nit. https://codereview.chromium.org/2066483010/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2066483010/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java#newcode232 chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:232: if (requestShipping && profile.getCountryCode() != ...
4 years, 6 months ago (2016-06-17 22:28:40 UTC) #14
please use gerrit instead
Sending to cq. https://codereview.chromium.org/2066483010/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java (right): https://codereview.chromium.org/2066483010/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java#newcode232 chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:232: if (requestShipping && profile.getCountryCode() != null ...
4 years, 6 months ago (2016-06-17 22:54:45 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2066483010/120001
4 years, 6 months ago (2016-06-17 22:59:46 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/89827)
4 years, 6 months ago (2016-06-17 23:49:23 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2066483010/140001
4 years, 6 months ago (2016-06-18 02:12:10 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_android_rel_ng/builds/89870)
4 years, 6 months ago (2016-06-18 03:21:51 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2066483010/180001
4 years, 6 months ago (2016-06-18 22:40:34 UTC) #28
commit-bot: I haz the power
Committed patchset #9 (id:180001)
4 years, 6 months ago (2016-06-18 23:12:56 UTC) #30
commit-bot: I haz the power
4 years, 6 months ago (2016-06-18 23:14:36 UTC) #32
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/cf584c715d7973f4c206bcacee348d64c0ea34e6
Cr-Commit-Position: refs/heads/master@{#400603}

Powered by Google App Engine
This is Rietveld 408576698