Overall LGTM with a few changes.
http://gwt-code-reviews.appspot.com/1254801/diff/14001/15001 File dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java (right): http://gwt-code-reviews.appspot.com/1254801/diff/14001/15001#newcode792 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:792: * Note: OpenJDK byte code doesn't create a type signature for non-static inner s/OpenJDK byte code/The OpenJDK compiler http://gwt-code-reviews.appspot.com/1254801/diff/14001/15001#newcode795 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:795: * represented in the type oracle. Mention that these differences also show up in java.lang.reflect. http://gwt-code-reviews.appspot.com/1254801/diff/14001/15002 File dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorFromByteCodeTest.java (right): http://gwt-code-reviews.appspot.com/1254801/diff/14001/15002#newcode27 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorFromByteCodeTest.java:27: * This test uses the byte code on the class path created when compiling this Add a one-line summary of the class above this note. http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004 File dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java (right): http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode127 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:127: DataInputStream instream = new DataInputStream(istream); Why use DataInputStream? http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode130 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:130: while (true) { Use com.google.gwt.util.Util here instead? http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode143 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:143: /* For building the type oracle from bytecode */ Single-line comment style http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode182 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:182: DataInputStream instream = new DataInputStream(istream); Same above about DataInputStream and Util. http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode196 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:196: return out.toString(); Note that using out.toString uses the default platform charset. (Bad). You can use out.toString(String encoding) instead, but if you just use Util.readStreamAsString, it uses UTF8. (Good). Typically when trying to read chars from a stream (instead of bytes), you'd use some kind of Reader to do the character conversion, like a BufferedReader. http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode196 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:196: return out.toString(); Most of this code is identical to getByteCode. Refactor? http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode214 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:214: String source = ""; Why are these two separate lines? http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode225 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:225: assertNotNull(source); Move the comment text into a message you provide in the assert? http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode243 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:243: Package p = aClass.getPackage(); FYI, getPackage can return null. A package only exists for a class if the ClassLoader that loaded it assigned one in the process. This is true for URLClassLoader but not others. Because you can always reliably determine a package name from a class name, it's better to use that. See com.google.gwt.dev.javac.Shared.getPackageNameFromBinary. http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode649 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:649: // assertNull(type.isDefaultInstantiable()); Did you fix this? http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode711 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:711: protected static final CheckedJavaResource CU_ReferencesGenericListConstant = new CheckedJavaResource( Wrapping? Also for following declarations. http://gwt-code-reviews.appspot.com/1254801/diff/14001/15004#newcode804 dev/core/test/com/google/gwt/dev/javac/TypeOracleMediatorTestBase.java:804: // assertFalse(publicTypeNameToTestCupMap.containsKey(qualifiedTypeName)); Why's this commented out? http://gwt-code-reviews.appspot.com/1254801/diff/14001/15008 File dev/core/test/com/google/gwt/dev/javac/mediatortest/BindToTypeScope.java (right): http://gwt-code-reviews.appspot.com/1254801/diff/14001/15008#newcode2 dev/core/test/com/google/gwt/dev/javac/mediatortest/BindToTypeScope.java:2: * This code must be kept in sync with {...@link com.google.gwt.dev.javac.TypeOracleMediatorTestBase} The wording on this confuses me. Maybe drop the comments on all of these "data" classes and just add package-level documentation? http://gwt-code-reviews.appspot.com/1254801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
