Many separate improvements, most to help with membranes (105065)

Addresses issues 1038, 1044, 1086, 1097, 1098, 1099, 1100.
Please see these issues themselves for documentation.
Some significant performance improvements on time to create a function
 (testFunctionClosure in both Cajita and Valija). 
callStackUnsealer now returns other disagnostic info which has been
 observed on thrown Errors on some browsers.
Date.prototype.toJSON is now whitelisted.
Trademark API is now much more expressive (and more E-like).
Incorporated David-Sarah's suggestion for making the applet
 classloader caching less sticky. Helps a lot!
Changed obsolete references to "ES3.1" to "ES5".

[DavidSarah, 2009-08-24 18:58:47]

http://codereview.appspot.com/109074/diff/52/70
File src/com/google/caja/cajita-debugmode.js (left):

http://codereview.appspot.com/109074/diff/52/70#oldcode242
Line 242: // Make sure that tamed Errors propogate the cajitaStack___,
sp: 'propagate'

http://codereview.appspot.com/109074/diff/52/70
File src/com/google/caja/cajita-debugmode.js (right):

http://codereview.appspot.com/109074/diff/52/70#newcode238
Line 238: // TODO(erights): What does the following comment mean?
I think the comment is trying to explain why the "&& !ex.cajitaStack___" is
necessary. That is, if we clobbered an existing ex.cajitaStack___ with the stack
at the point of the tameException, we would lose useful (sealed) information
about the original exception in the case of a rethrow.

(But it took me 3 minutes to work out that the comment was referring to just the
"&& !ex.cajitaStack___", so it definitely isn't clear.)

http://codereview.appspot.com/109074/diff/52/73
File src/com/google/caja/demos/applet/index.html (left):

http://codereview.appspot.com/109074/diff/52/73#oldcode172
Line 172: </applet>
"<!-- see http://java.sun.com/javase/6/webnotes/6u10/plugin2/index.html for
documentation of classloader_cache and separate_jvm parameters -->"

http://codereview.appspot.com/109074/diff/52/65
File src/com/google/caja/parser/js/Parser.java (right):

http://codereview.appspot.com/109074/diff/52/65#newcode570
Line 570: // restricted production.  See the grammar above and ES3 S7.9.1
"and ES3 or ES5 S7.9.1"

http://codereview.appspot.com/109074/diff/52/65#newcode782
Line 782: // "restricted productions" according to ES3 S7.9.1.
"ES3 or ES5 S7.9.1"

http://codereview.appspot.com/109074/diff/52/66
File src/com/google/caja/parser/js/UseSubsetDirective.java (right):

http://codereview.appspot.com/109074/diff/52/66#newcode32
Line 32: * "Use subsets" are defined in ES5.
Add "TODO: grammar no longer specifies a list of subsets."
Is there an issue for this?

http://codereview.appspot.com/109074/diff/52/67
File src/com/google/caja/parser/js/WithStmt.java (left):

http://codereview.appspot.com/109074/diff/52/67#oldcode28
Line 28: * chain.
"(ES5 expresses this in terms of environment records but with similar effect.)"

http://codereview.appspot.com/109074/diff/52/63
File src/com/google/caja/parser/quasiliteral/CajitaRewriter.java (right):

http://codereview.appspot.com/109074/diff/52/63#newcode330
Line 330: // in FF3, and ES5 is specifying the behavior of [] and {} in terms
"and ES5 specifies"

http://codereview.appspot.com/109074/diff/52/60
File src/com/google/caja/render/BufferingRenderer.java (left):

http://codereview.appspot.com/109074/diff/52/60#oldcode68
Line 68: // of semicolon insertion.
"... since ES3 and ES5 say ..."
"This is inconsistently implemented, but the rewriting works regardless of
whether an implementation actually treats the comment as a newline for semicolon
insertion."

http://codereview.appspot.com/109074/diff/52/59
File src/com/google/caja/render/JsRenderUtil.java (right):

http://codereview.appspot.com/109074/diff/52/59#newcode32
Line 32: // According to semicolon insertion rules in ES3 Section 7.9.1
"ES3 and ES5" (the rules have not changed)

[MarkM, 2009-08-25 00:12:38]

Thanks much!

http://codereview.appspot.com/109074/diff/52/70
File src/com/google/caja/cajita-debugmode.js (left):

http://codereview.appspot.com/109074/diff/52/70#oldcode242
Line 242: // Make sure that tamed Errors propogate the cajitaStack___,
On 2009/08/24 18:58:47, DavidSarah wrote:
> sp: 'propagate'

Done.

http://codereview.appspot.com/109074/diff/52/70
File src/com/google/caja/cajita-debugmode.js (right):

http://codereview.appspot.com/109074/diff/52/70#newcode238
Line 238: // TODO(erights): What does the following comment mean?
On 2009/08/24 18:58:47, DavidSarah wrote:
> I think the comment is trying to explain why the "&& !ex.cajitaStack___" is
> necessary. That is, if we clobbered an existing ex.cajitaStack___ with the
stack
> at the point of the tameException, we would lose useful (sealed) information
> about the original exception in the case of a rethrow.
> 
> (But it took me 3 minutes to work out that the comment was referring to just
the
> "&& !ex.cajitaStack___", so it definitely isn't clear.)

Done.

http://codereview.appspot.com/109074/diff/52/73
File src/com/google/caja/demos/applet/index.html (left):

http://codereview.appspot.com/109074/diff/52/73#oldcode172
Line 172: </applet>
On 2009/08/24 18:58:47, DavidSarah wrote:
> "<!-- see http://java.sun.com/javase/6/webnotes/6u10/plugin2/index.html for
> documentation of classloader_cache and separate_jvm parameters -->"

Done.

http://codereview.appspot.com/109074/diff/52/65
File src/com/google/caja/parser/js/Parser.java (right):

http://codereview.appspot.com/109074/diff/52/65#newcode570
Line 570: // restricted production.  See the grammar above and ES3 S7.9.1
On 2009/08/24 18:58:47, DavidSarah wrote:
> "and ES3 or ES5 S7.9.1"

Done.

http://codereview.appspot.com/109074/diff/52/65#newcode782
Line 782: // "restricted productions" according to ES3 S7.9.1.
On 2009/08/24 18:58:47, DavidSarah wrote:
> "ES3 or ES5 S7.9.1"

Done.

http://codereview.appspot.com/109074/diff/52/66
File src/com/google/caja/parser/js/UseSubsetDirective.java (right):

http://codereview.appspot.com/109074/diff/52/66#newcode32
Line 32: * "Use subsets" are defined in ES5.
On 2009/08/24 18:58:47, DavidSarah wrote:
> Add "TODO: grammar no longer specifies a list of subsets."
> Is there an issue for this?

Done.

http://codereview.appspot.com/109074/diff/52/67
File src/com/google/caja/parser/js/WithStmt.java (left):

http://codereview.appspot.com/109074/diff/52/67#oldcode28
Line 28: * chain.
On 2009/08/24 18:58:47, DavidSarah wrote:
> "(ES5 expresses this in terms of environment records but with similar
effect.)"

Done.

http://codereview.appspot.com/109074/diff/52/63
File src/com/google/caja/parser/quasiliteral/CajitaRewriter.java (right):

http://codereview.appspot.com/109074/diff/52/63#newcode330
Line 330: // in FF3, and ES5 is specifying the behavior of [] and {} in terms
On 2009/08/24 18:58:47, DavidSarah wrote:
> "and ES5 specifies"

Done.

http://codereview.appspot.com/109074/diff/52/60
File src/com/google/caja/render/BufferingRenderer.java (left):

http://codereview.appspot.com/109074/diff/52/60#oldcode68
Line 68: // of semicolon insertion.
On 2009/08/24 18:58:47, DavidSarah wrote:
> "... since ES3 and ES5 say ..."
> "This is inconsistently implemented, but the rewriting works regardless of
> whether an implementation actually treats the comment as a newline for
semicolon
> insertion."

Done.

http://codereview.appspot.com/109074/diff/52/59
File src/com/google/caja/render/JsRenderUtil.java (right):

http://codereview.appspot.com/109074/diff/52/59#newcode32
Line 32: // According to semicolon insertion rules in ES3 Section 7.9.1
On 2009/08/24 18:58:47, DavidSarah wrote:
> "ES3 and ES5" (the rules have not changed)

Done.

[DavidSarah, 2009-08-25 00:37:35]

http://codereview.appspot.com/109074/diff/81/1072
File src/com/google/caja/cajita-debugmode.js (right):

http://codereview.appspot.com/109074/diff/81/1072#newcode241
Line 241: // so it will available to code with access to the unsealer.
will be available

[metaweta, 2009-08-26 23:41:43]

http://codereview.appspot.com/109074/diff/52/70
File src/com/google/caja/cajita-debugmode.js (right):

http://codereview.appspot.com/109074/diff/52/70#newcode88
Line 88: * Returns a call stack, that will not be mutated by subsequent
remove comma

http://codereview.appspot.com/109074/diff/52/70#newcode242
Line 242: // > doesn't make ex unavailable to code with access to the unsealer.
Is this comment related to issue1110?

http://codereview.appspot.com/109074/diff/52/71
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/109074/diff/52/71#newcode790
Line 790: fail('cajita.freeze(obj) applies only to JSON Containers: ',
(and functions and Errors)

http://codereview.appspot.com/109074/diff/52/71#newcode1437
Line 1437: throw new TypeError("Can't read " + name + ' on ' + obj);
Double quotes

http://codereview.appspot.com/109074/diff/52/71#newcode1735
Line 1735: throw new TypeError("Can't call " + name + ' on ' + obj);
Double quotes

http://codereview.appspot.com/109074/diff/52/71#newcode1864
Line 1864: throw new TypeError("Can't delete " + name + ' on ' + obj);
Double quotes

http://codereview.appspot.com/109074/diff/52/71#newcode2844
Line 2844: * ejection. If these to their own non-local exit, that takes
If these *do* their own

http://codereview.appspot.com/109074/diff/52/71#newcode2887
Line 2887: * If <tt>opt_ejector</tt> is omitted, falsy, disabled, or returns
Can't omit the first argument.

http://codereview.appspot.com/109074/diff/52/71#newcode3004
Line 3004: // partial stamping.
Add one line here: Assumes single-threaded execution and non-mutating accessors.

http://codereview.appspot.com/109074/diff/52/71#newcode3402
Line 3402: case 'object': case 'function':
boolean?

http://codereview.appspot.com/109074/diff/52/65
File src/com/google/caja/parser/js/Parser.java (right):

http://codereview.appspot.com/109074/diff/52/65#newcode570
Line 570: // restricted production.  See the grammar above and ES3 S7.9.1
On 2009/08/25 00:12:38, MarkM wrote:
> On 2009/08/24 18:58:47, DavidSarah wrote:
> > "and ES3 or ES5 S7.9.1"
> 
> Done.

Did you intend not to mention ES5 here and below?

http://codereview.appspot.com/109074/diff/52/67
File src/com/google/caja/parser/js/WithStmt.java (left):

http://codereview.appspot.com/109074/diff/52/67#oldcode28
Line 28: * chain.
On 2009/08/25 00:12:38, MarkM wrote:
> On 2009/08/24 18:58:47, DavidSarah wrote:
> > "(ES5 expresses this in terms of environment records but with similar
> effect.)"
> 
> Done.

No mention of ES5 here?

http://codereview.appspot.com/109074/diff/52/67
File src/com/google/caja/parser/js/WithStmt.java (right):

http://codereview.appspot.com/109074/diff/52/67#newcode28
Line 28: * chain.
Still no mention of ES5?

http://codereview.appspot.com/109074/diff/52/54
File tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java
(right):

http://codereview.appspot.com/109074/diff/52/54#newcode420
Line 420: // should be figured out first.
Please add a new issue for this if you're going to close 1086.

[MarkM, 2009-08-28 05:58:06]

New comments, even on old patches, mean "as of patch set 5".

http://codereview.appspot.com/109074/diff/52/70
File src/com/google/caja/cajita-debugmode.js (right):

http://codereview.appspot.com/109074/diff/52/70#newcode88
Line 88: * Returns a call stack, that will not be mutated by subsequent
On 2009/08/26 23:41:43, metaweta wrote:
> remove comma

Done.

http://codereview.appspot.com/109074/diff/52/70#newcode242
Line 242: // > doesn't make ex unavailable to code with access to the unsealer.
On 2009/08/26 23:41:43, metaweta wrote:
> Is this comment related to issue1110?

No. See the new text as of patch 4 or later. Done.

http://codereview.appspot.com/109074/diff/52/71
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/109074/diff/52/71#newcode790
Line 790: fail('cajita.freeze(obj) applies only to JSON Containers: ',
On 2009/08/26 23:41:43, metaweta wrote:
> (and functions and Errors)

Done.

http://codereview.appspot.com/109074/diff/52/71#newcode1437
Line 1437: throw new TypeError("Can't read " + name + ' on ' + obj);
On 2009/08/26 23:41:43, metaweta wrote:
> Double quotes

For text containing an apostrophe, it is more readable to use double quotes than
to escape the apostrophe. So "won't fix" unless you insist.

http://codereview.appspot.com/109074/diff/52/71#newcode1735
Line 1735: throw new TypeError("Can't call " + name + ' on ' + obj);
On 2009/08/26 23:41:43, metaweta wrote:
> Double quotes

See earlier comment.

http://codereview.appspot.com/109074/diff/52/71#newcode1864
Line 1864: throw new TypeError("Can't delete " + name + ' on ' + obj);
On 2009/08/26 23:41:43, metaweta wrote:
> Double quotes

See earlier comment.

http://codereview.appspot.com/109074/diff/52/71#newcode2844
Line 2844: * ejection. If these to their own non-local exit, that takes
On 2009/08/26 23:41:43, metaweta wrote:
> If these *do* their own

Done.

http://codereview.appspot.com/109074/diff/52/71#newcode2887
Line 2887: * If <tt>opt_ejector</tt> is omitted, falsy, disabled, or returns
On 2009/08/26 23:41:43, metaweta wrote:
> Can't omit the first argument.

Done.

http://codereview.appspot.com/109074/diff/52/71#newcode3004
Line 3004: // partial stamping.
On 2009/08/26 23:41:43, metaweta wrote:
> Add one line here: Assumes single-threaded execution and non-mutating
accessors.

Done.

http://codereview.appspot.com/109074/diff/52/71#newcode3402
Line 3402: case 'object': case 'function':
On 2009/08/26 23:41:43, metaweta wrote:
> boolean?
> 

Added. Done.

http://codereview.appspot.com/109074/diff/52/65
File src/com/google/caja/parser/js/Parser.java (right):

http://codereview.appspot.com/109074/diff/52/65#newcode570
Line 570: // restricted production.  See the grammar above and ES3 S7.9.1
On 2009/08/26 23:41:43, metaweta wrote:
> On 2009/08/25 00:12:38, MarkM wrote:
> > On 2009/08/24 18:58:47, DavidSarah wrote:
> > > "and ES3 or ES5 S7.9.1"
> > 
> > Done.
> 
> Did you intend not to mention ES5 here and below?
> 

It was done on patch set 4. You're looking at patch set 3.

http://codereview.appspot.com/109074/diff/52/67
File src/com/google/caja/parser/js/WithStmt.java (left):

http://codereview.appspot.com/109074/diff/52/67#oldcode28
Line 28: * chain.
On 2009/08/26 23:41:43, metaweta wrote:
> On 2009/08/25 00:12:38, MarkM wrote:
> > On 2009/08/24 18:58:47, DavidSarah wrote:
> > > "(ES5 expresses this in terms of environment records but with similar
> > effect.)"
> > 
> > Done.
> 
> No mention of ES5 here?

Was fixed in patch set 4.

http://codereview.appspot.com/109074/diff/52/67
File src/com/google/caja/parser/js/WithStmt.java (right):

http://codereview.appspot.com/109074/diff/52/67#newcode28
Line 28: * chain.
On 2009/08/26 23:41:43, metaweta wrote:
> Still no mention of ES5?

Was fixed in patch set 4.

http://codereview.appspot.com/109074/diff/52/54
File tests/com/google/caja/parser/quasiliteral/CommonJsRewriterTestCase.java
(right):

http://codereview.appspot.com/109074/diff/52/54#newcode420
Line 420: // should be figured out first.
On 2009/08/26 23:41:43, metaweta wrote:
> Please add a new issue for this if you're going to close 1086.

It is now filed as issue 1114. Changed the comment accordingly. Done.

http://codereview.appspot.com/109074/diff/81/1072
File src/com/google/caja/cajita-debugmode.js (right):

http://codereview.appspot.com/109074/diff/81/1072#newcode241
Line 241: // so it will available to code with access to the unsealer.
On 2009/08/25 00:37:35, DavidSarah wrote:
> will be available

Done.

[metaweta, 2009-08-28 16:30:48]

LGTM