Bug 88278 – unprefix window.performance.webkitNow()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Masataka Yakura]

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

[James Simonsen]

Created attachment 157094 [details]
Patch

[James Simonsen]

Unprefixing it sounds good to me.

[James Robinson]

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]

(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 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]

Actually, before we land this, we should make the timebase compliant with the spec. I'll do that this afternoon.

[James Simonsen]

Created attachment 168283 [details]
Patch

[James Simonsen]

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 on attachment 168283 [details]
Patch

Also worth noting this is unprefixed in IE10 as well as moz.

[James Simonsen]

Created attachment 168293 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 168293 [details]
Patch for landing

Clearing flags on attachment: 168293

Committed r131106: <http://trac.webkit.org/changeset/131106>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

This patch broke magnitude-perf.js.

[Ojan Vafai]

(In reply to comment #13)
> This patch broke magnitude-perf.js.

How is that?

[Ojan Vafai]

I see https://bugs.webkit.org/show_bug.cgi?id=102848

[Adam Barth]

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]

(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.