PaymentRequest: Add support for shipping type. (in blink side)

PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270
  https://github.com/w3c/browser-payment-api/pull/285

BUG=648052
Committed: https://crrev.com/ac3f96f1115b079eec5e48700588022673b9986d
Cr-Commit-Position: refs/heads/master@{#420643}

[zino, 2016-09-18 06:29:50]

Description was changed from

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
 - https://github.com/w3c/browser-payment-api/pull/270

BUG=xxxxxx
==========

to

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
 - https://github.com/w3c/browser-payment-api/pull/270

BUG=xxxxxx
==========

[zino, 2016-09-18 14:31:32]

Description was changed from

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
 - https://github.com/w3c/browser-payment-api/pull/270

BUG=xxxxxx
==========

to

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
 - https://github.com/w3c/browser-payment-api/pull/270

BUG=648052
==========

[zino, 2016-09-18 14:31:44]

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

[zino, 2016-09-18 14:32:02]

PTAL

[please use gerrit instead, 2016-09-18 16:54:56]

Thank you so much for helping out here! :-D

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

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:169: var
request = new PaymentRequest([{'supportedMethods': ['foo']}], buildDetails(),
{'requestShipping': false, 'shippingType': 'shipping'});
Add a test for 'shippingType' being:

null
undefined
[]
0

Just in case, you know ;-)

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:189: var
request = new PaymentRequest([{'supportedMethods': ['foo']}], buildDetails(),
{'requestShipping': true, 'shippingType': 'shipping'});
Change 'shipping' to 'invalid'.

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentOptions.idl (right):

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentOptions.idl:32: DOMString
shippingType = "shipping";
I'm concerned about not using exactly the same words as were used in WebIDL. I
will try to get a resolution for this at TPAC this week.

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

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:370: };
Is this allocated on the stack? We would not want to return pointers that are
about to be invalidated. It's safer to place the array outside of the function,
unless you can show that I was wrong :-)

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.idl (right):

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.idl:5: //
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentre...
https://w3c.github.io/browser-payment-api/#paymentrequest-interface

[please use gerrit instead, 2016-09-18 16:55:29]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-18 16:55:44]

Dry run: 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-09-18 18:27:15]

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

[commit-bot: I haz the power, 2016-09-18 18:27:17]

Dry run: This issue passed the CQ dry run.

[zino, 2016-09-18 18:52:44]

Description was changed from

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
 - https://github.com/w3c/browser-payment-api/pull/270

BUG=648052
==========

to

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270

BUG=648052
==========

[zino, 2016-09-18 18:57:40]

Thank you for your review.

I addressed your comments. PTAL.

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

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:169: var
request = new PaymentRequest([{'supportedMethods': ['foo']}], buildDetails(),
{'requestShipping': false, 'shippingType': 'shipping'});
On 2016/09/18 16:54:56, rouslan wrote:
> Add a test for 'shippingType' being:
> 
> null
> undefined
> []
> 0
> 
> Just in case, you know ;-)

Done.

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:189: var
request = new PaymentRequest([{'supportedMethods': ['foo']}], buildDetails(),
{'requestShipping': true, 'shippingType': 'shipping'});
On 2016/09/18 16:54:56, rouslan wrote:
> Change 'shipping' to 'invalid'.

Done.

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentOptions.idl (right):

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentOptions.idl:32: DOMString
shippingType = "shipping";
On 2016/09/18 16:54:56, rouslan wrote:
> I'm concerned about not using exactly the same words as were used in WebIDL. I
> will try to get a resolution for this at TPAC this week.

Okay, Thanks.

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

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:370: };
On 2016/09/18 16:54:56, rouslan wrote:
> Is this allocated on the stack? We would not want to return pointers that are
> about to be invalidated. It's safer to place the array outside of the
function,
> unless you can show that I was wrong :-)

Good point.
I changed it to static const char* const type.
For readability, I thought it's better to place the array inside of this
function.
But if you don't like this, I'll make it place outside again. :)

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.idl (right):

https://codereview.chromium.org/2351533002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.idl:5: //
https://w3c.github.io/browser-payment-api/specs/paymentrequest.html#paymentre...
On 2016/09/18 16:54:56, rouslan wrote:
> https://w3c.github.io/browser-payment-api/#paymentrequest-interface

Done.

[please use gerrit instead, 2016-09-19 09:57:33]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-19 09:57:38]

Dry run: 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-09-19 11:17:17]

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

[commit-bot: I haz the power, 2016-09-19 11:17:18]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-09-20 08:32:16]

https://codereview.chromium.org/2351533002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentOptions.idl (right):

https://codereview.chromium.org/2351533002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentOptions.idl:31: // [2]
http://heycam.github.io/webidl/#example-363ff014
These comments are no longer necessary. See
https://github.com/w3c/browser-payment-api/commit/095c1cc7b9fc943dca48807150b...

[zino, 2016-09-20 15:55:15]

Description was changed from

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270

BUG=648052
==========

to

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270
  https://github.com/w3c/browser-payment-api/pull/285

BUG=648052
==========

[zino, 2016-09-20 15:56:10]

I removed the comment.

Thank you for review. PTAL

https://codereview.chromium.org/2351533002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentOptions.idl (right):

https://codereview.chromium.org/2351533002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentOptions.idl:31: // [2]
http://heycam.github.io/webidl/#example-363ff014
On 2016/09/20 08:32:16, rouslan (slow) wrote:
> These comments are no longer necessary. See
>
https://github.com/w3c/browser-payment-api/commit/095c1cc7b9fc943dca48807150b...

Done.

[zino, 2016-09-20 17:27:15]

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

[commit-bot: I haz the power, 2016-09-20 17:27:48]

Dry run: 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-09-20 19:06:09]

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

[commit-bot: I haz the power, 2016-09-20 19:06:10]

Dry run: This issue passed the CQ dry run.

[zino, 2016-09-22 15:44:26]

PTAL

[please use gerrit instead, 2016-09-23 11:21:10]

lgtm

[zino, 2016-09-23 13:41:53]

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

[commit-bot: I haz the power, 2016-09-23 13:42:01]

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-09-23 13:54:07]

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

[commit-bot: I haz the power, 2016-09-23 13:54:08]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[zino, 2016-09-23 15:42:54]

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

[commit-bot: I haz the power, 2016-09-23 15:43:09]

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-09-23 17:24:05]

Description was changed from

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270
  https://github.com/w3c/browser-payment-api/pull/285

BUG=648052
==========

to

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270
  https://github.com/w3c/browser-payment-api/pull/285

BUG=648052
==========

[commit-bot: I haz the power, 2016-09-23 17:24:06]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-23 17:26:53]

Description was changed from

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270
  https://github.com/w3c/browser-payment-api/pull/285

BUG=648052
==========

to

==========
PaymentRequest: Add support for shipping type. (in blink side)

This CL is adding support for shipping type. The API allows the page to
influence how the shipping address is described.

Related spec change:
  https://github.com/w3c/browser-payment-api/pull/270
  https://github.com/w3c/browser-payment-api/pull/285

BUG=648052

Committed: https://crrev.com/ac3f96f1115b079eec5e48700588022673b9986d
Cr-Commit-Position: refs/heads/master@{#420643}
==========

[commit-bot: I haz the power, 2016-09-23 17:26:54]

Patchset 6 (id:??) landed as
https://crrev.com/ac3f96f1115b079eec5e48700588022673b9986d
Cr-Commit-Position: refs/heads/master@{#420643}