PaymentApp: Blink side of payment request event dispatching in service worker.

PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

According to Onion Soup, we should connect browser and Blink directly but
there is no way right now because ServiceWorkerContextClient and others are
still in content/renderer side. For this reason, we have no choice but to add
many of WebTypes. However, once the SW related codes in content/renderer are
moved to Blink, we will able to remove them.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html

Committed: https://crrev.com/dbfe96630903e024d0b9fae92a8cd4859dbbbe5d
Cr-Commit-Position: refs/heads/master@{#435947}

[zino, 2016-11-23 15:46:46]

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

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

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-23 17:07:53]

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

[commit-bot: I haz the power, 2016-11-23 17:07:53]

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-23 17:32:13]

Description was changed from

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

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

to

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

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

[zino, 2016-11-23 17:32:13]

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

[zino, 2016-11-23 17:37:30]

PTAL.

haraken@ for overall blink.
rouslan@ for payments parts.
nhiroki@ for service worker parts.

Implementation note for review:
https://docs.google.com/document/d/1rWsvKQAwIboN2ZDuYYAkfce8GF27twi4UHTt0hcby...

Thanks.

[haraken, 2016-11-24 04:41:54]

Can we add tests?

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp
(right):

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp:37:
const WebString& stringifiedData) {

You need to enter ScriptState::Scope.

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/public/w...
File
third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h
(right):

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h:101:
const WebPaymentAppRequestData&) = 0;

Who calls this public method? I couldn't find it in the current code base.

Can we use mojo instead? Then we don't need to add a bunch of payment structs to
public/ APIs.

[shimazu, 2016-11-24 05:00:23]

shimazu@chromium.org changed reviewers:
+ shimazu@chromium.org

[shimazu, 2016-11-24 05:00:24]

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/public/w...
File
third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h
(right):

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/public/w...
third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h:101:
const WebPaymentAppRequestData&) = 0;
On 2016/11/24 04:41:54, haraken wrote:
> 
> Who calls this public method? I couldn't find it in the current code base.
> 
> Can we use mojo instead? Then we don't need to add a bunch of payment structs
to
> public/ APIs.
> 
> 

drive by:
I'm now mojofying IPCs related to dispatching events (but it's in
content/renderer): 
https://codereview.chromium.org/2490623005/diff/40001/content/common/service_...
I think this is fine for now though we need to move the codes from
content/renderer to blink later as a part of the onion soup.

[haraken, 2016-11-24 05:03:48]

On 2016/11/24 05:00:24, shimazu wrote:
>
https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/public/w...
> File
>
third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h
> (right):
> 
>
https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/public/w...
>
third_party/WebKit/public/web/modules/serviceworker/WebServiceWorkerContextProxy.h:101:
> const WebPaymentAppRequestData&) = 0;
> On 2016/11/24 04:41:54, haraken wrote:
> > 
> > Who calls this public method? I couldn't find it in the current code base.
> > 
> > Can we use mojo instead? Then we don't need to add a bunch of payment
structs
> to
> > public/ APIs.
> > 
> > 
> 
> drive by:
> I'm now mojofying IPCs related to dispatching events (but it's in
> content/renderer): 
>
https://codereview.chromium.org/2490623005/diff/40001/content/common/service_...
> I think this is fine for now though we need to move the codes from
> content/renderer to blink later as a part of the onion soup.

That sounds great!

[nhiroki, 2016-11-24 07:25:32]

service worker lgtm

[zino, 2016-11-26 18:27:18]

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

[commit-bot: I haz the power, 2016-11-26 18:27:22]

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-26 20:31:15]

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

[commit-bot: I haz the power, 2016-11-26 20:31:16]

Dry run: This issue passed the CQ dry run.

[zino, 2016-11-27 17:12:31]

Patchset #3 (id:40001) has been deleted

[zino, 2016-11-27 17:13:46]

Description was changed from

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

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

to

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

BUG=661608
TEST=PaymentAppRequestDataConversionTest
TEST=payment-request-event.html
==========

[zino, 2016-11-27 17:14:05]

Description was changed from

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

BUG=661608
TEST=PaymentAppRequestDataConversionTest
TEST=payment-request-event.html
==========

to

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html
==========

[zino, 2016-11-27 17:14:29]

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

[commit-bot: I haz the power, 2016-11-27 17:14:44]

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

[zino, 2016-11-27 17:17:25]

I added a unit test and a layout test.

PTAL.

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp
(right):

https://codereview.chromium.org/2528683002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp:37:
const WebString& stringifiedData) {
On 2016/11/24 04:41:54, haraken wrote:
> 
> You need to enter ScriptState::Scope.

Done.

https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/Sour...

[commit-bot: I haz the power, 2016-11-27 19:13:22]

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

[commit-bot: I haz the power, 2016-11-27 19:13:24]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-11-28 18:06:32]

haraken@: We should be using Mojo instead of
public/platform/modules/payments/WebPaymentAppRequestData.h and friends,
correct?

https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/payments/payment-request-event.html
(right):

https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/payments/payment-request-event.html:7:
// exposes the "hello" and "hello2" members.
"hello"?

https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h
(right):

https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h:1:
// Copyright 2016 The Chromium Authors. All rights reserved.
I was fairly certain that the Onion Soup project in Blink is eliminating the
need for public/platform/* data copying. Instead we're using Mojo to directly
communicate between Blink and its embedder. We have been able to avoid files in
public/platform/modules/payments/* so far. Let's continue this trend. Please
redesign using Mojo instead. See components/payments/*mojom for reference.

[please use gerrit instead, 2016-11-28 18:08:43]

Onion soup: http://crbug.com/561879

[zino, 2016-11-28 21:28:34]

Description was changed from

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html
==========

to

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

According to Onion Soup, we should connect browser and Blink directly but
there is no way right now because ServiceWorkerContextClient and others are
still in content/renderer side. For this reason, we have no choice but to add
many of WebTypes. However, once the SW related codes in content/renderer are
moved to Blink, we will able to remove them.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html
==========

[zino, 2016-11-28 21:29:47]

https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h
(right):

https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h:1:
// Copyright 2016 The Chromium Authors. All rights reserved.
On 2016/11/28 18:06:31, rouslan wrote:
> I was fairly certain that the Onion Soup project in Blink is eliminating the
> need for public/platform/* data copying. Instead we're using Mojo to directly
> communicate between Blink and its embedder. We have been able to avoid files
in
> public/platform/modules/payments/* so far. Let's continue this trend. Please
> redesign using Mojo instead. See components/payments/*mojom for reference.

Sure, I have already followed Onion Soup project in the last few CLs.
Here are my other CLs.
 - https://codereview.chromium.org/2472723003/
 - https://codereview.chromium.org/2476343002/
 - https://codereview.chromium.org/2497983002/

Event dispatching parts will also use mojo (in follow-up CL), but it will
connect the content/browser to content/renderer.
In service worker perspective, it's difficult to connect content/browser
to Blink directly right now because the ServiceWorkerContextClient and
others are still in content/renderer side. (In other words, we should move
SW related codes in content/renderer to Blink first.)

shimazu@ is working on mojofying service worker actively.
Please see his comment in #10.
Also, you can see Step3 in his design document:
https://docs.google.com/document/d/1DwPpvZG1gTCk5JAULmjPf0eCOHt01Pk59YdwRrbQW...

[haraken, 2016-11-29 02:11:05]

On 2016/11/28 21:29:47, zino wrote:
>
https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/publ...
> File
> third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h
> (right):
> 
>
https://codereview.chromium.org/2528683002/diff/60001/third_party/WebKit/publ...
>
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h:1:
> // Copyright 2016 The Chromium Authors. All rights reserved.
> On 2016/11/28 18:06:31, rouslan wrote:
> > I was fairly certain that the Onion Soup project in Blink is eliminating the
> > need for public/platform/* data copying. Instead we're using Mojo to
directly
> > communicate between Blink and its embedder. We have been able to avoid files
> in
> > public/platform/modules/payments/* so far. Let's continue this trend. Please
> > redesign using Mojo instead. See components/payments/*mojom for reference.
> 
> Sure, I have already followed Onion Soup project in the last few CLs.
> Here are my other CLs.
>  - https://codereview.chromium.org/2472723003/
>  - https://codereview.chromium.org/2476343002/
>  - https://codereview.chromium.org/2497983002/
> 
> Event dispatching parts will also use mojo (in follow-up CL), but it will
> connect the content/browser to content/renderer.
> In service worker perspective, it's difficult to connect content/browser
> to Blink directly right now because the ServiceWorkerContextClient and
> others are still in content/renderer side. (In other words, we should move
> SW related codes in content/renderer to Blink first.)
> 
> shimazu@ is working on mojofying service worker actively.
> Please see his comment in #10.
> Also, you can see Step3 in his design document:
>
https://docs.google.com/document/d/1DwPpvZG1gTCk5JAULmjPf0eCOHt01Pk59YdwRrbQW...

Correct. In the end, we should 1) migrate SW from content/renderer/ to Blink and
2) directly connect SW in Blink with content/browser/ using Mojo.

Until the migration is done, I think it would be okay to add some public APIs as
a intermediate step.

[please use gerrit instead, 2016-11-29 15:10:36]

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/payments/payment-request-event.html
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/payments/payment-request-event.html:1:
<!doctype html>
Please follow new layout test guidelines:

https://chromium.googlesource.com/chromium/src/+/master/docs/testing/writing_...

For example, you should add <meta charset=utf-8> to avoid console.log() warning
in Firefox. (Browsers often share layout tests.)

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp:37:
ScriptValue stringDataToScriptValue(ScriptState* scriptState,
Place this function immediately above the function where it's used. (Line 65)

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversionTest.cpp
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversionTest.cpp:28:
webMethodData.supportedMethods = WebVector<WebString>(&"foo", 1);
Eh, this is awkward. Can you do WebString::fromUTF8("foo") instead?

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversionTest.cpp:53:
ASSERT_TRUE(data.methodData()[0].hasSupportedMethods());
It's easier to read .front() instead of [0]

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h:15:

// https://w3c.github.io/webpayments-payment-apps-api/#sec-app-request-data

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentCurrencyAmount.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentCurrencyAmount.h:11:

// https://w3c.github.io/browser-payment-api/#paymentcurrencyamount-dictionary

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentDetailsModifier.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentDetailsModifier.h:13:

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

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/payments/WebPaymentItem.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentItem.h:12: 
// https://w3c.github.io/browser-payment-api/#paymentitem-dictionary

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/payments/WebPaymentMethodData.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentMethodData.h:15: 
// https://w3c.github.io/browser-payment-api/#paymentmethoddata-dictionary

[zino, 2016-11-30 16:09:33]

Thank you for review.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/payments/payment-request-event.html
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/payments/payment-request-event.html:1:
<!doctype html>
On 2016/11/29 15:10:35, rouslan wrote:
> Please follow new layout test guidelines:
> 
>
https://chromium.googlesource.com/chromium/src/+/master/docs/testing/writing_...
> 
> For example, you should add <meta charset=utf-8> to avoid console.log()
warning
> in Firefox. (Browsers often share layout tests.)

Oh, Thank you for information.

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp:37:
ScriptValue stringDataToScriptValue(ScriptState* scriptState,
On 2016/11/29 15:10:35, rouslan wrote:
> Place this function immediately above the function where it's used. (Line 65)

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversionTest.cpp
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversionTest.cpp:28:
webMethodData.supportedMethods = WebVector<WebString>(&"foo", 1);
On 2016/11/29 15:10:35, rouslan wrote:
> Eh, this is awkward. Can you do WebString::fromUTF8("foo") instead?

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversionTest.cpp:53:
ASSERT_TRUE(data.methodData()[0].hasSupportedMethods());
On 2016/11/29 15:10:35, rouslan wrote:
> It's easier to read .front() instead of [0]

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentAppRequestData.h:15:

On 2016/11/29 15:10:36, rouslan wrote:
> // https://w3c.github.io/webpayments-payment-apps-api/#sec-app-request-data

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentCurrencyAmount.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentCurrencyAmount.h:11:

On 2016/11/29 15:10:36, rouslan wrote:
> // https://w3c.github.io/browser-payment-api/#paymentcurrencyamount-dictionary

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File
third_party/WebKit/public/platform/modules/payments/WebPaymentDetailsModifier.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentDetailsModifier.h:13:

On 2016/11/29 15:10:36, rouslan wrote:
> //
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/payments/WebPaymentItem.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentItem.h:12: 
On 2016/11/29 15:10:36, rouslan wrote:
> // https://w3c.github.io/browser-payment-api/#paymentitem-dictionary

Done.

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/modules/payments/WebPaymentMethodData.h
(right):

https://codereview.chromium.org/2528683002/diff/80001/third_party/WebKit/publ...
third_party/WebKit/public/platform/modules/payments/WebPaymentMethodData.h:15: 
On 2016/11/29 15:10:36, rouslan wrote:
> // https://w3c.github.io/browser-payment-api/#paymentmethoddata-dictionary

Done.

[please use gerrit instead, 2016-11-30 17:19:19]

lgtm

[zino, 2016-12-01 02:07:38]

On 2016/11/30 17:19:19, rouslan wrote:
> lgtm

Thanks.

haraken@ PTAL

[zino, 2016-12-01 05:18:01]

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

[commit-bot: I haz the power, 2016-12-01 05:18:10]

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-12-01 07:10:05]

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

[commit-bot: I haz the power, 2016-12-01 07:10:06]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-12-01 08:24:27]

This CL LGTM but I have a question about how ScriptState::Scope should be used
in SW (to nhiroki@).

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp (right):

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:24: void
PaymentRequestEvent::data(PaymentAppRequestData& data) const {

Why don't you return PaymentAppRequestData instead of using a reference?

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right):

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
workerGlobalScope()->scriptController()->getScriptState());

You need to check scriptState->contextIsValid() before entering the scope,
because the context may be already gone.

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
workerGlobalScope()->scriptController()->getScriptState());

nhiroki@: I'm confused when SW should enter ScriptState::Scope.
dispatchPaymentRequestEvent, dispatchForeignFetchEvent and dispatchFetchEvent
are entering ScriptState::Scope, but other dispatchXXXEvents are not entering
ScriptState::Scope. Why?

[kinuko, 2016-12-01 13:01:51]

kinuko@chromium.org changed reviewers:
+ kinuko@chromium.org

[kinuko, 2016-12-01 13:01:52]

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right):

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
workerGlobalScope()->scriptController()->getScriptState());
On 2016/12/01 08:24:27, haraken wrote:
> 
> nhiroki@: I'm confused when SW should enter ScriptState::Scope.
> dispatchPaymentRequestEvent, dispatchForeignFetchEvent and dispatchFetchEvent
> are entering ScriptState::Scope, but other dispatchXXXEvents are not entering
> ScriptState::Scope. Why?

dispatchFetchEvent and dispatchForeignFetchEvent needs to enter
ScriptState::Scope because creating Response or Request requires it (see the
comment in Response.h and Request.h)-- that's my understanding.

[nhiroki, 2016-12-02 02:30:15]

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right):

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
workerGlobalScope()->scriptController()->getScriptState());
On 2016/12/01 13:01:52, kinuko wrote:
> On 2016/12/01 08:24:27, haraken wrote:
> > 
> > nhiroki@: I'm confused when SW should enter ScriptState::Scope.
> > dispatchPaymentRequestEvent, dispatchForeignFetchEvent and
dispatchFetchEvent
> > are entering ScriptState::Scope, but other dispatchXXXEvents are not
entering
> > ScriptState::Scope. Why?
> 
> dispatchFetchEvent and dispatchForeignFetchEvent needs to enter
> ScriptState::Scope because creating Response or Request requires it (see the
> comment in Response.h and Request.h)-- that's my understanding.

Thank you for pointing this out. As kinuko mentioned, this would be for creating
Response or Request. I'll make a patch to add comment about it.

zino, could you remove this line because PaymentRequest doesn't create them?

[kinuko, 2016-12-02 03:13:26]

kinuko@chromium.org changed reviewers:
- kinuko@chromium.org

[zino, 2016-12-02 03:13:49]

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp (right):

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:24: void
PaymentRequestEvent::data(PaymentAppRequestData& data) const {
On 2016/12/01 08:24:27, haraken wrote:
> 
> Why don't you return PaymentAppRequestData instead of using a reference?

Maybe, we can't return PaymentAppRequestData because it is a dictionary type.
 -
https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/modules/v8/...

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right):

https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
workerGlobalScope()->scriptController()->getScriptState());
On 2016/12/02 02:30:15, nhiroki wrote:
> On 2016/12/01 13:01:52, kinuko wrote:
> > On 2016/12/01 08:24:27, haraken wrote:
> > > 
> > > nhiroki@: I'm confused when SW should enter ScriptState::Scope.
> > > dispatchPaymentRequestEvent, dispatchForeignFetchEvent and
> dispatchFetchEvent
> > > are entering ScriptState::Scope, but other dispatchXXXEvents are not
> entering
> > > ScriptState::Scope. Why?
> > 
> > dispatchFetchEvent and dispatchForeignFetchEvent needs to enter
> > ScriptState::Scope because creating Response or Request requires it (see the
> > comment in Response.h and Request.h)-- that's my understanding.
> 
> Thank you for pointing this out. As kinuko mentioned, this would be for
creating
> Response or Request. I'll make a patch to add comment about it.
> 
> zino, could you remove this line because PaymentRequest doesn't create them?

The toPaymentAppRequestData() is also using ScriptState internally.
Therefore, I'm not sure but shouldn't it enter the ScriptState?

haraken@, What should I do for landing this?
Removing this line? VS adding contextIsValid() check?
Please correct me :)

[haraken, 2016-12-02 03:40:27]

On 2016/12/02 03:13:49, zino wrote:
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp
(right):
> 
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:24: void
> PaymentRequestEvent::data(PaymentAppRequestData& data) const {
> On 2016/12/01 08:24:27, haraken wrote:
> > 
> > Why don't you return PaymentAppRequestData instead of using a reference?
> 
> Maybe, we can't return PaymentAppRequestData because it is a dictionary type.
>  -
>
https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/modules/v8/...
> 
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right):
> 
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
> workerGlobalScope()->scriptController()->getScriptState());
> On 2016/12/02 02:30:15, nhiroki wrote:
> > On 2016/12/01 13:01:52, kinuko wrote:
> > > On 2016/12/01 08:24:27, haraken wrote:
> > > > 
> > > > nhiroki@: I'm confused when SW should enter ScriptState::Scope.
> > > > dispatchPaymentRequestEvent, dispatchForeignFetchEvent and
> > dispatchFetchEvent
> > > > are entering ScriptState::Scope, but other dispatchXXXEvents are not
> > entering
> > > > ScriptState::Scope. Why?
> > > 
> > > dispatchFetchEvent and dispatchForeignFetchEvent needs to enter
> > > ScriptState::Scope because creating Response or Request requires it (see
the
> > > comment in Response.h and Request.h)-- that's my understanding.
> > 
> > Thank you for pointing this out. As kinuko mentioned, this would be for
> creating
> > Response or Request. I'll make a patch to add comment about it.
> > 
> > zino, could you remove this line because PaymentRequest doesn't create them?
> 
> The toPaymentAppRequestData() is also using ScriptState internally.
> Therefore, I'm not sure but shouldn't it enter the ScriptState?
> 
> haraken@, What should I do for landing this?
> Removing this line? VS adding contextIsValid() check?
> Please correct me :)

You need to enter ScriptState because you're calling v8::JSON::Parse.

[haraken, 2016-12-02 03:41:42]

On 2016/12/02 03:40:27, haraken wrote:
> On 2016/12/02 03:13:49, zino wrote:
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp
> (right):
> > 
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:24: void
> > PaymentRequestEvent::data(PaymentAppRequestData& data) const {
> > On 2016/12/01 08:24:27, haraken wrote:
> > > 
> > > Why don't you return PaymentAppRequestData instead of using a reference?
> > 
> > Maybe, we can't return PaymentAppRequestData because it is a dictionary
type.
> >  -
> >
>
https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/modules/v8/...
> > 
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp
(right):
> > 
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
> > workerGlobalScope()->scriptController()->getScriptState());
> > On 2016/12/02 02:30:15, nhiroki wrote:
> > > On 2016/12/01 13:01:52, kinuko wrote:
> > > > On 2016/12/01 08:24:27, haraken wrote:
> > > > > 
> > > > > nhiroki@: I'm confused when SW should enter ScriptState::Scope.
> > > > > dispatchPaymentRequestEvent, dispatchForeignFetchEvent and
> > > dispatchFetchEvent
> > > > > are entering ScriptState::Scope, but other dispatchXXXEvents are not
> > > entering
> > > > > ScriptState::Scope. Why?
> > > > 
> > > > dispatchFetchEvent and dispatchForeignFetchEvent needs to enter
> > > > ScriptState::Scope because creating Response or Request requires it (see
> the
> > > > comment in Response.h and Request.h)-- that's my understanding.
> > > 
> > > Thank you for pointing this out. As kinuko mentioned, this would be for
> > creating
> > > Response or Request. I'll make a patch to add comment about it.
> > > 
> > > zino, could you remove this line because PaymentRequest doesn't create
them?
> > 
> > The toPaymentAppRequestData() is also using ScriptState internally.
> > Therefore, I'm not sure but shouldn't it enter the ScriptState?
> > 
> > haraken@, What should I do for landing this?
> > Removing this line? VS adding contextIsValid() check?
> > Please correct me :)
> 
> You need to enter ScriptState because you're calling v8::JSON::Parse.

Would it be an option to move the ScriptState::Scope into Response and Request
so that the caller side doesn't need to worry about when they should enter
ScriptState? We can pass in ScriptState* to Response and Request.

[nhiroki, 2016-12-02 03:43:33]

nhiroki@chromium.org changed reviewers:
+ yhirano@chromium.org

[nhiroki, 2016-12-02 03:44:03]

On 2016/12/02 03:41:42, haraken wrote:
> On 2016/12/02 03:40:27, haraken wrote:
> > On 2016/12/02 03:13:49, zino wrote:
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp
> > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:24:
void
> > > PaymentRequestEvent::data(PaymentAppRequestData& data) const {
> > > On 2016/12/01 08:24:27, haraken wrote:
> > > > 
> > > > Why don't you return PaymentAppRequestData instead of using a reference?
> > > 
> > > Maybe, we can't return PaymentAppRequestData because it is a dictionary
> type.
> > >  -
> > >
> >
>
https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/modules/v8/...
> > > 
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp
> (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
> > > workerGlobalScope()->scriptController()->getScriptState());
> > > On 2016/12/02 02:30:15, nhiroki wrote:
> > > > On 2016/12/01 13:01:52, kinuko wrote:
> > > > > On 2016/12/01 08:24:27, haraken wrote:
> > > > > > 
> > > > > > nhiroki@: I'm confused when SW should enter ScriptState::Scope.
> > > > > > dispatchPaymentRequestEvent, dispatchForeignFetchEvent and
> > > > dispatchFetchEvent
> > > > > > are entering ScriptState::Scope, but other dispatchXXXEvents are not
> > > > entering
> > > > > > ScriptState::Scope. Why?
> > > > > 
> > > > > dispatchFetchEvent and dispatchForeignFetchEvent needs to enter
> > > > > ScriptState::Scope because creating Response or Request requires it
(see
> > the
> > > > > comment in Response.h and Request.h)-- that's my understanding.
> > > > 
> > > > Thank you for pointing this out. As kinuko mentioned, this would be for
> > > creating
> > > > Response or Request. I'll make a patch to add comment about it.
> > > > 
> > > > zino, could you remove this line because PaymentRequest doesn't create
> them?
> > > 
> > > The toPaymentAppRequestData() is also using ScriptState internally.
> > > Therefore, I'm not sure but shouldn't it enter the ScriptState?
> > > 
> > > haraken@, What should I do for landing this?
> > > Removing this line? VS adding contextIsValid() check?
> > > Please correct me :)
> > 
> > You need to enter ScriptState because you're calling v8::JSON::Parse.
> 
> Would it be an option to move the ScriptState::Scope into Response and Request
> so that the caller side doesn't need to worry about when they should enter
> ScriptState? We can pass in ScriptState* to Response and Request.

+yhirano@, who is an owner of core/fetch

[yhirano, 2016-12-02 04:00:44]

On 2016/12/02 03:44:03, nhiroki wrote:
> On 2016/12/02 03:41:42, haraken wrote:
> > On 2016/12/02 03:40:27, haraken wrote:
> > > On 2016/12/02 03:13:49, zino wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > > File third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp
> > > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > > third_party/WebKit/Source/modules/payments/PaymentRequestEvent.cpp:24:
> void
> > > > PaymentRequestEvent::data(PaymentAppRequestData& data) const {
> > > > On 2016/12/01 08:24:27, haraken wrote:
> > > > > 
> > > > > Why don't you return PaymentAppRequestData instead of using a
reference?
> > > > 
> > > > Maybe, we can't return PaymentAppRequestData because it is a dictionary
> > type.
> > > >  -
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/modules/v8/...
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > > File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp
> > (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2528683002/diff/120001/third_party/WebKit/Sou...
> > > > third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:336:
> > > > workerGlobalScope()->scriptController()->getScriptState());
> > > > On 2016/12/02 02:30:15, nhiroki wrote:
> > > > > On 2016/12/01 13:01:52, kinuko wrote:
> > > > > > On 2016/12/01 08:24:27, haraken wrote:
> > > > > > > 
> > > > > > > nhiroki@: I'm confused when SW should enter ScriptState::Scope.
> > > > > > > dispatchPaymentRequestEvent, dispatchForeignFetchEvent and
> > > > > dispatchFetchEvent
> > > > > > > are entering ScriptState::Scope, but other dispatchXXXEvents are
not
> > > > > entering
> > > > > > > ScriptState::Scope. Why?
> > > > > > 
> > > > > > dispatchFetchEvent and dispatchForeignFetchEvent needs to enter
> > > > > > ScriptState::Scope because creating Response or Request requires it
> (see
> > > the
> > > > > > comment in Response.h and Request.h)-- that's my understanding.
> > > > > 
> > > > > Thank you for pointing this out. As kinuko mentioned, this would be
for
> > > > creating
> > > > > Response or Request. I'll make a patch to add comment about it.
> > > > > 
> > > > > zino, could you remove this line because PaymentRequest doesn't create
> > them?
> > > > 
> > > > The toPaymentAppRequestData() is also using ScriptState internally.
> > > > Therefore, I'm not sure but shouldn't it enter the ScriptState?
> > > > 
> > > > haraken@, What should I do for landing this?
> > > > Removing this line? VS adding contextIsValid() check?
> > > > Please correct me :)
> > > 
> > > You need to enter ScriptState because you're calling v8::JSON::Parse.
> > 
> > Would it be an option to move the ScriptState::Scope into Response and
Request
> > so that the caller side doesn't need to worry about when they should enter
> > ScriptState? We can pass in ScriptState* to Response and Request.
> 
> +yhirano@, who is an owner of core/fetch

Moving ScriptState::Scope inside Request / Response constructors will lead to
crashes. See https://codereview.chromium.org/2141383002.

[zino, 2016-12-02 04:18:33]

haraken@ PTAL

https://codereview.chromium.org/2528683002/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp
(right):

https://codereview.chromium.org/2528683002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp:57:
if (scriptState->contextIsValid())
Done.

https://codereview.chromium.org/2528683002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentAppRequestDataConversion.cpp:60:
ScriptState::Scope scope(
Done.

[haraken, 2016-12-02 04:22:44]

LGTM

[zino, 2016-12-02 04:25:09]

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

[zino, 2016-12-02 04:25:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
nhiroki@chromium.org, rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2528683002/#ps140001
(title: "PaymentApp: Blink side of payment request event dispatching in service
worker.")

[commit-bot: I haz the power, 2016-12-02 04:25:34]

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-12-02 04:36:38]

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

[commit-bot: I haz the power, 2016-12-02 04:36:40]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[zino, 2016-12-02 05:06:49]

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

[commit-bot: I haz the power, 2016-12-02 05:07:12]

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-12-02 06:47:25]

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

[commit-bot: I haz the power, 2016-12-02 06:47:26]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[zino, 2016-12-02 14:47:44]

Patchset #8 (id:160001) has been deleted

[zino, 2016-12-02 14:49:15]

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

[zino, 2016-12-02 14:49:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, nhiroki@chromium.org, rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2528683002/#ps180001
(title: "PaymentApp: Blink side of payment request event dispatching in service
worker.")

[commit-bot: I haz the power, 2016-12-02 14:49:32]

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-12-02 16:20:21]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1480690155722460,
"parent_rev": "c5fa67942ff8e7939e04f02586203b956a07d7cc", "commit_rev":
"391fb9a54bdfb03591c737e02e017e53b5ec0b49"}

[commit-bot: I haz the power, 2016-12-02 16:20:44]

Description was changed from

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

According to Onion Soup, we should connect browser and Blink directly but
there is no way right now because ServiceWorkerContextClient and others are
still in content/renderer side. For this reason, we have no choice but to add
many of WebTypes. However, once the SW related codes in content/renderer are
moved to Blink, we will able to remove them.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html
==========

to

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

According to Onion Soup, we should connect browser and Blink directly but
there is no way right now because ServiceWorkerContextClient and others are
still in content/renderer side. For this reason, we have no choice but to add
many of WebTypes. However, once the SW related codes in content/renderer are
moved to Blink, we will able to remove them.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html
==========

[commit-bot: I haz the power, 2016-12-02 16:20:46]

Committed patchset #8 (id:180001)

[commit-bot: I haz the power, 2016-12-02 16:22:25]

Description was changed from

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

According to Onion Soup, we should connect browser and Blink directly but
there is no way right now because ServiceWorkerContextClient and others are
still in content/renderer side. For this reason, we have no choice but to add
many of WebTypes. However, once the SW related codes in content/renderer are
moved to Blink, we will able to remove them.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html
==========

to

==========
PaymentApp: Blink side of payment request event dispatching in service worker.

The payment request event is initiated from browser side when the user selects
a payment app in payment request UI. The event is dispached from browser to
renderer and then from service worker context client in renderer to blink again.
As part of implementation for it, this CL implements interfaces to dispatch the
event from renderer to blink.

This CL provides the following things.
 - Add dispatchPaymentRequestEvent() & didHandlePaymentReuqestEvent() to SW.
 - Expose WebPaymentAppRequestData and related Web types.
 - Conversion util to convert web type to blink type.

According to Onion Soup, we should connect browser and Blink directly but
there is no way right now because ServiceWorkerContextClient and others are
still in content/renderer side. For this reason, we have no choice but to add
many of WebTypes. However, once the SW related codes in content/renderer are
moved to Blink, we will able to remove them.

BUG=661608
TEST=PaymentAppRequestDataConversionTest.cpp
TEST=payment-request-event.html

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

[commit-bot: I haz the power, 2016-12-02 16:22:27]

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