Fixes handling of non-public JsMethods. Change-Id: Ieb58280af7e076820e23ccae0774109562a351e0

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
index bf2d791..fc83adf 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java

@@ -360,17 +360,10 @@
       String name = x.getName();
       if (x.needsDynamicDispatch()) {
         if (polymorphicNames.get(x) == null) {
-          JsName polyName;
-          if (x.isPrivate()) {
-            polyName = interfaceScope.declareName(mangleNameForPrivatePoly(x), name);
-          } else if (x.isPackagePrivate()) {
-            polyName = interfaceScope.declareName(mangleNameForPackagePrivatePoly(x), name);
-          } else {
-            polyName =
-                x.getJsMemberType() != JsMemberType.NONE
-                    ? interfaceScope.declareUnobfuscatableName(x.getJsName())
-                    : interfaceScope.declareName(mangleNameForPoly(x), name);
-          }
+          JsName polyName =
+              x.getJsMemberType() != JsMemberType.NONE
+                  ? interfaceScope.declareUnobfuscatableName(x.getJsName())
+                  : interfaceScope.declareName(mangleNameForPoly(x), name);
           polymorphicNames.put(x, polyName);
         }
       }
@@ -2950,8 +2943,16 @@
   }
 
   private String mangleNameForPoly(JMethod method) {
-    assert !method.isPrivate() && !method.isStatic();
+    if (method.isPrivate()) {
+      return mangleNameForPrivatePoly(method);
+    } else if (method.isPackagePrivate()) {
+      return mangleNameForPackagePrivatePoly(method);
+    } else {
+      return mangleNameForPublicPoly(method);
+    }
+  }
 
+  private String mangleNameForPublicPoly(JMethod method) {
     return StringInterner.get().intern(
         JjsUtils.constructManglingSignature(method, JjsUtils.mangledNameString(method)));
   }

diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
index 8f8f4f3..d54b64c 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java

@@ -770,8 +770,16 @@
     // Methods that only differ in return types are Java overrides and need to be considered so
     // for local name collision checking.
     JMethod potentiallyOverriddenMethod = (JMethod) potentiallyOverriddenMember;
-    return method.getJsniSignature(false, false)
-        .equals(potentiallyOverriddenMethod.getJsniSignature(false, false));
+
+    // TODO(goktug): make this more precise to handle package visibilities.
+    boolean visibilitiesMatchesForOverride =
+        !method.isPackagePrivate() && !method.isPrivate()
+        && !potentiallyOverriddenMethod.isPackagePrivate()
+        && !potentiallyOverriddenMethod.isPrivate();
+
+    return visibilitiesMatchesForOverride
+        && method.getJsniSignature(false, false)
+               .equals(potentiallyOverriddenMethod.getJsniSignature(false, false));
   }
 
   private static String getDescription(HasSourceInfo hasSourceInfo) {

diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
index 719d98a..63b6205 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java

@@ -645,6 +645,21 @@
             + "both use the same JavaScript name 'foo'.");
   }
 
+  public void testShadowedSuperclassJsMethodFails() {
+    addSnippetImport("jsinterop.annotations.JsMethod");
+    addSnippetClassDecl(
+        "public static class ParentBuggy {",
+        "  @JsMethod private void foo() {}",
+        "}",
+        "public static class Buggy extends ParentBuggy {",
+        "  @JsMethod private void foo() {}",
+        "}");
+
+    assertBuggyFails(
+        "Line 8: 'void EntryPoint.Buggy.foo()' and 'void EntryPoint.ParentBuggy.foo()' cannot "
+            + "both use the same JavaScript name 'foo'.");
+  }
+
   public void testRenamedSuperclassJsMethodFails() {
     addSnippetImport("jsinterop.annotations.JsType");
     addSnippetImport("jsinterop.annotations.JsMethod");

diff --git a/user/test/com/google/gwt/core/client/interop/JsTypeTest.java b/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
index 242c9a3..9628a1c 100644
--- a/user/test/com/google/gwt/core/client/interop/JsTypeTest.java
+++ b/user/test/com/google/gwt/core/client/interop/JsTypeTest.java

@@ -25,6 +25,7 @@
 
 import javaemul.internal.annotations.DoNotInline;
 import jsinterop.annotations.JsFunction;
+import jsinterop.annotations.JsMethod;
 import jsinterop.annotations.JsOverlay;
 import jsinterop.annotations.JsPackage;
 import jsinterop.annotations.JsProperty;
@@ -539,4 +540,17 @@
     SomeConcreteSubclass o = new SomeConcreteSubclass();
     assertEquals(o, o.m());
   }
+
+  static class NonPublicJsMethodClass {
+    @JsMethod private String foo() { return "foo"; }
+    @JsMethod String bar() { return "bar"; }
+  }
+
+  public void testJsMethodWithDifferentVisiblities() {
+    NonPublicJsMethodClass instance = new NonPublicJsMethodClass();
+    assertEquals("foo", instance.foo());
+    assertEquals("bar", instance.bar());
+    assertEquals("foo", callFunction(instance, "foo", null));
+    assertEquals("bar", callFunction(instance, "bar", null));
+  }
 }