Bug 84912 - High res times should start at 0
: High res times should start at 0
Status: RESOLVED FIXED
: WebKit
Platform
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
: http://dvcs.w3.org/hg/webperf/raw-fil...
:
: 98223
: 66683 66684 88278
  Show dependency treegraph
 
Reported: 2012-04-25 17:19 PST by
Modified: 2012-10-10 20:06 PST (History)


Attachments
Patch (16.07 KB, patch)
2012-10-10 13:34 PST, James Simonsen
no flags Review Patch | Details | Formatted Diff | Diff
Patch (17.49 KB, patch)
2012-10-10 15:46 PST, James Simonsen
no flags Review Patch | Details | Formatted Diff | Diff
Patch (17.49 KB, patch)
2012-10-10 16:31 PST, James Simonsen
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (18.09 KB, patch)
2012-10-10 18:27 PST, James Simonsen
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-04-25 17:19:38 PST
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().
------- Comment #1 From 2012-10-10 13:34:31 PST -------
Created an attachment (id=168065) [details]
Patch
------- Comment #2 From 2012-10-10 14:17:00 PST -------
(From update of attachment 168065 [details])
Attachment 168065 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14252319
------- Comment #3 From 2012-10-10 15:46:36 PST -------
Created an attachment (id=168081) [details]
Patch
------- Comment #4 From 2012-10-10 15:58:33 PST -------
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.
------- Comment #5 From 2012-10-10 16:10:02 PST -------
(From update of attachment 168081 [details])
Attachment 168081 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14259241
------- Comment #6 From 2012-10-10 16:31:18 PST -------
Created an attachment (id=168090) [details]
Patch
------- Comment #7 From 2012-10-10 18:09:32 PST -------
(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.

> 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.
------- Comment #8 From 2012-10-10 18:27:28 PST -------
Created an attachment (id=168112) [details]
Patch for landing
------- Comment #9 From 2012-10-10 18:27:48 PST -------
(In reply to comment #7)
> (From update of attachment 168090 [details] [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.
------- Comment #10 From 2012-10-10 20:06:23 PST -------
(From update of attachment 168112 [details])
Clearing flags on attachment: 168112

Committed r131001: <http://trac.webkit.org/changeset/131001>
------- Comment #11 From 2012-10-10 20:06:27 PST -------
All reviewed patches have been landed.  Closing bug.