New snapshot, including merge with WeakMap algorithm change.
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js
File src/com/google/caja/ses/WeakMap.js (left):
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js#oldcode170
src/com/google/caja/ses/WeakMap.js:170: var hop =
Object.prototype.hasOwnProperty;
On 2014/01/29 19:32:49, MarkM wrote:
For understandable reasons, codereview diffing hit a jackpot,
reporting large
blocks are replaced with new large blocks. In these large blocks, I'm
generally
assuming that the parts that seem internally unmodified are. If that
means I'm
missing something, please bring it to my attention.
I don't think so, but I have reorganized the code so that it is in the
same order as originally, which should produce better diffs.
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js
File src/com/google/caja/ses/WeakMap.js (right):
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js#newcode194
src/com/google/caja/ses/WeakMap.js:194: // } catch (e) {}
On 2014/01/29 19:32:49, MarkM wrote:
Now that this code is within a test function, the commented out code
should also
always return something appropriate.
Done.
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js#newcode199
src/com/google/caja/ses/WeakMap.js:199: preSeverity:
severities.SAFE_SPEC_VIOLATION,
On 2014/01/29 19:32:49, MarkM wrote:
This preSeverity does not seem severe enough. Why would this be safe?
SAFE_SPEC_VIOLATION is described as "safe (in an integrity sense) even
if unrepaired. May still lead to inappropriate failures". Throwing is an
inappropriate failure. No?
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js#newcode201
src/com/google/caja/ses/WeakMap.js:201: urls: [],
On 2014/01/29 19:32:49, MarkM wrote:
I think we've (you've?) already filed FF issues on this, so you should
cite
those URLs here.
Done.
https://bugzilla.mozilla.org/show_bug.cgi?id=803844
Note we should figure out which objects are problematic and reopen this
bug explicitly about _all_ objects.
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js#newcode203
src/com/google/caja/ses/WeakMap.js:203: tests: []
On 2014/01/29 19:32:49, MarkM wrote:
Please add a todo on empty test section for things that test262 should
test.
Done.
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/WeakMap.js#newcode249
src/com/google/caja/ses/WeakMap.js:249: Proxy = undefined;
On 2014/01/29 19:32:49, MarkM wrote:
You also do this for the same reason at line 160 above. Can we safely
remove one
or kill the redundancy somehow?
I've moved this code into the repair for WEAKMAP_DROPS_FROZEN_KEYS in
order to make it more explicitly parallel.
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/repairES5.js
File src/com/google/caja/ses/repairES5.js (right):
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/repairES5.js#newcode140
src/com/google/caja/ses/repairES5.js:140: // TODO(kpreid): Replace uses
of this with new repair framework
On 2014/01/29 19:32:49, MarkM wrote:
Unless I'm missing something, doing this now isn't much harder than
adding this
TODO, so better to just do it.
updateMaxSeverity is used in such things as startSES's
property-traversal. I don't want to add reworking that to this change.
https://codereview.appspot.com/54450044/diff/1/src/com/google/caja/ses/repairES5.js#newcode3436
src/com/google/caja/ses/repairES5.js:3436: ////////////////////// Kludge
Records /////////////////////
On 2014/01/29 19:32:49, MarkM wrote:
Stale terminology. Please search for other possible occurrences.
Done.
https://codereview.appspot.com/54450044/
--
---
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.
