You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2010/02/17 02:10:35 UTC
svn commit: r910792 - in /tomcat/trunk/java/org/apache/jasper/compiler:
Generator.java Node.java ScriptingVariabler.java
Author: kkolinko
Date: Wed Feb 17 01:10:34 2010
New Revision: 910792
URL: http://svn.apache.org/viewvc?rev=910792&view=rev
Log:
Apply my patch from https://issues.apache.org/bugzilla/show_bug.cgi?id=48616#c20
This patch
- Reverts r905145,
- Provides an alternative fix for bug 48616 and bug 42390,
- Replaces Vector -> List, Hashtable -> HashMap in the affected API.
JspFragments are scriptless, so no need to declare or sync scripting
variables for fragments. Since errors in syncing the scripting variables for
JSP Fragments caused 48616 & 42390, this fixes both these bugs too.
Modified:
tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
tomcat/trunk/java/org/apache/jasper/compiler/Node.java
tomcat/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Generator.java?rev=910792&r1=910791&r2=910792&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Wed Feb 17 01:10:34 2010
@@ -167,26 +167,6 @@
return b.toString();
}
- /**
- * Finds the <jsp:body> subelement of the given parent node. If not
- * found, null is returned.
- */
- protected static Node.JspBody findJspBody(Node parent) {
- Node.JspBody result = null;
-
- Node.Nodes subelements = parent.getBody();
- for (int i = 0; (subelements != null) && (i < subelements.size()); i++) {
- Node n = subelements.getNode(i);
- if (n instanceof Node.JspBody) {
- result = (Node.JspBody) n;
- break;
- }
- }
-
- return result;
- }
-
-
private String createJspId() {
if (this.jspIdPrefix == null) {
StringBuilder sb = new StringBuilder(32);
@@ -358,6 +338,9 @@
@Override
public void visit(Node.CustomTag n) throws JasperException {
+ // XXX - Actually there is no need to declare those
+ // "_jspx_" + varName + "_" + nestingLevel variables when we are
+ // inside a JspFragment.
if (n.getCustomNestingLevel() > 0) {
TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
@@ -991,6 +974,25 @@
}
}
+ /**
+ * Finds the <jsp:body> subelement of the given parent node. If not
+ * found, null is returned.
+ */
+ private Node.JspBody findJspBody(Node parent) {
+ Node.JspBody result = null;
+
+ Node.Nodes subelements = parent.getBody();
+ for (int i = 0; (subelements != null) && (i < subelements.size()); i++) {
+ Node n = subelements.getNode(i);
+ if (n instanceof Node.JspBody) {
+ result = (Node.JspBody) n;
+ break;
+ }
+ }
+
+ return result;
+ }
+
@Override
public void visit(Node.ForwardAction n) throws JasperException {
Node.JspAttribute page = n.getPage();
@@ -2505,11 +2507,16 @@
}
private void declareScriptingVars(Node.CustomTag n, int scope) {
+ if (isFragment) {
+ // No need to declare Java variables, if we inside a
+ // JspFragment, because a fragment is always scriptless.
+ return;
+ }
- Vector<Object> vec = n.getScriptingVars(scope);
+ List<Object> vec = n.getScriptingVars(scope);
if (vec != null) {
for (int i = 0; i < vec.size(); i++) {
- Object elem = vec.elementAt(i);
+ Object elem = vec.get(i);
if (elem instanceof VariableInfo) {
VariableInfo varInfo = (VariableInfo) elem;
if (varInfo.getDeclare()) {
@@ -2552,6 +2559,14 @@
if (n.getCustomNestingLevel() == 0) {
return;
}
+ if (isFragment) {
+ // No need to declare Java variables, if we inside a
+ // JspFragment, because a fragment is always scriptless.
+ // Thus, there is no need to save/ restore/ sync them.
+ // Note, that JspContextWrapper.syncFoo() methods will take
+ // care of saving/ restoring/ sync'ing of JspContext attributes.
+ return;
+ }
TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
VariableInfo[] varInfos = n.getVariableInfos();
@@ -2559,13 +2574,15 @@
return;
}
+ List<Object> declaredVariables = n.getScriptingVars(scope);
+
if (varInfos.length > 0) {
for (int i = 0; i < varInfos.length; i++) {
if (varInfos[i].getScope() != scope)
continue;
// If the scripting variable has been declared, skip codes
// for saving and restoring it.
- if (n.getScriptingVars(scope).contains(varInfos[i]))
+ if (declaredVariables.contains(varInfos[i]))
continue;
String varName = varInfos[i].getVarName();
String tmpVarName = "_jspx_" + varName + "_"
@@ -2581,7 +2598,7 @@
continue;
// If the scripting variable has been declared, skip codes
// for saving and restoring it.
- if (n.getScriptingVars(scope).contains(tagVarInfos[i]))
+ if (declaredVariables.contains(tagVarInfos[i]))
continue;
String varName = tagVarInfos[i].getNameGiven();
if (varName == null) {
@@ -2612,6 +2629,14 @@
if (n.getCustomNestingLevel() == 0) {
return;
}
+ if (isFragment) {
+ // No need to declare Java variables, if we inside a
+ // JspFragment, because a fragment is always scriptless.
+ // Thus, there is no need to save/ restore/ sync them.
+ // Note, that JspContextWrapper.syncFoo() methods will take
+ // care of saving/ restoring/ sync'ing of JspContext attributes.
+ return;
+ }
TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
VariableInfo[] varInfos = n.getVariableInfos();
@@ -2619,13 +2644,15 @@
return;
}
+ List<Object> declaredVariables = n.getScriptingVars(scope);
+
if (varInfos.length > 0) {
for (int i = 0; i < varInfos.length; i++) {
if (varInfos[i].getScope() != scope)
continue;
// If the scripting variable has been declared, skip codes
// for saving and restoring it.
- if (n.getScriptingVars(scope).contains(varInfos[i]))
+ if (declaredVariables.contains(varInfos[i]))
continue;
String varName = varInfos[i].getVarName();
String tmpVarName = "_jspx_" + varName + "_"
@@ -2641,7 +2668,7 @@
continue;
// If the scripting variable has been declared, skip codes
// for saving and restoring it.
- if (n.getScriptingVars(scope).contains(tagVarInfos[i]))
+ if (declaredVariables.contains(tagVarInfos[i]))
continue;
String varName = tagVarInfos[i].getNameGiven();
if (varName == null) {
@@ -2666,6 +2693,15 @@
* given scope.
*/
private void syncScriptingVars(Node.CustomTag n, int scope) {
+ if (isFragment) {
+ // No need to declare Java variables, if we inside a
+ // JspFragment, because a fragment is always scriptless.
+ // Thus, there is no need to save/ restore/ sync them.
+ // Note, that JspContextWrapper.syncFoo() methods will take
+ // care of saving/ restoring/ sync'ing of JspContext attributes.
+ return;
+ }
+
TagVariableInfo[] tagVarInfos = n.getTagVariableInfos();
VariableInfo[] varInfos = n.getVariableInfos();
Modified: tomcat/trunk/java/org/apache/jasper/compiler/Node.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/Node.java?rev=910792&r1=910791&r2=910792&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/Node.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/Node.java Wed Feb 17 01:10:34 2010
@@ -1433,11 +1433,11 @@
private boolean implementsDynamicAttributes;
- private Vector<Object> atBeginScriptingVars;
+ private List<Object> atBeginScriptingVars;
- private Vector<Object> atEndScriptingVars;
+ private List<Object> atEndScriptingVars;
- private Vector<Object> nestedScriptingVars;
+ private List<Object> nestedScriptingVars;
private Node.CustomTag customTagParent;
@@ -1657,7 +1657,7 @@
return this.numCount;
}
- public void setScriptingVars(Vector<Object> vec, int scope) {
+ public void setScriptingVars(List<Object> vec, int scope) {
switch (scope) {
case VariableInfo.AT_BEGIN:
this.atBeginScriptingVars = vec;
@@ -1675,8 +1675,8 @@
* Gets the scripting variables for the given scope that need to be
* declared.
*/
- public Vector<Object> getScriptingVars(int scope) {
- Vector<Object> vec = null;
+ public List<Object> getScriptingVars(int scope) {
+ List<Object> vec = null;
switch (scope) {
case VariableInfo.AT_BEGIN:
Modified: tomcat/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java?rev=910792&r1=910791&r2=910792&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java (original)
+++ tomcat/trunk/java/org/apache/jasper/compiler/ScriptingVariabler.java Wed Feb 17 01:10:34 2010
@@ -59,11 +59,11 @@
static class ScriptingVariableVisitor extends Node.Visitor {
private ErrorDispatcher err;
- private Hashtable<String,Integer> scriptVars;
-
+ private Map<String, Integer> scriptVars;
+
public ScriptingVariableVisitor(ErrorDispatcher err) {
this.err = err;
- scriptVars = new Hashtable<String,Integer>();
+ scriptVars = new HashMap<String,Integer>();
}
@Override
@@ -83,7 +83,7 @@
return;
}
- Vector<Object> vec = new Vector<Object>();
+ List<Object> vec = new ArrayList<Object>();
Integer ownRange = null;
Node.CustomTag parent = n.getCustomTagParent();
@@ -107,11 +107,8 @@
String varName = varInfos[i].getVarName();
Integer currentRange = scriptVars.get(varName);
- // If a fragment helper has been used for the parent tag
- // the scripting variables always need to be declared
if (currentRange == null ||
- ownRange.compareTo(currentRange) > 0 ||
- parent != null && isImplementedAsFragment(parent)) {
+ ownRange.compareTo(currentRange) > 0) {
scriptVars.put(varName, ownRange);
vec.add(varInfos[i]);
}
@@ -134,11 +131,8 @@
}
Integer currentRange = scriptVars.get(varName);
- // If a fragment helper has been used for the parent tag
- // the scripting variables always need to be declared
if (currentRange == null ||
- ownRange.compareTo(currentRange) > 0 ||
- parent != null && isImplementedAsFragment(parent)) {
+ ownRange.compareTo(currentRange) > 0) {
scriptVars.put(varName, ownRange);
vec.add(tagVarInfos[i]);
}
@@ -149,22 +143,6 @@
}
}
- private static boolean isImplementedAsFragment(Node.CustomTag n) {
- // Replicates logic from Generator to determine if a fragment
- // helper will be used
- if (n.implementsSimpleTag()) {
- if (Generator.findJspBody(n) == null) {
- if (!n.hasEmptyBody()) {
- return true;
- }
- return false;
- }
- return true;
- }
- return false;
- }
-
-
public static void set(Node.Nodes page, ErrorDispatcher err)
throws JasperException {
page.visit(new CustomTagCounter());
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org