https://codereview.appspot.com/222570043/diff/80001/src/com/google/caja/ses/repairES5.js
File src/com/google/caja/ses/repairES5.js (right):
https://codereview.appspot.com/222570043/diff/80001/src/com/google/caja/ses/repairES5.js#newcode3693
src/com/google/caja/ses/repairES5.js:3693: ses.optForeignForIn =
inTestFrame(function(window) {
Without having any specific ideas in mind: note that inTestFrame wasn't
written to be defensive — I wrote it solely for doing cross-frame repair
tests, i.e. without more widely exposing its results. Might be good to
review inTestFrame with that in mind.
Also, please mark this as private in some way, such as a leading _
(precedent with ses._EarlyStringMap above). We need to work on defining
our public interface more precisely, so that future changes can be
evaluated for whether they can break clients, and this is a small first
step.
https://codereview.appspot.com/222570043/diff/80001/src/com/google/caja/ses/repairES5.js#newcode3715
src/com/google/caja/ses/repairES5.js:3715: // error. No reliable brand
test for error anyway.
"brand test for Error"
https://codereview.appspot.com/222570043/diff/80001/src/com/google/caja/ses/startSES.js
File src/com/google/caja/ses/startSES.js (right):
https://codereview.appspot.com/222570043/diff/80001/src/com/google/caja/ses/startSES.js#newcode266
src/com/google/caja/ses/startSES.js:266: // Note: Imperative update, but
should be ok.
The danger of an imperative update is not localized to the update.
Please document the fact that we're doing this at additional locations:
where whitelist is defined, and where it is next read — that is,
sufficient for a maintainer to tell that this code must not be
_reordered_.
https://codereview.appspot.com/222570043/
--
---
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/d/optout.
