On 13 Feb 2015, at 16:11, Alan Bateman <alan.bate...@oracle.com> wrote:
> On 10/02/2015 16:20, Chris Hegarty wrote: >> On 10 Feb 2015, at 11:35, Alan Bateman <alan.bate...@oracle.com> wrote: >> >>> On 09/02/2015 14:57, Chris Hegarty wrote: >>>> : >>>> >>>> Updated webrev [ spec and implementation] : >>>> http://cr.openjdk.java.net/~chegar/8064924/04/ >>>> >>> The updated javadoc looks good. I haven't had a chance yet to look at the >>> implementation but I think you will need to update >>> <top>/make/common/CORE_PKGS.gmk for the spi package. >> Sorry, I have the change locally but forgot it. I updated the webrev with >> the changes to the top level repo ( below ): >> > I think the approach is good. > > One thing in the implementation is detection for recursive initialization. In > other areas of the platforms (ClassLoader.getSystemClassLoader and > FileSystem.getDefault) then an exception is thrown. In an earlier revision recursion detection threw an Exception, but I was inspired by the the lookup mechanism in Charset, which returns null when it encounters recursion. I chose the latter as it means that we do not need to filter out specific protocols from lookup, like ‘jar’. Returning null just falls back to searching the built-in handlers. > I wonder if we need this here too. If the initialization of a > URLStreamHandlerFactory is dependent on the deployment of another then it > could be very tricky to diagnose as there isn't (yet) any control on the > ordering. I had not thought of that scenario, and I’m not sure how it would work, at least with ServiceLoader. Do you think it is important ? > I'd probably use isOverridable or canOverride for the method name. I’ll update from overrideable(String) -> isOverridable(String) > In the spi package-info.java then I assume it time that the second paragraph > will need to change as there will be other provider interfaces. Right. I think it is ok for now, and when additional providers are added it can be updated then. Thanks, -Chris. > -Alan. >
