PaymentRequest: Make the PaymentResponse interface serializable.

PaymentRequest: Make the PaymentResponse interface serializable.

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

BUG=648238
Committed: https://crrev.com/6a3cc7f4a4b8d30e4f9b6f305a279fa3853b24b9
Cr-Commit-Position: refs/heads/master@{#420044}

[zino, 2016-09-18 18:01:49]

Description was changed from

==========
PaymentRequest: Make the PaymentResponse interface serializable.

Make the PaymentResponse interface serializable.

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

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

to

==========
PaymentRequest: Make the PaymentResponse interface serializable.

Make the PaymentResponse interface serializable.

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

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

[zino, 2016-09-19 13:48:16]

Description was changed from

==========
PaymentRequest: Make the PaymentResponse interface serializable.

Make the PaymentResponse interface serializable.

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

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

to

==========
PaymentRequest: Make the PaymentResponse interface serializable.

Make the PaymentResponse interface serializable.

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

BUG=648238
==========

[zino, 2016-09-19 16:53:04]

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

[zino, 2016-09-19 16:53:19]

PTAL

[please use gerrit instead, 2016-09-20 08:15:46]

Please remove the part of the patch description that duplicates the title. If
there's nothing to add, let's be brief. :-)

Good job!

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

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.cpp:7: #include
"bindings/core/v8/ExceptionStatePlaceholder.h"
Why is this included?

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

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentResponse.cpp:42:
result.addNull("shippingAddress");
Do we need to add null shippingAddress, shippingOption, payerEmail, and
payerPhone? I would prefer omitting them instead.

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

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp:119:
mojom::blink::PaymentResponsePtr input = buildPaymentResponseForTest();
This function is just "mojom::blink::PaymentResponse::New()". Can we remove this
function throughout the code and use "mojom::blink::PaymentResponse::New()"
instead?

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp:135:
PaymentResponse output(std::move(input), completeCallback);
Inline the completeCallback, because you're using it only once.

PaymentResponse output(std::move(input), new MockPaymentCompleter);

[zino, 2016-09-20 16:33:39]

Description was changed from

==========
PaymentRequest: Make the PaymentResponse interface serializable.

Make the PaymentResponse interface serializable.

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

BUG=648238
==========

to

==========
PaymentRequest: Make the PaymentResponse interface serializable.

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

BUG=648238
==========

[zino, 2016-09-20 16:55:43]

Thank you for review.

I addressed your comments.

Could you please review again?

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

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAddress.cpp:7: #include
"bindings/core/v8/ExceptionStatePlaceholder.h"
On 2016/09/20 08:15:46, rouslan (slow) wrote:
> Why is this included?

Done.

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

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentResponse.cpp:42:
result.addNull("shippingAddress");
On 2016/09/20 08:15:46, rouslan (slow) wrote:
> Do we need to add null shippingAddress, shippingOption, payerEmail, and
> payerPhone? I would prefer omitting them instead.

I'm not sure but we can not omit them even if the value is null.
The attributes you mentioned are nullable types. So, they can be null.

According to the WebIDL spec, the null value is defined as serialized value.
 - https://www.w3.org/TR/WebIDL-1/#dfn-serialized-value
 - https://www.w3.org/TR/WebIDL-1/#dfn-convert-to-serialized-value

The following link is an existing another example in Chromium.
 -
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/peerco...

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

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp:119:
mojom::blink::PaymentResponsePtr input = buildPaymentResponseForTest();
On 2016/09/20 08:15:46, rouslan (slow) wrote:
> This function is just "mojom::blink::PaymentResponse::New()". Can we remove
this
> function throughout the code and use "mojom::blink::PaymentResponse::New()"
> instead?

Done.

https://codereview.chromium.org/2349133002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentResponseTest.cpp:135:
PaymentResponse output(std::move(input), completeCallback);
On 2016/09/20 08:15:46, rouslan (slow) wrote:
> Inline the completeCallback, because you're using it only once.
> 
> PaymentResponse output(std::move(input), new MockPaymentCompleter);

Done.

[zino, 2016-09-20 17:26:54]

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:30]

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 18:53:11]

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

[commit-bot: I haz the power, 2016-09-20 18:53:12]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[zino, 2016-09-20 19:00:01]

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 19:00: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:49:09]

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

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

Dry run: 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_...)

[please use gerrit instead, 2016-09-21 11:49:02]

lgtm

[haraken, 2016-09-21 12:33:21]

In terms of bindings, LGTM

[zino, 2016-09-21 12:37:13]

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

[commit-bot: I haz the power, 2016-09-21 12:37:26]

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-21 14:05:27]

Description was changed from

==========
PaymentRequest: Make the PaymentResponse interface serializable.

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

BUG=648238
==========

to

==========
PaymentRequest: Make the PaymentResponse interface serializable.

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

BUG=648238
==========

[commit-bot: I haz the power, 2016-09-21 14:05:28]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-21 14:08:07]

Description was changed from

==========
PaymentRequest: Make the PaymentResponse interface serializable.

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

BUG=648238
==========

to

==========
PaymentRequest: Make the PaymentResponse interface serializable.

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

BUG=648238

Committed: https://crrev.com/6a3cc7f4a4b8d30e4f9b6f305a279fa3853b24b9
Cr-Commit-Position: refs/heads/master@{#420044}
==========

[commit-bot: I haz the power, 2016-09-21 14:08:08]

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