Whitelist iterator.next unconditionally (issue 287160043 by erights@gmail.com)

[re...@codereview-hr.appspotmail.com]

Reviewers: kpreid_google,

Description:
Technically, for SES-on-ES5, we should not need to whitelist
'next'. However, browsers are accidentally relying on it
https://bugs.chromium.org/p/v8/issues/detail?id=4769#
https://bugs.webkit.org/show_bug.cgi?id=154475 and we will be
whitelisting it as we transition to ES6 anyway, so we unconditionally
whitelist it now.

Please review this at https://codereview.appspot.com/287160043/

Affected files (+10, -26 lines):
M src/com/google/caja/ses/startSES.js
M src/com/google/caja/ses/whitelist.js


Index: src/com/google/caja/ses/startSES.js
diff --git a/src/com/google/caja/ses/startSES.js
b/src/com/google/caja/ses/startSES.js
index
15a29d7620e9fe5c6035d436352415ffc50cdbd1..f385e37562ab32d6b662b442f6f61c9cc3943917
100644
--- a/src/com/google/caja/ses/startSES.js
+++ b/src/com/google/caja/ses/startSES.js
@@ -285,17 +285,6 @@ ses.startSES = function(global,
ses.es5ProblemReports.NONCONFIGURABLE_OWN_PROTO.afterFailure;
var INCREMENT_IGNORES_FROZEN =
ses.es5ProblemReports.INCREMENT_IGNORES_FROZEN.afterFailure;
- var CROSS_FRAME_FOR_IN_NEEDS_INHERITED_NEXT =
- ses.es5ProblemReports.CROSS_FRAME_FOR_IN_NEEDS_INHERITED_NEXT.
- afterFailure;
-
- if (CROSS_FRAME_FOR_IN_NEEDS_INHERITED_NEXT) {
- // Note: Imperative update, but should be ok. Please maintain
- // this note together with corresponding notes where whitelist is
- // defined in whitelist.js, and where it is first read below.
- whitelist.cajaVM.anonIntrinsics.IteratorPrototype.next = '*';
- // Whether this has the desired effect is tested after cleaning.
- }

var dirty = true;

@@ -1802,12 +1791,6 @@ ses.startSES = function(global,
* non-enumerable since ES5.1 specifies that all these properties
* are non-enumerable on the global object.
*/
- // Note that, on browsers suffering from
- // CROSS_FRAME_FOR_IN_NEEDS_INHERITED_NEXT, startSES does an
- // imperative update of the whitelist above, but should be ok.
- // Please maintain this note together with corresponding notes in
- // whitelist.js where whitelist is defined, and in startSES.js above
- // where whitelist is updated
keys(whitelist).forEach(function(name) {
var desc = gopd(global, name);
if (desc) {
Index: src/com/google/caja/ses/whitelist.js
diff --git a/src/com/google/caja/ses/whitelist.js
b/src/com/google/caja/ses/whitelist.js
index
6c96928651b6d73c1a44573fcb91efdb4fd36d9c..ed701206bb438c74ce1a656b90f054e5da6b77d9
100644
--- a/src/com/google/caja/ses/whitelist.js
+++ b/src/com/google/caja/ses/whitelist.js
@@ -72,7 +72,7 @@ var ses;
*
* <p>TODO: We want to do for constructor: something weaker than '*',
* but rather more like what we do for [[Prototype]] links, which is
- * that it is whitelisted only if it points as an object which is
+ * that it is whitelisted only if it points at an object which is
* otherwise reachable by a whitelisted path.
*
* <p>The members of the whitelist are either
@@ -110,11 +110,6 @@ var ses;
var t = true;
var TypedArrayWhitelist; // defined and used below

- // Note that, on browsers suffering from
- // CROSS_FRAME_FOR_IN_NEEDS_INHERITED_NEXT, startSES does an
- // imperative update of the whitelist, but should be ok. Please
- // maintain this note together with corresponding notes in
- // startSES.js where whitelist is updated and where it first read.
ses.whitelist = {
cajaVM: { // Caja support
// The accessible intrinsics which are not reachable by own
@@ -127,9 +122,15 @@ var ses;
anonIntrinsics: {
ThrowTypeError: {},
IteratorPrototype: {
- constructor: false // suppress inherited '*'
- // Note that startSES may add a "next: '*'" here, depending on
- // CROSS_FRAME_FOR_IN_NEEDS_INHERITED_NEXT
+ // Technically, for SES-on-ES5, we should not need to
+ // whitelist 'next'. However, browsers are accidentally
+ // relying on it
+ // https://bugs.chromium.org/p/v8/issues/detail?id=4769#
+ // https://bugs.webkit.org/show_bug.cgi?id=154475
+ // and we will be whitelisting it as we transition to ES6
+ // anyway, so we unconditionally whitelist it now.
+ next: '*',
+ constructor: false
},
ArrayIteratorPrototype: {},
StringIteratorPrototype: {},

[re...@codereview-hr.appspotmail.com]

Technically, for SES-on-ES5, we should not need to whitelist
'next'. However, browsers are accidentally relying on it
https://bugs.chromium.org/p/v8/issues/detail?id=4769#
https://bugs.webkit.org/show_bug.cgi?id=154475 and we will be
whitelisting it as we transition to ES6 anyway, so we unconditionally
whitelist it now.

https://codereview.appspot.com/287160043/

[eri...@gmail.com]

New snapshot, mostly getting rid of code that's no longer needed.

https://codereview.appspot.com/287160043/

[kpr...@google.com]

LGTM

https://codereview.appspot.com/287160043/

[re...@codereview-hr.appspotmail.com]

Technically, for SES-on-ES5, we should not need to whitelist
'next'. However, browsers are accidentally relying on it
https://bugs.chromium.org/p/v8/issues/detail?id=4769#
https://bugs.webkit.org/show_bug.cgi?id=154475 and we will be
whitelisting it as we transition to ES6 anyway, so we unconditionally
whitelist it now.

We also get rid of _optForeignForIn and
CROSS_FRAME_FOR_IN_NEEDS_INHERITED_NEXT since these are no longer
needed.
Merging two unrelated changes


https://codereview.appspot.com/287160043/