Revision: 4452
Author: mikesamuel
Date: Mon Apr 25 23:22:07 2011
Log: Fix issue 1348
http://codereview.appspot.com/4432067
Fix problem with optimizing dot operators in the minifier.
[email protected]
http://code.google.com/p/google-caja/source/detail?r=4452
Modified:
/trunk/src/com/google/caja/ancillary/opt/StatementSimplifier.java
/trunk/src/com/google/caja/demos/playground/server/AppEngineJobCache.java
/trunk/src/com/google/caja/demos/playground/server/AppEngineJobCacheKey.java
/trunk/src/com/google/caja/lang/css/JSRE.java
/trunk/src/com/google/caja/lang/html/HtmlDefinitions.java
/trunk/src/com/google/caja/parser/AbstractParseTreeNode.java
/trunk/src/com/google/caja/parser/js/Operation.java
/trunk/src/com/google/caja/plugin/CssRuleRewriter.java
/trunk/src/com/google/caja/plugin/stages/StubJobCache.java
/trunk/tests/com/google/caja/ancillary/opt/JsOptimizerTest.java
/trunk/tests/com/google/caja/parser/html/NodesTest.java
/trunk/tests/com/google/caja/plugin/PipelineCacheTest.java
=======================================
--- /trunk/src/com/google/caja/ancillary/opt/StatementSimplifier.java Thu
Nov 18 11:05:16 2010
+++ /trunk/src/com/google/caja/ancillary/opt/StatementSimplifier.java Mon
Apr 25 23:22:07 2011
@@ -42,6 +42,7 @@
import com.google.caja.parser.js.Reference;
import com.google.caja.parser.js.ReturnStmt;
import com.google.caja.parser.js.Statement;
+import com.google.caja.parser.js.StringLiteral;
import com.google.caja.parser.js.SwitchCase;
import com.google.caja.parser.js.SwitchStmt;
import com.google.caja.parser.js.ThrowStmt;
@@ -645,11 +646,26 @@
&& ParseTreeNodes.deepEquals(xoperands.get(0),
yoperands.get(0))
) {
// c ? (x + 1 : x + 2) -> x + (c ? 1 : 2)
- return Operation.create(
- pos, xoper, xoperands.get(0),
- optimizeExpressionFlow(
- Operation.createTernary(
- c, xoperands.get(1), yoperands.get(1))));
+ if (xoper != Operator.MEMBER_ACCESS) {
+ return Operation.create(
+ pos, xoper, xoperands.get(0),
+ optimizeExpressionFlow(
+ Operation.createTernary(
+ c, xoperands.get(1), yoperands.get(1))));
+ } else {
+ Reference xref = (Reference) xoperands.get(1);
+ Reference yref = (Reference) yoperands.get(1);
+ StringLiteral xname = StringLiteral.valueOf(
+ xref.getFilePosition(), xref.getIdentifierName());
+ StringLiteral yname = StringLiteral.valueOf(
+ yref.getFilePosition(), yref.getIdentifierName());
+ return Operation.create(
+ pos, Operator.SQUARE_BRACKET,
+ xoperands.get(0),
+ optimizeExpressionFlow(
+ Operation.createTernary(
+ c, xname, yname)));
+ }
}
}
}
=======================================
---
/trunk/src/com/google/caja/demos/playground/server/AppEngineJobCache.java
Fri Apr 8 23:31:20 2011
+++
/trunk/src/com/google/caja/demos/playground/server/AppEngineJobCache.java
Mon Apr 25 23:22:07 2011
@@ -18,7 +18,6 @@
import java.util.Collections;
import net.sf.jsr107cache.Cache;
-import net.sf.jsr107cache.CacheException;
import net.sf.jsr107cache.CacheManager;
import com.google.caja.parser.ParseTreeNode;
@@ -34,7 +33,7 @@
*/
final class AppEngineJobCache extends JobCache {
private Cache l1cache;
-
+
AppEngineJobCache() {
try {
this.l1cache = CacheManager.getInstance().getCacheFactory()
@@ -44,10 +43,13 @@
}
}
+ @Override
public AppEngineJobCacheKey forJob(ContentType type, ParseTreeNode node)
{
return new AppEngineJobCacheKey(type, node);
}
+ @SuppressWarnings("unchecked")
+ @Override
public List<? extends Job> fetch(JobCache.Key k) {
if (null == l1cache) { return null; }
if (!(k instanceof AppEngineJobCacheKey)) { return null; }
@@ -59,6 +61,7 @@
return cloneJobs(cachedJobs);
}
+ @Override
public void store(Key k, List<? extends Job> derivatives) {
if (!(k instanceof AppEngineJobCacheKey)) {
throw new IllegalArgumentException(k.getClass().getName());
=======================================
---
/trunk/src/com/google/caja/demos/playground/server/AppEngineJobCacheKey.java
Fri Apr 8 23:31:20 2011
+++
/trunk/src/com/google/caja/demos/playground/server/AppEngineJobCacheKey.java
Mon Apr 25 23:22:07 2011
@@ -111,8 +111,9 @@
}
private void hash(Node node) {
- hash((short) node.getNodeType());
- switch (node.getNodeType()) {
+ short nodeType = node.getNodeType();
+ hash(nodeType);
+ switch (nodeType) {
case Node.ATTRIBUTE_NODE:
case Node.ELEMENT_NODE:
hash(node.getNodeName());
@@ -125,7 +126,7 @@
hash((short) node.getChildNodes().getLength());
- if (node.getNodeType() == Node.ELEMENT_NODE) {
+ if (nodeType == Node.ELEMENT_NODE) {
NamedNodeMap attrs = node.getAttributes();
int nAttrs = attrs.getLength();
hash((short) nAttrs);
=======================================
--- /trunk/src/com/google/caja/lang/css/JSRE.java Fri Apr 8 23:31:20 2011
+++ /trunk/src/com/google/caja/lang/css/JSRE.java Mon Apr 25 23:22:07 2011
@@ -18,7 +18,6 @@
import com.google.caja.util.Lists;
import com.google.caja.util.Sets;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -390,7 +389,7 @@
int i = 0;
while (i < n && a.get(i).equals(b.get(i))) { ++i; }
// Wrapping in a new array list because List.subList is not
serializable
- return new ArrayList(a.subList(0, i));
+ return Lists.<T>newArrayList(a.subList(0, i));
}
private static <T> List<T> commonSuffix(List<T> a, List<T> b) {
@@ -400,7 +399,7 @@
// work done in condition
}
// Wrapping in a new array list because List.subList is not
serializable
- return new ArrayList(a.subList(i + 1, m));
+ return Lists.<T>newArrayList(a.subList(i + 1, m));
}
static final class Concatenation extends JSRE {
=======================================
--- /trunk/src/com/google/caja/lang/html/HtmlDefinitions.java Fri Feb 18
13:40:38 2011
+++ /trunk/src/com/google/caja/lang/html/HtmlDefinitions.java Mon Apr 25
23:22:07 2011
@@ -97,7 +97,7 @@
private static final AttribKey SCRIPT_SRC = AttribKey.forHtmlAttrib(
ElKey.forHtmlElement("script"), "src");
-
+
private static <U> ExpressionStmt mapFromEnum(
Iterable<U> entries, String key,
Function<U, String> keyMaker, Function<U, Integer> valueMaker) {
@@ -115,7 +115,7 @@
"k", new ParseTreeNodeContainer(keys),
"v", new ParseTreeNodeContainer(values)));
}
-
+
public static Block generateJavascriptDefinitions(HtmlSchema schema) {
final FilePosition unk = FilePosition.UNKNOWN;
Map<AttribKey, HTML.Attribute.Type> atypes = attributeTypes(schema);
@@ -140,7 +140,7 @@
}
})
);
-
+
{
List<StringLiteral> keys = new ArrayList<StringLiteral>();
List<IntegerLiteral> values = new ArrayList<IntegerLiteral>();
@@ -160,7 +160,7 @@
"k", new ParseTreeNodeContainer(keys),
"v", new ParseTreeNodeContainer(values))));
}
-
+
definitions.appendChild(mapFromEnum(
EnumSet.allOf(EFlag.class),
"eflags",
@@ -343,9 +343,9 @@
}
return attributeFlags;
}
-
+
private interface SchemaExtractor<Result> { Result
extract(HTML.Attribute attr); }
-
+
private static <A> Map<AttribKey, A> deriveMapFromSchema(
HtmlSchema schema, SchemaExtractor<A> extractor) {
Map<AttribKey, A> result = Maps.newTreeMap();
@@ -364,7 +364,6 @@
private static Map<AttribKey, UriEffect> uriEffects(
HtmlSchema schema) {
return deriveMapFromSchema(schema, new SchemaExtractor<UriEffect>() {
- @Override
public UriEffect extract(Attribute attr) {
return attr.getUriEffect();
}
@@ -374,7 +373,6 @@
private static Map<AttribKey, LoaderType> loaderTypes(
HtmlSchema schema) {
return deriveMapFromSchema(schema, new SchemaExtractor<LoaderType>() {
- @Override
public LoaderType extract(Attribute attr) {
return attr.getLoaderType();
}
=======================================
--- /trunk/src/com/google/caja/parser/AbstractParseTreeNode.java Fri Apr 8
23:31:20 2011
+++ /trunk/src/com/google/caja/parser/AbstractParseTreeNode.java Mon Apr 25
23:22:07 2011
@@ -20,6 +20,7 @@
import com.google.caja.reporting.MessageContext;
import com.google.caja.reporting.MessagePart;
import com.google.caja.util.Join;
+import com.google.caja.util.Lists;
import com.google.caja.util.SyntheticAttributeKey;
import com.google.caja.util.SyntheticAttributes;
@@ -79,8 +80,8 @@
@SuppressWarnings("unchecked")
protected <T2> List<T2> childrenPart(
int start, int end, Class<T2> cl) {
- List<ParseTreeNode> sub =
- new ArrayList(children.getImmutableFacet().subList(start, end));
+ List<ParseTreeNode> sub = Lists.newArrayList(
+ children.getImmutableFacet().subList(start, end));
for (ParseTreeNode el : sub) {
if (!cl.isInstance(el)) {
throw new ClassCastException(
=======================================
--- /trunk/src/com/google/caja/parser/js/Operation.java Fri Apr 8 23:31:20
2011
+++ /trunk/src/com/google/caja/parser/js/Operation.java Mon Apr 25 23:22:07
2011
@@ -64,7 +64,8 @@
@Override
protected void childrenChanged() {
super.childrenChanged();
- int nChildren = children().size();
+ List<? extends Expression> children = children();
+ int nChildren = children.size();
if (nChildren < minArity(op)) {
throw new IllegalArgumentException(
"Too few of children " + nChildren + " for operator " + op);
@@ -72,6 +73,19 @@
throw new IllegalArgumentException(
"Too many children " + nChildren + " for operator " + op);
}
+ if (Operator.MEMBER_ACCESS == op) {
+ if (!(children.get(1) instanceof Reference)) {
+ throw new IllegalArgumentException(
+ "Bad child of . operator : " + children().get(1));
+ }
+ }
+ if (OperatorCategory.ASSIGNMENT == op.getCategory()) {
+ if (!children.get(0).isLeftHandSide()) {
+ throw new IllegalArgumentException(
+ "Invalid assignment " + op + " with left hand side "
+ + children.get(0));
+ }
+ }
}
public static Operation create(
=======================================
--- /trunk/src/com/google/caja/plugin/CssRuleRewriter.java Mon Apr 4
11:32:33 2011
+++ /trunk/src/com/google/caja/plugin/CssRuleRewriter.java Mon Apr 25
23:22:07 2011
@@ -193,7 +193,6 @@
super(pos, value);
}
- @Override
public void render(RenderContext r) {
TokenConsumer tc = r.getOut();
tc.mark(getFilePosition());
@@ -290,7 +289,6 @@
return arrayElements;
}
- @Override
public void mark(FilePosition pos) {
cssTokenConsumer.mark(pos);
if (inJsString && positionAtStartOfStringLiteral == null) {
@@ -299,7 +297,6 @@
last = pos;
}
- @Override
public void consume(String text) {
if (!inJsString) {
startPartialJsStringLiteral();
@@ -313,7 +310,6 @@
inJsString = true;
}
- @Override
public void noMoreTokens() {
cssTokenConsumer.noMoreTokens();
endArrayElement(false);
=======================================
--- /trunk/src/com/google/caja/plugin/stages/StubJobCache.java Sat Feb 5
15:36:47 2011
+++ /trunk/src/com/google/caja/plugin/stages/StubJobCache.java Mon Apr 25
23:22:07 2011
@@ -42,7 +42,6 @@
public TrivialJobCacheKey(String pos) { this.pos = pos; }
- @Override
public JobCache.Keys asSingleton() {
return new TrivialJobCacheKeys(
Collections.<JobCache.Key>singletonList(this));
@@ -56,7 +55,6 @@
TrivialJobCacheKeys(List<JobCache.Key> keys) { this.keys = keys; }
- @Override
public JobCache.Keys union(JobCache.Keys other) {
if (!other.iterator().hasNext()) { return this; }
List<JobCache.Key> keys = Lists.newArrayList();
@@ -65,7 +63,7 @@
return new TrivialJobCacheKeys(Collections.unmodifiableList(keys));
}
- @Override public Iterator<JobCache.Key> iterator() {
+ public Iterator<JobCache.Key> iterator() {
return keys.iterator();
}
=======================================
--- /trunk/tests/com/google/caja/ancillary/opt/JsOptimizerTest.java Tue May
25 12:11:32 2010
+++ /trunk/tests/com/google/caja/ancillary/opt/JsOptimizerTest.java Mon Apr
25 23:22:07 2011
@@ -22,6 +22,7 @@
import com.google.caja.render.JsMinimalPrinter;
import com.google.caja.reporting.RenderContext;
import com.google.caja.util.CajaTestCase;
+import com.google.caja.util.Join;
import java.util.Collections;
@@ -83,6 +84,28 @@
js(fromString("alert(function(){ if (a) { foo(); bar(); return
baz(); }"
+ "else return boo(); }());")));
}
+
+ public final void testIssue1348() throws Exception {
+ String input = Join.join(
+ "\n",
+ "if (XMLHttpRequest && 'withCredentials' in new XMLHttpRequest())
{",
+ " // FF 3.5+ and Safari 4",
+ " request = requestFunctions['w3cxhr'];",
+ "} else if (XDomainRequest) {",
+ " // IE8",
+ " request = requestFunctions['msxdr'];",
+ "} else {",
+ " // Older browser; fallback",
+ " request = requestFunctions['jsonp'];",
+ "}");
+ assertOptimized(
+ js(fromString(
+ "XMLHttpRequest && 'withCredentials' in new XMLHttpRequest"
+ + "? (request = requestFunctions.w3cxhr)"
+ + ": (request =
requestFunctions[XDomainRequest ? 'msxdr' : 'jsonp'])"
+ )),
+ js(fromString(input)));
+ }
private void assertOptimized(Statement golden, Block... inputs) {
for (Block input : inputs) { opt.addInput(input); }
=======================================
--- /trunk/tests/com/google/caja/parser/html/NodesTest.java Fri Apr 22
15:35:40 2011
+++ /trunk/tests/com/google/caja/parser/html/NodesTest.java Mon Apr 25
23:22:07 2011
@@ -25,7 +25,6 @@
import com.google.caja.util.MoreAsserts;
import java.util.Arrays;
-import java.io.IOException;
import java.io.StringReader;
import org.w3c.dom.Attr;
@@ -154,7 +153,7 @@
assertRendersUnsafe(RENDER_WITH_COMMENTS, el, MarkupRenderMode.HTML);
assertRendersUnsafe(RENDER_WITH_COMMENTS, el, MarkupRenderMode.XML);
}
-
+
public final void testIllegalCharactersInComment() throws Exception {
assertFailsToRenderUnsafe("<!-- -- -->", MarkupRenderMode.HTML,
"XML/HTML comment", "contains '--'");
@@ -171,7 +170,8 @@
FilePosition.startOfFile(is), new StringReader(xml), true, true);
return new DomParser(tq, true, mq).parseFragment();
}
-
+
+ @SuppressWarnings("deprecation")
private void assertRendersUnsafe(String expected, Node el,
MarkupRenderMode mode) throws Exception {
try {
@@ -181,8 +181,9 @@
}
}
+ @SuppressWarnings("deprecation")
private void assertFailsToRenderUnsafe(
- String xml, MarkupRenderMode mode, String... messages)
+ String xml, MarkupRenderMode mode, String... messages)
throws Exception {
try {
TokenQueue<HtmlTokenType> tq = DomParser.makeTokenQueue(
=======================================
--- /trunk/tests/com/google/caja/plugin/PipelineCacheTest.java Sat Feb 5
15:36:47 2011
+++ /trunk/tests/com/google/caja/plugin/PipelineCacheTest.java Mon Apr 25
23:22:07 2011
@@ -507,7 +507,6 @@
plm.populate(pl.getStages());
pl.getStages().add(new PipelineStoreStage(cache));
pl.getStages().add(new Pipeline.Stage<Jobs>() {
- @Override
public boolean apply(Jobs jobs) {
for (ListIterator<JobEnvelope> it = jobs.getJobs().listIterator();
it.hasNext();) {
