PaymentApp: Add interfaces for PaymentRequestEvent.

PaymentApp: Add interfaces for PaymentRequestEvent.

The event is initiated by payment request in order to process payment in
payment app. This CL is just adding interfaces in blink side and implementing
them in follow-up CL. Also, the interfaces are behind a new runtime flag.

Intent to implement:
  https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/2ojnMk_T9_c

BUG=661608
Committed: https://crrev.com/c37cee81d6fd96e5ac04933ec06c163ca9d7686f
Cr-Commit-Position: refs/heads/master@{#433863}

[zino, 2016-11-19 07:41:23]

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

[commit-bot: I haz the power, 2016-11-19 07:41:35]

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-11-19 09:04:05]

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

[commit-bot: I haz the power, 2016-11-19 09:04:06]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[zino, 2016-11-19 18:22:31]

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

[commit-bot: I haz the power, 2016-11-19 18:22:34]

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-11-19 21:13:33]

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

[commit-bot: I haz the power, 2016-11-19 21:13:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[zino, 2016-11-20 14:02:12]

Patchset #1 (id:1) has been deleted

[zino, 2016-11-20 14:02:21]

Patchset #1 (id:20001) has been deleted

[zino, 2016-11-20 14:02:27]

Patchset #1 (id:40001) has been deleted

[zino, 2016-11-20 14:09:29]

Description was changed from

==========
dsdf

BUG=
==========

to

==========
PaymentApp: Add interfaces for PaymentRequestEvent.

The event is initiated by payment request in order to process payment in
payment app. This CL is just adding interfaces in blink side and implementing
them in follow-up CL. Also, the interfaces are behind a new runtime flag.

Intent to implement:
  https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/2ojnMk_T9_c

BUG=661608
==========

[zino, 2016-11-20 14:50:30]

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

[zino, 2016-11-20 14:50:31]

PTAL

[haraken, 2016-11-21 00:15:55]

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

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:1: #include
"modules/payments/PaymentRequestEvent.h"

Add a copyright.

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:11: void
PaymentRequestEvent::data(PaymentAppRequestData& data) const {}

Do you want to add NOT_IMPLEMENTED to these methods?

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h (right):

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:32: explicit
PaymentRequestEvent(const AtomicString& type,

Drop explicit.

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

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl:12: void
respondWith(Promise<PaymentResponse> r);

Nit: I want to rename |r| to a more descriptive name in the spec though.

[zino, 2016-11-21 09:31:05]

Thank you for review.

I addressed all comments. PTAL

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

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:1: #include
"modules/payments/PaymentRequestEvent.h"
On 2016/11/21 00:15:54, haraken wrote:
> 
> Add a copyright.

Done.

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:11: void
PaymentRequestEvent::data(PaymentAppRequestData& data) const {}
On 2016/11/21 00:15:55, haraken wrote:
> 
> Do you want to add NOT_IMPLEMENTED to these methods?

Done.

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h (right):

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:32: explicit
PaymentRequestEvent(const AtomicString& type,
On 2016/11/21 00:15:55, haraken wrote:
> 
> Drop explicit.

Done.

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

https://codereview.chromium.org/2510833002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl:12: void
respondWith(Promise<PaymentResponse> r);
On 2016/11/21 00:15:55, haraken wrote:
> 
> Nit: I want to rename |r| to a more descriptive name in the spec though.

Done.

[please use gerrit instead, 2016-11-21 20:31:37]

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/payments/PaymentAppServiceWorkerGlobalScope.h
(right):

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppServiceWorkerGlobalScope.h:13:
STATIC_ONLY(PaymentAppServiceWorkerGlobalScope);
Please #inclued "wtf/Allocator.h" for STATIC_ONLY.

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h (right):

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:15: }  //
namespace WTF
Nit: Short blocks of forward declarations usually don't have "  // namespace
<name>" endings, because the reader can easily see the start of the namespace 2
lines above.

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:34:
WaitUntilObserver*);
Please provide a destructor with "override" keyword.

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

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl:12: void
respondWith(Promise<PaymentResponse> response);
The spec says:

void respondWith((Promise<PaymentResponse> or PaymentResponse) r);

[zino, 2016-11-21 23:22:13]

Thank you for review.

I addressed all comments.

PTAL

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/payments/PaymentAppServiceWorkerGlobalScope.h
(right):

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppServiceWorkerGlobalScope.h:13:
STATIC_ONLY(PaymentAppServiceWorkerGlobalScope);
On 2016/11/21 20:31:36, rouslan wrote:
> Please #inclued "wtf/Allocator.h" for STATIC_ONLY.

Done.

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h (right):

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:15: }  //
namespace WTF
On 2016/11/21 20:31:36, rouslan wrote:
> Nit: Short blocks of forward declarations usually don't have "  // namespace
> <name>" endings, because the reader can easily see the start of the namespace
2
> lines above.

Done.

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.h:34:
WaitUntilObserver*);
On 2016/11/21 20:31:36, rouslan wrote:
> Please provide a destructor with "override" keyword.

Done.

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

https://codereview.chromium.org/2510833002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl:12: void
respondWith(Promise<PaymentResponse> response);
On 2016/11/21 20:31:36, rouslan wrote:
> The spec says:
> 
> void respondWith((Promise<PaymentResponse> or PaymentResponse) r);

IMHO, the spec should be respondWith(Promise<PaymentResponse> response);
We can refer to existing example (in service worker):
https://github.com/w3c/ServiceWorker/issues/724
Another example is updateWith in PaymentRequestUpdateEvent.idl.

So, I sent a pull request to fix this to the spec repo.
 - https://github.com/w3c/webpayments-payment-apps-api/pull/71

[please use gerrit instead, 2016-11-21 23:26:27]

https://codereview.chromium.org/2510833002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl (right):

https://codereview.chromium.org/2510833002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl:12: void
respondWith(Promise<PaymentResponse> response);
Please add a comment with an explanation of why you can't follow the spec and
link to your pull request that fixes the spec.

[zino, 2016-11-22 00:05:36]

On 2016/11/21 23:26:27, rouslan wrote:
>
https://codereview.chromium.org/2510833002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl
(right):
> 
>
https://codereview.chromium.org/2510833002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/payments/PaymentRequestEvent.idl:12: void
> respondWith(Promise<PaymentResponse> response);
> Please add a comment with an explanation of why you can't follow the spec and
> link to your pull request that fixes the spec.

I left a comment with related links.

Thanks :)

[please use gerrit instead, 2016-11-22 00:06:40]

lgtm

[zino, 2016-11-22 09:10:05]

On 2016/11/22 00:06:40, rouslan wrote:
> lgtm

haraken@ PTAL

[haraken, 2016-11-22 09:11:36]

On 2016/11/22 09:10:05, zino wrote:
> On 2016/11/22 00:06:40, rouslan wrote:
> > lgtm
> 
> haraken@ PTAL

LGTM

[zino, 2016-11-22 13:05:49]

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

[commit-bot: I haz the power, 2016-11-22 13:06: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, 2016-11-22 15:22:40]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1479819949916640,
"parent_rev": "60fa8dd73bae3b37a49a6f02b3458d90595f1a60", "commit_rev":
"0c2e3224fcfb1ae851cdd11e3dc5cbc7a6448f97"}

[commit-bot: I haz the power, 2016-11-22 15:23:03]

Description was changed from

==========
PaymentApp: Add interfaces for PaymentRequestEvent.

The event is initiated by payment request in order to process payment in
payment app. This CL is just adding interfaces in blink side and implementing
them in follow-up CL. Also, the interfaces are behind a new runtime flag.

Intent to implement:
  https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/2ojnMk_T9_c

BUG=661608
==========

to

==========
PaymentApp: Add interfaces for PaymentRequestEvent.

The event is initiated by payment request in order to process payment in
payment app. This CL is just adding interfaces in blink side and implementing
them in follow-up CL. Also, the interfaces are behind a new runtime flag.

Intent to implement:
  https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/2ojnMk_T9_c

BUG=661608
==========

[commit-bot: I haz the power, 2016-11-22 15:23:04]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2016-11-22 15:25:39]

Description was changed from

==========
PaymentApp: Add interfaces for PaymentRequestEvent.

The event is initiated by payment request in order to process payment in
payment app. This CL is just adding interfaces in blink side and implementing
them in follow-up CL. Also, the interfaces are behind a new runtime flag.

Intent to implement:
  https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/2ojnMk_T9_c

BUG=661608
==========

to

==========
PaymentApp: Add interfaces for PaymentRequestEvent.

The event is initiated by payment request in order to process payment in
payment app. This CL is just adding interfaces in blink side and implementing
them in follow-up CL. Also, the interfaces are behind a new runtime flag.

Intent to implement:
  https://groups.google.com/a/chromium.org/forum/m/#!topic/blink-dev/2ojnMk_T9_c

BUG=661608

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

[commit-bot: I haz the power, 2016-11-22 15:25:40]

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