Rescue JSOs crossing JsInterop borders.
Although JSO usage is now discouraged in favor of JsInterop the two
features should behave consistently and allow users to incrementaly
migrate from JSOs to JsInterop constructs.
To help make the transition, JSOs are now considered reachable if
they cross a JsInterop frontier, i.e. passed to a method that
can be referenced externally, etc.
Change-Id: I931571072197ad3762110b93a3ade98b589aaf4a
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
index cbc44f4..aa4e865 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
@@ -376,7 +376,7 @@
}
}
- if (argsToRescueIfParameterRead == null || method.canBePolymorphic()
+ if (argumentsToRescueIfParameterRead == null || method.canBePolymorphic()
|| call instanceof JsniMethodRef) {
return true;
}
@@ -619,7 +619,7 @@
if (dependencyRecorder != null) {
curMethodStack.remove(curMethodStack.size() - 1);
}
- if (method.isJsniMethod()) {
+ if (method.isJsniMethod() || method.canBeImplementedExternally()) {
// Returning from this method passes a value from JavaScript into Java.
maybeRescueJavaScriptObjectPassingIntoJava(method.getType());
}
@@ -627,6 +627,9 @@
for (JParameter param : method.getParams()) {
// Parameters in JsExport, JsType, JsFunction methods should not be pruned in order to
// keep the API intact.
+ if (method.canBeReferencedExternally()) {
+ maybeRescueJavaScriptObjectPassingIntoJava(param.getType());
+ }
rescue(param);
if (param.isVarargs()) {
assert method.isJsMethodVarargs();
@@ -720,47 +723,54 @@
// Already rescued.
return;
}
- membersToRescueIfTypeIsInstantiated.remove(var);
- if (var == getClassField) {
- rescueClassLiteralsIfGetClassIsLive();
- }
- if (isStaticFieldInitializedToLiteral(var)) {
- /*
- * Rescue literal initializers when the field is rescued, not when
- * the static initializer runs. This allows fields initialized to
- * string literals to only need the string literals when the field
- * itself becomes live.
- *
- * NOTE: needs to be in sync with {@link JTypeOracle.CheckClinitVistior}.
- */
- accept(((JField) var).getLiteralInitializer());
- } else if (var instanceof JField
- && (program.getTypeClassLiteralHolder().equals(((JField) var).getEnclosingType()))) {
- /*
- * Rescue just slightly less than what would normally be rescued for
- * a field reference to the literal's field. Rescue the field
- * itself, and its initializer, but do NOT rescue the whole
- * enclosing class. That would pull in the clinit of that class,
- * which has initializers for all the class literals, which in turn
- * have all of the strings of all of the class names.
- *
- * This work is done in rescue() to allow JSNI references to class
- * literals (via the @Foo::class syntax) to correctly rescue class
- * literal initializers.
- *
- * TODO: Model ClassLiteral access a different way to avoid special
- * handling. See
- * Pruner.transformToNullFieldRef()/transformToNullMethodCall().
- */
+ if (var instanceof JField) {
JField field = (JField) var;
- accept(field.getInitializer());
- referencedTypes.add(field.getEnclosingType());
- liveFieldsAndMethods.add(field.getEnclosingType().getClinitMethod());
- } else if (argsToRescueIfParameterRead != null && var instanceof JParameter) {
- List<JExpression> list = argsToRescueIfParameterRead.removeAll(var);
- for (JExpression arg : list) {
- this.accept(arg);
+
+ membersToRescueIfTypeIsInstantiated.remove(field);
+
+ if (field.canBeReferencedExternally() || field.canBeImplementedExternally()) {
+ maybeRescueJavaScriptObjectPassingIntoJava(field.getType());
+ }
+
+ if (field == getClassField) {
+ rescueClassLiteralsIfGetClassIsLive();
+ }
+
+ if (isStaticFieldInitializedToLiteral(field)) {
+ /*
+ * Rescue literal initializers when the field is rescued, not when
+ * the static initializer runs. This allows fields initialized to
+ * string literals to only need the string literals when the field
+ * itself becomes live.
+ *
+ * NOTE: needs to be in sync with {@link JTypeOracle.CheckClinitVistior}.
+ */
+ accept(field.getLiteralInitializer());
+ } else if (program.getTypeClassLiteralHolder().equals(field.getEnclosingType())) {
+ /*
+ * Rescue just slightly less than what would normally be rescued for
+ * a field reference to the literal's field. Rescue the field
+ * itself, and its initializer, but do NOT rescue the whole
+ * enclosing class. That would pull in the clinit of that class,
+ * which has initializers for all the class literals, which in turn
+ * have all of the strings of all of the class names.
+ *
+ * This work is done in rescue() to allow JSNI references to class
+ * literals (via the @Foo::class syntax) to correctly rescue class
+ * literal initializers.
+ *
+ * TODO: Model ClassLiteral access a different way to avoid special
+ * handling. See
+ * Pruner.transformToNullFieldRef()/transformToNullMethodCall().
+ */
+ accept(field.getInitializer());
+ referencedTypes.add(field.getEnclosingType());
+ liveFieldsAndMethods.add(field.getEnclosingType().getClinitMethod());
+ }
+ } else if (var instanceof JParameter && argumentsToRescueIfParameterRead != null) {
+ for (JExpression arg : argumentsToRescueIfParameterRead.removeAll(var)) {
+ accept(arg);
}
}
}
@@ -786,7 +796,7 @@
this.accept(arg);
continue;
}
- argsToRescueIfParameterRead.put(param, arg);
+ argumentsToRescueIfParameterRead.put(param, arg);
}
// Visit any "extra" arguments that exceed the param list.
for (int c = args.size(); i < c; ++i) {
@@ -894,7 +904,7 @@
* read, we will need to rescue the associated arguments. See comments in
* {@link #rescueArgumentsIfParametersCanBeRead}.
*/
- private ListMultimap<JParameter, JExpression> argsToRescueIfParameterRead;
+ private ListMultimap<JParameter, JExpression> argumentsToRescueIfParameterRead;
private final JMethod asyncFragmentOnLoad;
@@ -940,9 +950,9 @@
liveStrings = Sets.newHashSet(cfa.liveStrings);
membersToRescueIfTypeIsInstantiated =
Sets.newHashSet(cfa.membersToRescueIfTypeIsInstantiated);
- if (cfa.argsToRescueIfParameterRead != null) {
- argsToRescueIfParameterRead =
- ArrayListMultimap.create(cfa.argsToRescueIfParameterRead);
+ if (cfa.argumentsToRescueIfParameterRead != null) {
+ argumentsToRescueIfParameterRead =
+ ArrayListMultimap.create(cfa.argumentsToRescueIfParameterRead);
}
rescuer = new RescueVisitor();
}
@@ -1001,8 +1011,8 @@
}
public void setForPruning() {
- assert argsToRescueIfParameterRead == null;
- argsToRescueIfParameterRead = ArrayListMultimap.create();
+ assert argumentsToRescueIfParameterRead == null;
+ argumentsToRescueIfParameterRead = ArrayListMultimap.create();
}
/**
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzerTest.java
index deaa8f5..2f26cb5 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzerTest.java
@@ -105,87 +105,27 @@
* circumstances where values can pass from JS into Java.
*/
public void testRescueJavaScriptObjectFromJsni() throws Exception {
- sourceOracle.addOrReplace(new MockJavaResource("test.JsoIntf") {
- @Override
- public CharSequence getContent() {
- return Joiner.on("\n").join(
- "package test;",
- "import com.google.gwt.core.client.JavaScriptObject;",
- "public interface JsoIntf {",
- " public int getAny();",
- "}");
- }
- });
-
- sourceOracle.addOrReplace(new MockJavaResource("test.UpRefIntf") {
- @Override
- public CharSequence getContent() {
- return Joiner.on("\n").join(
- "package test;",
- "import com.google.gwt.core.client.JavaScriptObject;",
- "public interface UpRefIntf {",
- " public int getFoo();",
- "}");
- }
- });
-
- sourceOracle.addOrReplace(new MockJavaResource("test.NonImplementor") {
- @Override
- public CharSequence getContent() {
- return Joiner.on("\n").join(
- "package test;",
- "import com.google.gwt.core.client.JavaScriptObject;",
- "public class NonImplementor extends JavaScriptObject {",
- " protected NonImplementor() {}",
- " final public native int getFoo() /*-{ return 0; }-*/;",
- "}");
- }
- });
-
- sourceOracle.addOrReplace(new MockJavaResource("test.VirtualUpRef") {
- @Override
- public CharSequence getContent() {
- return Joiner.on("\n").join(
- "package test;",
- "import com.google.gwt.core.client.JavaScriptObject;",
- "final public class VirtualUpRef extends NonImplementor implements UpRefIntf {",
- " protected VirtualUpRef() {}",
- " public static native VirtualUpRef create() /*-{ return {}; }-*/;",
- "}");
- }
- });
-
- sourceOracle.addOrReplace(new MockJavaResource("test.SingleJso") {
- @Override
- public CharSequence getContent() {
- return Joiner.on("\n").join(
- "package test;",
- "import com.google.gwt.core.client.JavaScriptObject;",
- "final public class SingleJso extends JavaScriptObject implements JsoIntf {",
- " protected SingleJso() {}",
- " public native int getAny() /*-{ return 1; }-*/;",
- " public static native JsoIntf returnsJsoIntf() /*-{ return {}; }-*/;",
- " public static native SingleJso returnsJso() /*-{ return {}; }-*/;",
- "}");
- }
- });
-
- sourceOracle.addOrReplace(new MockJavaResource("test.Foo") {
- @Override
- public CharSequence getContent() {
- return Joiner.on("\n").join(
- "package test;",
- "import com.google.gwt.core.client.JavaScriptObject;",
- "public class Foo {",
- " public static native JavaScriptObject returnsJso() /*-{ return {}; }-*/;",
- " public static native void assignsJsoField() /*-{ @test.Foo::jsoField = {}; }-*/;",
- " public static native void readsJsoField() /*-{ var x = @test.Foo::jsoField; }-*/;",
- " public static native void passesJsoParam() /*-{ @test.Foo::calledFromJsni(Lcom/google/gwt/core/client/JavaScriptObject;)({}); }-*/;",
- " private static JavaScriptObject jsoField = null;",
- " private static void calledFromJsni(JavaScriptObject arg) { }",
- "}");
- }
- });
+ addJsoResources();
+ addOrReplaceResource("test.VirtualUpRef",
+ "package test;",
+ "import com.google.gwt.core.client.JavaScriptObject;",
+ "final public class VirtualUpRef extends NonImplementor implements UpRefIntf {",
+ " protected VirtualUpRef() {}",
+ " public static native VirtualUpRef create() /*-{ return {}; }-*/;",
+ "}");
+ addOrReplaceResource("test.Foo",
+ "package test;",
+ "import com.google.gwt.core.client.JavaScriptObject;",
+ "public class Foo {",
+ " public static native JavaScriptObject returnsJso() /*-{ return {}; }-*/;",
+ " public static native void assignsJsoField() /*-{ @test.Foo::jsoField = {}; }-*/;",
+ " public static native void readsJsoField() /*-{ var x = @test.Foo::jsoField; }-*/;",
+ " public static native void passesJsoParam() /*-{ @test.Foo::calledFromJsni(Lcom/google/gwt/core/client/JavaScriptObject;)({}); }-*/;",
+ " private static JavaScriptObject jsoField = null;",
+ " private static void calledFromJsni(JavaScriptObject arg) { }",
+ " public static native JsoIntf returnsJsoIntf() /*-{ return {}; }-*/;",
+ " public static native SingleJso returnsSingleJso() /*-{ return {}; }-*/;",
+ "}");
addSnippetImport("test.Foo");
analyzeSnippet("").assertOnlyInstantiatedTypes(Empty.STRINGS);
@@ -207,11 +147,11 @@
"JavaScriptObject", "Object");
// Returning a JSO subType instantiates it
- analyzeSnippet("SingleJso.returnsJso();").assertOnlyInstantiatedTypes(
+ analyzeSnippet("Foo.returnsSingleJso();").assertOnlyInstantiatedTypes(
"SingleJso", "JavaScriptObject", "Object", "JsoIntf");
// Returning a JSO SingleJsoImpl instantiates it and the implementor
- analyzeSnippet("SingleJso.returnsJsoIntf();").assertOnlyInstantiatedTypes(
+ analyzeSnippet("Foo.returnsJsoIntf();").assertOnlyInstantiatedTypes(
"SingleJso", "JavaScriptObject", "Object", "JsoIntf");
// A virtual upref should still be rescued
@@ -235,17 +175,11 @@
* instantiated in JSNI.
*/
public void testRescueArraysFromJSNI() throws Exception {
- sourceOracle.addOrReplace(new MockJavaResource("test.Foo") {
- @Override
- public CharSequence getContent() {
- return Joiner.on("\n").join(
- "package test;",
- "public class Foo {",
- " public static native int[] create_array() /*-{ return {}; }-*/;",
- " public static native int[][] create_2d_array() /*-{ return {}; }-*/;",
- "}");
- }
- });
+ addOrReplaceResource("test.Foo",
+ "package test;", "public class Foo {",
+ " public static native int[] create_array() /*-{ return {}; }-*/;",
+ " public static native int[][] create_2d_array() /*-{ return {}; }-*/;",
+ "}");
addSnippetImport("test.Foo");
analyzeSnippet("").assertOnlyInstantiatedTypes(Empty.STRINGS);
@@ -259,6 +193,133 @@
"int[][]", "Object[]", "Object");
}
+ public void testRescueJavaScriptObjectReturnedFromExternallyImplementedMethod() throws Exception {
+ addJsoResources();
+ addSnippetImport("test.Test");
+
+ addOrReplaceResource("test.Test",
+ "package test;",
+ "import jsinterop.annotations.JsMethod;",
+ "public class Test {",
+ " @JsMethod",
+ " public static native JsoIntf returnsJsoInterface();",
+ "}"
+ );
+ analyzeSnippet("Test.returnsJsoInterface();").assertOnlyInstantiatedTypes(
+ "JsoIntf", "SingleJso", "JavaScriptObject", "Object");
+
+ addOrReplaceResource("test.Test",
+ "package test;",
+ "import jsinterop.annotations.JsMethod;",
+ "public class Test {",
+ " @JsMethod",
+ " public static native SingleJso returnsJso();",
+ "}"
+ );
+ analyzeSnippet("Test.returnsJso();").assertOnlyInstantiatedTypes(
+ "JsoIntf", "SingleJso", "JavaScriptObject", "Object");
+ }
+
+ public void testRescueJavaScriptObjectParameterToJsInteropEntryPoint() throws Exception {
+ addJsoResources();
+ addSnippetImport("test.Test");
+
+ addOrReplaceResource("test.Test",
+ "package test;",
+ "import jsinterop.annotations.JsMethod;",
+ "public class Test {",
+ " public static void receivesJsoInterface(JsoIntf i) {};",
+ "}"
+ );
+ analyzeSnippet("Test.receivesJsoInterface(null);").assertOnlyInstantiatedTypes(
+ Empty.STRINGS);
+
+ addOrReplaceResource("test.Test",
+ "package test;",
+ "import jsinterop.annotations.JsMethod;",
+ "public class Test {",
+ " @JsMethod",
+ " public static void receivesJsoInterface(JsoIntf i) {};",
+ "}"
+ );
+ analyzeSnippet("Test.receivesJsoInterface(null);").assertOnlyInstantiatedTypes(
+ "JsoIntf", "SingleJso", "JavaScriptObject", "Object");
+ }
+
+ /**
+ * Tests that the JavaScriptObject type gets rescued in the three specific
+ * circumstances where values can pass from JS into Java.
+ */
+ public void testRescueJavaScriptObjectReturnedFromFieldReference() throws Exception {
+ addJsoResources();
+ addSnippetImport("test.Test");
+
+ addOrReplaceResource("test.Test",
+ "package test;",
+ "import jsinterop.annotations.JsType;",
+ "@JsType(isNative = true)",
+ "public class Test {",
+ " public static JsoIntf field;",
+ "}"
+ );
+ analyzeSnippet("Test.field.hashCode();").assertOnlyInstantiatedTypes(
+ "JsoIntf", "SingleJso", "JavaScriptObject", "Object",
+ // These are all live because of the method Test.toString can be referenced externally.
+ "String", "Comparable", "CharSequence", "Serializable");
+
+ addOrReplaceResource("test.Test",
+ "package test;",
+ "import jsinterop.annotations.JsType;",
+ "@JsType(isNative = true)",
+ "public class Test {",
+ " public static SingleJso field;",
+ "}"
+ );
+ analyzeSnippet("Test.field.hashCode();").assertOnlyInstantiatedTypes(
+ "JsoIntf", "SingleJso", "JavaScriptObject", "Object",
+ // These are all live because of the method Test.toString can be referenced externally.
+ "String", "Comparable", "CharSequence", "Serializable");
+ }
+
+ private void addOrReplaceResource(String qualifiedTypeName, final String... source) {
+ sourceOracle.addOrReplace(
+ new MockJavaResource(qualifiedTypeName) {
+ @Override
+ public CharSequence getContent() {
+ return Joiner.on("\n").join(source);
+ }
+ });
+ }
+
+ private void addJsoResources() {
+ addOrReplaceResource("test.JsoIntf",
+ "package test;",
+ "import com.google.gwt.core.client.JavaScriptObject;",
+ "public interface JsoIntf {",
+ " public int getAny();",
+ "}");
+ addOrReplaceResource("test.UpRefIntf",
+ "package test;",
+ "import com.google.gwt.core.client.JavaScriptObject;",
+ "public interface UpRefIntf {",
+ " public int getFoo();",
+ "}");
+ addOrReplaceResource("test.NonImplementor",
+ "package test;",
+ "import com.google.gwt.core.client.JavaScriptObject;",
+ "public class NonImplementor extends JavaScriptObject {",
+ " protected NonImplementor() {}",
+ " final public native int getFoo() /*-{ return 0; }-*/;",
+ "}");
+ addOrReplaceResource("test.SingleJso",
+ "package test;",
+ "import com.google.gwt.core.client.JavaScriptObject;",
+ "final public class SingleJso extends JavaScriptObject implements JsoIntf {",
+ " protected SingleJso() {}",
+ " public native int getAny() /*-{ return 1; }-*/;",
+ "}");
+ }
+
private Result analyzeSnippet(String codeSnippet)
throws UnableToCompleteException {
JProgram program = compileSnippet("void", codeSnippet, true);
@@ -266,4 +327,5 @@
cfa.traverseFrom(findMainMethod(program));
return new Result(program, cfa);
}
+
}
