RFR (JAXP) 8152912: SAX XMLReaderFactory needs to be ServiceLoader compliant

[Daniel Fuchs]

Hi Joe,

This look good but the implementation might be overly complex,
which makes it difficult to read.

First:
141         ClassLoader     cl = ss.getContextClassLoader();
is misnamed, because as far as I can see this method returns
the context class loader if not null, otherwise the system
class loader.

So cl is either the context class loader or the system class
loader and is guaranteed to be non null.
I would suggest adding a comment to make it clear.

This means in turn that the new jarLookup(ClassLoader)
method can be greatly simplified - you can do:
final ClassLoader cl = Objects.requiresNonNull(loader);
and then simplify the if (cl != null) else ... logic,
because you now know that cl cannot be null.

Best regards,

-- daniel



On 02/05/16 20:30, huizhe wang wrote:
> Hi,
>
> Please review a change that adds a step using the ServiceLoader to
> XMLReaderFactory's provider-lookup process. Meanwhile, XMLReaderFactory
> is deprecated in favor of javax.xml.parsers.SAXParserFactory.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8152912
> Webrev: http://cr.openjdk.java.net/~joehw/jdk9/8152912/webrev/
>
> The change has been verified by SQE test that Frank will submit soon for
> review (that is, when he starts to work at his local time).
>
> Thanks,
> Joe