LGTM
https://codereview.appspot.com/54750043/diff/130003/src/com/google/caja/ses/WeakMap.js
File src/com/google/caja/ses/WeakMap.js (right):
https://codereview.appspot.com/54750043/diff/130003/src/com/google/caja/ses/WeakMap.js#newcode445
src/com/google/caja/ses/WeakMap.js:445: keys.push(key);
If I were feeling especially paranoid (considering such things as
slow-script-stopping), I might check here the invariant that keys and
values are the same length, since a violation would result in
mis-associating all future keys and values, a particularly horrible
outcome.
https://codereview.appspot.com/54750043/diff/130003/src/com/google/caja/ses/WeakMap.js#newcode461
src/com/google/caja/ses/WeakMap.js:461: values.splice(index, 1);
Splicing is probably O(n); instead, move the last entry into this index.
Since WeakMaps are non-enumerable the ordering makes no observable
difference.
(Should you choose to adopt the abovementioned paranoia, erase either
the key or value of the destination, copy the other, and copy the
blanked value.)
https://codereview.appspot.com/54750043/diff/130003/tests/com/google/caja/ses/test-ses-parts.js
File tests/com/google/caja/ses/test-ses-parts.js (right):
https://codereview.appspot.com/54750043/diff/130003/tests/com/google/caja/ses/test-ses-parts.js#newcode15
tests/com/google/caja/ses/test-ses-parts.js:15: var preFrozen =
Object.freeze({});
+1 for thoroughness
https://codereview.appspot.com/54750043/diff/130003/tests/com/google/caja/ses/test-ses-parts.js#newcode38
tests/com/google/caja/ses/test-ses-parts.js:38: var wm1 = new WeakMap();
Either give these more descriptive names or add comments explaining
which roles they play in the test, so that one doesn't have to figure
out the logic of the tests by reading all the asserts.
https://codereview.appspot.com/54750043/diff/130003/tests/com/google/caja/ses/test-ses-parts.js#newcode43
tests/com/google/caja/ses/test-ses-parts.js:43:
assertEquals(wm1.set(postFrozen, 10), wm1);
Expected values go first, actual values go second. Jsunit error messages
are written to assume that.
https://codereview.appspot.com/54750043/
--
---
You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
