Alan, Thanks for the review. I'll send out a new webrev to incorporate
your feedback. Yes, I file a CR for the http protocol handler:
6882384: Update http protocol handler to use PlatformLogger
Several 2D and sun.font classes are refactored in the 2D repository that
will be integrated for b74. Some classes I modified in the 2D repo are
not in TL repo. To simplify the integration, I file a new CR 6882376
for the core-libs changes including changes in java.util classes that
will go in TL repo that I think I can make b73. The AWT/2D/Swing
changes will go in 2D repo as a fix for 6879044.
I'll generate two new webrevs to separate the core-libs and awt/2d/swing
stuff for the review.
Thanks
Mandy
Alan Bateman wrote:
Mandy Chung wrote:
6879044: Eliminate the dependency of logging from the JRE
core/awt/swing classes
Webrev:
http://cr.openjdk.java.net/~mchung/6879044/webrev.00/
The approach seems reasonable to me. It is a bit strange to redirect the
platform logging to j.u.logging if something up the stack starts logging
but I think we can live with this because these loggers are for
debugging purposes.
The changes to the AWT code are mostly mechanical, so I've only skimmed
these (assuming that someone from the AWT team with do a detailed check).
Have you tested this with a security manager set? I'm just wondering if
PlatformLogger's static initializer should do the property lookup in a
doPrivileged block. Also in the LoggerProxy for the line separator (BTW:
lineSeparator can be final).
Is isLoggingEnabled() used anywhere? I can't see it and wonder if it
should be removed. If it is used, then I assume it needs to be
synchronized with redirectPlatformLoggers.
You allow the PlatformLoggers to be GC'ed but the entries will remain in
the loggers map. I don't think this is a big deal because the AWT
classes will not be unloaded but might be worth putting a note in a
comment to re-visit this some time.
The static initializer in the forwarding proxy seems a bit messy. It
might look neater if changed to something like:
private static final Class<?> loggerClass =
getClass("java.lang.Logger"); private static
final Method getLoggerMethod = getMethod(loggerClass, "getLogger",
String.class);
where getClass does the Class.forName, returning null if CNF, getMethod
returns null if passed a null class, etc.
Minor nit but there are a few style/formatting issues in PlatformLogger.
For example, in JavaLogger there isn't a blank line between methods. It
might be worthing taking a pass over this to have it as neat as possible.
Do you have a bugID to track updating the http protocol handler? Jessie
did push some changes to eliminate the static dependency on logging and
it would be good if that code used PlatformLogger.
-Alan.
