Request Review: JDK-6479237 (cl) Add support for classloader names

[Mandy Chung]

Hi Daniel,

Here is the updated webrev incorporating your feedback:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6479237/webrev.03

> On Nov 1, 2016, at 7:04 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> :
> 334             s += declaringClass;
> 
> 
> but should line 334 instead be
> 
>   s = (s.isEmpty() ? declaringClass : s + "/" + declaringClass;
> 
> Otherwise "/" will be missing after module at version..
> 

Good catch.  I added a test for this.

> Also should there be some asserts somewhere verifying that moduleVersion
> is null or empty when moduleName is null or empty? At the moment the
> constructor will blindly accept a version for an unnamed module,
> and I am assuming this is wrong - am I right?
> 

Good point.  The constructor should throw IAE if module name/version and class loader name is an empty string.

> toLoaderModuleClassName will call ClassLoader.getName().
> If a class loader overrides getName() then that might be a different
> name than what StackTraceElement.getClassLoaderName() returns.
> 
> Is that an issue?
> 

I added a test to verify StackTraceElement that uses the ClassLoader's name field.  I also added @apiNote in ClassLoader::getName.

> Also I wonder whether you should be considering including a fix
> for https://bugs.openjdk.java.net/browse/JDK-8167099 as part
> of this change (though arguably the bug has been there since
> new fields were added to StackTraceElement in 9).
> 

It is related but it’s better to separate it from this issue.  Do you have cycle to help write a test case?  I can help fix JDK-8167099 later.

Mandy