Skip to content

Commit

Permalink
Fix constant evaluation.
Browse files Browse the repository at this point in the history
If you try to access a constant through CssResource interface
with a definition referring to a function or another constant,
the returned value was not correct.

This patch fixes also the AssertionError introduced in
a previous (reverted) patch when using constants with custom
function.

Bug: issue 9113
Change-Id: Icc5adc94990ddb627d119f8bab06870ffc66450a
  • Loading branch information
jDramaix authored and Gerrit Code Review committed Apr 28, 2015
1 parent 230c633 commit 8a557dc
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 84 deletions.
@@ -0,0 +1,52 @@
/*
* Copyright 2014 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.google.gwt.resources.gss;

import com.google.gwt.thirdparty.common.css.compiler.ast.CssCompilerPass;
import com.google.gwt.thirdparty.common.css.compiler.ast.CssDefinitionNode;
import com.google.gwt.thirdparty.common.css.compiler.ast.CssTree;
import com.google.gwt.thirdparty.common.css.compiler.ast.MutatingVisitController;
import com.google.gwt.thirdparty.common.css.compiler.passes.CollectConstantDefinitions;

/**
* A pass that collects the constant definitions inside the tree. This pass removes the constant
* definitions during the collect.
*/
public class CollectAndRemoveConstantDefinitions extends CollectConstantDefinitions
implements CssCompilerPass {

private final MutatingVisitController visitController;

public CollectAndRemoveConstantDefinitions(CssTree tree) {
this(tree.getMutatingVisitController());
}

public CollectAndRemoveConstantDefinitions(MutatingVisitController visitController) {
super(visitController);

this.visitController = visitController;
}

@Override
public boolean enterDefinition(CssDefinitionNode node) {
super.enterDefinition(node);

visitController.removeCurrentNode();

return false;
}
}
74 changes: 0 additions & 74 deletions user/src/com/google/gwt/resources/gss/ConstantResolver.java

This file was deleted.

Expand Up @@ -45,7 +45,7 @@
import com.google.gwt.resources.ext.ResourceGeneratorUtil; import com.google.gwt.resources.ext.ResourceGeneratorUtil;
import com.google.gwt.resources.ext.SupportsGeneratorResultCaching; import com.google.gwt.resources.ext.SupportsGeneratorResultCaching;
import com.google.gwt.resources.gss.BooleanConditionCollector; import com.google.gwt.resources.gss.BooleanConditionCollector;
import com.google.gwt.resources.gss.ConstantResolver; import com.google.gwt.resources.gss.CollectAndRemoveConstantDefinitions;
import com.google.gwt.resources.gss.CreateRuntimeConditionalNodes; import com.google.gwt.resources.gss.CreateRuntimeConditionalNodes;
import com.google.gwt.resources.gss.CssPrinter; import com.google.gwt.resources.gss.CssPrinter;
import com.google.gwt.resources.gss.ExtendedEliminateConditionalNodes; import com.google.gwt.resources.gss.ExtendedEliminateConditionalNodes;
Expand Down Expand Up @@ -719,7 +719,6 @@ private void finalizeTree(CssTree cssTree) throws UnableToCompleteException {
new ProcessKeyframes(cssTree.getMutatingVisitController(), errorManager, true, true).runPass(); new ProcessKeyframes(cssTree.getMutatingVisitController(), errorManager, true, true).runPass();
new ProcessRefiners(cssTree.getMutatingVisitController(), errorManager, true).runPass(); new ProcessRefiners(cssTree.getMutatingVisitController(), errorManager, true).runPass();
new MarkNonFlippableNodes(cssTree.getMutatingVisitController(), errorManager).runPass(); new MarkNonFlippableNodes(cssTree.getMutatingVisitController(), errorManager).runPass();
new ConstantResolver(cssTree, cssTree.getMutatingVisitController()).runPass();
} }


private ConstantDefinitions optimizeTree(CssParsingResult cssParsingResult, ResourceContext context, private ConstantDefinitions optimizeTree(CssParsingResult cssParsingResult, ResourceContext context,
Expand Down Expand Up @@ -761,7 +760,7 @@ private ConstantDefinitions optimizeTree(CssParsingResult cssParsingResult, Reso
collectConstantDefinitionsPass.runPass(); collectConstantDefinitionsPass.runPass();


ReplaceConstantReferences replaceConstantReferences = new ReplaceConstantReferences(cssTree, ReplaceConstantReferences replaceConstantReferences = new ReplaceConstantReferences(cssTree,
collectConstantDefinitionsPass.getConstantDefinitions(), true, errorManager, false); collectConstantDefinitionsPass.getConstantDefinitions(), false, errorManager, false);
replaceConstantReferences.runPass(); replaceConstantReferences.runPass();


new ImageSpriteCreator(cssTree.getMutatingVisitController(), context, errorManager).runPass(); new ImageSpriteCreator(cssTree.getMutatingVisitController(), context, errorManager).runPass();
Expand All @@ -770,6 +769,10 @@ private ConstantDefinitions optimizeTree(CssParsingResult cssParsingResult, Reso
new ResolveCustomFunctionNodes(cssTree.getMutatingVisitController(), errorManager, new ResolveCustomFunctionNodes(cssTree.getMutatingVisitController(), errorManager,
gssFunctionMap, true, allowedNonStandardFunctions).runPass(); gssFunctionMap, true, allowedNonStandardFunctions).runPass();


// collect the final value of the constants and remove them.
collectConstantDefinitionsPass = new CollectAndRemoveConstantDefinitions(cssTree);
collectConstantDefinitionsPass.runPass();

if (simplifyCss) { if (simplifyCss) {
// Eliminate empty rules. // Eliminate empty rules.
new EliminateEmptyRulesetNodes(cssTree.getMutatingVisitController()).runPass(); new EliminateEmptyRulesetNodes(cssTree.getMutatingVisitController()).runPass();
Expand Down
Expand Up @@ -180,9 +180,11 @@ public void testCharset() {
} }


public void testConstantAccess() { public void testConstantAccess() {
assertEquals("#012345", res().constants().COLOR1()); assertEquals("10px", res().constants().padding2());
assertEquals("#012345", res().constants().MYCOLOR()); assertEquals("#012345", res().constants().color1());
assertEquals("#012345", res().constants().MYCOLOR1()); assertEquals("#012345", res().constants().mycolor());
assertEquals("#012345", res().constants().mycolor1());
assertEquals(10, res().constants().margin());
} }


public void testEmpty() { public void testEmpty() {
Expand Down
12 changes: 9 additions & 3 deletions user/test/com/google/gwt/resources/client/gss/TestResources.java
Expand Up @@ -166,9 +166,15 @@ interface Charset extends CssResource {
* Used to test constants that use other constants. * Used to test constants that use other constants.
*/ */
interface Constants extends CssResource { interface Constants extends CssResource {
String COLOR1(); String color1();
String MYCOLOR();
String MYCOLOR1(); int margin();

String mycolor();

String mycolor1();

String padding2();
} }


/** /**
Expand Down
15 changes: 14 additions & 1 deletion user/test/com/google/gwt/resources/client/gss/constants.gss
@@ -1,3 +1,16 @@
@def COLOR1 #012345; @if (is("custom.one", "bar")) {
@def COLOR1 #012345;
}
@else {
@def COLOR1 #543210;
}

@def MYCOLOR COLOR1; @def MYCOLOR COLOR1;
@def MYCOLOR1 MYCOLOR; @def MYCOLOR1 MYCOLOR;

@def PADDING add(5px, 5px);
@def PADDING2 PADDING;

@def MARGIN1 5px;
@def MARGIN2 5px;
@def MARGIN add(MARGIN1, MARGIN2);

0 comments on commit 8a557dc

Please sign in to comment.