Harmful performance.now() polyfill

[nicjansma]

Hello!

Per the code below:
https://github.com/julianshapiro/velocity/blob/master/velocity.js#L498

var performance = (function() {
    var perf = window.performance || {};

    if (!Object.prototype.hasOwnProperty.call(perf, "now")) {
        var nowOffset = perf.timing && perf.timing.domComplete
            ? perf.timing.domComplete : (new Date()).getTime();

        perf.now = function() {
            return (new Date()).getTime() - nowOffset;
        };
    }
    return perf;
})();

I'm not quite sure if this is intentional, but this code is changing performance.now() in browsers that support it from a high-precision, monotonically non-decreasing timer (performance.now()) with an epoch of navigationStart to using a low-precision, system-time-affected timer (Date) with an epoch of domComplete.

:(

The problem is that if performance.now() exists (e.g. is supported by the browser), !Object.prototype.hasOwnProperty.call(window.performance, "now") will return true and thus performance.now() will be overwritten to use Date instead.

Was it intended to just polyfill performance.now() if it didn't already exist? If so, a simple typeof performance.now === 'function' should suffice.

The second question is, why does it use DOM Complete as its epoch? Is there something in velocity.js that expects this? Using DOM Complete changes the definition of the time origin, which should be Navigation Start.

Unfortunately this change directly affects any third party library that is measuring timestamps and comparing them to other DOMHighResTimeStamp timestamps on the page, i.e. those in NavigationTiming, ResourceTiming, UserTiming, etc.

We stumbled upon this because it is causing several measurements (e.g. Page Load) to be incorrect in boomerang. If one reads performance.now() as the first function in the onload event, it should be essentially equal to performance.timing.loadEventStart. By offsetting it by DOM Complete, the two measurements could have values 1000s of ms apart. If you're doing any comparisons of performance.now() to these other timestamps, your math will be off.

I'm assuming the above wasn't intentional -- I've opened #763 to fix these two issues.

Thanks!

[Rycochet]

Not quite sure what you mean here - it's specifically checking if the method doesn't exist on the object, and adding in that case. If it does exist then the if() check fails and nothing gets done... Or is .now() not actually a member and used via the prototype chain instead?

[MattyBalaam]

I've just done a quick check in the console on the latest Firefox and Chrome, and the test is returning true.

var perf = window.performance || {};
undefined
!Object.prototype.hasOwnProperty.call(perf, "now")
true

[Rycochet]

Prototype chain it is, meh - nice catch!

[Rycochet]

Closed via #763

[nicjansma]

Thanks for the quick turnaround!

[Rycochet]

I still need to bake it into a release later today ;-)