Duplicate payment method identifiers in methodData and details.modififiers should throw TypeError.

Duplicate payment method identifiers in methodData and details.modififiers should throw TypeError.

BUG=587995
Committed: https://crrev.com/7bd82467d47867c4c6787666eb7578026fdf92cd
Cr-Commit-Position: refs/heads/master@{#404159}

[pals, 2016-07-05 11:55:20]

Description was changed from

==========
Duplicate payment method identifiers in methodData and details.modififiers
should throw TypeError.

BUG=587995
==========

to

==========
Duplicate payment method identifiers in methodData and details.modififiers
should throw TypeError.

BUG=587995
==========

[pals, 2016-07-05 11:55:20]

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

[pals, 2016-07-05 11:56:20]

Spec says "If a payment method identifier appears more than once in the
methodData or details.modififiers sequences, then throw a TypeError." PTAL.

[please use gerrit instead, 2016-07-06 07:31:56]

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/payments/payment-request-interface.html
(right):

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:217: },
'Duplicate supported payment method identifiers in modifiers should throw
TypeError.');
Can you put your assert_throws() tests together inside of the generate_tests()
below? That saves 50% of lines of code. I'm not sure why the other
assert_throws() tests are not in the generate_tests(), but you don't have to fix
them in your patch.

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

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:27: #include
"public/platform/ServiceRegistry.h"
#include "wtf/HashSet.h"

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:232: } else {
No need for "else", because the previous clause returns.

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:306: } else {
Ditto.

[pals, 2016-07-07 03:59:03]

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

[commit-bot: I haz the power, 2016-07-07 03:59:17]

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

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

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

Dry run: This issue passed the CQ dry run.

[pals, 2016-07-07 05:14:13]

Fixed the comments. PTAL.

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/payments/payment-request-interface.html
(right):

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/payments/payment-request-interface.html:217: },
'Duplicate supported payment method identifiers in modifiers should throw
TypeError.');
On 2016/07/06 07:31:56, Rouslan (ツ) wrote:
> Can you put your assert_throws() tests together inside of the generate_tests()
> below? That saves 50% of lines of code. I'm not sure why the other
> assert_throws() tests are not in the generate_tests(), but you don't have to
fix
> them in your patch.

Done.

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

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:27: #include
"public/platform/ServiceRegistry.h"
On 2016/07/06 07:31:56, Rouslan (ツ) wrote:
> #include "wtf/HashSet.h"

Done.

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:232: } else {
On 2016/07/06 07:31:56, Rouslan (ツ) wrote:
> No need for "else", because the previous clause returns.

Done.

https://codereview.chromium.org/2117213003/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:306: } else {
On 2016/07/06 07:31:56, Rouslan (ツ) wrote:
> Ditto.

Done.

[please use gerrit instead, 2016-07-07 14:04:45]

lgtm

[pals, 2016-07-07 14:37:03]

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

[commit-bot: I haz the power, 2016-07-07 14:37:29]

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-07-07 16:25:52]

Description was changed from

==========
Duplicate payment method identifiers in methodData and details.modififiers
should throw TypeError.

BUG=587995
==========

to

==========
Duplicate payment method identifiers in methodData and details.modififiers
should throw TypeError.

BUG=587995
==========

[commit-bot: I haz the power, 2016-07-07 16:25:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-07 16:25:56]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 16:27:47]

Description was changed from

==========
Duplicate payment method identifiers in methodData and details.modififiers
should throw TypeError.

BUG=587995
==========

to

==========
Duplicate payment method identifiers in methodData and details.modififiers
should throw TypeError.

BUG=587995

Committed: https://crrev.com/7bd82467d47867c4c6787666eb7578026fdf92cd
Cr-Commit-Position: refs/heads/master@{#404159}
==========

[commit-bot: I haz the power, 2016-07-07 16:27:48]

Patchset 2 (id:??) landed as
https://crrev.com/7bd82467d47867c4c6787666eb7578026fdf92cd
Cr-Commit-Position: refs/heads/master@{#404159}