PaymentHandler: Implement PaymentInstruments.delete().

PaymentHandler: Implement PaymentInstruments.delete().

Remove the stored payment instrument in associated service worker registration.

Related Spec Link:
  https://w3c.github.io/webpayments-payment-handler/#paymentinstruments-interface

BUG=661608
TEST=payment_manager_unittest.cc, payment-instruments.html

Review-Url: https://codereview.chromium.org/2844463002
Cr-Commit-Position: refs/heads/master@{#467304}
Committed: https://chromium.googlesource.com/chromium/src/+/bd27d5fb3a918c21d8231b32fa0ec929ec66afae

[zino, 2017-04-25 14:41:34]

Description was changed from

==========
PaymentHandler: Implement PaymentInstruments.delete().

Remove the stored payment instrument in associated service worker registration.

Related Spec Link:
 
https://w3c.github.io/webpayments-payment-handler/#paymentinstruments-interface

BUG=661608
TEST=payment_manager_unittest.cc, payment-instruments.html
==========

to

==========
PaymentHandler: Implement PaymentInstruments.delete().

Remove the stored payment instrument in associated service worker registration.

Related Spec Link:
 
https://w3c.github.io/webpayments-payment-handler/#paymentinstruments-interface

BUG=661608
TEST=payment_manager_unittest.cc, payment-instruments.html
==========

[zino, 2017-04-25 14:41:34]

jinho.bang@samsung.com changed reviewers:
+ haraken@chromium.org, rouslan@chromium.org, tsepez@chromium.org

[zino, 2017-04-25 14:41:43]

PTAL

[please use gerrit instead, 2017-04-25 15:21:40]

https://codereview.chromium.org/2844463002/diff/1/components/payments/mojom/p...
File components/payments/mojom/payment_app.mojom (right):

https://codereview.chromium.org/2844463002/diff/1/components/payments/mojom/p...
components/payments/mojom/payment_app.mojom:51: DeletePaymentInstrument(string
instrumentKey)
Let's use under_score_variable_names everywhere in the file.

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_app_database.cc (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_database.cc:121:
base::Passed(std::move(callback))));
Is base::Passed() necessary? I believe std::move() is sufficient.

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_database.cc:281: instrument_key,
base::Passed(std::move(callback))));
Same comment about base::Passed().

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_app_database.h (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_database.h:53: const std::string&
instrumentKey,
hacker_case_variable_names in C++ please.

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_manager.cc (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_manager.cc:61: const std::string&
instrumentKey,
hacker_case

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_manager.h (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_manager.h:40: const std::string& instrumentKey,
hacker_case

[Tom Sepez, 2017-04-25 16:23:23]

mojom LGTM

[zino, 2017-04-25 21:48:51]

Thank you for review.

PTAL

https://codereview.chromium.org/2844463002/diff/1/components/payments/mojom/p...
File components/payments/mojom/payment_app.mojom (right):

https://codereview.chromium.org/2844463002/diff/1/components/payments/mojom/p...
components/payments/mojom/payment_app.mojom:51: DeletePaymentInstrument(string
instrumentKey)
On 2017/04/25 15:21:40, ಠ_ಠ wrote:
> Let's use under_score_variable_names everywhere in the file.

Done.

I have to modify 61 line and 64 line but the change requires service worker
part.
Then, I will have to add a reviewer in this CL. So, I'll fix it in separated CL.

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_app_database.cc (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_database.cc:121:
base::Passed(std::move(callback))));
On 2017/04/25 15:21:40, ಠ_ಠ wrote:
> Is base::Passed() necessary? I believe std::move() is sufficient.

The FindReadyRegistrationForPattern requires legacy callback type.
So, we can not yet..
(Please see my comment in my past CL:
https://codereview.chromium.org/2806133002/diff/100001/content/browser/paymen...)

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_database.cc:281: instrument_key,
base::Passed(std::move(callback))));
On 2017/04/25 15:21:40, ಠ_ಠ wrote:
> Same comment about base::Passed().

I already left a comment in above your comment.

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_app_database.h (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_database.h:53: const std::string&
instrumentKey,
On 2017/04/25 15:21:40, ಠ_ಠ wrote:
> hacker_case_variable_names in C++ please.

Done.

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_manager.cc (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_manager.cc:61: const std::string&
instrumentKey,
On 2017/04/25 15:21:40, ಠ_ಠ wrote:
> hacker_case

Done.

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_manager.h (right):

https://codereview.chromium.org/2844463002/diff/1/content/browser/payments/pa...
content/browser/payments/payment_manager.h:40: const std::string& instrumentKey,
On 2017/04/25 15:21:40, ಠ_ಠ wrote:
> hacker_case

Done.

[zino, 2017-04-25 21:59:02]

https://codereview.chromium.org/2844463002/diff/1/components/payments/mojom/p...
File components/payments/mojom/payment_app.mojom (right):

https://codereview.chromium.org/2844463002/diff/1/components/payments/mojom/p...
components/payments/mojom/payment_app.mojom:51: DeletePaymentInstrument(string
instrumentKey)
I uploaded separated CL.

https://codereview.chromium.org/2839813005

[please use gerrit instead, 2017-04-25 22:27:07]

lgtm

[zino, 2017-04-26 10:38:43]

The CQ bit was checked by jinho.bang@samsung.com

[zino, 2017-04-26 10:38:44]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2844463002/#ps60001
(title: "PaymentHandler: Implement PaymentInstruments.delete().")

[commit-bot: I haz the power, 2017-04-26 10:38:55]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 12:09:38]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493203123766950,
"parent_rev": "e9592f40c7a6e6959be58a19af9f1483c148ed3b", "commit_rev":
"bd27d5fb3a918c21d8231b32fa0ec929ec66afae"}

[commit-bot: I haz the power, 2017-04-26 12:09:46]

Description was changed from

==========
PaymentHandler: Implement PaymentInstruments.delete().

Remove the stored payment instrument in associated service worker registration.

Related Spec Link:
 
https://w3c.github.io/webpayments-payment-handler/#paymentinstruments-interface

BUG=661608
TEST=payment_manager_unittest.cc, payment-instruments.html
==========

to

==========
PaymentHandler: Implement PaymentInstruments.delete().

Remove the stored payment instrument in associated service worker registration.

Related Spec Link:
 
https://w3c.github.io/webpayments-payment-handler/#paymentinstruments-interface

BUG=661608
TEST=payment_manager_unittest.cc, payment-instruments.html

Review-Url: https://codereview.chromium.org/2844463002
Cr-Commit-Position: refs/heads/master@{#467304}
Committed:
https://chromium.googlesource.com/chromium/src/+/bd27d5fb3a918c21d8231b32fa0e...
==========

[commit-bot: I haz the power, 2017-04-26 12:09:47]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/bd27d5fb3a918c21d8231b32fa0e...