Bug 102848 - REGRESSION(r131106): magnitude-perf.js calls bind on undefined
Summary: REGRESSION(r131106): magnitude-perf.js calls bind on undefined
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-20 16:12 PST by Ryosuke Niwa
Modified: 2012-11-20 17:11 PST (History)
6 users (show)

See Also:


Attachments
Fixes the regression (1.36 KB, patch)
2012-11-20 16:13 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-11-20 16:12:27 PST
http://trac.webkit.org/changeset/131106/trunk/LayoutTests/resources/magnitude-perf.js
replaced
    var nowFunction = window.performance.now || Date.now;
with
    var nowFunction = window.performance.now.bind(window.performance) || Date.now; 

Calling bind on undefined object will results in an exception being thrown and "|| Date.now" is useless.
Comment 1 Ryosuke Niwa 2012-11-20 16:13:32 PST
Created attachment 175298 [details]
Fixes the regression
Comment 2 Ojan Vafai 2012-11-20 16:34:11 PST
Comment on attachment 175298 [details]
Fixes the regression

I don't see why we need to call bind at all. Why can't this just be:
var nowFunction = window.performance.now || Date.now;

The original patch that added the bind call doesn't explain this.
Comment 3 James Simonsen 2012-11-20 16:50:58 PST
(In reply to comment #2)
> (From update of attachment 175298 [details])
> I don't see why we need to call bind at all. Why can't this just be:
> var nowFunction = window.performance.now || Date.now;
> 
> The original patch that added the bind call doesn't explain this.

You get a TypeError: Illegal invocation without it. You can test it in the console. Try storing now in a variable and then invoke that variable.

I don't fully understand why. I think this answer might explain it:

http://stackoverflow.com/questions/1007340/javascript-function-aliasing-doesnt-seem-to-work
Comment 4 Ojan Vafai 2012-11-20 16:52:44 PST
It seems wrong to me that you can alias Date.now but not window.performance.now.
Comment 5 James Simonsen 2012-11-20 16:59:01 PST
(In reply to comment #4)
> It seems wrong to me that you can alias Date.now but not window.performance.now.

performance.now() _does_ vary depending on which window you call it on. It measures the time since that window object started navigating.
Comment 6 Ojan Vafai 2012-11-20 17:01:40 PST
oic. Sad. I wonder if we can make aliasing of these things work. Probably not. :(
Comment 7 WebKit Review Bot 2012-11-20 17:11:44 PST
Comment on attachment 175298 [details]
Fixes the regression

Clearing flags on attachment: 175298

Committed r135335: <http://trac.webkit.org/changeset/135335>
Comment 8 WebKit Review Bot 2012-11-20 17:11:48 PST
All reviewed patches have been landed.  Closing bug.