fix: convert PaymentMethodData.data

[marcoscaceres]

Closes #813

Part 1 of #753

Converts PaymentMethodData.data to IDL type or object.

The following tasks have been completed:

• Confirmed there are no ReSpec errors/warnings.
• Modified Web platform tests
• Modified MDN Docs - obscure detail, not required.

Implementation commitment:

• Safari
• Chrome - Implemented
• Firefox - Implemented
• Edge (public signal)

Optional, Impact on Payment Handler spec?

---

Preview | Diff

[marcoscaceres]

Both Chrome and Firefox do the IDL conversion already.

[marcoscaceres]

@domenic hopefully good now.

[marcoscaceres]

@domenic, because in implementations, PaymentMethod.data's JS-to-IDL conversion happens in the PaymentRequest constructor (instead of on .show()), it means we don't need the [[serialized*]] data internal slots because:

• We can do the "JSON serializable" check at construction time and when updateWith() is called: this gives us assurances that .data doesn't contain any un-serializable JSON types and circular references....and, if ever needed, .data can be serialized to a string in the future.
• And because of the above, we can just internally hold converted IDL values for each PaymentRequest instance, without the need to hold them in string form.

This also fixes what would otherwise be a mess when show()ing or updateWith()ing a payment sheet: the [[serializedModifierData]] array positions would need to match the position of the deleted .data members.

So, with this PR, when a payment handler shows or gets updated, it always operates on its expected IDL type, without needing to redo any redundant JSON parsing, re-conversions, etc.

[domenic]

Can you separate the editorial changes (removing lots of data-lts) from the normative ones, to make this easier to review? I think I found the big normative change and the issue with it, but it's hard to tell what else might be there.

[domenic]

So the problem with not storing the JSON serialized version is that the JSON serialization now becomes meaningless. Consider the following:

const bcr1 = { supportedNetworks: new Set("foo", "bar") };

With the new spec, the JSON-serialization will be { "supportedNetworks": {} }. But the value stored in paymentMethods will be { supportedNetworks: ["foo", "bar"] } since Web IDL dictionary conversion converts all iterables to sequences.

This means that if someone, e.g., passed the JSON serialization across a process boundary and used it to attempt to reconstruct the result on the other side, they would not be able to match the spec. I thought our original spec was motivated exactly by that design.

[marcoscaceres]

I believe the original design was motivated by some kind of security concern that was supposed to be mitigated by doing a JSON serialization on .data whose type is object to remove things like Function being added as properties.

[domenic]

It may be good to check with implementations then. But a spec that JSON serializes then disregards the result seems incoherent.

[marcoscaceres]

Gecko only does the IDL conversion, because we don't care about any payment method apart from "Basic Card". So, we don't store any [[serializedMethodData]] in our code... in fact, I don't see is doing any we JSON serialization of data at all in Gecko's code (@edenchuang?).

From the constructor, we check the method's details:
https://github.com/mozilla/gecko-dev/blob/master/dom/payments/PaymentRequest.cpp#L574

We then just check IsValidBasicCardRequest:
https://github.com/mozilla/gecko-dev/blob/master/dom/payments/PaymentRequest.cpp#L293

And then we convert it (to check throw if it fails):
https://github.com/mozilla/gecko-dev/blob/master/dom/payments/BasicCardPayment.cpp#L53

WebKit does do the serialization and stores it. However, @aestes also seems to suggest a preference to not store the JSON serialization: "Browsers could convert from a JS object directly to a IDL dictionary rather than the current JS object > JSON string > JS object > IDL dictionary conversion algorithm".

[domenic]

Thanks. @rsolomakhin?

[marcoscaceres]

Went back to look at #346 ... I'd forgotten that the ideas was to support actually sending the JSON-serialized data no just across process boundaries (which I was imagining was just as per-origin process isolation) - but to other native applications acting as payment handlers.

As such, I concur that it still makes sense to keep the JSON serialization.