https://codereview.appspot.com/7678045/diff/30025/build.xml
File build.xml (right):
https://codereview.appspot.com/7678045/diff/30025/build.xml#newcode1228
build.xml:1228: <include name="**/caja/tracing.js"/>
On 2013/08/01 21:48:39, kpreid_google wrote:
I think tracing should have its own package (caja/tracing/tracing.js,
caja/tracing/TracingRewriter.java,
caja/tracing/TracingRewriterMain.java).
(I agree that this does not entirely match the existing package
structure, but I
think it's better and will help with disentangling things in the
future.)
Done.
https://codereview.appspot.com/7678045/diff/30025/build.xml#newcode1834
build.xml:1834: <echo>Next step will succeed if you don't have Graphviz
installed</echo>
On 2013/08/01 21:48:39, kpreid_google wrote:
wrong change
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/demos/playground/client/policy.js
File src/com/google/caja/demos/playground/client/policy.js (right):
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/demos/playground/client/policy.js#newcode18
src/com/google/caja/demos/playground/client/policy.js:18:
imports.console = frameGroup.tame(frameGroup.markReadOnlyRecord({
On 2013/08/01 21:48:39, kpreid_google wrote:
Please add a TODO to add a standardized taming of console somewhere in
Caja. We
should have fewer copies of this code.
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/js/PlainModule.java
File src/com/google/caja/parser/js/PlainModule.java (right):
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/js/PlainModule.java#newcode1
src/com/google/caja/parser/js/PlainModule.java:1: // Copyright (C) 2013
Google Inc.
On 2013/08/01 21:48:39, kpreid_google wrote:
This file appears to be marked as a copy, but not be sufficiently
similar to its
original to be relevant.
Yeah, sorry. :(
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/js/PlainModule.java#newcode30
src/com/google/caja/parser/js/PlainModule.java:30: /**
On 2013/08/01 21:48:39, kpreid_google wrote:
Please document, as its class name is not obvious.
If I understand what this does (basically what the comment inside
render()
says), I'm surprised we don't already have something that does it.
Shouldn't the
minifier etc. need this functionality already?
Yeah I dunno, but the minifier is under ancillary/ and is a nontrivial
codebase to plumb. This is just a simple utility that does precisely
what I need. :)
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/js/PlainModule.java#newcode66
src/com/google/caja/parser/js/PlainModule.java:66: public final
TokenConsumer makeRenderer(
On 2013/08/01 21:48:39, kpreid_google wrote:
Why does this belong specifically on this class?
Should be static and package-private?
It's inherited from Renderable....
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java
File src/com/google/caja/parser/quasiliteral/TracingRewriter.java
(right):
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode45
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:45: *
@author [email protected]
On 2013/08/01 21:48:39, kpreid_google wrote:
Document purpose. If nothing else, at least explain that this is not
part of the
cajoler.
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode45
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:45: *
@author [email protected]
On 2013/08/01 21:48:39, kpreid_google wrote:
Document purpose. If nothing else, at least explain that this is not
part of the
cajoler.
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode48
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:48:
Deferred.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode49
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:49: private
static StringLiteral nodeEqualsTo(ParseTreeNode n) {
Deferred.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode50
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:50:
StringBuffer sb = new StringBuffer();
On 2013/08/01 21:48:39, kpreid_google wrote:
StringBuilder is preferred when thread-safety is not required.
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode118
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:118: ((expr
instanceof Reference )
On 2013/08/01 21:48:39, kpreid_google wrote:
unnecessary parens (all "comparison" operators are higher precedence
than ?:,
and this is common across all C-style syntax I've heard of)
stray whitespace
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode126
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:126:
((fc.getIdentifierName() != null)
On 2013/08/01 21:48:39, kpreid_google wrote:
ditto
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode161
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:161:
synopsis="Cajole an UncajoledModule into a PlainModule.",
On 2013/08/01 21:48:39, kpreid_google wrote:
"Cajole" is wrong
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode223
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:223: + "
@fh*;"
On 2013/08/01 21:48:39, kpreid_google wrote:
This substitution pattern appears not to be used. The parameter can be
omitted.
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/parser/quasiliteral/TracingRewriter.java#newcode339
src/com/google/caja/parser/quasiliteral/TracingRewriter.java:339: + "
return TRACING.@popCallsite('return', new ctor(@actuals*));"
On 2013/08/01 21:48:39, kpreid_google wrote:
long line
Done.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/plugin/TracingRewriterMain.java
File src/com/google/caja/plugin/TracingRewriterMain.java (right):
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/plugin/TracingRewriterMain.java#newcode58
src/com/google/caja/plugin/TracingRewriterMain.java:58: new UriFetcher()
{
Deferred.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/plugin/TracingRewriterMain.java#newcode74
src/com/google/caja/plugin/TracingRewriterMain.java:74: if
(mq.hasMessageAtLevel(MessageLevel.WARNING)) {
Deferred.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/plugin/TracingRewriterMain.java#newcode89
src/com/google/caja/plugin/TracingRewriterMain.java:89: mq.addMessage(
Deferred.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/service/TracingHandler.java
File src/com/google/caja/service/TracingHandler.java (right):
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/service/TracingHandler.java#newcode97
src/com/google/caja/service/TracingHandler.java:97: return null;
Deferred.
https://codereview.appspot.com/7678045/diff/30025/src/com/google/caja/service/TracingHandler.java#newcode140
src/com/google/caja/service/TracingHandler.java:140: new
Callback<IOException>() {
Deferred.
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js
File
tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js
(right):
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js#newcode30
tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js:30:
function stringify(o) {
Deferred.
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js#newcode33
tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js:33:
if (k === 'parent' || k === 'result' || k === 'level') { return
undefined; }
Deferred.
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js#newcode57
tests/com/google/caja/parser/quasiliteral/tracingRewriterTestAssertTraces.js:57:
Object.getOwnPropertyNames(expected).map(add);
On 2013/08/01 21:48:39, kpreid_google wrote:
use forEach instead of map
Done.
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/parser/quasiliteral/tracingRewriterTestCases.js
File
tests/com/google/caja/parser/quasiliteral/tracingRewriterTestCases.js
(right):
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/parser/quasiliteral/tracingRewriterTestCases.js#newcode24
tests/com/google/caja/parser/quasiliteral/tracingRewriterTestCases.js:24:
return funcExprRE.exec(f.toSource())[1];
This code is run under Rhino.
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/parser/quasiliteral/tracingRewriterTestCases.js#newcode43
tests/com/google/caja/parser/quasiliteral/tracingRewriterTestCases.js:43:
function () {
On 2013/08/01 21:48:39, kpreid_google wrote:
no space before (
Done.
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/util/RhinoExecutor.java
File tests/com/google/caja/util/RhinoExecutor.java (right):
https://codereview.appspot.com/7678045/diff/30025/tests/com/google/caja/util/RhinoExecutor.java#newcode159
tests/com/google/caja/util/RhinoExecutor.java:159: // Deny reflective
access up front. This should not be triggered due
On 2013/08/01 21:48:39, kpreid_google wrote:
Why this change?
I don't recall specifically, but it was necessary and seemed safe at the
time.
https://codereview.appspot.com/7678045/diff/30025/tools/postfile.py
File tools/postfile.py (right):
https://codereview.appspot.com/7678045/diff/30025/tools/postfile.py#newcode18
tools/postfile.py:18: Simple HTTP server for saving files from test
harnesses.
Deferred.
https://codereview.appspot.com/7678045/
--
---
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.
