PaymentHandler: PaymentRequestEvent.total should be |object| type.

PaymentHandler: PaymentRequestEvent.total should be |object| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec:
  https://w3c.github.io/payment-handler/#the-paymentrequestevent

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608, 720027
Review-Url: https://codereview.chromium.org/2933023002
Cr-Commit-Position: refs/heads/master@{#479010}
Committed: https://chromium.googlesource.com/chromium/src/+/e5cab2a724f496f18a5f0403e135f2494e852da1

[zino, 2017-06-11 14:56:11]

Description was changed from

==========
PaymentHandler: PaymentRequestEvent.total should be |any| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec changes:
  https://github.com/w3c/payment-handler/issues/175
  https://github.com/w3c/payment-handler/issues/175

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027
==========

to

==========
PaymentHandler: PaymentRequestEvent.total should be |any| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec changes:
  https://github.com/w3c/payment-handler/issues/175
  https://github.com/w3c/payment-handler/pull/177

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027
==========

[zino, 2017-06-11 14:58:07]

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

[zino, 2017-06-11 14:58:37]

PTAL

[please use gerrit instead, 2017-06-12 14:02:34]

Thank you for the patch! Note that I've reviewed your pull request on github for
the spec and have one comment in there as well.

https://codereview.chromium.org/2933023002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h (left):

https://codereview.chromium.org/2933023002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:40: void
total(PaymentItem& value) const;
Does keeping the old signature of total() in here work with the new WebIDL?

https://codereview.chromium.org/2933023002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:59: PaymentItem
total_;
Can we try to keep PaymentItem on stack? I don't think garbage collection works
properly if you keep a ScriptValue as a member variable.

[zino, 2017-06-12 15:52:11]

Patchset #2 (id:20001) has been deleted

[zino, 2017-06-12 15:57:19]

> Thank you for the patch! Note that I've reviewed your pull request on 
> github for the spec and have one comment in there as well.

Thank you for review!
BTW, I didn't see your comments in a pull request for this :-)
(If you meant the pull request for ImageObject, then I already left a reply.)

https://codereview.chromium.org/2933023002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h (left):

https://codereview.chromium.org/2933023002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:40: void
total(PaymentItem& value) const;
On 2017/06/12 14:02:34, rouslan wrote:
> Does keeping the old signature of total() in here work with the new WebIDL?

The signature is changed by WebIDL compiler because it doesn't return dictionary
type anymore.

https://codereview.chromium.org/2933023002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:59: PaymentItem
total_;
On 2017/06/12 14:02:34, rouslan wrote:
> Can we try to keep PaymentItem on stack? I don't think garbage collection
works
> properly if you keep a ScriptValue as a member variable.

Done.

[please use gerrit instead, 2017-06-12 16:46:13]

On 2017/06/12 15:57:19, zino wrote:
> BTW, I didn't see your comments in a pull request for this :-)

I forgot to push "Publish Comment" on github. It's published now.

[zino, 2017-06-12 17:28:52]

Description was changed from

==========
PaymentHandler: PaymentRequestEvent.total should be |any| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec changes:
  https://github.com/w3c/payment-handler/issues/175
  https://github.com/w3c/payment-handler/pull/177

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027
==========

to

==========
PaymentHandler: PaymentRequestEvent.total should be |object| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec changes:
  https://github.com/w3c/payment-handler/issues/175
  https://github.com/w3c/payment-handler/pull/177

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027
==========

[zino, 2017-06-12 17:29:57]

Description was changed from

==========
PaymentHandler: PaymentRequestEvent.total should be |object| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec changes:
  https://github.com/w3c/payment-handler/issues/175
  https://github.com/w3c/payment-handler/pull/177

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027
==========

to

==========
PaymentHandler: PaymentRequestEvent.total should be |object| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec:
  https://w3c.github.io/payment-handler/#the-paymentrequestevent

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027
==========

[zino, 2017-06-12 17:30:56]

On 2017/06/12 16:46:13, rouslan wrote:
> On 2017/06/12 15:57:19, zino wrote:
> > BTW, I didn't see your comments in a pull request for this :-)
> 
> I forgot to push "Publish Comment" on github. It's published now.

Done. Thanks. I updated this patch as well.

[please use gerrit instead, 2017-06-12 17:32:46]

lgtm

[haraken, 2017-06-13 00:59:32]

LGTM

[zino, 2017-06-13 12:41:47]

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

[commit-bot: I haz the power, 2017-06-13 12:42:00]

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-06-13 14:29:01]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1497357707009170,
"parent_rev": "539f27bb466c912de33897a7e16cde207cf53e5e", "commit_rev":
"e5cab2a724f496f18a5f0403e135f2494e852da1"}

[commit-bot: I haz the power, 2017-06-13 14:29:15]

Description was changed from

==========
PaymentHandler: PaymentRequestEvent.total should be |object| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec:
  https://w3c.github.io/payment-handler/#the-paymentrequestevent

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027
==========

to

==========
PaymentHandler: PaymentRequestEvent.total should be |object| type.

The total field is currently defined as PaymentItem dictionary but WebIDL spec
says that attributes should not be dictionaries[1].

Related spec:
  https://w3c.github.io/payment-handler/#the-paymentrequestevent

[1] https://heycam.github.io/webidl/#idl-dictionaries

BUG=661608,720027

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

[commit-bot: I haz the power, 2017-06-13 14:29:16]

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