Add data parameter to payment details modifier.

Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0AgAJ

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926
Committed: https://crrev.com/736ccd21ff339cf3243ee5857d3de0a661dfec3c
Cr-Commit-Position: refs/heads/master@{#435780}

[please use gerrit instead, 2016-10-31 19:03:25]

Patchset #2 (id:20001) has been deleted

[please use gerrit instead, 2016-10-31 19:03:29]

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

[please use gerrit instead, 2016-10-31 19:03:49]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This change also results in saving around 100 lines
of code.

BUG=FILEME
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This change also results in saving around 75 lines
of code.

BUG=FILEME
==========

[commit-bot: I haz the power, 2016-10-31 19:03:51]

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

[please use gerrit instead, 2016-10-31 19:04:52]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This change also results in saving around 75 lines
of code.

BUG=FILEME
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 75 lines of code.

BUG=FILEME
==========

[please use gerrit instead, 2016-10-31 19:09:28]

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

[commit-bot: I haz the power, 2016-10-31 19:09:37]

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

[please use gerrit instead, 2016-10-31 19:09:46]

Patchset #1 (id:1) has been deleted

[please use gerrit instead, 2016-10-31 19:10:18]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 75 lines of code.

BUG=FILEME
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 102 lines of code.

BUG=FILEME
==========

[please use gerrit instead, 2016-10-31 19:10:45]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 102 lines of code.

BUG=FILEME
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=FILEME
==========

[please use gerrit instead, 2016-10-31 19:12:08]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=FILEME
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=660926
==========

[please use gerrit instead, 2016-10-31 19:12:28]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializeable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=660926
==========

[commit-bot: I haz the power, 2016-10-31 20:28:38]

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

[commit-bot: I haz the power, 2016-10-31 20:28:39]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-10-31 20:34:35]

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

[please use gerrit instead, 2016-10-31 20:34:36]

mathp, ptal

Ideally all these changes would be reviewed by mek@, but he's ooo until at least
next week.

[Mathieu, 2016-11-01 14:39:17]

lgtm, mostly nits

I'm still unclear about the difference between PaymentMethodData and
PaymentDetailsModifier and how both can have this stringified_data, but that's
just my own ignorance. So I can learn, are there actual examples or tests of how
this can be used in the real world?

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

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:115: void
validateShippingOptionOrPaymentItem(const T& item,
curious: are you worried that the exception is going to be too generic given
that this function is used to validate many things? Or will the dev tools
provide a clear indication of what's wrong?

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:152: String
stringifyData(const ScriptValue& data, ExceptionState& exceptionState) {
Not sure of the convention for documentation in Blink, but this would benefit
from a quick function comment to mention what are valid values and invalid
values.

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:164: if
(value->isNull())
combine with previous block?

if (!value || value->isNull())

Unless isNull is a valid state where no exception should be returned?

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:300: void
validateAndConvertPaymentMethodData(
would benefit from a comment such as 

// Takes the PaymentMethodData as specified by the website/javascript and
creates a validated Mojo PaymentMethodDataPtr object to be passed to the
embedder/browser.

[please use gerrit instead, 2016-11-02 16:23:55]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

Can be observed in:
https://w3c.github.io/browser-payment-api/#paymentdetailsmodifier-dictionary

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:

https://github.com/w3c/browser-payment-api/pull/261/files

Spec:

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

Example:

new PaymentRequest([{
      supportedMethods: ["foo", "bar"],
      data: {
        commonParameter: "abc"
      }
    }], {
      total: {
        label: "Total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      },
      displayItems: [{
        label: "Sub-total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      }],
      modifiers: [{
          supportedMethods: ["bar"],
          total: {
            label: "Total",
            amount: {
              currency: "USD",
              value: "99.00"
            }
          },
          additionalDisplayItems: [{
            label: "Bar discount",
            amount: {
              currency: "USD",
              value: "-1.00"
            }
          }],
          data: {
            discountParameter: "xyz"
          }]
      });

The "commonParameter" in PaymentMethodData and "discountParameter" in
PaymentDetails are payment method specific and not standardized. They
are stringified and passed to the browser as JSON strings.

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=660926
==========

[please use gerrit instead, 2016-11-02 16:24:22]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:

https://github.com/w3c/browser-payment-api/pull/261/files

Spec:

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

Example:

new PaymentRequest([{
      supportedMethods: ["foo", "bar"],
      data: {
        commonParameter: "abc"
      }
    }], {
      total: {
        label: "Total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      },
      displayItems: [{
        label: "Sub-total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      }],
      modifiers: [{
          supportedMethods: ["bar"],
          total: {
            label: "Total",
            amount: {
              currency: "USD",
              value: "99.00"
            }
          },
          additionalDisplayItems: [{
            label: "Bar discount",
            amount: {
              currency: "USD",
              value: "-1.00"
            }
          }],
          data: {
            discountParameter: "xyz"
          }]
      });

The "commonParameter" in PaymentMethodData and "discountParameter" in
PaymentDetails are payment method specific and not standardized. They
are stringified and passed to the browser as JSON strings.

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 100 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:

https://github.com/w3c/browser-payment-api/pull/261/files

Spec:

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

Example:

new PaymentRequest([{
      supportedMethods: ["foo", "bar"],
      data: {
        commonParameter: "abc"
      }
    }], {
      total: {
        label: "Total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      },
      displayItems: [{
        label: "Sub-total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      }],
      modifiers: [{
          supportedMethods: ["bar"],
          total: {
            label: "Total",
            amount: {
              currency: "USD",
              value: "99.00"
            }
          },
          additionalDisplayItems: [{
            label: "Bar discount",
            amount: {
              currency: "USD",
              value: "-1.00"
            }
          }],
          data: {
            discountParameter: "xyz"
          }]
      });

The "commonParameter" in PaymentMethodData and "discountParameter" in
PaymentDetails are payment method specific and not standardized. They
are stringified and passed to the browser as JSON strings.

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 75 lines of code.

BUG=660926
==========

[please use gerrit instead, 2016-11-02 16:24:56]

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

[commit-bot: I haz the power, 2016-11-02 16:25:17]

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

[please use gerrit instead, 2016-11-02 16:27:09]

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

[please use gerrit instead, 2016-11-02 16:27:10]

tsepez, ptal payment_request.mojom.

On 2016/11/01 14:39:17, Mathieu Perreault wrote:
> I'm still unclear about the difference between PaymentMethodData and
> PaymentDetailsModifier and how both can have this stringified_data, but that's
> just my own ignorance. So I can learn, are there actual examples or tests of
how
> this can be used in the real world?

Added an example to the bug description. Modifier is also used in test code
here:

http://rsolomakhin.github.io/pr/pr.js

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

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:115: void
validateShippingOptionOrPaymentItem(const T& item,
On 2016/11/01 14:39:16, Mathieu Perreault wrote:
> curious: are you worried that the exception is going to be too generic given
> that this function is used to validate many things? Or will the dev tools
> provide a clear indication of what's wrong?

Improving these error messages are a nice to have, but not high priority at the
moment. So far web developer feedback is that the API is easy to use.

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:152: String
stringifyData(const ScriptValue& data, ExceptionState& exceptionState) {
On 2016/11/01 14:39:16, Mathieu Perreault wrote:
> Not sure of the convention for documentation in Blink, but this would benefit
> from a quick function comment to mention what are valid values and invalid
> values.

Done.

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:164: if
(value->isNull())
On 2016/11/01 14:39:16, Mathieu Perreault wrote:
> Unless isNull is a valid state where no exception should be returned?

That's right.

"!value" catches objects that cannot be parsed into a JSON value. For example:

var foo = {};
foo.bar = foo;

Recursion is valid for objects, but cannot be parsed into JSON. That's not valid
according to the spec.

"value->isNull()" handles null objects. For example.

var foo = {"bar": null};

That's perfectly valid. So we stringify it as "", which means "no data" for us.

https://codereview.chromium.org/2470463002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:300: void
validateAndConvertPaymentMethodData(
On 2016/11/01 14:39:16, Mathieu Perreault wrote:
> would benefit from a comment such as 
> 
> // Takes the PaymentMethodData as specified by the website/javascript and
> creates a validated Mojo PaymentMethodDataPtr object to be passed to the
> embedder/browser.

Done. Also added a similar comment to validatedAndConvertPaymentDetails().

[commit-bot: I haz the power, 2016-11-02 17:54:42]

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

[commit-bot: I haz the power, 2016-11-02 17:54:43]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-02 20:23:21]

tsepez@chromium.org changed reviewers:
+ jschuh@chromium.org

[Tom Sepez, 2016-11-02 20:23:22]

https://codereview.chromium.org/2470463002/diff/60001/components/payments/pay...
File components/payments/payment_request.mojom (right):

https://codereview.chromium.org/2470463002/diff/60001/components/payments/pay...
components/payments/payment_request.mojom:100: // A JSON string built by the
renderer from a JavaScript object that the
Parsing such an object in the browser is generally frowned upon; we use JSON in
the browser, but only when coming from a trusted source.

Do we need to process this in the browser or just pass it thru?  Presumably the
payment app can defend itself from malformed input?

[please use gerrit instead, 2016-11-02 20:35:17]

https://codereview.chromium.org/2470463002/diff/60001/components/payments/pay...
File components/payments/payment_request.mojom (right):

https://codereview.chromium.org/2470463002/diff/60001/components/payments/pay...
components/payments/payment_request.mojom:100: // A JSON string built by the
renderer from a JavaScript object that the
On 2016/11/02 20:23:22, Tom Sepez wrote:
> Parsing such an object in the browser is generally frowned upon; we use JSON
in
> the browser, but only when coming from a trusted source.
> 
> Do we need to process this in the browser or just pass it thru?  Presumably
the
> payment app can defend itself from malformed input?

PaymentRequestImpl.java sanitizes and parses the string to JSONObject to
validate that the renderer is giving a valid JSON string to the browser. If
that's not the case, the browser tears down the Mojo pipe and closes the
payments UI.

AndroidPayPaymentApp.java and AndroidPayPaymentInstrument.java (not public)
process the JSONObject to determine the flags to use when calling into Android
Pay.

Once 3rd party payment apps are implemented, they will start receiving this as a
string. They will be responsible for protecting themselves.

[Tom Sepez, 2016-11-02 20:46:04]

> PaymentRequestImpl.java sanitizes and parses the string to JSONObject to
> validate that the renderer is giving a valid JSON string to the browser. If
> that's not the case, the browser tears down the Mojo pipe and closes the
> payments UI.
>

Here's an alternate suggestion:  can the renderer strip out the fields that
the browser needs to know about and pass them as separate IPC parameters,
leaving
the varying JSON blob as a string that doesn't get inspected in the browser?

[haraken, 2016-11-03 15:21:31]

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

[haraken, 2016-11-03 15:21:32]

Just drive-by.

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

https://codereview.chromium.org/2470463002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:167:
v8::JSON::Stringify(data.context(), data.v8Value().As<v8::Object>());

Can you use ToLocal instead of MaybeLocal?

[Tom Sepez, 2016-11-03 16:48:21]

Unfortunately, we know android's java implementation is not memory safe so we do
not rely on it for security purposes.

We'll need to pass both a parsed representation as obtained by the renderer and
the JSON blob itself, and use one or the other depending upon whether we're
handling it vs. a plugin.

[Bernhard Bauer, 2016-11-17 10:05:21]

bauerb@chromium.org changed reviewers:
+ bauerb@chromium.org, rsesek@chromium.org

[Bernhard Bauer, 2016-11-17 10:05:22]

On 2016/11/03 16:48:21, Tom Sepez wrote:
> Unfortunately, we know android's java implementation is not memory safe so we
do
> not rely on it for security purposes.

+rsesek

[Citation needed]? We do have couple of other places in the code where we do
exactly this.

[Robert Sesek, 2016-11-18 20:58:01]

rsesek@chromium.org changed reviewers:
+ palmer@chromium.org

[Robert Sesek, 2016-11-18 20:58:02]

+palmer who I discussed this with

On 2016/11/17 10:05:22, Bernhard Bauer wrote:
> On 2016/11/03 16:48:21, Tom Sepez wrote:
> > Unfortunately, we know android's java implementation is not memory safe so
we
> do
> > not rely on it for security purposes.
> 
> +rsesek
> 
> [Citation needed]? We do have couple of other places in the code where we do
> exactly this.

I think this notion stems from this note about DEX verification:
https://android.googlesource.com/platform/dalvik/+/81d713a/docs/verifier.html#23

But that was for Dalvik and similar language does not exist for ART as far as we
can tell. It also wouldn't really affect a JSON parser that is written entirely
in Java and performs no native calls. That said, if we can parse in the
renderer, that's the better place to do it.

[please use gerrit instead, 2016-11-18 20:59:23]

On 2016/11/18 20:58:02, Robert Sesek wrote:
> That said, if we can parse in the
> renderer, that's the better place to do it.

Yep, I've moved all parsing into the renderer. Sorry that this patch looks
abandoned. Some other priorities came up. I will get back to this in 1-2 weeks.

[please use gerrit instead, 2016-11-18 21:00:33]

On 2016/11/18 20:59:23, rouslan wrote:
> On 2016/11/18 20:58:02, Robert Sesek wrote:
> > That said, if we can parse in the
> > renderer, that's the better place to do it.
> 
> Yep, I've moved all parsing into the renderer. Sorry that this patch looks
> abandoned. Some other priorities came up. I will get back to this in 1-2
weeks.

Patch that moved parsing into the renderer: http://crrev.com/2500183002

[please use gerrit instead, 2016-11-23 17:54:56]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:

https://github.com/w3c/browser-payment-api/pull/261/files

Spec:

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

Example:

new PaymentRequest([{
      supportedMethods: ["foo", "bar"],
      data: {
        commonParameter: "abc"
      }
    }], {
      total: {
        label: "Total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      },
      displayItems: [{
        label: "Sub-total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      }],
      modifiers: [{
          supportedMethods: ["bar"],
          total: {
            label: "Total",
            amount: {
              currency: "USD",
              value: "99.00"
            }
          },
          additionalDisplayItems: [{
            label: "Bar discount",
            amount: {
              currency: "USD",
              value: "-1.00"
            }
          }],
          data: {
            discountParameter: "xyz"
          }]
      });

The "commonParameter" in PaymentMethodData and "discountParameter" in
PaymentDetails are payment method specific and not standardized. They
are stringified and passed to the browser as JSON strings.

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 75 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
new PaymentRequest([{
      supportedMethods: ["foo", "bar"],
      data: {
        commonParameter: "abc"
      }
    }], {
      total: {
        label: "Total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      },
      displayItems: [{
        label: "Sub-total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      }],
      modifiers: [{
          supportedMethods: ["bar"],
          total: {
            label: "Total",
            amount: {
              currency: "USD",
              value: "99.00"
            }
          },
          additionalDisplayItems: [{
            label: "Bar discount",
            amount: {
              currency: "USD",
              value: "-1.00"
            }
          }],
          data: {
            discountParameter: "xyz"
          }]
      });

The "commonParameter" in PaymentMethodData and "discountParameter" in
PaymentDetails are payment method specific and not standardized. They
are stringified and passed to the browser as JSON strings.

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 75 lines of code.

BUG=660926
==========

[please use gerrit instead, 2016-11-28 22:16:05]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
new PaymentRequest([{
      supportedMethods: ["foo", "bar"],
      data: {
        commonParameter: "abc"
      }
    }], {
      total: {
        label: "Total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      },
      displayItems: [{
        label: "Sub-total",
        amount: {
          currency: "USD",
          value: "100.00"
        }
      }],
      modifiers: [{
          supportedMethods: ["bar"],
          total: {
            label: "Total",
            amount: {
              currency: "USD",
              value: "99.00"
            }
          },
          additionalDisplayItems: [{
            label: "Bar discount",
            amount: {
              currency: "USD",
              value: "-1.00"
            }
          }],
          data: {
            discountParameter: "xyz"
          }]
      });

The "commonParameter" in PaymentMethodData and "discountParameter" in
PaymentDetails are payment method specific and not standardized. They
are stringified and passed to the browser as JSON strings.

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData
and PaymentDetails. This results in saving around 75 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 30 lines of code.

BUG=660926
==========

[please use gerrit instead, 2016-11-28 22:16:39]

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

[commit-bot: I haz the power, 2016-11-28 22:17:44]

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

[please use gerrit instead, 2016-11-28 22:20:41]

mathp, tsepez, haraken: ptal patch 3.

JSON parsing has been moved to the renderer, so this patch is largely rewritten
and requires a new review.

[commit-bot: I haz the power, 2016-11-28 22:30:06]

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

[commit-bot: I haz the power, 2016-11-28 22:30:07]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[palmer, 2016-11-29 00:24:55]

https://codereview.chromium.org/2470463002/diff/80001/components/payments/pay...
File components/payments/payment_request.mojom (right):

https://codereview.chromium.org/2470463002/diff/80001/components/payments/pay...
components/payments/payment_request.mojom:116: // no one format for this object,
so richer types cannot be used. A simple
Nit: s/richer/more specific/

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

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:192: const
PaymentItem& item = input[i];
Nit: If you want, you could declare |i| and |item| with auto.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:207: const
PaymentShippingOption& option = input[i];
Here, too.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:350: const
PaymentDetailsModifier& modifier = input[i];
Here, too.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:389: for (int i =
shippingOptions.size() - 1; i >= 0; --i) {
If this is a wtf::Vector, you should IWYU, and it returns size_t. Hence, a cast
to int is a change in signedness and possibly width. (There is a width
difference in principle, although not in practice, because Vector::m_size is
declared as `unsigned`.) I think this code should produce a compiler warning?
Even if not, it should.

If you declare |i| as auto or size_t, the `i >= 0` condition will always be
true, so you'll need to come up with a different loop construct that doesn't
also loop forever or read OOB memory. If, instead, you stick with int, consider
base::checked_cast or the like to make sure nothing weird happens.

[haraken, 2016-11-29 01:58:26]

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

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:209:
exceptionState.throwTypeError("ShippingOption id required");

Don't we need to clear output?

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:222: return;

Ditto.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:315: String
stringifiedData = "";

'= ""' won't be needed.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:356: return;

Ditto. Do you need to clear output?

[please use gerrit instead, 2016-11-29 16:19:23]

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

[commit-bot: I haz the power, 2016-11-29 16:19:33]

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

[please use gerrit instead, 2016-11-29 16:19:52]

palmer, haraken: ptal patch 4.

https://codereview.chromium.org/2470463002/diff/80001/components/payments/pay...
File components/payments/payment_request.mojom (right):

https://codereview.chromium.org/2470463002/diff/80001/components/payments/pay...
components/payments/payment_request.mojom:116: // no one format for this object,
so richer types cannot be used. A simple
On 2016/11/29 00:24:55, palmer wrote:
> Nit: s/richer/more specific/

Done.

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

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:192: const
PaymentItem& item = input[i];
On 2016/11/29 00:24:55, palmer wrote:
> Nit: If you want, you could declare |i| and |item| with auto.

I prefer to be explicit, if there's enough space on the line. auto is only
useful when I want to save some space, IMHO. Are there better arguments to use
auto?

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:207: const
PaymentShippingOption& option = input[i];
On 2016/11/29 00:24:55, palmer wrote:
> Here, too.

Acknowledged.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:209:
exceptionState.throwTypeError("ShippingOption id required");
On 2016/11/29 01:58:25, haraken wrote:
> 
> Don't we need to clear output?

No need to clear output when throwing an exception. Output is declared on stack
by the caller of this method. The caller aborts own execution when this function
returns an exception. This deletes the output, so there's no need to also clear
it first.

Added comments to that effect.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:222: return;
On 2016/11/29 01:58:25, haraken wrote:
> 
> Ditto.

Acknowledged.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:315: String
stringifiedData = "";
On 2016/11/29 01:58:25, haraken wrote:
> 
> '= ""' won't be needed.

Removed.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:350: const
PaymentDetailsModifier& modifier = input[i];
On 2016/11/29 00:24:55, palmer wrote:
> Here, too.

Acknowledged.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:356: return;
On 2016/11/29 01:58:25, haraken wrote:
> 
> Ditto. Do you need to clear output?

Acknowledged.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:389: for (int i =
shippingOptions.size() - 1; i >= 0; --i) {
On 2016/11/29 00:24:55, palmer wrote:
> If this is a wtf::Vector, you should IWYU, and it returns size_t. Hence, a
cast
> to int is a change in signedness and possibly width. (There is a width
> difference in principle, although not in practice, because Vector::m_size is
> declared as `unsigned`.) I think this code should produce a compiler warning?
> Even if not, it should.
> 
> If you declare |i| as auto or size_t, the `i >= 0` condition will always be
> true, so you'll need to come up with a different loop construct that doesn't
> also loop forever or read OOB memory. If, instead, you stick with int,
consider
> base::checked_cast or the like to make sure nothing weird happens.

Switched to looping over all elements in the natural order instead.

As for Vector.h, it is included in PaymentRequest.h:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/paymen...

The style guide says IWYU with exception of headers already included in the .h
file corresponding to this .cpp file.

https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

"any includes present in the related header do not need to be included again in
the related cc (i.e., foo.cc can rely on foo.h's includes)."

[please use gerrit instead, 2016-11-29 16:21:30]

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

[please use gerrit instead, 2016-11-29 16:21:30]

krb, ptal components/payments/payment_details_validation.cc in patch 4. Payment
method names can have duplicates now.

[Tom Sepez, 2016-11-29 17:13:06]

lgtm

[Kevin Bailey, 2016-11-29 17:27:59]

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

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:192: const
PaymentItem& item = input[i];
On 2016/11/29 16:19:52, rouslan wrote:
> On 2016/11/29 00:24:55, palmer wrote:
> > Nit: If you want, you could declare |i| and |item| with auto.
> 
> I prefer to be explicit, if there's enough space on the line. auto is only
> useful when I want to save some space, IMHO. Are there better arguments to use
> auto?

Does HeapVector<> support range-based loops?

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:389: for (int i =
shippingOptions.size() - 1; i >= 0; --i) {
On 2016/11/29 16:19:52, rouslan wrote:
> On 2016/11/29 00:24:55, palmer wrote:
> > If this is a wtf::Vector, you should IWYU, and it returns size_t. Hence, a
> cast
> > to int is a change in signedness and possibly width. (There is a width
> > difference in principle, although not in practice, because Vector::m_size is
> > declared as `unsigned`.) I think this code should produce a compiler
warning?
> > Even if not, it should.
> > 
> > If you declare |i| as auto or size_t, the `i >= 0` condition will always be
> > true, so you'll need to come up with a different loop construct that doesn't
> > also loop forever or read OOB memory. If, instead, you stick with int,
> consider
> > base::checked_cast or the like to make sure nothing weird happens.
> 
> Switched to looping over all elements in the natural order instead.
> 
> As for Vector.h, it is included in PaymentRequest.h:
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/paymen...
> 
> The style guide says IWYU with exception of headers already included in the .h
> file corresponding to this .cpp file.
> 
> https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
> 
> "any includes present in the related header do not need to be included again
in
> the related cc (i.e., foo.cc can rely on foo.h's includes)."

I can appreciate the concern in case this size goes over 2 billion :) but in
that case, do we really want to scan through all 2B ? How about a reverse
iterator, or offsetting the index by 1 ?

https://codereview.chromium.org/2470463002/diff/100001/components/payments/pa...
File components/payments/payment_details_validation.cc (right):

https://codereview.chromium.org/2470463002/diff/100001/components/payments/pa...
components/payments/payment_details_validation.cc:112: if
(!modifier->method_data) {
This looks fine; it just concerns me that the IDL continues to change this late.

[commit-bot: I haz the power, 2016-11-29 17:55:02]

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

[commit-bot: I haz the power, 2016-11-29 17:55:03]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-11-29 19:26:31]

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

[please use gerrit instead, 2016-11-29 19:26:53]

krb, ptal patch 5.

haraken, should I IWYU for DCHECK() and arraysize()? Including "base/logging.h"
and "base/macros.h" from third_party/WebKit feels unorthodox.

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

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:192: const
PaymentItem& item = input[i];
On 2016/11/29 17:27:59, Kevin Bailey wrote:
> On 2016/11/29 16:19:52, rouslan wrote:
> > On 2016/11/29 00:24:55, palmer wrote:
> > > Nit: If you want, you could declare |i| and |item| with auto.
> > 
> > I prefer to be explicit, if there's enough space on the line. auto is only
> > useful when I want to save some space, IMHO. Are there better arguments to
use
> > auto?
> 
> Does HeapVector<> support range-based loops?

Yes it does. Vector used to not have .append() method. But now that I look at
Vector.h again, I see that .append() and .back() were added. This eliminates the
need for using Vector::resize(n) and index operator.

https://codereview.chromium.org/2470463002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:389: for (int i =
shippingOptions.size() - 1; i >= 0; --i) {
On 2016/11/29 17:27:59, Kevin Bailey wrote:
> On 2016/11/29 16:19:52, rouslan wrote:
> > On 2016/11/29 00:24:55, palmer wrote:
> > > If this is a wtf::Vector, you should IWYU, and it returns size_t. Hence, a
> > cast
> > > to int is a change in signedness and possibly width. (There is a width
> > > difference in principle, although not in practice, because Vector::m_size
is
> > > declared as `unsigned`.) I think this code should produce a compiler
> warning?
> > > Even if not, it should.
> > > 
> > > If you declare |i| as auto or size_t, the `i >= 0` condition will always
be
> > > true, so you'll need to come up with a different loop construct that
doesn't
> > > also loop forever or read OOB memory. If, instead, you stick with int,
> > consider
> > > base::checked_cast or the like to make sure nothing weird happens.
> > 
> > Switched to looping over all elements in the natural order instead.
> > 
> > As for Vector.h, it is included in PaymentRequest.h:
> > 
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/paymen...
> > 
> > The style guide says IWYU with exception of headers already included in the
.h
> > file corresponding to this .cpp file.
> > 
> >
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes
> > 
> > "any includes present in the related header do not need to be included again
> in
> > the related cc (i.e., foo.cc can rely on foo.h's includes)."
> 
> I can appreciate the concern in case this size goes over 2 billion :) but in
> that case, do we really want to scan through all 2B ? How about a reverse
> iterator, or offsetting the index by 1 ?

The concern was compiler warning, which should be triggered, but has not yet.
Forward loop makes the code more clear and eliminates the possibility of a
warning.

https://codereview.chromium.org/2470463002/diff/100001/components/payments/pa...
File components/payments/payment_details_validation.cc (right):

https://codereview.chromium.org/2470463002/diff/100001/components/payments/pa...
components/payments/payment_details_validation.cc:112: if
(!modifier->method_data) {
On 2016/11/29 17:27:59, Kevin Bailey wrote:
> This looks fine; it just concerns me that the IDL continues to change this
late.

Acknowledged.

[commit-bot: I haz the power, 2016-11-29 19:27:01]

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

[please use gerrit instead, 2016-11-29 19:27:31]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 30 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926
==========

[commit-bot: I haz the power, 2016-11-30 01:12:00]

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

[commit-bot: I haz the power, 2016-11-30 01:12:01]

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

[haraken, 2016-11-30 04:49:22]

LGTM

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

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

[commit-bot: I haz the power, 2016-11-30 17:59:44]

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-30 22:09:27]

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

[commit-bot: I haz the power, 2016-11-30 22:09:28]

Dry run: This issue passed the CQ dry run.

[Kevin Bailey, 2016-12-01 17:48:31]

lgtm

[Bernhard Bauer, 2016-12-01 17:57:26]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926
==========

[Bernhard Bauer, 2016-12-01 17:57:26]

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

[please use gerrit instead, 2016-12-01 18:19:06]

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

[please use gerrit instead, 2016-12-01 19:52:42]

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

[commit-bot: I haz the power, 2016-12-01 19:53: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-12-01 21:50:07]

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

[commit-bot: I haz the power, 2016-12-01 21:50:09]

Dry run: This issue passed the CQ dry run.

[palmer, 2016-12-01 23:10:00]

lgtm

[please use gerrit instead, 2016-12-01 23:50:51]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-12-01 23:50:51]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org,
tsepez@chromium.org, haraken@chromium.org, krb@chromium.org
Link to the patchset: https://codereview.chromium.org/2470463002/#ps160001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-01 23:51:42]

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 00:00:11]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1480636251293660,
"parent_rev": "401dc1c00bcbcedad93ec3a72f4fd43e11525c0f", "commit_rev":
"034328abeb06e8ac9b5120af6cb1a0c077264b9e"}

[commit-bot: I haz the power, 2016-12-02 00:00:38]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926
==========

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

Committed patchset #7 (id:160001)

[commit-bot: I haz the power, 2016-12-02 00:03:24]

Description was changed from

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926
==========

to

==========
Add data parameter to payment details modifier.

This patch adds the data parameter for payment details modifier that is
a JSON-serializable object with optional information that might be
needed by the supported payment methods. For example, {"gateway":
"company"}. The data parameter is behind a runtime flag.

Added in:
https://github.com/w3c/browser-payment-api/pull/261/files

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

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/Dpc5Ftqgxlg/ZYenHNR0...

Example:
var supportedMethods = [{supportedMethods: ["basic-card"]}];
var shoppingCart = {total: {title: "Total",
                            currency: {code: "USD", value: "5.00"}}};
shoppingCart.modifiers = [{
    data: {supportedType: ["debit"]}, // THE NEW FEATURE
    supportedMethods: ["basic-card"],
    total: {title: "Total",
            currency: {code: "USD", value: "4.63"}}}];
new PaymentRequest(supportedMethods, shoppingCart);

To better share the object validation and stringification logic, this
patch merges the validation and conversion code for PaymentMethodData,
PaymentDetails, and PaymentDetailsModifier. This results in saving
around 50 lines of code.

BUG=660926

Committed: https://crrev.com/736ccd21ff339cf3243ee5857d3de0a661dfec3c
Cr-Commit-Position: refs/heads/master@{#435780}
==========

[commit-bot: I haz the power, 2016-12-02 00:03:26]

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