Bug 88278

Summary: unprefix window.performance.webkitNow()
Product: WebKit Reporter: Masataka Yakura <myakura.web>
Component: WebCore Misc.Assignee: James Simonsen <simonjam>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eoconnor, gyuyoung.kim, jamesr, nduca, ojan, paulirish, rakuco, rniwa, sam, simonjam, syoichi, tonyg, webkit.review.bot
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 84912, 98953    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Masataka Yakura
Reported 2012-06-04 18:55:19 PDT
The spec has reached Candidate Recommendation and Gecko has implemented it without prefix. We can drop the prefix. http://www.w3.org/TR/hr-time/ https://bugzilla.mozilla.org/show_bug.cgi?id=539095
Attachments
Patch (9.42 KB, patch)
2012-08-07 20:12 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Patch (14.78 KB, patch)
2012-10-11 15:02 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Patch for landing (14.77 KB, patch)
2012-10-11 15:42 PDT, James Simonsen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
James Simonsen
Comment 1 2012-08-07 20:12:30 PDT
Created attachment 157094 [details] Patch
James Simonsen
Comment 2 2012-08-07 20:13:18 PDT
Unprefixing it sounds good to me.
James Robinson
Comment 3 2012-08-07 20:47:25 PDT
Comment on attachment 157094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157094&action=review > Source/WebCore/page/Performance.idl:56 > - double webkitNow(); > + double now(); I agree we want to unprefix, but could/should we keep webkitNow() as an alias for now (or perhaps collect some data about how many pages are using this entry point)?
James Simonsen
Comment 4 2012-08-08 11:40:49 PDT
(In reply to comment #3) > (From update of attachment 157094 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=157094&action=review > > > Source/WebCore/page/Performance.idl:56 > > - double webkitNow(); > > + double now(); > > I agree we want to unprefix, but could/should we keep webkitNow() as an alias for now (or perhaps collect some data about how many pages are using this entry point)? Personally, I'd rather just make the cut and get it done with. If we keep it around, they'll just continue using it. Better to break it earlier rather than later. We did the same thing with Navigation Timing, so this WG has some precedent for it. It didn't end up being a problem.
James Robinson
Comment 5 2012-08-08 12:51:00 PDT
Comment on attachment 157094 [details] Patch Sounds good to me. Make sure that devrel is aware of the change and has a chance to update their docs/tutorials/etc before this hits users.
James Simonsen
Comment 6 2012-08-08 12:59:54 PDT
Actually, before we land this, we should make the timebase compliant with the spec. I'll do that this afternoon.
James Simonsen
Comment 7 2012-10-11 15:02:24 PDT
Created attachment 168283 [details] Patch
James Simonsen
Comment 8 2012-10-11 15:03:52 PDT
After much spec discussion, I'm planning to land this for real now. I've uploaded a new patch. I'd appreciate an r+ on it.
Tony Gentilcore
Comment 9 2012-10-11 15:33:47 PDT
Comment on attachment 168283 [details] Patch Also worth noting this is unprefixed in IE10 as well as moz.
James Simonsen
Comment 10 2012-10-11 15:42:52 PDT
Created attachment 168293 [details] Patch for landing
WebKit Review Bot
Comment 11 2012-10-11 16:08:13 PDT
Comment on attachment 168293 [details] Patch for landing Clearing flags on attachment: 168293 Committed r131106: <http://trac.webkit.org/changeset/131106>
WebKit Review Bot
Comment 12 2012-10-11 16:08:19 PDT
All reviewed patches have been landed. Closing bug.
Ryosuke Niwa
Comment 13 2012-11-20 16:10:11 PST
This patch broke magnitude-perf.js.
Ojan Vafai
Comment 14 2012-11-20 16:31:23 PST
(In reply to comment #13) > This patch broke magnitude-perf.js. How is that?
Ojan Vafai
Comment 15 2012-11-20 16:37:24 PST
I see https://bugs.webkit.org/show_bug.cgi?id=102848
Adam Barth
Comment 16 2012-11-20 16:37:58 PST
Comment on attachment 168293 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=168293&action=review > LayoutTests/resources/magnitude-perf.js:204 > - var nowFunction = window.performance.now || Date.now; > + var nowFunction = window.performance.now.bind(window.performance) || Date.now; This didn't fix it?
Ojan Vafai
Comment 17 2012-11-20 16:43:55 PST
(In reply to comment #16) > (From update of attachment 168293 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=168293&action=review > > > LayoutTests/resources/magnitude-perf.js:204 > > - var nowFunction = window.performance.now || Date.now; > > + var nowFunction = window.performance.now.bind(window.performance) || Date.now; > > This didn't fix it? This looks backwards to me. The old code looks correct. The new code will throw an exception of window.performance.now is undefined.
Note You need to log in before you can comment on or make changes to this bug.