High res times should start at 0

• Component: Platform
• Status: RESOLVED FIXED

[James Simonsen]

The spec has changed to define 0 as the value of navigationStart for the document. All times returned by window.performance.webkitNow() should be relative to that.

convertMonotonicTimeToDocumentTime() should automatically subtract off the value of navigationStart.

Note that doing so will break the time values returned by window.performance.timing. These values are supposed to use the monotonic clock, but not rebase their values to 0. To fix this, perhaps we could add a legacy accessor that doesn't subtract out navigationStart. This should be clearly marked as legacy, because all future uses are expected to use convertMonotonicTimeToDocumentTime().

[James Simonsen]

Created attachment 168065 [details]
Patch

[Build Bot]

Comment on attachment 168065 [details]
Patch

Attachment 168065 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14252319

[James Simonsen]

Created attachment 168081 [details]
Patch

[James Simonsen]

I think this is ready for review. This should get us in line with the spec for performance.now() without breaking Navigation Timing. Next, I'll import the W3 test suite for now() and work on unprefixing that.

[Build Bot]

Comment on attachment 168081 [details]
Patch

Attachment 168081 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14259241

[James Simonsen]

Created attachment 168090 [details]
Patch

[Tony Gentilcore]

Comment on attachment 168090 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168090&action=review

> Source/WebCore/loader/DocumentLoadTiming.cpp:57
> +double DocumentLoadTiming::convertMonotonicTimeToLegacyDocumentTime(double monotonicTime) const

I'd consider s/Legacy/WallBased/ with a comment that new users should use zero-based. That name is more synergistic with ZeroBasedDocumentTime and better describes what the method does.

> Source/WebCore/loader/DocumentLoadTiming.h:41
> +    double convertMonotonicTimeToZeroBasedDocumentTime(double) const;

You could consider dropping the term "convert" from these methods. The name will be just as descriptive but a little more terse.

[James Simonsen]

Created attachment 168112 [details]
Patch for landing

[James Simonsen]

(In reply to comment #7)
> (From update of attachment 168090 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168090&action=review
> 
> > Source/WebCore/loader/DocumentLoadTiming.cpp:57
> > +double DocumentLoadTiming::convertMonotonicTimeToLegacyDocumentTime(double monotonicTime) const
> 
> I'd consider s/Legacy/WallBased/ with a comment that new users should use zero-based. That name is more synergistic with ZeroBasedDocumentTime and better describes what the method does.

Went with PseudoWallTime

> 
> > Source/WebCore/loader/DocumentLoadTiming.h:41
> > +    double convertMonotonicTimeToZeroBasedDocumentTime(double) const;
> 
> You could consider dropping the term "convert" from these methods. The name will be just as descriptive but a little more terse.

Done.

[WebKit Review Bot]

Comment on attachment 168112 [details]
Patch for landing

Clearing flags on attachment: 168112

Committed r131001: <http://trac.webkit.org/changeset/131001>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.