Comments inline. NativeString -- In htmlUnit, after a call like: String foo = new String(); 'foo' is of type NativeString. While on the java side, the object is assumed to be of type String. So, we need to the appropriate massaging in makeValueFromJsVal.
http://gwt-code-reviews.appspot.com/64817/diff/1/2 File dev/oophm/overlay/com/google/gwt/dev/shell/JsValueGlue.java (right): http://gwt-code-reviews.appspot.com/64817/diff/1/2#newcode177 Line 177: */ Just to be clear, the current JsValueGlue, i.e., in gwt-dev, throws an IllegalArgumentException. So, while the HostedModeException looks more appropriate, this might break other code (besides this test) where an IllegalArgumentException is expected. I still think we should make this change. Do you still agree ? On 2009/09/16 19:30:16, jat wrote: > I think HostedModeException is correct here, as this something that is only > happening because of hosted mode. Unless there is a bug in GWT, this should > only happen when you call Java code from JSNI with the wrong parameter types. Done. http://gwt-code-reviews.appspot.com/64817/diff/1/5 File dev/oophm/src/com/google/gwt/dev/shell/BrowserChannel.java (right): http://gwt-code-reviews.appspot.com/64817/diff/1/5#newcode77 Line 77: } JsObjectRef is now also used on the client side where the static was causing an error. On 2009/09/16 19:30:16, jat wrote: > Why was this moved outside the constructor? http://gwt-code-reviews.appspot.com/64817/diff/1/6 File dev/oophm/src/com/google/gwt/dev/shell/JsValueOOPHM.java (right): http://gwt-code-reviews.appspot.com/64817/diff/1/6#newcode147 Line 147: JsObjectRef.checkIdMap(jsRefId); I just isolated the code using the static. On 2009/09/16 19:30:16, jat wrote: > I'm not sure I understand what is going on with this change and the > corresponding one in BrowserChannel. http://gwt-code-reviews.appspot.com/64817/diff/1/10 File user/src/com/google/gwt/junit/JUnitShell.java (left): http://gwt-code-reviews.appspot.com/64817/diff/1/10#oldcode697 Line 697: } On 2009/09/16 19:30:16, jat wrote: > I assume with this removal, SWT no longer works, right? Yes. http://gwt-code-reviews.appspot.com/64817/diff/1/10 File user/src/com/google/gwt/junit/JUnitShell.java (right): http://gwt-code-reviews.appspot.com/64817/diff/1/10#newcode430 Line 430: private static final int TEST_BEGIN_TIMEOUT_MILLIS = 6000000; On 2009/09/16 19:30:16, jat wrote: > This needs to get lowered. Done. http://gwt-code-reviews.appspot.com/64817/diff/1/10#newcode621 Line 621: setHeadless(GraphicsEnvironment.isHeadless()); typo. Will remove the unnecessary first 'setHeadless(..)' statement. Also, not sure if the setHeadless(GraphicsEnvironment.isHeadless()) does what we want. On 2009/09/16 19:30:16, jat wrote: > Why set this twice? http://gwt-code-reviews.appspot.com/64817/diff/1/10#newcode651 Line 651: consoleLogger.setMaxDetail(TreeLogger.INFO); Not needed. will remove it. Previously, the call was: consoleLogger.setMaxDetail(getCompilerOptions().getLogLevel()); On 2009/09/16 19:30:16, jat wrote: > Why is this needed? http://gwt-code-reviews.appspot.com/64817 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
