ES5 repairs for Chrome array mutability bugs.

* Patch Object.freeze to explicitly defineProperty an array's .length.
  This prevents some array mutation methods from modifying frozen
  arrays, including .push(), and avoids the need to patch .push()
  itself.
* Patch Array.prototype.{shift,unshift,splice} which may mutate frozen
  arrays.
* Adjust testRepairReport logic so that a problem configured doNotRepair
  is not considered to be 'accidentally repaired' if the same repair
  was used elsewhere.

Fixes <https://code.google.com/p/google-caja/issues/detail?id=1816>.

@r5551

[kpreid2, 2013-07-16 00:33:31]

I'm not sure whether this is ready to go yet, but I'm putting it out for review
to get some more eyeballs on it.

[MarkM, 2013-07-16 01:05:57]

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
File src/com/google/caja/ses/repairES5.js (right):

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:2105: unrepairedArrayPush.call(x, 3);
I didn't understand at first why you're calling unrepairedArrayPush here rather
than push. If push has been repaired such that this test passes, hasn't that
repaired the problem being tested? IIUC, the answer is no, because the point of
this test has nothing to do with push. Needs a better comment.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:2111: return 'Unexpected modification w/o
throw';
If, for example, a throw happened, x.length was unchanged, but x[0] !== 1, then
the catch clause will fall thru to here, so the diagnostic "w/o throw" will be
wrong.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:3357: existingMethod(O);
Just "return existingMethod(O);"

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:3538: id: (prop +
'_IGNORES_SEALED').toUpperCase(),
Cool. I didn't see that one coming.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:3549: tests: [] // TODO(erights): Add to
test262
Make *all* of these *not* "TODO(erights)". 

Consider TODO(jasvir). See https://bugs.ecmascript.org/show_bug.cgi?id=1578

[kpreid2, 2013-07-16 16:47:46]

* Patch Object.freeze to explicitly defineProperty an array's .length.
  This prevents some array mutation methods from modifying frozen
  arrays, including .push(), and avoids the need to patch .push()
  itself.
* Patch Array.prototype.{shift,unshift,splice} which may mutate frozen
  arrays.
* Adjust testRepairReport logic so that a problem configured doNotRepair
  is not considered to be 'accidentally repaired' if the same repair
  was used elsewhere.

[kpreid2, 2013-07-16 16:48:55]

New snapshot.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
File src/com/google/caja/ses/repairES5.js (right):

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:2105: unrepairedArrayPush.call(x, 3);
On 2013/07/16 01:05:57, MarkM wrote:
> I didn't understand at first why you're calling unrepairedArrayPush here
> rather than push. If push has been repaired such that this test passes, 
> hasn't that repaired the problem being tested? IIUC, the answer is no,
> because the point of this test has nothing to do with push. Needs a
> better comment.

Done.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:2111: return 'Unexpected modification w/o
throw';
On 2013/07/16 01:05:57, MarkM wrote:
> If, for example, a throw happened, x.length was unchanged, but 
> x[0] !== 1, then the catch clause will fall thru to here, so the
> diagnostic "w/o throw" will be wrong.

Rewrote the whole thing to test what it cares about more directly.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:3357: existingMethod(O);
On 2013/07/16 01:05:57, MarkM wrote:
> Just "return existingMethod(O);"

I deliberately wrote it this way, on the off chance that it could be optimized
better since there is no unnecessary data-flow dependency.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:3538: id: (prop +
'_IGNORES_SEALED').toUpperCase(),
On 2013/07/16 01:05:57, MarkM wrote:
> Cool. I didn't see that one coming.

Unfortunately, it breaks grepping, which is why I added the comments with the
full name near the uses. I tried to pass the ids in originally, but it didn't
work out cleanly.

https://codereview.appspot.com/11312043/diff/1/src/com/google/caja/ses/repair...
src/com/google/caja/ses/repairES5.js:3549: tests: [] // TODO(erights): Add to
test262
On 2013/07/16 01:05:57, MarkM wrote:
> Make *all* of these *not* "TODO(erights)". 
> 
> Consider TODO(jasvir). See https://bugs.ecmascript.org/show_bug.cgi?id=1578 

Done. If he's working on that I'll pass on the ones labeled w/ myself too :)

[MarkM, 2013-07-16 17:06:59]

LGTM