Hi Kevin, > On May 1, 2017, at 5:21 PM, Kevin Rushforth <kevin.rushfo...@oracle.com> > wrote: > > This review is being cross-posted to both openjfx-dev and jigsaw-dev. > > Please review the proposed fix for: > > https://bugs.openjdk.java.net/browse/JDK-8177566 > http://cr.openjdk.java.net/~kcr/8177566/webrev.00/complete-webrev/
First pass of comment: javafx.base/src/main/java/com/sun/javafx/property/PropertyReference.java 196 try { 197 return (ReadOnlyProperty<T>)MethodHelper.invoke(propertyGetter, bean, (Object[])null); 198 } catch (Exception ex) { 199 throw new RuntimeException(ex); 200 } Do you have an example exception thrown if the package is not open to javafx.base? IAE is thrown by MethodHelper.invoke. Are you detecting this and throw an exception with friendlier message? javafx.base/src/main/java/com/sun/javafx/property/MethodHelper.java javafx.fxml/src/main/java/com/sun/javafx/fxml/MethodHelper.java javafx.web/src/main/java/com/sun/webkit/MethodHelper.java 45 public static Object invoke(Method m, Object obj, Object[] params) To avoid 3 ModuleHelper classes, the invoke method can take the callerModule argument to replace this line: 56 final Module thisModule = MethodHelper.class.getModule(); javafx.base/src/main/java/com/sun/javafx/reflect/MethodUtil.java There are a few other public methods which I think JavaFX doesn’t need and can be removed. Mandy