Fix toString dispatch for subtypes of native JsTypes.

Change-Id: If5b8dbfd75c41e49c8dd0dd7b6ff36bbd143d73d

dev/core/src/com/google/gwt/dev/jjs/ast/RuntimeConstants.java

@@ -92,6 +92,7 @@ public class RuntimeConstants {
   public static final String RUNTIME_GET_CLASS_PROTOTYPE = "Runtime.getClassPrototype";
   public static final String RUNTIME_MAKE_LAMBDA_FUNCTION = "Runtime.makeLambdaFunction";
   public static final String RUNTIME_PROVIDE = "Runtime.provide";
+  public static final String RUNTIME_TO_STRING = "Runtime.toString";
   public static final String RUNTIME_TYPE_MARKER_FN = "Runtime.typeMarkerFn";
   public static final String RUNTIME_UNIQUE_ID = "Runtime.uniqueId";
 

dev/core/src/com/google/gwt/dev/jjs/impl/Devirtualizer.java

@@ -245,8 +245,12 @@ private void ensureDevirtualVersionExists(JMethod method) {
       // and optimizations need to be strong enough to perform the same kind of size reductions
       // achieved by keeping track of singleImpls.
 
-      //
-      if (!program.typeOracle.isDualJsoInterface(targetType) &&
+      if (method.getSignature().equals("toString()Ljava/lang/String;")) {
+        // Object.toString is special because: 1) every JS object has it and 2) GWT creates
+        // a bridge from toString to its implementation method.
+        devirtualMethodByMethod.put(
+            method, program.getIndexedMethod(RuntimeConstants.RUNTIME_TO_STRING));
+      } else if (!program.typeOracle.isDualJsoInterface(targetType) &&
           program.typeOracle.isSingleJsoImpl(targetType)) {
         // Optimize the trampoline away when there is ONLY JSO dispatch.
         // TODO(rluble): verify that this case can not arise in optimized mode and if so

dev/core/src/com/google/gwt/dev/jjs/impl/MethodCallTightener.java

@@ -72,33 +72,57 @@ public void endVisit(JMethodCall x, Context ctx) {
         return;
       }
 
-      JMethodCall newCall = maybeUpgradeToNonPolymorphicCall(x);
-
-      // If the call is still polymorphic, try tightening the method.
-      if (newCall.canBePolymorphic()) {
-        newCall = maybeTightenMethodCall(newCall);
+      JMethod mostSpecificTarget = getMostSpecificOverride(x);
+      if (mostSpecificTarget.getEnclosingType().isJsNative()) {
+        // Never tighten to instance methods in native types. This done because java.lang.Object
+        // methods are implicitly implemented by all objects but may or may not be present in the
+        // native type implementation. The dispatch for these is eventually done through a
+        // trampoline {@see Devirtualizer} that makes the proper checks and invokes the native
+        // implementation if present.
+        assert x.getTarget().getEnclosingType().isJavaLangObject()
+            || x.getTarget().getEnclosingType().isJsNative();
+        return;
       }
 
+      // Tighten the method call if a more specific override is available.
+      JMethodCall newCall = maybeReplaceTargetMethod(x, mostSpecificTarget);
+      maybeUpgradeToNonPolymorphicCall(newCall);
+
       if (newCall != x) {
         ctx.replaceMe(newCall);
       }
     }
 
-    private JMethodCall maybeUpgradeToNonPolymorphicCall(JMethodCall x) {
+    private JMethod getMostSpecificOverride(final JMethodCall methodCall) {
+      JMethod original = methodCall.getTarget();
+      JClassType underlyingType =
+          (JClassType) methodCall.getInstance().getType().getUnderlyingType();
+
+      return program.typeOracle.findMostSpecificOverride(underlyingType, original);
+    }
+
+    private JMethodCall maybeReplaceTargetMethod(JMethodCall methodCall, JMethod newTargetMethod) {
+      if (methodCall.getTarget() == newTargetMethod) {
+        return methodCall;
+      }
+      return new JMethodCall(
+          methodCall.getSourceInfo(),
+          methodCall.getInstance(),
+          newTargetMethod,
+          methodCall.getArgs());
+    }
+
+    private void maybeUpgradeToNonPolymorphicCall(JMethodCall x) {
       JReferenceType instanceType = (JReferenceType) x.getInstance().getType();
 
-      if (!instanceType.canBeSubclass()) {
-        // Mark a call as non-polymorphic if the targeted type is guaranteed to be not a subclass.
-        x = maybeTightenMethodCall(x);
-        x.setCannotBePolymorphic();
-        madeChanges();
-      } else if (!hasPotentialOverride(instanceType, x.getTarget())) {
-        // Mark a call as non-polymorphic if the targeted method is the only possible dispatch.
+      if (!instanceType.canBeSubclass() || !hasPotentialOverride(instanceType, x.getTarget())) {
+        assert getMostSpecificOverride(x) == x.getTarget();
+
+        // Mark a call as non-polymorphic if the targeted type is guaranteed to be not a subclass
+        // or there are no overriding implementations.
         x.setCannotBePolymorphic();
         madeChanges();
        }
-
-      return x;
     }
 
     private boolean hasPotentialOverride(JReferenceType instanceType, JMethod target) {
@@ -117,27 +141,6 @@ private boolean hasPotentialOverride(JReferenceType instanceType, JMethod target
       return false;
     }
 
-    private JMethodCall maybeTightenMethodCall(final JMethodCall methodCall) {
-      JMethod original = methodCall.getTarget();
-      JClassType underlyingType =
-          (JClassType) methodCall.getInstance().getType().getUnderlyingType();
-
-      JMethod mostSpecificOverride =
-          program.typeOracle.findMostSpecificOverride(underlyingType, original);
-
-      if (mostSpecificOverride == original
-          // Never tighten object methods to native implementations. This decision forces
-          // the use of the Object trampoline for hashcCode, equals and toString.
-          || (original.getEnclosingType().isJavaLangObject()
-              && mostSpecificOverride.isJsNative())) {
-        return methodCall;
-      }
-      JMethodCall newCall = new JMethodCall(
-          methodCall.getSourceInfo(), methodCall.getInstance(), mostSpecificOverride);
-      newCall.addArgs(methodCall.getArgs());
-      return newCall;
-    }
-
     @Override
     public void endVisit(JNewInstance x, Context ctx) {
       // Do not tighten new operations.

dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Runtime.java

@@ -212,4 +212,11 @@ static native void defineProperties(
     }
     Object.defineProperties(proto,  propertyDefinition);
   }-*/;
+
+  public static native String toString(Object object) /*-{
+    if (Array.isArray(object) && @Util::hasTypeMarker(*)(object)) {
+       return @Object::toString(Ljava/lang/Object;)(object);
+    }
+    return object.toString();
+  }-*/;
 }

dev/core/test/com/google/gwt/dev/javac/typemodel/JParameterizedTypeTest.java

@@ -433,7 +433,8 @@ private void validateInheritableOrOverridableMethods(List<JMethod> expected,
     expectedMethods.addAll(expected);
     if (addObjectMethods) {
       TypeOracle oracle = moduleContext.getOracle();
-      expectedMethods.addAll(Arrays.asList(oracle.getJavaLangObject().getMethods()));
+      expectedMethods.addAll(
+          Arrays.asList(oracle.getJavaLangObject().getOverridableMethods()));
     }
 
     for (JMethod method : actual) {

dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java

@@ -292,6 +292,7 @@ public CharSequence getContent() {
                   "  public static void bootstrap() {}",
                   "  public static void emptyMethod() {}",
                   "  public static void getClassPrototype() {}",
+                  "  public static String toString(Object object) { return null; }",
                   "  static native void typeMarkerFn() /*-{}-*/;",
                   "}");
         }

dev/core/test/com/google/gwt/dev/jjs/impl/DevirtualizerTest.java

@@ -151,8 +151,78 @@ public void testDevirtualizeJsOverlay() throws UnableToCompleteException {
     result.intoString(expected);
   }
 
+  public void testDevirtualizeObjectMethods() throws UnableToCompleteException {
+    addSnippetImport("jsinterop.annotations.JsType");
+    addSnippetImport("jsinterop.annotations.JsOverlay");
+    addSnippetClassDecl(
+        "@JsType(isNative=true) public static class NativeClass {",
+        "}",
+        "public static class NativeClassSubclass extends NativeClass {",
+        "}");
+
+    String code = Joiner.on('\n').join(
+        "NativeClass nativeClass = null;",
+        "nativeClass.toString();",
+        "nativeClass.equals(nativeClass);",
+        "nativeClass.hashCode();",
+        "NativeClassSubclass subclass = null;",
+        "subclass.toString();",
+        "subclass.equals(subclass);",
+        "subclass.hashCode();"
+        );
+
+    String expected = Joiner.on('\n').join(
+        "EntryPoint$NativeClass nativeClass = null;",
+        "Runtime.toString(nativeClass);",
+        "Object.equals_Ljava_lang_Object__Z__devirtual$(nativeClass, nativeClass);",
+        "Object.hashCode__I__devirtual$(nativeClass);",
+        "EntryPoint$NativeClassSubclass subclass = null;",
+        "Runtime.toString(subclass);",
+        "Object.equals_Ljava_lang_Object__Z__devirtual$(subclass, subclass);",
+        "Object.hashCode__I__devirtual$(subclass);");
+    Result result = optimize("void", code);
+    result.intoString(expected);
+  }
+
+  public void testDevirtualizeObjectMethodsExplicitelyDefined() throws UnableToCompleteException {
+    addSnippetImport("jsinterop.annotations.JsType");
+    addSnippetImport("jsinterop.annotations.JsOverlay");
+    addSnippetClassDecl(
+        "@JsType(isNative=true) public static class NativeClass {",
+        "  public native String toString();",
+        "  public native int hashCode();",
+        "  public native boolean equals(Object o);",
+        "}",
+        "public static class NativeClassSubclass extends NativeClass {",
+        "}");
+
+    String code = Joiner.on('\n').join(
+        "NativeClass nativeClass = null;",
+        "nativeClass.toString();",
+        "nativeClass.equals(nativeClass);",
+        "nativeClass.hashCode();",
+        "NativeClassSubclass subclass = null;",
+        "subclass.toString();",
+        "subclass.equals(subclass);",
+        "subclass.hashCode();"
+    );
+
+    String expected = Joiner.on('\n').join(
+        "EntryPoint$NativeClass nativeClass = null;",
+        "Runtime.toString(nativeClass);",
+        "Object.equals_Ljava_lang_Object__Z__devirtual$(nativeClass, nativeClass);",
+        "Object.hashCode__I__devirtual$(nativeClass);",
+        "EntryPoint$NativeClassSubclass subclass = null;",
+        "Runtime.toString(subclass);",
+        "Object.equals_Ljava_lang_Object__Z__devirtual$(subclass, subclass);",
+        "Object.hashCode__I__devirtual$(subclass);");
+    Result result = optimize("void", code);
+    result.intoString(expected);
+  }
+
   @Override
   protected boolean doOptimizeMethod(TreeLogger logger, JProgram program, JMethod method) {
+    ReplaceCallsToNativeJavaLangObjectOverrides.exec(program);
     Devirtualizer.exec(program);
     return true;
   }

user/super/com/google/gwt/emul/java/lang/Object.java

@@ -77,9 +77,12 @@ public int hashCode() {
   }
 
   public String toString() {
-    return getClass().getName() + '@' + Integer.toHexString(hashCode());
+    return toString(this);
   }
 
+  private static String toString(Object object) {
+    return object.getClass().getName() + '@' + Integer.toHexString(object.hashCode());
+  }
   /**
    * Never called; here for JRE compatibility.
    *

user/test/com/google/gwt/core/interop/NativeJsTypeTest.java

@@ -471,9 +471,15 @@ public void testMainWindowIsNotIFrameWindow() {
 
   @JsType(isNative = true, namespace = JsPackage.GLOBAL, name = "Error")
   private static class NativeError {
+    public String message;
   }
 
+  private static final String ERROR_FROM_NATIVE_ERROR_SUBCLASS = "error from NativeErrorSubclass";
+
   private static class NativeErrorSubclass extends NativeError {
+    public NativeErrorSubclass() {
+      message = ERROR_FROM_NATIVE_ERROR_SUBCLASS;
+    }
   }
 
   public void testObjectPropertiesAreCopied() {
@@ -482,8 +488,6 @@ public void testObjectPropertiesAreCopied() {
     // Make sure the subclass is a proper Java object (the typeMarker should be one of the
     // properties copied from java.lang.Object).
     assertFalse(error instanceof JavaScriptObject);
-    // TODO(rluble): NativeErrorSubclass should have inherited Error toString behavior not
-    // j.l.Object.toString behavior.
-    // assertTrue(error.toString().matches("[0-9a-zA-Z$_.]+@[0-9a-fA-F]+"));
+    assertTrue(error.toString().contains(ERROR_FROM_NATIVE_ERROR_SUBCLASS));
   }
 }