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 <gok...@google.com>

--
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 google-web-toolkit-contributors+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to