Allow JsInliner to inline functions defined in JSNI blocks.

Change-Id: I4ec8e1d2d9dce39f720c5232f26c71af9dc87346
gwtproject · Sep 10, 2015 · 03f973a · 03f973a
1 parent daa2249
commit 03f973a
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 15 deletions.
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
@@ -3117,6 +3117,14 @@ public void endVisit(JMethod x, Context ctx) {
         if (function != null && function.getBody() != null) {
           functionsForJsInlining.add(function);
         }
+        // Add all functions declared inside JSNI blocks as well.
+        assert function != null;
+        new JsModVisitor() {
+          @Override
+          public void endVisit(JsFunction x, JsContext ctx) {
+            functionsForJsInlining.add(x);
+          }
+        }.accept(function);
       }
 
       currentMethod = null;

diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAstTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAstTest.java
@@ -49,34 +49,54 @@ public void setUp() throws Exception {
   }
 
   public void testInlineJSNIMethod() throws UnableToCompleteException {
-    StringBuilder code = new StringBuilder();
+    String code = Joiner.on('\n').join(
-    code.append("package test;\n");
+        "package test;",
-    code.append("import com.google.gwt.core.client.GWT;\n");
+        "import com.google.gwt.core.client.GWT;",
-    code.append("import com.google.gwt.core.client.RunAsyncCallback;\n");
+        "import com.google.gwt.core.client.RunAsyncCallback;",
-    code.append("public class EntryPoint {\n");
+        "public class EntryPoint {",
-    code.append("  public static native void inlinableJSNI() /*-{ $wnd; }-*/; ");
+        "  public static native void inlinableJSNI() /*-{ $wnd; }-*/; ",
-    code.append("  public static void functionA() { onModuleLoad(); }\n");
+        "  public static void functionA() { onModuleLoad(); }",
-    code.append("  public static void onModuleLoad() {\n");
+        "  public static void onModuleLoad() {",
-    code.append("    inlinableJSNI();");
+        "    inlinableJSNI();",
-    code.append("  }\n");
+        "  }",
-    code.append("}\n");
+        "}");
 
-    Set<JsNode> functionForJsInlining = compileSnippetToJS(code.toString()).getRight();
+    Set<JsNode> functionForJsInlining = compileSnippetToJS(code).getRight();
     assertContainsAll(functionForJsInlining, "onModuleLoad", "inlinableJSNI");
     assertDoesNotContainsAny(functionForJsInlining, "functionA");
   }
 
+  public void testInlineFunctionDefinedInJSNI() throws UnableToCompleteException {
+    String code = Joiner.on('\n').join(
+        "package test;",
+        "import com.google.gwt.core.client.GWT;",
+        "import com.google.gwt.core.client.RunAsyncCallback;",
+        "public class EntryPoint {",
+        "  public static native void inlinableJSNI() /*-{ (function () { return $wnd;})(); }-*/;",
+        "  public static void functionA() { onModuleLoad(); }",
+        "  public static void onModuleLoad() {",
+        "    inlinableJSNI();",
+        "  }",
+        "}");
+
+    Set<JsNode> functionForJsInlining = compileSnippetToJS(code.toString()).getRight();
+    assertContainsAll(functionForJsInlining, "onModuleLoad", "inlinableJSNI",
+        "function(){ return $wnd; }");
+    assertDoesNotContainsAny(functionForJsInlining, "functionA");
+  }
+
   private void assertContainsAll(Set<JsNode> functionsForJsInlining,
-      String... functionNames) {
+      String... functionNamesorContents) {
-    Set<String> remainingFunctions = Sets.newHashSet(functionNames);
+    Set<String> remainingFunctions = Sets.newHashSet(functionNamesorContents);
     for (JsFunction function : Iterables.filter(functionsForJsInlining, JsFunction.class)) {
       JsName name = function.getName();
       if (name == null) {
+        remainingFunctions.remove(function.toString().replaceAll("\\s+"," ").trim());
         continue;
       }
       remainingFunctions.remove(name.getShortIdent());
     }
-    assertTrue("{" + (Joiner.on(",").join(remainingFunctions)) + "} marked for consideration in "
+    assertTrue("{" + (Joiner.on(",").join(remainingFunctions)) + "} not marked for consideration in "
         + "JsInliner", remainingFunctions.isEmpty());
   }