Define the EpochTimeStamp typedef #124

marcoscaceres · 2021-09-09T06:28:54Z

Closes #123

See whatwg/webidl#2 for discussion. Bugs have been filed with the relevant specifications telling them to move away from using DOMTimeStamp.

In particular, if the change results in potential backwards compatibility issues and content breakage, it needs to be justified.

As this is a typedef, it has no impact on web compat itself.

For normative spec changes, please get implementation commitments anf file issues on the various engines during the review process. All tasks should be complete before merging.

Implementation commitment - primarily for "change" and other normative changes:

Tasks:

[Added tests] - WPT will handle this automatically once each dependent spec is updated.

Preview | Diff

index.html

yoavweiss · 2021-09-23T08:50:56Z

Maybe instead of calling this LegacyTimestamp we can just call it EpochTimestamp? That's the functional difference between it and DOMHighResTimestamp, which could be useful for some, even in the future.

marcoscaceres · 2021-09-29T08:12:41Z

index.html

+        <li>If |epoch| was not passed, set |epoch| to 00:00:00 UTC on 1 January
+        1970.
+        </li>
+        <li>Return the number of milliseconds from |epoch| to the current time.


The ECMAScript spec uses "current time", so I just went with that... is that ok?

index.html

annevk

Thanks for working on this. My main feedback is on the algorithm, along similar lines to @yoavweiss's remark.

index.html

yoavweiss

LGTM

annevk

This looks great, the one thing you might want to add is an Assert that the time passed is after 1970. Probably no need for a before assert as it's so far into the future...

marcoscaceres · 2021-10-07T19:46:06Z

@annevk, ok to merge?

annevk · 2021-10-08T05:08:02Z

Yeah!

index.html

marcoscaceres · 2021-10-11T03:12:36Z

Filed gecko bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1735098

yoavweiss · 2021-10-11T04:07:10Z

Chromium bug

marcoscaceres · 2021-10-11T04:19:07Z

File webkit bug and updated the template.

https://bugs.webkit.org/show_bug.cgi?id=231496 Reviewed by Sam Weinig. Source/WebCore: DOMTimeStamp was renamed EpochTimeStamp. There is no observable behavioral change - it's just a name change. Relevant WebIDL discussions/issue: - whatwg/webidl#2 Which lead to: - w3c/hr-time#124 * Headers.cmake: * Modules/geolocation/Geolocation.cpp: (WebCore::createGeolocationPosition): (WebCore::Geolocation::haveSuitableCachedPosition): * Modules/geolocation/GeolocationPosition.h: (WebCore::GeolocationPosition::create): (WebCore::GeolocationPosition::timestamp const): (WebCore::GeolocationPosition::GeolocationPosition): * Modules/geolocation/GeolocationPosition.idl: * Modules/notifications/Notification.idl: * Modules/push-api/PushSubscription.cpp: (WebCore::PushSubscription::PushSubscription): (WebCore::PushSubscription::expirationTime const): * Modules/push-api/PushSubscription.h: * Modules/push-api/PushSubscription.idl: * Modules/push-api/PushSubscriptionJSON.h: * Modules/push-api/PushSubscriptionJSON.idl: * Modules/push-api/PushSubscriptionOptions.h: * WebCore.xcodeproj/project.pbxproj: * bindings/scripts/IDLParser.pm: (addBuiltinTypedefs): * bindings/scripts/test/TestTypedefs.idl: * dom/EpochTimeStamp.h: Renamed from Source/WebCore/dom/DOMTimeStamp.h. (WebCore::convertSecondsToEpochTimeStamp): (WebCore::convertEpochTimeStampToSeconds): * testing/Internals.cpp: (WebCore::Internals::createPushSubscription): * testing/Internals.h: * testing/Internals.idl: Source/WebKitLegacy/mac: * DOM/DOMEvent.h: * DOM/DOMEvent.mm: (-[DOMEvent timeStamp]): * DOM/DOMObject.h: Source/WebKitLegacy/win: * DOMEventsClasses.cpp: (DOMEvent::timeStamp): * DOMEventsClasses.h: (DOMUIEvent::timeStamp): (DOMKeyboardEvent::timeStamp): (DOMMouseEvent::timeStamp): (DOMMutationEvent::timeStamp): (DOMOverflowEvent::timeStamp): (DOMWheelEvent::timeStamp): * Interfaces/DOMEvents.idl: Canonical link: https://commits.webkit.org/242783@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@283912 268f45cc-cd09-0410-ab3c-d52691b4dbfc

marcoscaceres · 2021-10-12T07:17:02Z

cdumez updated WebKit already 🤯: https://bugs.webkit.org/show_bug.cgi?id=231496

https://bugs.webkit.org/show_bug.cgi?id=231496 Reviewed by Sam Weinig. Source/WebCore: DOMTimeStamp was renamed EpochTimeStamp. There is no observable behavioral change - it's just a name change. Relevant WebIDL discussions/issue: - whatwg/webidl#2 Which lead to: - w3c/hr-time#124 * Headers.cmake: * Modules/geolocation/Geolocation.cpp: (WebCore::createGeolocationPosition): (WebCore::Geolocation::haveSuitableCachedPosition): * Modules/geolocation/GeolocationPosition.h: (WebCore::GeolocationPosition::create): (WebCore::GeolocationPosition::timestamp const): (WebCore::GeolocationPosition::GeolocationPosition): * Modules/geolocation/GeolocationPosition.idl: * Modules/notifications/Notification.idl: * Modules/push-api/PushSubscription.cpp: (WebCore::PushSubscription::PushSubscription): (WebCore::PushSubscription::expirationTime const): * Modules/push-api/PushSubscription.h: * Modules/push-api/PushSubscription.idl: * Modules/push-api/PushSubscriptionJSON.h: * Modules/push-api/PushSubscriptionJSON.idl: * Modules/push-api/PushSubscriptionOptions.h: * WebCore.xcodeproj/project.pbxproj: * bindings/scripts/IDLParser.pm: (addBuiltinTypedefs): * bindings/scripts/test/TestTypedefs.idl: * dom/EpochTimeStamp.h: Renamed from Source/WebCore/dom/DOMTimeStamp.h. (WebCore::convertSecondsToEpochTimeStamp): (WebCore::convertEpochTimeStampToSeconds): * testing/Internals.cpp: (WebCore::Internals::createPushSubscription): * testing/Internals.h: * testing/Internals.idl: Source/WebKitLegacy/mac: * DOM/DOMEvent.h: * DOM/DOMEvent.mm: (-[DOMEvent timeStamp]): * DOM/DOMObject.h: Source/WebKitLegacy/win: * DOMEventsClasses.cpp: (DOMEvent::timeStamp): * DOMEventsClasses.h: (DOMUIEvent::timeStamp): (DOMKeyboardEvent::timeStamp): (DOMMouseEvent::timeStamp): (DOMMutationEvent::timeStamp): (DOMOverflowEvent::timeStamp): (DOMWheelEvent::timeStamp): * Interfaces/DOMEvents.idl: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@283912 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Add the EpochTimeStamp, which can be used to incrementally rename/remove DOMTimeStamp. See also w3c/hr-time#124 and whatwg/webidl#1021 Differential Revision: https://phabricator.services.mozilla.com/D128030

Define the LegacyTimeStamp typedef

4383c8d

This was referenced Sep 9, 2021

Remove DOMTimeStamp whatwg/webidl#1021

Merged

DOMTimeStamp confusion whatwg/webidl#2

Closed

yoavweiss reviewed Sep 9, 2021

View reviewed changes

index.html Outdated Show resolved Hide resolved

marcoscaceres changed the title ~~Define the LegacyTimeStamp typedef~~ Define the EpochTimeStamp typedef Sep 29, 2021

rename EpochTimeStamp + steps

a4deab8

marcoscaceres requested a review from yoavweiss September 29, 2021 08:11

marcoscaceres commented Sep 29, 2021

View reviewed changes

slightly change format

c41d0d8

marcoscaceres marked this pull request as ready for review October 1, 2021 01:36

marcoscaceres mentioned this pull request Oct 1, 2021

Switch DOMTimeStamp to EpochTimeStamp w3c/geolocation-api#104

Merged

4 tasks

yoavweiss reviewed Oct 1, 2021

View reviewed changes

index.html Outdated Show resolved Hide resolved

annevk reviewed Oct 1, 2021

View reviewed changes

index.html Outdated Show resolved Hide resolved

Review feedback

71a3e74

yoavweiss approved these changes Oct 6, 2021

View reviewed changes

annevk approved these changes Oct 6, 2021

View reviewed changes

add assert

74d4c4b

marcoscaceres merged commit 160cde6 into gh-pages Oct 9, 2021

marcoscaceres deleted the legacyTimestamp branch October 9, 2021 08:38

TimothyGu reviewed Oct 9, 2021

View reviewed changes

index.html Show resolved Hide resolved

marcoscaceres mentioned this pull request Oct 11, 2021

Switch to EpochTimeStamp w3c/push-api#338

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define the EpochTimeStamp typedef #124

Define the EpochTimeStamp typedef #124

marcoscaceres commented Sep 9, 2021 •

edited

yoavweiss commented Sep 23, 2021

marcoscaceres Sep 29, 2021

annevk left a comment

yoavweiss left a comment

annevk left a comment

marcoscaceres commented Oct 7, 2021

annevk commented Oct 8, 2021

marcoscaceres commented Oct 11, 2021

yoavweiss commented Oct 11, 2021

marcoscaceres commented Oct 11, 2021

marcoscaceres commented Oct 12, 2021

Define the EpochTimeStamp typedef #124

Define the EpochTimeStamp typedef #124

Conversation

marcoscaceres commented Sep 9, 2021 • edited

yoavweiss commented Sep 23, 2021

marcoscaceres Sep 29, 2021

Choose a reason for hiding this comment

annevk left a comment

Choose a reason for hiding this comment

yoavweiss left a comment

Choose a reason for hiding this comment

annevk left a comment

Choose a reason for hiding this comment

marcoscaceres commented Oct 7, 2021

annevk commented Oct 8, 2021

marcoscaceres commented Oct 11, 2021

yoavweiss commented Oct 11, 2021

marcoscaceres commented Oct 11, 2021

marcoscaceres commented Oct 12, 2021

marcoscaceres commented Sep 9, 2021 •

edited