w3c / push-api

Push API
https://w3c.github.io/push-api/
Other
145 stars 40 forks source link

Migrate the serializer to a toJSON method. #267

Closed beverloo closed 7 years ago

beverloo commented 7 years ago

Fixes #266. How's this, @tobie?


Preview | Diff

tobie commented 7 years ago

@domenic, do you want to give my comments a quick second look to see if we're in agreement, here?

beverloo commented 7 years ago

Thank you for the feedback!

I guess it's a bit of both, underlying value refers to the attribute's behaviour so for the dictionary we can use it. Not having branches is significant here from a timing point of view, as these operations expose cryptographic data to the developer.

tobie commented 7 years ago

Yeah, from my perspective, the serialization happens in JSON.stringify, not when calling obj.toJSON() (which just returns a JS object). But others might see things differently.

beverloo commented 7 years ago

Cheers again, all applied. Two follow-ups: the ReSpec issue (https://github.com/w3c/respec/issues/1297) and deciding on adding a dependency on infra. I moved the constant time statement to the attribute.

I'll submit this, happy to address any other follow-ups :).