Fix arguments leakage in fn.bind polyfill

[Fishrock123]

Bluebird has done a bunch of research into Javascript optimization blockers, which includes strict arguments usage:

https://github.com/petkaantonov/bluebird/wiki/Optimization-killers#3-managing-arguments

[englercj]

Pretty sure this is going to pass along the thisArg that was passed in since you are not slicing off the first param of arguments. Would you mind making some tests for this function and trying it out?

[Fishrock123]

Whoops I didn't notice the top one sliced an arg off, I'll get it patched up tonight!

Edit: yeah I'll also cook up some tests.

[Fishrock123]

@englercj Maybe a better question is, do we need this?

I'm not sure that any browser that has a decently running canvas wouldn't have fn.bind

In any case I'm actually not sure how to test this, since our test env will not use our polyfill but rather the native version.

[englercj]

To test it, export it as another function PIXI.utils.bind or something. Then test against that.

Do we need it? That is a great question, I would be surprised if we did...

[Fishrock123]

@englercj this is the fixed version. I'd rather just test the logic separately but it is up to you guys.

Here's the compatibility tables for fn.bind

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind#Browser_compatibility
http://kangax.github.io/compat-table/es5/#Function.prototype.bind

Seems to be missing from Safari 5 / 5.1

[englercj]

That stinks! Canvas was added to safari in 2.0, so we do need this polyfill I guess...

I will merge this in if you write a few simple tests for it :D THANKS!

[Fishrock123]

Preliminary tests / benchmarks against an existing fn.bind polyfill on jsperf: http://jsperf.com/bind-vs-custom/5

[englercj]

Very cool! Seems like there is a perf increase for sure using this method instead of what we have now!

(btw what I meant by tests were just a couple unit tests in the PR itself 😄)

[Fishrock123]

Yeah I know, I'll write them tomorrow haha

[Fishrock123]

@englercj did you do any testing? Sorry I've been away and busy. :s

[englercj]

Nah, just going through and cleaning some PRs decided to merge this for now. Feel free to add tests though still :D

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.