JDK-8195799 : Use System logger instead of platform logger in javafx modules
  • Type: Bug
  • Component: javafx
  • Sub-Component: other
  • Affected Version: 9,10,openjfx11
  • Priority: P2
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-01-19
  • Updated: 2019-01-10
  • Resolved: 2018-03-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
openjfx11Fixed
Related Reports
Blocks :  
Blocks :  
Relates :  
Relates :  
Relates :  
Description
In an effort to eliminate the use of internal packages from core module by javafx.* modules, we need to switch from using the internal platform logger, in the internal sun.util.logging package, to the light-weight System logger.

We can (and should) consider providing an adapter class to minimize the change in those classes that use the existing platform logger.
Comments
This looks good to me. +1
26-03-2018

Thanks all for the review. I have addressed the review comments in - http://cr.openjdk.java.net/~aghaisas/fx/8195799/webrev.1/ The changes are - 1. Addressed the (c) year inaccuracies from files - modules/javafx.base/src/main/java/com/sun/javafx/collections/SetListenerHelper.java modules/javafx.graphics/src/main/java/javafx/scene/Scene.java 2. Removed tabs from modules/javafx.base/src/test/java/test/com/sun/javafx/binding/SelectBindingTest.java 3. Removed unused methods from com.sun.javafx.logging.PlatformLogger. Also, I have created a new bug JDK-8200236 to address some of the valid suggestions from Mandy and Daniel. [~jvos] The unsupported methods in PlatformLogger are not to be used. They are there to achieve correct logging of calling class / method name. JDK internal loggers know how to skip any calls from System.Logger classes. Hence, JavaFX PlatformLogger has been made a System.Logger. I have noted this in class description. 41 * PlatformLogger implements System.Logger as to facilitate logging of 42 * calling class and method. 43 * Note : JDK internal loggers know to skip any calls from System.Logger 44 * classes when looking for the calling class / method.
26-03-2018

Overall this approach looks ok to me. There are a number of unsupported overridden methods (e.g. getName()) in PlatformLogger that are probably not used now, but might be in the future? There are a few places where the (c) year conversions failed: --- old/modules/javafx.base/src/main/java/com/sun/javafx/collections/SetListenerHelper.java 2018-03-23 20:57:21.071911563 +0530 +++ new/modules/javafx.base/src/main/java/com/sun/javafx/collections/SetListenerHelper.java 2018-03-23 20:57:20.855911563 +0530 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 201, Oracle and/or its affiliates. All rights reserved. And similiar in modules/javafx.graphics/src/main/java/javafx/scene/Scene.java
26-03-2018

The change is basically implementation of option-2 as listed out above by Kevin. ---> 2. Provide an adapter class that implements the System.Logger interface I have added an internal logger class to JavaFX - namely - com.sun.javafx.logging.PlatformLogger. I have replaced sun.util.logging.PlatformLogger usage in JavaFX codebase with this newly added (internal to JavaFX) com.sun.javafx.logging.PlatformLogger. Webrev : http://cr.openjdk.java.net/~aghaisas/fx/8195799/webrev.0/ Request you to review.
23-03-2018

This is a primary priority change
14-03-2018

While working on another internal dependency issue I discovered another place where we need to remove qualified exports to make sure that our unit tests are not trying to access internal packages. Each module has a src/test/addExports file with additional exports, so sun.util.logging should be removed from those files as well. Here are the ones I found: modules/javafx.base/src/test/addExports:--add-exports java.base/sun.util.logging=ALL-UNNAMED modules/javafx.controls/src/test/addExports:--add-exports java.base/sun.util.logging=ALL-UNNAMED modules/javafx.graphics/src/test/addExports:--add-exports java.base/sun.util.logging=ALL-UNNAMED tests/system/src/test/addExports:--add-exports java.base/sun.util.logging=ALL-UNNAMED
28-02-2018

Usage of sun.util.logging package can be found by issuing a grep command in rt/modules directory - grep -inr "import sun.util.logging." *
06-02-2018

Once all calls to sun.util.logging have been eliminated, then the qualified export can be removed from dependencies/java.base/module-info.java.extra and build.gradle. See the "REMOVING QUALIFIED EXPORTS" comment in JDK-8195798 for instructions.
23-01-2018

As a related RFE, javafx.fxml and javafx.web both use java.util.logging (from the java.logging module). We should consider a separate RFE to convert them to the lighter-weight System.Logger. I filed JDK-8195974 to track this.
23-01-2018

There are two possible approaches to consider: 1. Provide a convenience "adapter" class that delegates to System.Logger 2. Provide an adapter class that implements the System.Logger interface The former will allow fewer code changes in code that uses the logger, but the latter has the advantage that the java.util.logger and the simple console logger know to skip any calls from System.Logger classes when looking for the calling class / method. Other things to note: PlatformLogger.setLevel can't be converted to System.Logger since the configuration side is moved to the providers (callers should not programmatically set the level). We only call setLevel in a couple places, so it should be fairly easy to eliminate these calls. Note that one of the calls to setLevel is in native code in: rt/modules/javafx.media/src/main/native/jfxmedia/jni/Logger.cpp
23-01-2018