Fix bad edge case in bubble sort

[islemaster]

Turns out the Array.prototype.sort polyfill, implemented as a bubble sort, is broken.

Brought to our attention by a Zendesk ticket and easily reproduced in App Lab with the following code:

// Array.sort doesn't work properly
console.log([5, 4, 5, 1].sort());
// Expected: [1,4,5,5]
// Actual  : [4,1,5,5]

The problem is that this Array.prototype.sort polyfill, which uses a bubble sort, is implemented incorrectly:

JS-Interpreter/interpreter.js

Lines 1100 to 1117 in 37933e4

	"Object.defineProperty(Array.prototype, 'sort', {configurable: true, value:",
	"function(opt_comp) {",
	"for (var i = 0; i < this.length; i++) {",
	"var changes = 0;",
	"for (var j = 0; j < this.length - i - 1; j++) {",
	"if (opt_comp ?" +
	"opt_comp(this[j], this[j + 1]) > 0 : this[j] > this[j + 1]) {",
	"var swap = this[j];",
	"this[j] = this[j + 1];",
	"this[j + 1] = swap;",
	"changes++;",
	"}",
	"}",
	"if (changes <= 1) break;",
	"}",
	"return this;",
	"}",
	"});",

The exact issue is the if (changes <= 1) break; line. Stepping through the algorithm for the broken case, we can see that we're breaking too early, when only one swap has occurred in the pass but there's still more work to do.

i is 0
  begin 5,4,5,1
  j is 0
    swap 5 and 4
  j is 1
  j is 2
    swap 5 and 1
  end 4,5,1,5
i is 1
  begin 4,5,1,5
  j is 0
  j is 1
    swap 5 and 1
  end 4,1,5,5
[4,1,5,5]

The solution is to check changes < 1 or just !changes. That's what's happened upstream in addition to some other optimizations we should eventually grab, but for now I want a very targeted bugfix.

[Bjvanminnen]

LGTM. I know Paul did a bunch of work on testing the interpreter. It would be good to (a) see if this changes the pass/fail state of any tests and (b) perhaps add a test for it if not.

[islemaster]

I'm working on a regression unit test for this in the code-dot-org repo right now. The tests Paul set up for the interpreter are running in Circle on this PR right now. It runs the test262 EcmaScript Conformance Test Suite, which I suspect will not hit the particular sort edge case we're fixing here, but I also don't feel this merits the effort to set up a second test harness on this repo for custom tests.

[domino14]

why bubble sort