RFR [JDK8]: 7169894: JAXP Plugability Layer: using service loader

[Alan Bateman]

  On 24/08/2012 17:52, Joe Wang wrote:
> Hi,
>
> Here is a modified patch: 
> http://cr.openjdk.java.net/~joehw/jdk8/7169894/webrev/
>
> The factory finders contain some format changes, a NetBeans format 
> work. The real changes are as the follows:
>
> 1) In factory classes: reinstated the implementation resolution 
> mechanism, the 3rd step mainly
>
> 2) In factory finders: replaced findJarServiceProvider with 
> findServiceProvider
>
> 3) In factory finders: removed ConfigurationError class, using 
> FactoryConfigurationError instead
I've taken a pass over this, it looks much better now (duplicate 
FactoryFinders aside).

In DocumentBuilderFactory and SAXParserFactory the javadoc reads " 
Otherwise the default implementation is returned if it is on the 
classpath or installed as a module". I think this statement needs to be 
re-worked, first to remove the word "module" as it is not defined here, 
and also because I don't think it's the classpath. If I read the code 
correctly then initiating loader may actually be the context class 
loader and assuming the usual delegation then the default should be 
loaded by the boot class loader. It may be possible to borrow some of 
the wording from the ServiceLoader javadoc to help here.

In DatatypeFactory.java then it would be good to use {@link ..} or 
{@code ..} around the error and exception types that are referenced in 
the javadoc. Same thing in several other classes.

In src/javax/xml/datatype/FactoryFinder.java then is there any reason 
why this class has to use Object? I realize that some of the factory 
finders are called for several types but there are few (like this one) 
where there is only one type involved.

DocumentBuilderFactory.java it has:
   catch (FactoryConfigurationError e) { throw e; }
I assume the catch is not needed, same thing in a few other places.

Also in DocumentBuilderFatcory: " If ServiceConfigurationError is 
thrown, it is interpreted as an invalid provider is encountered. The 
process shall continue the search until all are exhausted."
I think this could be re-worded, minimally "as an invalid" to "as if an 
invalid".  Also "until all are exhausted" isn't completely clear to me, 
maybe it needs to re-iterate that it falls back to the system default.

Minor nit in src/javax/xml/validation/FactoryConfigurationError.java but 
it seems to be using 2-space indents rather than 4-space indents.

I think one thing that would be really useful here is to get a summary 
of the behavioral and API changes that this changes brings. 
SchemeFactory now throws FactoryConfigurationError whereas previously 
errors were ignored. It would be good to summarize the others.

-Alan.