Initial implementation for feature policy - PaymentRequest

Initial implementation for feature policy - PaymentRequest

Disable PaymentRequest API unless enabled through feature policy.

BUG=679385
Review-Url: https://codereview.chromium.org/2618383002
Cr-Commit-Position: refs/heads/master@{#444797}
Committed: https://chromium.googlesource.com/chromium/src/+/662473565c1dd46ec06077c8e6519c195814b46d

[lunalu1, 2017-01-09 17:01:27]

The CQ bit was checked by lunalu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 17:02:24]

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, 2017-01-09 19:46:59]

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

[commit-bot: I haz the power, 2017-01-09 19:46:59]

Dry run: This issue passed the CQ dry run.

[lunalu1, 2017-01-11 19:03:03]

The CQ bit was checked by lunalu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 19:03:47]

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, 2017-01-11 20:57:48]

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

[commit-bot: I haz the power, 2017-01-11 20:57:49]

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

[lunalu1, 2017-01-12 16:06:37]

The CQ bit was checked by lunalu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 16:07:08]

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, 2017-01-12 17:17:25]

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

[commit-bot: I haz the power, 2017-01-12 17:17:26]

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

[lunalu1, 2017-01-12 18:23:21]

The CQ bit was checked by lunalu@chromium.org to run a CQ dry run

[lunalu1, 2017-01-12 18:23:29]

Patchset #3 (id:40001) has been deleted

[lunalu1, 2017-01-12 18:23:42]

lunalu@chromium.org changed reviewers:
+ iclelland@chromium.org

[commit-bot: I haz the power, 2017-01-12 18:24:41]

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, 2017-01-12 21:49:29]

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

[commit-bot: I haz the power, 2017-01-12 21:49:29]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-01-18 15:00:19]

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

[please use gerrit instead, 2017-01-18 15:00:20]

Sorry for the drive-by.

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

https://codereview.chromium.org/2618383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:571: // and
nit: reflow the comment please.

https://codereview.chromium.org/2618383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:583: //
TODO(lunalu): clean up the code once FP iframe is supported
nit: link to the bug that tracks this task.

[iclelland, 2017-01-18 16:17:31]

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

https://codereview.chromium.org/2618383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:583: //
TODO(lunalu): clean up the code once FP iframe is supported
On 2017/01/18 15:00:19, rouslan wrote:
> nit: link to the bug that tracks this task.

Thanks, rouslan -- loonybear, I've created crbug.com/682280 for this, if you
want to link it.

[lunalu1, 2017-01-18 17:12:13]

Hi, 
Thanks for reviewing my CL. I have made the changes. Please take another look.

Thanks,
Luna

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

https://codereview.chromium.org/2618383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:571: // and
On 2017/01/18 15:00:19, rouslan wrote:
> nit: reflow the comment please.

Done.

https://codereview.chromium.org/2618383002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:583: //
TODO(lunalu): clean up the code once FP iframe is supported
On 2017/01/18 16:17:31, iclelland wrote:
> On 2017/01/18 15:00:19, rouslan wrote:
> > nit: link to the bug that tracks this task.
> 
> Thanks, rouslan -- loonybear, I've created crbug.com/682280 for this, if you
> want to link it.

Done.

[please use gerrit instead, 2017-01-18 18:59:53]

Description was changed from

==========
Initial implementation for feature policy - PaymentRequest

Disable PaymentRequest API unless enabled through feature policy.

BUG=679385
==========

to

==========
Initial implementation for feature policy - PaymentRequest

Disable PaymentRequest API unless enabled through feature policy.

BUG=679385
==========

[please use gerrit instead, 2017-01-18 18:59:53]

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

[please use gerrit instead, 2017-01-18 19:00:22]

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

[please use gerrit instead, 2017-01-18 19:00:23]

I can provide my rubber stamp after Ian's approval.

[iclelland, 2017-01-18 19:31:00]

https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt
(right):

https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt:23:
FAIL No iframe may construct PaymentRequest when disabled. assert_equals:
expected "SecurityError" but got "Error"
Do you know why this assert is failing? I expected this to fail, since the
Feature Policy runtime flag is not enabled in this test, but not for that
reason. I'm wondering why the "error" is being thrown.

https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt:31:
FAIL No iframe may construct PaymentRequest when disabled. assert_equals:
expected "SecurityError" but got "Error"
Same here

[lunalu1, 2017-01-19 16:53:49]

The CQ bit was checked by lunalu@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 16:54:07]

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

[lunalu1, 2017-01-19 16:54:08]

Done. Thanks

https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt
(right):

https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt:23:
FAIL No iframe may construct PaymentRequest when disabled. assert_equals:
expected "SecurityError" but got "Error"
On 2017/01/18 19:31:00, iclelland wrote:
> Do you know why this assert is failing? I expected this to fail, since the
> Feature Policy runtime flag is not enabled in this test, but not for that
> reason. I'm wondering why the "error" is being thrown.
Error is from assert_unreached. But I have made the error more explicit.
Done.

https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt:31:
FAIL No iframe may construct PaymentRequest when disabled. assert_equals:
expected "SecurityError" but got "Error"
On 2017/01/18 19:31:00, iclelland wrote:
> Same here

Done.

[iclelland, 2017-01-19 17:12:37]

On 2017/01/19 16:54:08, loonybear wrote:
> Done. Thanks
> 
>
https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
> File
>
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt
> (right):
> 
>
https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt:23:
> FAIL No iframe may construct PaymentRequest when disabled. assert_equals:
> expected "SecurityError" but got "Error"
> On 2017/01/18 19:31:00, iclelland wrote:
> > Do you know why this assert is failing? I expected this to fail, since the
> > Feature Policy runtime flag is not enabled in this test, but not for that
> > reason. I'm wondering why the "error" is being thrown.
> Error is from assert_unreached. But I have made the error more explicit.
> Done.
> 
>
https://codereview.chromium.org/2618383002/diff/80001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/http/tests/feature-policy/payment-disabled-expected.txt:31:
> FAIL No iframe may construct PaymentRequest when disabled. assert_equals:
> expected "SecurityError" but got "Error"
> On 2017/01/18 19:31:00, iclelland wrote:
> > Same here
> 
> Done.

The new output looks much better -- much clearer what is going wrong in the
FeaturePolicy-flag-disabled case. Thanks!

LGTM

[please use gerrit instead, 2017-01-19 18:14:28]

lgtm

[lunalu1, 2017-01-19 18:14:48]

The CQ bit was checked by lunalu@chromium.org

[commit-bot: I haz the power, 2017-01-19 18:15:03]

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

[commit-bot: I haz the power, 2017-01-19 18:36:19]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484849688253440,
"parent_rev": "4ac83ec457ecc1ed922bc137fe65fc60f3f28d97", "commit_rev":
"662473565c1dd46ec06077c8e6519c195814b46d"}

[commit-bot: I haz the power, 2017-01-19 18:36:50]

Description was changed from

==========
Initial implementation for feature policy - PaymentRequest

Disable PaymentRequest API unless enabled through feature policy.

BUG=679385
==========

to

==========
Initial implementation for feature policy - PaymentRequest

Disable PaymentRequest API unless enabled through feature policy.

BUG=679385

Review-Url: https://codereview.chromium.org/2618383002
Cr-Commit-Position: refs/heads/master@{#444797}
Committed:
https://chromium.googlesource.com/chromium/src/+/662473565c1dd46ec06077c8e651...
==========

[commit-bot: I haz the power, 2017-01-19 18:36:52]

Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/662473565c1dd46ec06077c8e651...