[OpenJDK 2D-Dev] Review Request for 6879044
Mandy.Chung at Sun.COM
Tue Sep 15 23:26:40 UTC 2009
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.
Alan Bateman wrote:
> Mandy Chung wrote:
>> 6879044: Eliminate the dependency of logging from the JRE
>> core/awt/swing classes
> 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",
> 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.
More information about the 2d-dev