Deprecate and disable CollectorLegacy, simplify startup (#10185)

CollectorLegacy shouldn't be needed in any modern browser, though the
Error.stack property is still non-standard. This patch leaves it in, so
that it is at least possible for projects to restore it if they need it,
but won't add an extra step to startup for most GWT applications.

With only a single implementation, the static final field isn't needed
and likewise doesn't need to be eagerly initialized. This will remove
clinits for both Impl and StackTraceCreator everywhere they are used,
and by default should still compile out the collector instance into just
a set of static functions.

Author: niloc132

user/src/com/google/gwt/core/client/impl/Impl.java

@@ -28,12 +28,6 @@
  */
 public final class Impl {
 
-  static {
-    if (GWT.isScript() && StackTraceCreator.collector != null) {
-      // Just enforces loading of StackTraceCreator early on, nothing else to do here...
-    }
-  }
-
   private static final int WATCHDOG_ENTRY_DEPTH_CHECK_INTERVAL_MS = 2000;
 
   /**

user/src/com/google/gwt/core/client/impl/StackTraceCreator.java

@@ -65,6 +65,7 @@ abstract static class Collector {
    * This legacy {@link Collector} simply crawls <code>arguments.callee.caller</code> for browsers
    * that doesn't support {@code Error.stack} property.
    */
+  @Deprecated
   static class CollectorLegacy extends Collector {
 
     @Override
@@ -306,11 +307,11 @@ public StackTraceElement[] getStackTrace(Object ignored) {
    * Collect necessary information to construct stack trace trace later in time.
    */
   public static void captureStackTrace(Object error) {
-    collector.collect(error);
+    getCollector().collect(error);
   }
 
   public static StackTraceElement[] constructJavaStackTrace(Throwable thrown) {
-    StackTraceElement[] stackTrace = collector.getStackTrace(thrown);
+    StackTraceElement[] stackTrace = getCollector().getStackTrace(thrown);
     return dropInternalFrames(stackTrace);
   }
 
@@ -338,26 +339,10 @@ private static <T> void splice(Object[] arr, int length) {
     }
   }
 
-  // Visible for testing
-  static final Collector collector;
-
-  static {
-    // Ensure old Safari falls back to legacy Collector implementation.
-    boolean enforceLegacy = !supportsErrorStack();
-    Collector c = GWT.create(Collector.class);
-    collector = (c instanceof CollectorModern && enforceLegacy) ? new CollectorLegacy() : c;
+  static Collector getCollector() {
+    return GWT.create(Collector.class);
   }
 
-  private static native boolean supportsErrorStack() /*-{
-    // Error.stackTraceLimit is cheaper to check and available in both IE and Chrome
-    if (Error.stackTraceLimit > 0) {
-      $wnd.Error.stackTraceLimit = Error.stackTraceLimit = 64;
-      return true;
-    }
-
-    return "stack" in new Error();
-  }-*/;
-
   private static native JsArrayString getFnStack(Object e) /*-{
     return (e && e["fnStack"]) ? e["fnStack"] : [];
   }-*/;

user/test/com/google/gwt/core/client/impl/StackTraceEmulTest.java

@@ -37,7 +37,7 @@ protected String[] getTraceJse(Object thrown) {
 
   @Override
   public void testCollectorType() {
-    assertTrue(StackTraceCreator.collector instanceof StackTraceCreator.CollectorEmulated);
+    assertTrue(StackTraceCreator.getCollector() instanceof StackTraceCreator.CollectorEmulated);
   }
 
   /**

user/test/com/google/gwt/core/client/impl/StackTraceNativeTest.java

@@ -17,7 +17,6 @@
 
 import static com.google.gwt.core.client.impl.StackTraceExamples.TYPE_ERROR;
 
-import com.google.gwt.core.client.impl.StackTraceCreator.CollectorLegacy;
 import com.google.gwt.core.client.impl.StackTraceCreator.CollectorModern;
 import com.google.gwt.junit.DoNotRunWith;
 import com.google.gwt.junit.Platform;
@@ -49,7 +48,7 @@ protected String[] getTraceJava() {
   @Override
   protected String[] getTraceRecursion() {
     // First two frames are optional as they are automatically stripped by EDGE.
-    final String[] expectedModern = {
+    return new String[]{
         "?" + Impl.getNameOf("@java.lang.Throwable::new(Ljava/lang/String;)"),
         "?" + Impl.getNameOf("@java.lang.Exception::new(Ljava/lang/String;)"),
         Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceExamples::throwException2(*)"),
@@ -62,16 +61,6 @@ protected String[] getTraceRecursion() {
         Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceExamples::getLiveException(*)"),
         Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceTestBase::testTraceRecursion()"),
     };
-
-    final String[] expectedLegacy = {
-        Impl.getNameOf("@java.lang.Throwable::new(Ljava/lang/String;)"),
-        Impl.getNameOf("@java.lang.Exception::new(Ljava/lang/String;)"),
-        Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceExamples::throwException2(*)"),
-        Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceExamples::throwException1(*)"),
-        Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceExamples::throwRecursive(*)"),
-    };
-
-    return isLegacyCollector() ? expectedLegacy : expectedModern;
   }
 
   @Override
@@ -87,12 +76,6 @@ protected String[] getTraceJse(Object thrown) {
         Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceTestBase::assertJse(*)"),
     };
 
-    final String[] limited_wrap = {
-        Impl.getNameOf("@com.google.gwt.lang.Exceptions::toJava(*)"),
-        Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceExamples::getLiveException(*)"),
-        Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceTestBase::assertJse(*)"),
-    };
-
     final String[] limited_fillInStackTrace = {
         Impl.getNameOf("@java.lang.Throwable::fillInStackTrace()"),
         Impl.getNameOf("@com.google.gwt.core.client.impl.StackTraceExamples::getLiveException(*)"),
@@ -101,28 +84,14 @@ protected String[] getTraceJse(Object thrown) {
 
     // For legacy browsers and non-error javascript exceptions (e.g. throw "string"), we can only
     // construct stack trace from the catch block and below.
-    return isLegacyCollector()
-        ? limited_wrap : (thrown != TYPE_ERROR ? limited_fillInStackTrace : full);
+    return thrown != TYPE_ERROR ? limited_fillInStackTrace : full;
   }
 
   public void testCollectorType() {
-    if (isSafari5()) {
-      assertTrue(isLegacyCollector());
-    } else {
-      assertTrue(isModernCollector());
-    }
-  }
-
-  private static boolean isLegacyCollector() {
-    return StackTraceCreator.collector instanceof CollectorLegacy;
+    assertTrue(isModernCollector());
   }
 
   private static boolean isModernCollector() {
-    return StackTraceCreator.collector instanceof CollectorModern;
+    return StackTraceCreator.getCollector() instanceof CollectorModern;
   }
-
-  private static native boolean isSafari5() /*-{
-    return navigator.userAgent.match(' Safari/') && !navigator.userAgent.match(' Chrom')
-        && !!navigator.userAgent.match(' Version/5.');
-  }-*/;
 }