WICG / cookie-store

Asynchronous access to cookies from JavaScript
https://wicg.github.io/cookie-store/
Apache License 2.0
143 stars 35 forks source link

Normative: Change use of EpochTimeStamp to DOMHighResTimeStamp #214

Closed inexorabletash closed 1 year ago

inexorabletash commented 1 year ago

EpochTimeStamp is a typedef for unsigned long long and deprecated, and DOMHighResTimeStamp is a typedef for double and the preferred type going forward. Semantics are the same.

Resolves #205


Preview | Diff

domenic commented 1 year ago

Semantics of the two are not the same: DOMHighResTimeStamp is time since document creation; EpochTimeStamp is time since the epoch.

domenic commented 1 year ago

Filed https://github.com/w3c/hr-time/issues/149

ayuishii commented 1 year ago

Thanks @domenic for the clarification, and filing the issue!

inexorabletash commented 1 year ago

Hmmm... "The DOMHighResTimeStamp type is used to store a duration in milliseconds. Depending on its context, it may represent the moment that is this duration after a base moment like a time origin or the Unix epoch." source (emphasis mine)

The algorithms https://wicg.github.io/cookie-store/#as-a-timestamp and https://wicg.github.io/cookie-store/#date-serialize are explicit about the base moment and specify ignoring leap seconds, so at least from cookie store's perspective they're the same I think?

But thanks for jumping in @domenic ! Your guidance is always welcome.

domenic commented 1 year ago

Oh, I might just be wrong then... I didn't recall any instances of DOMHighResTimeStamp being epoch-relative, but looking through Webdex I found that WebTransport is using it that way. So this is probably fine to proceed, although I still find the naming of the two types confusing and will discuss that in the issue I opened.

inexorabletash commented 1 year ago

We're not in a rush to land this (also blocked on Bikeshed etc fun) so I'm happy to wait for the other discussion to make progress.

domenic commented 1 year ago

My tentative conclusion (https://github.com/w3c/hr-time/issues/149#issuecomment-1432299178) is that landing this, and doing the same thing to the Notifications API, would be a good thing because then we could kill EpochTimeStamp entirely and I would be less confused. But indeed, hearing from a few other folks (like the HR-TIME editors) would be welcome.

inexorabletash commented 1 year ago

Going ahead and merging this. We haven't figured out a way to test it from script. Also I revisited the hr-time definitions and I don't see our conversions to or from a timestamp being simplified (although we could align wording slightly better).