PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

The PaymentDetailsModifier allows details to modify the PaymentDetails based on
payment method identifier. It is used to add a discount or surcharge line item
indicating the reason for the different total amount, for example.

This CL is adding the dictionary, mojo interface and related tests but doesn't
include UI change to indicate total amount as per payment method identifier.

Related spec link:
  https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

BUG=618857
Committed: https://crrev.com/434828bc5ea0c5e62dafdd87045a65a8d61349e0
Cr-Commit-Position: refs/heads/master@{#402297}

[zino, 2016-06-26 18:25:49]

Description was changed from

==========
PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

The PaymentDetailsModifier allows details to modify the PaymentDetails based on
payment method identifier. It is used to add a discount or surcharge line item
indicating the reason for the different total amount, for example.

This CL is adding the dictionary, mojo interface and related tests but doesn't
include UI change to indicate total amount as per payment method identifier.

Related spec link:
  https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

BUG=618857
==========

to

==========
PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

The PaymentDetailsModifier allows details to modify the PaymentDetails based on
payment method identifier. It is used to add a discount or surcharge line item
indicating the reason for the different total amount, for example.

This CL is adding the dictionary, mojo interface and related tests but doesn't
include UI change to indicate total amount as per payment method identifier.

Related spec link:
  https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

BUG=618857
==========

[zino, 2016-06-26 18:25:50]

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

[zino, 2016-06-26 18:26:09]

jinho.bang@samsung.com changed reviewers:
+ palmer@chromium.org

[zino, 2016-06-26 18:27:31]

Patchset #1 (id:1) has been deleted

[zino, 2016-06-26 18:28:38]

PTAL

[zino, 2016-06-26 18:29:15]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-26 18:29:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[please use gerrit instead, 2016-06-26 19:45:26]

Great work!

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/payments/payment-request-interface.html
(right):

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:196: new
PaymentRequest([{'supportedMethods': ['foo']}], {'total': buildItem(),
'modifiers': [{'total' : buildItem()}]});
no space between the second 'total' and :

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:219: new
PaymentRequest([{'supportedMethods': ['foo']}], {'total': buildItem(),
'modifiers': [{'supportedMethods': ['foo'], 'total': buildItem({'value':
'0.0'})}]});
Value format checks should really be done in "Invalid amount formats" section
below. You can either:

(a) Add a TODO(rouslan) to move the value check into "Invalid amount formats"
section below and land this patch with the TODO, or

(b) Modify buildDetails() above to be able to alter the 'modifiers' as well and 
put 'modifiers' in the "var detailNames" list on line 278.

It's up to you which route you choose.

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentDetails.idl (right):

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentDetails.idl:5: //
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentde...
https://w3c.github.io/browser-payment-api/#paymentdetails-dictionary

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:36: using
blink::mojom::blink::PaymentDetailsModifier;
Please sort this list. PaymentDetailsModifier* should be above
PaymentDetailPtr*, because M<P.

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:88: output->total
= PaymentItem::From(input.total());
Total is not required in the spec, so check for it first:

if (input.hasTotal())
    output->total = PaymentItem::From(input.total());
else
    output->total = PaymentItem::New();

The "else" clause is necessary because Mojo requires "total".

[commit-bot: I haz the power, 2016-06-26 20:27:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-26 20:27:58]

Dry run: This issue passed the CQ dry run.

[zino, 2016-06-27 17:29:24]

Thank you for review.

I addressed your comments. PTAL.

Thanks.

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/payments/payment-request-interface.html
(right):

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:196: new
PaymentRequest([{'supportedMethods': ['foo']}], {'total': buildItem(),
'modifiers': [{'total' : buildItem()}]});
On 2016/06/26 19:45:24, Rouslan (ツ) wrote:
> no space between the second 'total' and :

Done.

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:219: new
PaymentRequest([{'supportedMethods': ['foo']}], {'total': buildItem(),
'modifiers': [{'supportedMethods': ['foo'], 'total': buildItem({'value':
'0.0'})}]});
On 2016/06/26 19:45:24, Rouslan (ツ) wrote:
> Value format checks should really be done in "Invalid amount formats" section
> below. You can either:
> 
> (a) Add a TODO(rouslan) to move the value check into "Invalid amount formats"
> section below and land this patch with the TODO, or
> 
> (b) Modify buildDetails() above to be able to alter the 'modifiers' as well
and 
> put 'modifiers' in the "var detailNames" list on line 278.
> 
> It's up to you which route you choose.

I added tests :)
Done.

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentDetails.idl (right):

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentDetails.idl:5: //
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentde...
On 2016/06/26 19:45:25, Rouslan (ツ) wrote:
> https://w3c.github.io/browser-payment-api/#paymentdetails-dictionary

Done.

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:36: using
blink::mojom::blink::PaymentDetailsModifier;
On 2016/06/26 19:45:25, Rouslan (ツ) wrote:
> Please sort this list. PaymentDetailsModifier* should be above
> PaymentDetailPtr*, because M<P.

Done.

https://codereview.chromium.org/2101463002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:88: output->total
= PaymentItem::From(input.total());
On 2016/06/26 19:45:25, Rouslan (ツ) wrote:
> Total is not required in the spec, so check for it first:
> 
> if (input.hasTotal())
>     output->total = PaymentItem::From(input.total());
> else
>     output->total = PaymentItem::New();
> 
> The "else" clause is necessary because Mojo requires "total".

Done.

[zino, 2016-06-27 17:31:44]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-27 17:33:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[please use gerrit instead, 2016-06-27 17:58:19]

lgtm

[zino, 2016-06-27 18:08:26]

jinho.bang@samsung.com changed reviewers:
+ kenrb@chromium.org
- palmer@chromium.org

[zino, 2016-06-27 18:09:05]

+kenrb@ for payment_request.mojom

PTAL.

Thank you.

[commit-bot: I haz the power, 2016-06-27 19:08:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-27 19:08:35]

Dry run: This issue passed the CQ dry run.

[kenrb, 2016-06-27 20:49:41]

lgtm

[please use gerrit instead, 2016-06-27 21:33:02]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2016-06-27 21:34:24]

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, 2016-06-27 21:42:44]

Description was changed from

==========
PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

The PaymentDetailsModifier allows details to modify the PaymentDetails based on
payment method identifier. It is used to add a discount or surcharge line item
indicating the reason for the different total amount, for example.

This CL is adding the dictionary, mojo interface and related tests but doesn't
include UI change to indicate total amount as per payment method identifier.

Related spec link:
  https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

BUG=618857
==========

to

==========
PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

The PaymentDetailsModifier allows details to modify the PaymentDetails based on
payment method identifier. It is used to add a discount or surcharge line item
indicating the reason for the different total amount, for example.

This CL is adding the dictionary, mojo interface and related tests but doesn't
include UI change to indicate total amount as per payment method identifier.

Related spec link:
  https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

BUG=618857
==========

[commit-bot: I haz the power, 2016-06-27 21:42:45]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-06-27 21:45:16]

Description was changed from

==========
PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

The PaymentDetailsModifier allows details to modify the PaymentDetails based on
payment method identifier. It is used to add a discount or surcharge line item
indicating the reason for the different total amount, for example.

This CL is adding the dictionary, mojo interface and related tests but doesn't
include UI change to indicate total amount as per payment method identifier.

Related spec link:
  https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

BUG=618857
==========

to

==========
PaymentRequest: Implement PaymentDetailsModifier dictionary (in blink side).

The PaymentDetailsModifier allows details to modify the PaymentDetails based on
payment method identifier. It is used to add a discount or surcharge line item
indicating the reason for the different total amount, for example.

This CL is adding the dictionary, mojo interface and related tests but doesn't
include UI change to indicate total amount as per payment method identifier.

Related spec link:
  https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

BUG=618857

Committed: https://crrev.com/434828bc5ea0c5e62dafdd87045a65a8d61349e0
Cr-Commit-Position: refs/heads/master@{#402297}
==========

[commit-bot: I haz the power, 2016-06-27 21:45:17]

Patchset 2 (id:??) landed as
https://crrev.com/434828bc5ea0c5e62dafdd87045a65a8d61349e0
Cr-Commit-Position: refs/heads/master@{#402297}