Goktug Gokdogan has uploaded a new change for review.

  https://gwt-review.googlesource.com/3640


Change subject: Adds support for unwrapping of thrown non-jso objects from JavaScriptException.
......................................................................

Adds support for unwrapping of thrown non-jso objects from JavaScriptException.

This is the second part of exception wrap/unwrap fix which adds support to unwrapping
of non-jso objects thrown by native code.

Change-Id: I09c92ac03abbdb930085e2aaf97001b4d495abd9
---
M dev/core/src/com/google/gwt/dev/shell/MethodDispatch.java
M dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
M dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
M user/src/com/google/gwt/core/client/JavaScriptException.java
M user/src/com/google/gwt/core/client/impl/StackTraceCreator.java
M user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
6 files changed, 96 insertions(+), 76 deletions(-)



diff --git a/dev/core/src/com/google/gwt/dev/shell/MethodDispatch.java b/dev/core/src/com/google/gwt/dev/shell/MethodDispatch.java
index 8f1c062..fb5b3a8 100644
--- a/dev/core/src/com/google/gwt/dev/shell/MethodDispatch.java
+++ b/dev/core/src/com/google/gwt/dev/shell/MethodDispatch.java
@@ -109,21 +109,14 @@
   /**
    * Send an exception back to the client. This will either wrap a Java
* Throwable as a Java Object to be sent over the wire, or if the exception is
-   * a JavaScriptObject unwinding through the stack, send the JSO instead.
+ * a JavaScriptException unwinding through the stack, send the original thrown
+   * object instead.
    */
   private void wrapException(JsValue returnValue, Throwable t) {
     ModuleSpace.setThrownJavaException(t);

-    // See if we're in the process of throwing a JavaScriptObject; remove
- // it from the JavaScriptException object and throw the JS object instead
-    Object jsoException = ModuleSpace.getJavaScriptExceptionException(
-        classLoader, t);
-
-    if (jsoException != null) {
-      JsValueGlue.set(returnValue, classLoader, jsoException.getClass(),
-          jsoException);
-    } else {
-      JsValueGlue.set(returnValue, classLoader, t.getClass(), t);
-    }
+    Object thrown = ModuleSpace.getThrownObject(classLoader, t);
+    Class<?> type = thrown == null ? Object.class : thrown.getClass();
+    JsValueGlue.set(returnValue, classLoader, type, thrown);
   }
 }
diff --git a/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java b/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
index 4798106..6094f17 100644
--- a/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
+++ b/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
@@ -101,15 +101,16 @@
   }

   /**
- * Get the JavaScriptObject wrapped by a JavaScriptException. We have to do + * Get the original thrown object. If the exception is JavaScriptException,
+   * gets the object wrapped by a JavaScriptException. We have to do
    * this reflectively, since the JavaScriptException object is from an
* arbitrary classloader. If the object is not a JavaScriptException, or is
-   * not from the given ClassLoader, we'll return null.
+ * not from the given ClassLoader, or the exception is not set we'll return
+   * exception itself.
    */
-  static Object getJavaScriptExceptionException(ClassLoader cl,
-      Object javaScriptException) {
-    if (javaScriptException.getClass().getClassLoader() != cl) {
-      return null;
+  static Object getThrownObject(ClassLoader cl, Object exception) {
+    if (exception.getClass().getClassLoader() != cl) {
+      return exception;
     }

     Exception caught;
@@ -117,12 +118,16 @@
       Class<?> javaScriptExceptionClass = Class.forName(
           "com.google.gwt.core.client.JavaScriptException", true, cl);

-      if (!javaScriptExceptionClass.isInstance(javaScriptException)) {
+      if (!javaScriptExceptionClass.isInstance(exception)) {
         // Not a JavaScriptException
-        return null;
+        return exception;
       }
- Method getException = javaScriptExceptionClass.getMethod("getException");
-      return getException.invoke(javaScriptException);
+ Method isThrownSet = javaScriptExceptionClass.getMethod("isThrownSet");
+      if (!((Boolean) isThrownSet.invoke(exception))) {
+        return exception;
+      }
+      Method getThrown = javaScriptExceptionClass.getMethod("getThrown");
+      return getThrown.invoke(exception);
     } catch (NoSuchMethodException e) {
       caught = e;
     } catch (ClassNotFoundException e) {
diff --git a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
index 4e075f5..2a2301b 100644
--- a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java +++ b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
@@ -16,7 +16,6 @@
 package com.google.gwt.lang;

 import com.google.gwt.core.client.JavaScriptException;
-import com.google.gwt.core.client.JavaScriptObject;

 /**
  * This is a magic class the compiler uses to throw and check exceptions.
@@ -32,9 +31,9 @@

   static Object unwrap(Object e) {
     if (e instanceof JavaScriptException) {
- JavaScriptObject exception = ((JavaScriptException) e).getException();
-      if (exception != null) {
-        return exception;
+      JavaScriptException jse = ((JavaScriptException) e);
+      if (jse.isThrownSet()) {
+        return jse.getThrown();
       }
     }
     return e;
diff --git a/user/src/com/google/gwt/core/client/JavaScriptException.java b/user/src/com/google/gwt/core/client/JavaScriptException.java
index 8f15edd..85d90f0 100644
--- a/user/src/com/google/gwt/core/client/JavaScriptException.java
+++ b/user/src/com/google/gwt/core/client/JavaScriptException.java
@@ -45,6 +45,8 @@
  */
 public final class JavaScriptException extends RuntimeException {

+  private static final Object NOT_SET = new Object();
+
   private static String getExceptionDescription(Object e) {
     if (e instanceof JavaScriptObject) {
       return getExceptionDescription0((JavaScriptObject) e);
@@ -133,7 +135,7 @@
     this.message = "JavaScript " + name + " exception: " + description;
     this.name = name;
     this.description = description;
-    this.e = null;
+    this.e = NOT_SET;
   }

   /**
@@ -144,7 +146,21 @@
   protected JavaScriptException(String message) {
     super(message);
     this.message = this.description = message;
-    this.e = null;
+    this.e = NOT_SET;
+  }
+
+  /**
+   * Returns {@code true} if a thrown object is not set for the exception.
+   */
+  public boolean isThrownSet() {
+    return e != NOT_SET;
+  }
+
+  /**
+ * Returns the original thrown object from javascript; may be {@code null}.
+   */
+  public Object getThrown() {
+    return e == NOT_SET ? null : e;
   }

   /**
@@ -152,24 +168,23 @@
    * <code>null</code>.
    */
   public String getDescription() {
-    if (message == null) {
-      init();
-    }
+    ensureInit();
     return description;
   }

   /**
* Returns the original JavaScript the exception; may be <code>null</code>.
+   *
+ * @deprecated deprecated in favor for {@link #getThrown()} and {@link #isThrownSet()}
    */
+  @Deprecated
   public JavaScriptObject getException() {
     return (e instanceof JavaScriptObject) ? (JavaScriptObject) e : null;
   }

   @Override
   public String getMessage() {
-    if (message == null) {
-      init();
-    }
+    ensureInit();
     return message;
   }

@@ -178,16 +193,17 @@
    * <code>null</code>.
    */
   public String getName() {
-    if (message == null) {
-      init();
-    }
+    ensureInit();
     return name;
   }

-  private void init() {
-    name = getExceptionName(e);
-    description = description + ": " + getExceptionDescription(e);
-    message = "(" + name + ") " + getExceptionProperties(e) + description;
+  private void ensureInit() {
+    if (message == null) {
+      Object exception = getThrown();
+      name = getExceptionName(exception);
+ description = description + ": " + getExceptionDescription(exception); + message = "(" + name + ") " + getExceptionProperties(exception) + description;
+    }
   }

 }
diff --git a/user/src/com/google/gwt/core/client/impl/StackTraceCreator.java b/user/src/com/google/gwt/core/client/impl/StackTraceCreator.java
index 8d804b1..58bb0f4 100644
--- a/user/src/com/google/gwt/core/client/impl/StackTraceCreator.java
+++ b/user/src/com/google/gwt/core/client/impl/StackTraceCreator.java
@@ -74,7 +74,7 @@
     }-*/;

     public void createStackTrace(JavaScriptException e) {
-      JsArrayString stack = inferFrom(e.getException());
+      JsArrayString stack = inferFrom(e.getThrown());

StackTraceElement[] stackTrace = new StackTraceElement[stack.length()];
       for (int i = 0, j = stackTrace.length; i < j; i++) {
@@ -121,7 +121,7 @@
      *
      * @param e a JavaScriptObject
      */
-    public JsArrayString inferFrom(JavaScriptObject e) {
+    public JsArrayString inferFrom(Object e) {
       return JavaScriptObject.createArray().cast();
     }

@@ -206,7 +206,7 @@
     }

     @Override
-    public JsArrayString inferFrom(JavaScriptObject e) {
+    public JsArrayString inferFrom(Object e) {
       throw new RuntimeException("Should not reach here");
     }

@@ -237,8 +237,9 @@
     }

     @Override
-    public JsArrayString inferFrom(JavaScriptObject e) {
-      JsArrayString stack = getStack(e);
+    public JsArrayString inferFrom(Object e) {
+ JavaScriptObject jso = (e instanceof JavaScriptObject) ? (JavaScriptObject) e : null;
+      JsArrayString stack = getStack(jso);
       for (int i = 0, j = stack.length(); i < j; i++) {
         stack.set(i, extractName(stack.get(i)));
       }
@@ -296,7 +297,7 @@

     @Override
     public void createStackTrace(JavaScriptException e) {
-      JsArrayString stack = inferFrom(e.getException());
+      JsArrayString stack = inferFrom(e.getThrown());
       parseStackTrace(e, stack);
     }

@@ -307,7 +308,7 @@
     }

     @Override
-    public JsArrayString inferFrom(JavaScriptObject e) {
+    public JsArrayString inferFrom(Object e) {
       JsArrayString stack = super.inferFrom(e);
       if (stack.length() == 0) {
         // Safari should fall back to default Collector:
diff --git a/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java b/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
index ef3acdf..af1272d 100644
--- a/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
+++ b/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
@@ -98,7 +98,7 @@

private static void assertJavaScriptException(Object expected, Throwable exception) {
     assertTrue(exception instanceof JavaScriptException);
- assertEquals(expected, ((JavaScriptException) exception).getException());
+    assertEquals(expected, ((JavaScriptException) exception).getThrown());
   }

private static void assertJsoProperties(boolean extraPropertiesShouldBePresent) {
@@ -109,7 +109,8 @@
     } catch (JavaScriptException e) {
       assertEquals("myName", e.getName());
       assertDescription(e, "myDescription");
-      assertSame(jso, e.getException());
+      assertTrue(e.isThrownSet());
+      assertSame(jso, e.getThrown());
       assertTrue(e.getMessage().contains("myName"));
       assertTrue(e.getMessage().contains(e.getDescription()));
       if (extraPropertiesShouldBePresent) {
@@ -182,18 +183,19 @@
     e = makeJSO();
     assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));

-    // TODO(goktug): non-jso objects will be supported in follow up CL
-    //
-    // e = "testing";
- // assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));
-    //
-    // e = makeJavaObject();
- // assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));
-    //
-    // e = null;
- // assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));
+    e = "testing";
+    assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));
+
+    e = null;
+    assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));
+
+    e = makeJavaObject();
+    assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));

     e = new RuntimeException();
+    assertSame(e, catchJava(createThrowNativeRunnable(e)));
+
+    e = new JavaScriptException("exception message"); // Thrown is not set
     assertSame(e, catchJava(createThrowNativeRunnable(e)));

     e = new JavaScriptException(makeJSO());
@@ -207,21 +209,22 @@
     e = makeJSO();
     assertSame(e, nativeJavaNativeSandwich(e));

-    // TODO(goktug): non-jso objects will be supported in follow up CL
-    //
-    // e = "testing";
-    // assertEquals(e, nativeJavaNativeSandwich(e));
- // if (GWT.isScript()) { // Devmode will not preserve the same String instance
-    //   assertSame(e, nativeJavaNativeSandwich(e));
-    // }
-    //
-    // e = makeJavaObject();
- // assertJavaScriptException(e, catchJava(createThrowNativeRunnable(e)));
-    //
-    // e = null;
-    // assertSame(e, nativeJavaNativeSandwich(e));
+    e = "testing";
+    assertEquals(e, nativeJavaNativeSandwich(e));
+ if (GWT.isScript()) { // Devmode will not preserve the same String instance
+      assertSame(e, nativeJavaNativeSandwich(e));
+    }
+
+    e = null;
+    assertSame(e, nativeJavaNativeSandwich(e));
+
+    e = makeJavaObject();
+    assertSame(e, nativeJavaNativeSandwich(e));

     e = new RuntimeException();
+    assertSame(e, nativeJavaNativeSandwich(e));
+
+    e = new JavaScriptException("exception message"); // Thrown is not set
     assertSame(e, nativeJavaNativeSandwich(e));

     JavaScriptObject jso = makeJSO();
@@ -283,7 +286,8 @@
     } catch (JavaScriptException e) {
       assertEquals("null", e.getName());
       assertDescription(e, "null");
-      assertEquals(null, e.getException());
+      assertTrue(e.isThrownSet());
+      assertEquals(null, e.getThrown());
       assertTrue(e.getMessage().contains("null"));
     }
   }
@@ -296,7 +300,8 @@
     } catch (JavaScriptException e) {
       assertEquals(o.getClass().getName(), e.getName());
       assertDescription(e, "myLameObject");
-      assertEquals(null, e.getException());
+      assertTrue(e.isThrownSet());
+      assertEquals(o, e.getThrown());
       assertTrue(e.getMessage().contains(o.getClass().getName()));
       assertTrue(e.getMessage().contains(e.getDescription()));
     }
@@ -309,7 +314,8 @@
     } catch (JavaScriptException e) {
       assertEquals("String", e.getName());
       assertDescription(e, "foobarbaz");
-      assertEquals(null, e.getException());
+      assertTrue(e.isThrownSet());
+      assertEquals("foobarbaz", e.getThrown());
       assertTrue(e.getMessage().contains(e.getDescription()));
     }
   }

--
To view, visit https://gwt-review.googlesource.com/3640
To unsubscribe, visit https://gwt-review.googlesource.com/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I09c92ac03abbdb930085e2aaf97001b4d495abd9
Gerrit-PatchSet: 1
Gerrit-Project: gwt
Gerrit-Branch: master
Gerrit-Owner: Goktug Gokdogan <[email protected]>

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--- You received this message because you are subscribed to the Google Groups "GWT Contributors" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to