iframes with allowpaymentrequest attribute are allowed to make payment requests.

iframes with allowpaymentrequest attribute are allowed to make payment requests.

Specification: https://w3c.github.io/browser-payment-api/

There are some circumstances where a cross-origin iframe
wants to make a payment request. A cross-origin iframe
needs explicit permission from the embedding page to invoke
the payment request API.

BUG=652148
Committed: https://crrev.com/310d33fb9aa25f2bcf58b4c64e4e0c1fbcc73bdb
Cr-Commit-Position: refs/heads/master@{#426513}

[pals, 2016-10-10 05:39:05]

Description was changed from

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

BUG=652148
==========

to

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

BUG=652148
==========

[pals, 2016-10-10 05:39:05]

sanjoy.pal@samsung.com changed reviewers:
+ rouslan@chromium.org

[pals, 2016-10-10 05:39:17]

PTAL.

[pals, 2016-10-10 05:48:00]

Description was changed from

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

BUG=652148
==========

to

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

Specification: https://w3c.github.io/browser-payment-api/

There are some circumstances where a cross-origin iframe
wants to make a payment request. A cross-origin iframe
needs explicit permission from the embedding page to invoke
the payment request API.

BUG=652148
==========

[please use gerrit instead, 2016-10-13 00:26:47]

rouslan@chromium.org changed reviewers:
+ haraken@chromium.org

[please use gerrit instead, 2016-10-13 00:26:48]

+haraken@: I'm not super familiar with IFrame attribute extension. Can you
verify we're doing everything right here?

sanjoy.pal@: Thank you so much for the patch! This is not trivial.

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/payments/payment-request-in-iframe-allowed.html
(right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/payment-request-in-iframe-allowed.html:6:
<iframe allowpaymentrequest  srcdoc="
Only one space between allowpaymentrequest and srcdoc, please.

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html
(right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html:6:
<iframe allowpaymentrequest  srcdoc="
Ditto

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested2.html
(right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested2.html:6:
<iframe allowpaymentrequest  srcdoc="
Ditto

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.idl (right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLIFrameElement.idl:30: [CEReactions,
Reflect] attribute boolean allowPaymentRequest;
Instead of modifying the global iframe IDL here, you should try to define your
own partial interface file in modules/payments/HTMLIFrameElementPayments.idl
with the contents from the spec.

// https://w3c.github.io/browser-payment-api/#paymentrequest-and-iframes

[
    ImplementedAs=HTMLIFrameElementPayments,
] partial interface HTMLIFrameElement {
    [CEReactions, Reflect] attribute boolean allowPaymentRequest;
};

Then you can put the implementation in
modules/payments/HTMLIFrameElementPayments.h/cpp, or something like that.

See
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/Wi...
and
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/DO...
for an example.

[haraken, 2016-10-13 02:20:01]

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.idl (right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLIFrameElement.idl:30: [CEReactions,
Reflect] attribute boolean allowPaymentRequest;
On 2016/10/13 00:26:48, rouslan (slow 10.10-10.17) wrote:
> Instead of modifying the global iframe IDL here, you should try to define your
> own partial interface file in modules/payments/HTMLIFrameElementPayments.idl
> with the contents from the spec.
> 
> // https://w3c.github.io/browser-payment-api/#paymentrequest-and-iframes
> 
> [
>     ImplementedAs=HTMLIFrameElementPayments,
> ] partial interface HTMLIFrameElement {
>     [CEReactions, Reflect] attribute boolean allowPaymentRequest;
> };
> 
> Then you can put the implementation in
> modules/payments/HTMLIFrameElementPayments.h/cpp, or something like that.
> 
> See
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/Wi...
> and
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/DO...
> for an example.

Yes, this is a right way forward.

[pals, 2016-10-14 08:11:18]

Extended the HTMLIFrameElemnt with partial interface. PTAL.

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/payments/payment-request-in-iframe-allowed.html
(right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/payment-request-in-iframe-allowed.html:6:
<iframe allowpaymentrequest  srcdoc="
On 2016/10/13 00:26:48, rouslan (slow 10.10-10.17) wrote:
> Only one space between allowpaymentrequest and srcdoc, please.

Done.

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html
(right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html:6:
<iframe allowpaymentrequest  srcdoc="
On 2016/10/13 00:26:48, rouslan (slow 10.10-10.17) wrote:
> Ditto

Done.

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested2.html
(right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested2.html:6:
<iframe allowpaymentrequest  srcdoc="
On 2016/10/13 00:26:48, rouslan (slow 10.10-10.17) wrote:
> Ditto

Done.

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.idl (right):

https://codereview.chromium.org/2394473002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLIFrameElement.idl:30: [CEReactions,
Reflect] attribute boolean allowPaymentRequest;
On 2016/10/13 02:20:01, haraken wrote:
> On 2016/10/13 00:26:48, rouslan (slow 10.10-10.17) wrote:
> > Instead of modifying the global iframe IDL here, you should try to define
your
> > own partial interface file in modules/payments/HTMLIFrameElementPayments.idl
> > with the contents from the spec.
> > 
> > // https://w3c.github.io/browser-payment-api/#paymentrequest-and-iframes
> > 
> > [
> >     ImplementedAs=HTMLIFrameElementPayments,
> > ] partial interface HTMLIFrameElement {
> >     [CEReactions, Reflect] attribute boolean allowPaymentRequest;
> > };
> > 
> > Then you can put the implementation in
> > modules/payments/HTMLIFrameElementPayments.h/cpp, or something like that.
> > 
> > See
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/Wi...
> > and
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/timing/DO...
> > for an example.
> 
> Yes, this is a right way forward.

Done.

[pals, 2016-10-14 11:03:02]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 11:03:16]

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-10-14 11:05:47]

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

[commit-bot: I haz the power, 2016-10-14 11:05:48]

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

[pals, 2016-10-14 12:06:24]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 12:06:36]

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-10-14 12:32:05]

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

[commit-bot: I haz the power, 2016-10-14 12:32:06]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  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-...)

[pals, 2016-10-14 15:45:27]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-14 15:45:53]

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-10-14 17:53:05]

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

[commit-bot: I haz the power, 2016-10-14 17:53:06]

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_...)

[pals, 2016-10-15 01:56:46]

I shall update the failed tests.

[pals, 2016-10-17 09:28:26]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-17 09:28:43]

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-10-17 11:02:20]

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

[commit-bot: I haz the power, 2016-10-17 11:02:21]

Dry run: This issue passed the CQ dry run.

[pals, 2016-10-17 11:41:10]

On 2016/10/15 01:56:46, pals wrote:
> I shall update the failed tests.

Rebaselined tests. Please review.

[please use gerrit instead, 2016-10-17 13:57:48]

This is some complex stuff! I'm not even sure myself of how to do this "the
right way." haraken@?

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/payments/payment-request-in-iframe.html
(right):

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-in-iframe.html:6:
<iframe allowpaymentrequest srcdoc="
You also need one test file that's exactly like this file, but does not have
"allowpaymentrequest" attribute. That test should expect a SecurityError.

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html
(right):

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html:1:
<!DOCTYPE html>
"nested1" and "nested2" are not very descriptive names. Let's use
"nested-expect-success" and "nested-expect-failure" instead. That points out the
difference between the two "nested" files more clearly.

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

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/HTMLIFrameElementPayments.idl:6: //
https://w3c.github.io/browser-payment-api/#paymentrequest-and-iframes
Please remove the duplicate line.

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/HTMLIFrameElementPayments.idl:9:
RuntimeEnabled=PaymentRequest
You don't need to specify 'ImplementedAs=HTMLIFrameElementPayments" here?

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

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventInit.idl:5:
// https://w3c.github.io/browser-payment-api/#paymentrequest-and-iframes
This line does not seem necessary, unless I am missing something. Added by
mistake?

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebLocalFrameImpl.cpp (right):

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebLocalFrameImpl.cpp:1629:
ownerElement->allowFullscreen(), ownerElement->csp(),
You seem to have passed in "allowFullscreen()" twice. One of these should be
"allowPaymentRequest()". Right?

[haraken, 2016-10-17 15:02:12]

The Supplement implementation looks good to me.

[pals, 2016-10-18 04:39:21]

I'm sorry. Most of the issues were introduced during rebase from patch set 2 to
patch set 3. Corrected now. Please review.

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/payments/payment-request-in-iframe.html
(right):

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/payment-request-in-iframe.html:6:
<iframe allowpaymentrequest srcdoc="
On 2016/10/17 13:57:48, rouslan (slow 10.10-10.17) wrote:
> You also need one test file that's exactly like this file, but does not have
> "allowpaymentrequest" attribute. That test should expect a SecurityError.

Messed up during rebase. Corrected now.

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html
(right):

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/payments/resources/payment-request-in-iframe-nested1.html:1:
<!DOCTYPE html>
On 2016/10/17 13:57:48, rouslan (slow 10.10-10.17) wrote:
> "nested1" and "nested2" are not very descriptive names. Let's use
> "nested-expect-success" and "nested-expect-failure" instead. That points out
the
> difference between the two "nested" files more clearly.

Done.

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

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/HTMLIFrameElementPayments.idl:6: //
https://w3c.github.io/browser-payment-api/#paymentrequest-and-iframes
On 2016/10/17 13:57:48, rouslan (slow 10.10-10.17) wrote:
> Please remove the duplicate line.

Messed up during rebase. Corrected now.

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/HTMLIFrameElementPayments.idl:9:
RuntimeEnabled=PaymentRequest
On 2016/10/17 13:57:48, rouslan (slow 10.10-10.17) wrote:
> You don't need to specify 'ImplementedAs=HTMLIFrameElementPayments" here?

As I see from existing blink code, if idl file name and implementation file
names are same, we do not need to specify ImplementedAs.

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

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventInit.idl:5:
// https://w3c.github.io/browser-payment-api/#paymentrequest-and-iframes
On 2016/10/17 13:57:48, rouslan (slow 10.10-10.17) wrote:
> This line does not seem necessary, unless I am missing something. Added by
> mistake?

Its a mistake. Messed up during rebase from patch set 2 to patch set 3.
Corrected now.

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebLocalFrameImpl.cpp (right):

https://codereview.chromium.org/2394473002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebLocalFrameImpl.cpp:1629:
ownerElement->allowFullscreen(), ownerElement->csp(),
On 2016/10/17 13:57:48, rouslan (slow 10.10-10.17) wrote:
> You seem to have passed in "allowFullscreen()" twice. One of these should be
> "allowPaymentRequest()". Right?

Its a mistake. Messed up during rebase from patch set 2 to patch set 3.
Corrected now.

[pals, 2016-10-18 04:39:31]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 04:39:59]

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-10-18 06:25:06]

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

[commit-bot: I haz the power, 2016-10-18 06:25:07]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-10-18 14:04:49]

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.h (right):

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLIFrameElement.h:82: bool
allowPaymentRequest() const override { return m_allowPaymentRequest; }
I was under impression that this getter would live in
HTMLIFrameElementPayments.h when you specify
ImplementedAs=HTMLIframeElementPayments in HTMLIFrameElementPayments.idl. If
that's not possible for some reason or is the wrong thing to do, please let me
know. I'm also learning this area of the code for the first time.

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLIFrameElement.h:94: bool
m_allowPaymentRequest;
Ditto

[pals, 2016-10-19 05:09:53]

@haraken, could please suggest how we can move the implementaion in
HTMLIFrameElementPayments.h|cpp files?

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.h (right):

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLIFrameElement.h:82: bool
allowPaymentRequest() const override { return m_allowPaymentRequest; }
On 2016/10/18 14:04:49, rouslan wrote:
> I was under impression that this getter would live in
> HTMLIFrameElementPayments.h when you specify
> ImplementedAs=HTMLIframeElementPayments in HTMLIFrameElementPayments.idl. If
> that's not possible for some reason or is the wrong thing to do, please let me
> know. I'm also learning this area of the code for the first time.

I am trying to move "m_allowPaymentRequest" to HTMLIFrameElementPayments.h but
not able to get allowpaymentrequestAttr in HTMLIFrameElementPayments.cpp. I am
not able find existing code where idl filename and ImplementedAs are having same
name. @haraken, could please suggest how this can be achived.

[haraken, 2016-10-19 09:04:09]

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/html/HTMLIFrameElement.h (right):

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/html/HTMLIFrameElement.h:82: bool
allowPaymentRequest() const override { return m_allowPaymentRequest; }
On 2016/10/19 05:09:53, pals wrote:
> On 2016/10/18 14:04:49, rouslan wrote:
> > I was under impression that this getter would live in
> > HTMLIFrameElementPayments.h when you specify
> > ImplementedAs=HTMLIframeElementPayments in HTMLIFrameElementPayments.idl. If
> > that's not possible for some reason or is the wrong thing to do, please let
me
> > know. I'm also learning this area of the code for the first time.
> 
> I am trying to move "m_allowPaymentRequest" to HTMLIFrameElementPayments.h but
> not able to get allowpaymentrequestAttr in HTMLIFrameElementPayments.cpp. I am
> not able find existing code where idl filename and ImplementedAs are having
same
> name. @haraken, could please suggest how this can be achived. 

Nice catch! rouslan is right. We should put m_allowPaymentRequest in
HTMLIFrameElementPayments.h.

Why do you need [ImplementedAs] in the first place? What happens if you simply
put m_allowPaymentRequest in HTMLIFrameElementPayments.h without using
[ImplementedAs]?

[pals, 2016-10-19 12:53:12]

Removed need of variable in HTMLIFrameElement.h but we need to override
allowPaymentRequest() in HTMLIFrameElement.h. I'm also not sure how to move
allowPaymentRequest() in HTMLIFrameElementPayments.h.

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:897:
frameElement->allowPaymentRequest(), frameElement->csp(),
HTMLIFrameElement needs to override allowPaymentRequest()as it is being used
here.

[please use gerrit instead, 2016-10-19 14:21:19]

sanjoy.pal@: Sounds like you've done everything you could. I'm out of concrete
ideas on how to make this code better. So LGTM from me on modules/payments,
which I own.

[haraken, 2016-10-19 15:37:56]

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:897:
frameElement->allowPaymentRequest(), frameElement->csp(),
On 2016/10/19 12:53:12, pals wrote:
> HTMLIFrameElement needs to override allowPaymentRequest()as it is being used
> here. 

Can we avoid the override by changing the line to something liek:

  HTMLIFrameElementPayments::from(frameElement)->allowPaymentRequest()

?

[pals, 2016-10-20 09:25:57]

Patchset #8 (id:140001) has been deleted

[pals, 2016-10-20 09:30:07]

Thank you. Now the code looks much simpler now. PTAL.

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp (right):

https://codereview.chromium.org/2394473002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/FrameLoaderClientImpl.cpp:897:
frameElement->allowPaymentRequest(), frameElement->csp(),
On 2016/10/19 15:37:56, haraken wrote:
> On 2016/10/19 12:53:12, pals wrote:
> > HTMLIFrameElement needs to override allowPaymentRequest()as it is being used
> > here. 
> 
> Can we avoid the override by changing the line to something liek:
> 
>   HTMLIFrameElementPayments::from(frameElement)->allowPaymentRequest()
> 
> ?

Done.

[pals, 2016-10-20 09:30:14]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 09:30:33]

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-10-20 09:32:21]

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

[commit-bot: I haz the power, 2016-10-20 09:32:21]

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

[haraken, 2016-10-20 09:36:59]

Looks much nicer. LGTM.

[pals, 2016-10-20 11:37:44]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-20 11:38:04]

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-10-20 13:15:45]

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

[commit-bot: I haz the power, 2016-10-20 13:15:46]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-10-20 15:07:01]

still lgtm from me

[pals, 2016-10-20 16:55:39]

The CQ bit was checked by sanjoy.pal@samsung.com

[pals, 2016-10-20 16:55:39]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2394473002/#ps180001
(title: "Rebased")

[commit-bot: I haz the power, 2016-10-20 16:56:16]

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-10-20 17:15:51]

Description was changed from

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

Specification: https://w3c.github.io/browser-payment-api/

There are some circumstances where a cross-origin iframe
wants to make a payment request. A cross-origin iframe
needs explicit permission from the embedding page to invoke
the payment request API.

BUG=652148
==========

to

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

Specification: https://w3c.github.io/browser-payment-api/

There are some circumstances where a cross-origin iframe
wants to make a payment request. A cross-origin iframe
needs explicit permission from the embedding page to invoke
the payment request API.

BUG=652148
==========

[commit-bot: I haz the power, 2016-10-20 17:15:53]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2016-10-21 13:19:27]

Description was changed from

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

Specification: https://w3c.github.io/browser-payment-api/

There are some circumstances where a cross-origin iframe
wants to make a payment request. A cross-origin iframe
needs explicit permission from the embedding page to invoke
the payment request API.

BUG=652148
==========

to

==========
iframes with allowpaymentrequest attribute are allowed to make payment requests.

Specification: https://w3c.github.io/browser-payment-api/

There are some circumstances where a cross-origin iframe
wants to make a payment request. A cross-origin iframe
needs explicit permission from the embedding page to invoke
the payment request API.

BUG=652148

Committed: https://crrev.com/310d33fb9aa25f2bcf58b4c64e4e0c1fbcc73bdb
Cr-Commit-Position: refs/heads/master@{#426513}
==========

[commit-bot: I haz the power, 2016-10-21 13:19:29]

Patchset 9 (id:??) landed as
https://crrev.com/310d33fb9aa25f2bcf58b4c64e4e0c1fbcc73bdb
Cr-Commit-Position: refs/heads/master@{#426513}