Bug 88278 - unprefix window.performance.webkitNow()
Summary: unprefix window.performance.webkitNow()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Simonsen
URL:
Keywords: WebExposed
Depends on: 84912 98953
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-04 18:55 PDT by Masataka Yakura
Modified: 2012-11-20 16:43 PST (History)
14 users (show)

See Also:


Attachments
Patch (9.42 KB, patch)
2012-08-07 20:12 PDT, James Simonsen
no flags Details | Formatted Diff | Diff
Patch (14.78 KB, patch)
2012-10-11 15:02 PDT, James Simonsen
no flags Details | Formatted Diff | Diff
Patch for landing (14.77 KB, patch)
2012-10-11 15:42 PDT, James Simonsen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Masataka Yakura 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
Comment 1 James Simonsen 2012-08-07 20:12:30 PDT
Created attachment 157094 [details]
Patch
Comment 2 James Simonsen 2012-08-07 20:13:18 PDT
Unprefixing it sounds good to me.
Comment 3 James Robinson 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)?
Comment 4 James Simonsen 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.
Comment 5 James Robinson 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.
Comment 6 James Simonsen 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.
Comment 7 James Simonsen 2012-10-11 15:02:24 PDT
Created attachment 168283 [details]
Patch
Comment 8 James Simonsen 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.
Comment 9 Tony Gentilcore 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.
Comment 10 James Simonsen 2012-10-11 15:42:52 PDT
Created attachment 168293 [details]
Patch for landing
Comment 11 WebKit Review Bot 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>
Comment 12 WebKit Review Bot 2012-10-11 16:08:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Ryosuke Niwa 2012-11-20 16:10:11 PST
This patch broke magnitude-perf.js.
Comment 14 Ojan Vafai 2012-11-20 16:31:23 PST
(In reply to comment #13)
> This patch broke magnitude-perf.js.

How is that?
Comment 15 Ojan Vafai 2012-11-20 16:37:24 PST
I see https://bugs.webkit.org/show_bug.cgi?id=102848
Comment 16 Adam Barth 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?
Comment 17 Ojan Vafai 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.