Bug 84912

Summary: High res times should start at 0
Product: WebKit Reporter: James Simonsen <simonjam>
Component: PlatformAssignee: James Simonsen <simonjam>
Status: RESOLVED FIXED    
Severity: Normal CC: japhet, nduca, sullivan, tonyg, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://dvcs.w3.org/hg/webperf/raw-file/tip/specs/HighResolutionTime/Overview.html
Bug Depends on: 98223    
Bug Blocks: 66683, 66684, 88278    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

James Simonsen
Reported 2012-04-25 17:19:38 PDT
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().
Attachments
Patch (16.07 KB, patch)
2012-10-10 13:34 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Patch (17.49 KB, patch)
2012-10-10 15:46 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Patch (17.49 KB, patch)
2012-10-10 16:31 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Patch for landing (18.09 KB, patch)
2012-10-10 18:27 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
James Simonsen
Comment 1 2012-10-10 13:34:31 PDT
Created attachment 168065 [details] Patch
Build Bot
Comment 2 2012-10-10 14:17:00 PDT
Comment on attachment 168065 [details] Patch Attachment 168065 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14252319
James Simonsen
Comment 3 2012-10-10 15:46:36 PDT
Created attachment 168081 [details] Patch
James Simonsen
Comment 4 2012-10-10 15:58:33 PDT
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 5 2012-10-10 16:10:02 PDT
Comment on attachment 168081 [details] Patch Attachment 168081 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14259241
James Simonsen
Comment 6 2012-10-10 16:31:18 PDT
Created attachment 168090 [details] Patch
Tony Gentilcore
Comment 7 2012-10-10 18:09:32 PDT
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
Comment 8 2012-10-10 18:27:28 PDT
Created attachment 168112 [details] Patch for landing
James Simonsen
Comment 9 2012-10-10 18:27:48 PDT
(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 10 2012-10-10 20:06:23 PDT
Comment on attachment 168112 [details] Patch for landing Clearing flags on attachment: 168112 Committed r131001: <http://trac.webkit.org/changeset/131001>
WebKit Review Bot
Comment 11 2012-10-10 20:06:27 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.