Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(18)

Issue 2958333002: [Payments] Implement web payment app manifest (Closed)

Created:
4 months, 4 weeks ago by gogerald1
Modified:
4 months, 3 weeks ago
CC:
chromium-reviews, qsr+mojo_chromium.org, tzik, yzshen+watch_chromium.org, kinuko+watch, jsbell+serviceworker_chromium.org, viettrungluu+watch_chromium.org, jam, abarth-chromium, dglazkov+blink, darin-cc_chromium.org, blink-reviews, blink-reviews-api_chromium.org, blink-worker-reviews_chromium.org, nhiroki, rouslan+payments_chromium.org, haraken, michaeln, shimazu+serviceworker_chromium.org, serviceworker-reviews, gogerald+paymentswatch_chromium.org, Aaron Boodman, kinuko+serviceworker, mahmadi+paymentswatch_chromium.org, horo+watch_chromium.org, darin (slow to review), sebsg+paymentswatch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement web payment app manifest The web page registering web payment app needs to add <link rel="manifest" href="manifest.json"> in the page to provide the web app manifest. BUG=735063 Review-Url: https://codereview.chromium.org/2958333002 Cr-Commit-Position: refs/heads/master@{#484193} Committed: https://chromium.googlesource.com/chromium/src/+/70b9dcb8dae67c7c101a1c4c8f17da2adbcf98ab

Patch Set 1 #

Total comments: 6

Patch Set 2 : address comments #

Patch Set 3 : fix tests and add comments #

Total comments: 2

Patch Set 4 : update comments #

Total comments: 3

Patch Set 5 : use url::Origin #

Total comments: 4

Patch Set 6 : rebase #

Patch Set 7 : rename and comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+602 lines, -51 lines) Patch
M chrome/browser/android/payments/service_worker_payment_app_bridge.cc View 1 chunk +5 lines, -2 lines 0 comments Download
M content/browser/BUILD.gn View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/payments/payment_app.proto View 1 chunk +13 lines, -7 lines 0 comments Download
M content/browser/payments/payment_app_browsertest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M content/browser/payments/payment_app_content_unittest_base.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M content/browser/payments/payment_app_database.h View 1 7 chunks +31 lines, -4 lines 0 comments Download
M content/browser/payments/payment_app_database.cc View 1 2 3 4 10 chunks +152 lines, -13 lines 0 comments Download
A content/browser/payments/payment_app_info_fetcher.h View 1 1 chunk +57 lines, -0 lines 0 comments Download
A content/browser/payments/payment_app_info_fetcher.cc View 1 1 chunk +157 lines, -0 lines 0 comments Download
M content/browser/payments/payment_app_provider_impl_unittest.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M content/browser/payments/payment_manager.h View 2 chunks +7 lines, -1 line 0 comments Download
M content/browser/payments/payment_manager.cc View 2 chunks +30 lines, -3 lines 0 comments Download
M content/browser/payments/payment_manager_unittest.cc View 1 2 5 chunks +27 lines, -5 lines 0 comments Download
M content/browser/service_worker/service_worker_context_wrapper.h View 1 2 3 4 5 6 1 chunk +5 lines, -0 lines 0 comments Download
M content/browser/service_worker/service_worker_context_wrapper.cc View 1 2 3 4 5 6 1 chunk +18 lines, -0 lines 0 comments Download
M content/public/browser/BUILD.gn View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M content/public/browser/payment_app_provider.h View 2 chunks +2 lines, -4 lines 0 comments Download
A content/public/browser/stored_payment_app.h View 1 2 3 4 1 chunk +42 lines, -0 lines 0 comments Download
A content/public/browser/stored_payment_app.cc View 1 chunk +13 lines, -0 lines 0 comments Download
M content/public/browser/stored_payment_instrument.h View 1 chunk +0 lines, -3 lines 0 comments Download
A content/test/data/payments/manifest.json View 1 chunk +9 lines, -0 lines 0 comments Download
M content/test/data/payments/payment_app_invocation.html View 1 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/payments/PaymentInstruments.cpp View 1 2 3 2 chunks +16 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/modules/payments/PaymentManager.cpp View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/public/platform/modules/payments/payment_app.mojom View 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 78 (61 generated)
gogerald1
Hi rouslan@, ptal,
4 months, 4 weeks ago (2017-06-29 19:30:37 UTC) #26
please use gerrit instead
Please make sure that a failure to retrieve payment app's label and icon does not ...
4 months, 4 weeks ago (2017-06-29 21:15:39 UTC) #27
falken
https://codereview.chromium.org/2958333002/diff/80001/content/browser/service_worker/service_worker_context_wrapper.h File content/browser/service_worker/service_worker_context_wrapper.h (right): https://codereview.chromium.org/2958333002/diff/80001/content/browser/service_worker/service_worker_context_wrapper.h#newcode250 content/browser/service_worker/service_worker_context_wrapper.h:250: ServiceWorkerContextCore* context(); This represents a big shift in this ...
4 months, 4 weeks ago (2017-06-30 01:14:36 UTC) #31
gogerald1
Thanks, another look? https://codereview.chromium.org/2958333002/diff/80001/content/browser/payments/payment_app_info_fetcher.cc File content/browser/payments/payment_app_info_fetcher.cc (right): https://codereview.chromium.org/2958333002/diff/80001/content/browser/payments/payment_app_info_fetcher.cc#newcode20 content/browser/payments/payment_app_info_fetcher.cc:20: #define PAYMENT_APP_MINIMUM_ICON_SIZE 0 On 2017/06/29 21:15:39, ...
4 months, 3 weeks ago (2017-06-30 02:55:23 UTC) #34
gogerald1
4 months, 3 weeks ago (2017-06-30 15:36:04 UTC) #44
please use gerrit instead
LGTM % comments. Please reformat the CL description to be at most 72 characters wide. ...
4 months, 3 weeks ago (2017-06-30 15:43:31 UTC) #47
gogerald1
Thanks, Hi nasko@, please review changes in content/public/* and content/browser/BUILD.gn https://codereview.chromium.org/2958333002/diff/140001/third_party/WebKit/Source/modules/payments/PaymentInstruments.cpp File third_party/WebKit/Source/modules/payments/PaymentInstruments.cpp (right): https://codereview.chromium.org/2958333002/diff/140001/third_party/WebKit/Source/modules/payments/PaymentInstruments.cpp#newcode61 ...
4 months, 3 weeks ago (2017-06-30 16:25:09 UTC) #52
nasko
https://codereview.chromium.org/2958333002/diff/160001/content/public/browser/stored_payment_app.h File content/public/browser/stored_payment_app.h (right): https://codereview.chromium.org/2958333002/diff/160001/content/public/browser/stored_payment_app.h#newcode28 content/public/browser/stored_payment_app.h:28: GURL origin; Why not url::Origin? Using GURLs to represent ...
4 months, 3 weeks ago (2017-06-30 17:32:47 UTC) #53
gogerald1
Hi nasko@, another look? https://codereview.chromium.org/2958333002/diff/160001/content/public/browser/stored_payment_app.h File content/public/browser/stored_payment_app.h (right): https://codereview.chromium.org/2958333002/diff/160001/content/public/browser/stored_payment_app.h#newcode28 content/public/browser/stored_payment_app.h:28: GURL origin; On 2017/06/30 17:32:47, ...
4 months, 3 weeks ago (2017-06-30 18:18:51 UTC) #56
nasko
IPC LGTM https://codereview.chromium.org/2958333002/diff/160001/content/public/browser/stored_payment_app.h File content/public/browser/stored_payment_app.h (right): https://codereview.chromium.org/2958333002/diff/160001/content/public/browser/stored_payment_app.h#newcode28 content/public/browser/stored_payment_app.h:28: GURL origin; On 2017/06/30 18:18:51, gogerald1 wrote: ...
4 months, 3 weeks ago (2017-06-30 21:02:34 UTC) #59
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/2958333002/180001
4 months, 3 weeks ago (2017-07-05 01:34:12 UTC) #62
commit-bot: I haz the power
Try jobs failed on following builders: ios-device on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds/244746) ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, ...
4 months, 3 weeks ago (2017-07-05 01:36:22 UTC) #64
falken
https://codereview.chromium.org/2958333002/diff/180001/content/browser/service_worker/service_worker_context_wrapper.cc File content/browser/service_worker/service_worker_context_wrapper.cc (right): https://codereview.chromium.org/2958333002/diff/180001/content/browser/service_worker/service_worker_context_wrapper.cc#newcode583 content/browser/service_worker/service_worker_context_wrapper.cc:583: render_frames->push_back( This isn't necessarily a render_frame. I would call ...
4 months, 3 weeks ago (2017-07-05 01:55:05 UTC) #65
gogerald1
Thanks, addressed your comments, https://codereview.chromium.org/2958333002/diff/180001/content/browser/service_worker/service_worker_context_wrapper.cc File content/browser/service_worker/service_worker_context_wrapper.cc (right): https://codereview.chromium.org/2958333002/diff/180001/content/browser/service_worker/service_worker_context_wrapper.cc#newcode583 content/browser/service_worker/service_worker_context_wrapper.cc:583: render_frames->push_back( On 2017/07/05 01:55:05, falken ...
4 months, 3 weeks ago (2017-07-05 02:26:04 UTC) #70
falken
Thanks, service worker lgtm
4 months, 3 weeks ago (2017-07-05 02:27:43 UTC) #71
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/2958333002/220001
4 months, 3 weeks ago (2017-07-05 03:32:42 UTC) #75
commit-bot: I haz the power
4 months, 3 weeks ago (2017-07-05 05:29:07 UTC) #78
Message was sent while issue was closed.
Committed patchset #7 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/70b9dcb8dae67c7c101a1c4c8f17...

Powered by Google App Engine
This is Rietveld efc10ee0f