Hi Phil,
Few questions about this patch.
I wonder if IcedTeaWeb will be broken by such change removing the support
of multiple AppContexts in FontManager.
I would have prefered keeping maybeMultiAppContext() relying on a system
property than removing all the internal mechanism, that could be maintained
externally but would mean having a forked openjdk to keep this specific
code.
What are the possible side effects if this change is applied on application
using multiple AppContexts ?
Laurent
Le mer. 30 janv. 2019 à 21:56, Phil Race <philip.r...@oracle.com> a écrit :
> The synopsis - which I rephrased for better English, turns out to still
> not represent most of what is happening here. The synopsis would
> make one think this is mostly about removing an unused abstract class
> and the font changes are a foot note to that. In fact this is MOSTLY
> about removing per-appcontext code in the font code - code which
> just asked for an appcontext and had no direct relationship with the
> AWTSecurityManager class except to check for it in a one time call as a
> cheap way
> of knowing we needed to be prepared to check for multiple contexts.
> If you are removing AWTSecurityManager, the logical thing would be
> to find another way to see if it is multiple appcontexts, not remove all
> the code
> that handles such a case.
> In other words there is an extrapolation here from "AWTSecurityManager is
> gone"
> to "multiple appcontexts are gone" which happens to be true, but does not
> logically follow.
>
> So I thought I was reviewing one thing and it turns out to be quite
> different
> and better summarised as :
> "Remove multiple appcontext support from the font code because
> AWTSecurityManager is being removed"
> The removal of AWTSecurityManager is < 10% of this change ..
>
> Alternatively everything changed in the font code could have been handled
> with this :-
>
> private static boolean maybeMultiAppContext() {
> // Do we have multiple app contexts any more ?
> // Or do we need a new way to check for multiple app contexts ?
> // If not, can we remove this code + its downstream dependencies too in a
> separate fix ?
> return false;
> }
>
>
> Are there any tests that are also obsoleted ?
>
> I am approving this change (modulo the answer about tests) since the end
> result will
> be the same but personally I would have split it into an AWT bug and a 2D bug
> each
> with an appropriate synopsis.
>
> -phil.
>
> On 1/15/19 11:49 AM, Sergey Bylokhov wrote:
>
> Hello.
> Please review the fix for jdk 13.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8216592
> Webrev: http://cr.openjdk.java.net/~serb/8216592/webrev.01
>
> The AWTSecurityManager class is an abstract class which provided
> the AppContext objects through SecurityManager extensions. The plugin
> provided a concrete implementation of this class. Since plugin was
> removed the AWTSecurityManager itself can be removed as well.
>
> Since we currently never set AWTSecurityManager some of our checks like:
> "sm instanceof sun.awt.AWTSecurityManager"
> are always "false". And I dropped the code which uses such
> checks(mostly in the SunFontManager)
>
>
>
>
