Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-17 Thread Kevin Rushforth

That all sounds good.

As for the packager, it will likely continue to be a source of issues 
like this now that it is gone from the JDK, and isn't part of the 
unbundled FX builds.


I just filed a bug [1] to remove the qualified export from 
javafx.graphics to jdk.packager (and to disable building the packager 
even when building using an Oracle JDK) -- see my recent reply to the 
OpenJFX updates thread [1]. Perhaps I should expand that issue to remove 
references to it from the IDE files as well.


Ultimately, we should delete modules/jdk.packager* and the references to 
it in build.gradle, etc., but that can be done as a follow-on cleanup 
issue, which I just filed [3]. Note that the source code would still 
available for anyone who wants it, either from the openjfx/10u-dev/rt 
repo or from the openjfx/jfx-dev/rt repo using the jdk-11+13 tag.


-- Kevin

[1] https://bugs.openjdk.java.net/browse/JDK-8203378
[2] http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-May/021894.html
[3] https://bugs.openjdk.java.net/browse/JDK-8203379


On 5/17/2018 2:07 PM, Nir Lisker wrote:


There was one other place where isDebug() was called, that being
the mLog method.Did you replace the call ti isDebug() with a
direct call to isLoggable instead?


Yes, I should have been clearer: I inlined `isDebug` into `mLog`, then 
commented out `isDebug` so that if someone reads the commented-out 
code in `main` they'll know what it is referring to. I just didn't see 
a point in leaving a private static method just for this one use.


In any case, since "HelloService" is a test program, I see no
problem with it continuing to use java.util.logging.


For Eclipse users this it still a problem since Eclipse includes the 
tests in the module (it was discussed in the JIRA issue). Since the 
packager is out of scope, I'll just patch the last remnant 
in test.com.sun.javafx.binding.ErrorLoggingUtiltity and submit a 
Webrev tomorrow or this weekend since all other changes are already 
prepared (and mostly trivial).


- Nir

On Thu, May 17, 2018 at 11:39 PM, Kevin Rushforth 
> wrote:


I think it isn't worth doing anything for the packager. In any
case, since "HelloService" is a test program, I see no problem
with it continuing to use java.util.logging.

Regarding your other comment:


Thanks Murali, Iv'e also commented out isDebug() (though maybe it
can be completely removed, only that the commented out code
relies on it).



There was one other place where isDebug() was called, that being
the mLog method. Did you replace the call ti isDebug() with a
direct call to isLoggable instead? Note that without the
isLoggable check you will incur the redundant cost of the method
calls + string construction.

-- Kevin



On 5/17/2018 1:25 PM, Nir Lisker wrote:

Thanks Murali, Iv'e also commented out isDebug() (though maybe it
can be completely removed, only that the commented out code
relies on it).

Kevin, after this change, HelloService in the packager is the
only one requiring file handling. What is the future of the
packager now? Is it worth supplying an alternative for these classes?

- Nir

On Thu, May 17, 2018 at 8:49 PM, Murali Billa
> wrote:

Hi Nir,

Regarding “com.sun.javafx.webkit.drt.DumpRenderTree”, you can
comment out the below code in “main” method from
DumpRenderTree.java file.

if ( isDebug() ) {

log.setLevel(Level.FINEST);

FileHandler handler = new FileHandler("drt.log", true);

handler.setFormatter(new Formatter() {

@Override

public String format(LogRecord record) {

return formatMessage(record) + "\n";

}

});

log.addHandler(handler);

}

Thanks,

MUrali

*From:*Murali Billa
*Sent:* Thursday, May 03, 2018 1:52 AM
*To:* Nir Lisker >
*Cc:* openjfx-dev@openjdk.java.net
 Mailing
>
*Subject:* RE: JDK-8195974: Replace use of java.util.logging
in javafx with System logger

Hi Nir,

1)Regarding “verbose” flag usage:

·Currently verbose flag is used to show log Levels
(FINER/FINE/INFO/WARNING) in WCMediaPlayer &
WCMediaPlayerImpl. I feel it is not desirable to remove this
flag as all these logs will start appearing now by default.

·We can try 2 options:

a)1^st Option: We can change all INFO log messages to FINE
 under verbose flag (by leaving all log messages that use
Level other than INFO unchanged) and verbose flag can be removed.

b)If 1^st option results 

Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-17 Thread Nir Lisker
>
> There was one other place where isDebug() was called, that being the mLog
> method. Did you replace the call ti isDebug() with a direct call to
> isLoggable instead?
>

Yes, I should have been clearer: I inlined `isDebug` into `mLog`, then
commented out `isDebug` so that if someone reads the commented-out code in
`main` they'll know what it is referring to. I just didn't see a point in
leaving a private static method just for this one use.

In any case, since "HelloService" is a test program, I see no problem with
> it continuing to use java.util.logging.


For Eclipse users this it still a problem since Eclipse includes the tests
in the module (it was discussed in the JIRA issue). Since the packager is
out of scope, I'll just patch the last remnant
in test.com.sun.javafx.binding.ErrorLoggingUtiltity and submit a Webrev
tomorrow or this weekend since all other changes are already prepared (and
mostly trivial).

- Nir

On Thu, May 17, 2018 at 11:39 PM, Kevin Rushforth <
kevin.rushfo...@oracle.com> wrote:

> I think it isn't worth doing anything for the packager. In any case, since
> "HelloService" is a test program, I see no problem with it continuing to
> use java.util.logging.
>
> Regarding your other comment:
>
> Thanks Murali, Iv'e also commented out isDebug() (though maybe it can be
> completely removed, only that the commented out code relies on it).
>
>
> There was one other place where isDebug() was called, that being the mLog
> method. Did you replace the call ti isDebug() with a direct call to
> isLoggable instead? Note that without the isLoggable check you will incur
> the redundant cost of the method calls + string construction.
>
> -- Kevin
>
>
>
> On 5/17/2018 1:25 PM, Nir Lisker wrote:
>
> Thanks Murali, Iv'e also commented out isDebug() (though maybe it can be
> completely removed, only that the commented out code relies on it).
>
> Kevin, after this change, HelloService in the packager is the only one
> requiring file handling. What is the future of the packager now? Is it
> worth supplying an alternative for these classes?
>
> - Nir
>
> On Thu, May 17, 2018 at 8:49 PM, Murali Billa 
> wrote:
>
>> Hi Nir,
>>
>>
>>
>> Regarding “com.sun.javafx.webkit.drt.DumpRenderTree”, you can comment
>> out the below code in “main” method from DumpRenderTree.java file.
>>
>>
>>
>> if ( isDebug() ) {
>>
>> log.setLevel(Level.FINEST);
>>
>> FileHandler handler = new FileHandler("drt.log", true);
>>
>> handler.setFormatter(new Formatter() {
>>
>> @Override
>>
>> public String format(LogRecord record) {
>>
>> return formatMessage(record) + "\n";
>>
>> }
>>
>> });
>>
>> log.addHandler(handler);
>>
>> }
>>
>>
>>
>> Thanks,
>>
>> MUrali
>>
>> *From:* Murali Billa
>> *Sent:* Thursday, May 03, 2018 1:52 AM
>> *To:* Nir Lisker 
>> *Cc:* openjfx-dev@openjdk.java.net Mailing 
>> *Subject:* RE: JDK-8195974: Replace use of java.util.logging in javafx
>> with System logger
>>
>>
>>
>> Hi Nir,
>>
>>
>>
>> 1)  Regarding “verbose” flag usage:
>>
>> · Currently verbose flag is used to show log Levels
>> (FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl.  I feel
>> it is not desirable to remove this flag as all these logs will start
>> appearing now by default.
>>
>> · We can try 2 options:
>>
>> a)   1st Option: We can change all INFO log messages to FINE  under
>> verbose flag (by leaving all log messages that use Level other than INFO
>> unchanged) and verbose flag can be removed.
>>
>> b)  If 1st option results in too much noise for WARNING log
>> messages, then we can keep the verbose flag and introduce a System Property
>> (for ex: javafx.web.verbose) to enable the flag. I won’t suggest reading
>> level value from log/config file.
>>
>>
>>
>> 2)  Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”,  I need
>> to check few more things (since we use “addHandler” in drt) and will get
>> back to you.
>>
>>
>>
>> Please let me know, if you have any queries for 1.
>>
>> Thanks,
>>
>> Murali
>>
>> *From:* Nir Lisker 
>> *Sent:* Saturday, April 28, 2018 1:06 AM
>> *To:* Murali Billa 
>> *Cc:* openjfx-dev@openjdk.java.net Mailing 
>> *Subject:* JDK-8195974: Replace use of java.util.logging in javafx with
>> System logger
>>
>>
>>
>> Hi Murali,
>>
>>
>>
>> Can you have a look at https://bugs.openjdk.java.net/browse/JDK-8195974
>> please?
>>
>>
>>
>> There are some usages of j.u.l in the web module I'd like your opinion
>> on. I'm not familiar with the intent of these pieces of code and would like
>> to know what the options are for advancing with this issue on that front.
>>
>>
>>
>> Thanks,
>>
>> Nir
>>
>
>
>


Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-17 Thread Kevin Rushforth
I think it isn't worth doing anything for the packager. In any case, 
since "HelloService" is a test program, I see no problem with it 
continuing to use java.util.logging.


Regarding your other comment:

Thanks Murali, Iv'e also commented out isDebug() (though maybe it can 
be completely removed, only that the commented out code relies on it).




There was one other place where isDebug() was called, that being the 
mLog method. Did you replace the call ti isDebug() with a direct call to 
isLoggable instead? Note that without the isLoggable check you will 
incur the redundant cost of the method calls + string construction.


-- Kevin


On 5/17/2018 1:25 PM, Nir Lisker wrote:
Thanks Murali, Iv'e also commented out isDebug() (though maybe it can 
be completely removed, only that the commented out code relies on it).


Kevin, after this change, HelloService in the packager is the only one 
requiring file handling. What is the future of the packager now? Is it 
worth supplying an alternative for these classes?


- Nir

On Thu, May 17, 2018 at 8:49 PM, Murali Billa > wrote:


Hi Nir,

Regarding “com.sun.javafx.webkit.drt.DumpRenderTree”, you can
comment out the below code in “main” method from
DumpRenderTree.java file.

if ( isDebug() ) {

log.setLevel(Level.FINEST);

FileHandler handler = new FileHandler("drt.log", true);

handler.setFormatter(new Formatter() {

@Override

public String format(LogRecord record) {

return formatMessage(record) + "\n";

}

});

log.addHandler(handler);

}

Thanks,

MUrali

*From:*Murali Billa
*Sent:* Thursday, May 03, 2018 1:52 AM
*To:* Nir Lisker >
*Cc:* openjfx-dev@openjdk.java.net
 Mailing
>
*Subject:* RE: JDK-8195974: Replace use of java.util.logging in
javafx with System logger

Hi Nir,

1)Regarding “verbose” flag usage:

·Currently verbose flag is used to show log Levels
(FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl. I
feel it is not desirable to remove this flag as all these logs
will start appearing now by default.

·We can try 2 options:

a)1^st Option: We can change all INFO log messages to FINE  under
verbose flag (by leaving all log messages that use Level other
than INFO unchanged) and verbose flag can be removed.

b)If 1^st option results in too much noise for WARNING log
messages, then we can keep the verbose flag and introduce a System
Property (for ex: javafx.web.verbose) to enable the flag. I won’t
suggest reading level value from log/config file.

2)Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”, I need to
check few more things (since we use “addHandler” in drt) and will
get back to you.

Please let me know, if you have any queries for 1.

Thanks,

Murali

*From:*Nir Lisker >
*Sent:* Saturday, April 28, 2018 1:06 AM
*To:* Murali Billa >
*Cc:* openjfx-dev@openjdk.java.net
 Mailing
>
*Subject:* JDK-8195974: Replace use of java.util.logging in javafx
with System logger

Hi Murali,

Can you have a look at
https://bugs.openjdk.java.net/browse/JDK-8195974
 please?

There are some usages of j.u.l in the web module I'd like your
opinion on. I'm not familiar with the intent of these pieces of
code and would like to know what the options are for advancing
with this issue on that front.

Thanks,

Nir






Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-17 Thread Nir Lisker
Thanks Murali, Iv'e also commented out isDebug() (though maybe it can be
completely removed, only that the commented out code relies on it).

Kevin, after this change, HelloService in the packager is the only one
requiring file handling. What is the future of the packager now? Is it
worth supplying an alternative for these classes?

- Nir

On Thu, May 17, 2018 at 8:49 PM, Murali Billa 
wrote:

> Hi Nir,
>
>
>
> Regarding “com.sun.javafx.webkit.drt.DumpRenderTree”, you can comment out
> the below code in “main” method from DumpRenderTree.java file.
>
>
>
> if ( isDebug() ) {
>
> log.setLevel(Level.FINEST);
>
> FileHandler handler = new FileHandler("drt.log", true);
>
> handler.setFormatter(new Formatter() {
>
> @Override
>
> public String format(LogRecord record) {
>
> return formatMessage(record) + "\n";
>
> }
>
> });
>
> log.addHandler(handler);
>
> }
>
>
>
> Thanks,
>
> MUrali
>
> *From:* Murali Billa
> *Sent:* Thursday, May 03, 2018 1:52 AM
> *To:* Nir Lisker 
> *Cc:* openjfx-dev@openjdk.java.net Mailing 
> *Subject:* RE: JDK-8195974: Replace use of java.util.logging in javafx
> with System logger
>
>
>
> Hi Nir,
>
>
>
> 1)  Regarding “verbose” flag usage:
>
> · Currently verbose flag is used to show log Levels
> (FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl.  I feel
> it is not desirable to remove this flag as all these logs will start
> appearing now by default.
>
> · We can try 2 options:
>
> a)   1st Option: We can change all INFO log messages to FINE  under
> verbose flag (by leaving all log messages that use Level other than INFO
> unchanged) and verbose flag can be removed.
>
> b)  If 1st option results in too much noise for WARNING log messages,
> then we can keep the verbose flag and introduce a System Property (for ex:
> javafx.web.verbose) to enable the flag. I won’t suggest reading level value
> from log/config file.
>
>
>
> 2)  Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”,  I need to
> check few more things (since we use “addHandler” in drt) and will get back
> to you.
>
>
>
> Please let me know, if you have any queries for 1.
>
> Thanks,
>
> Murali
>
> *From:* Nir Lisker 
> *Sent:* Saturday, April 28, 2018 1:06 AM
> *To:* Murali Billa 
> *Cc:* openjfx-dev@openjdk.java.net Mailing 
> *Subject:* JDK-8195974: Replace use of java.util.logging in javafx with
> System logger
>
>
>
> Hi Murali,
>
>
>
> Can you have a look at https://bugs.openjdk.java.net/browse/JDK-8195974
> please?
>
>
>
> There are some usages of j.u.l in the web module I'd like your opinion on.
> I'm not familiar with the intent of these pieces of code and would like to
> know what the options are for advancing with this issue on that front.
>
>
>
> Thanks,
>
> Nir
>


Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-03 Thread Kevin Rushforth

Also, why is webkit.mediaplayer special in its usage of the
logger (that it requires its own property)?



That's the real question: are the needs of the WebView media component 
so special that it justifies its own mechanism / property. I doubt it, 
which is why removing the verbose flag altogether seems the better 
choice as long as it isn't too intrusive / noisy.
Is the intrusiveness something that needs to be and can be tested? Can 
we remove the flag and if needed later reinstate it via a system property?


Yes, this seems OK to me.


 it wouldn't surprise me that some of the WARNING log messages are
things that the user shouldn't necessarily be warned about.


From my point of view, if the warning messages are of no interest to 
the user, maybe they shouldn't be warning level.


Agreed. Some testing will be needed to see whether that is the case.

-- Kevin



On 5/3/2018 7:07 AM, Nir Lisker wrote:



Also, why is webkit.mediaplayer special in its usage of the
logger (that it requires its own property)?



That's the real question: are the needs of the WebView media
component so special that it justifies its own mechanism /
property. I doubt it, which is why removing the verbose flag
altogether seems the better choice as long as it isn't too
intrusive / noisy.

Is the intrusiveness something that needs to be and can be tested? Can 
we remove the flag and if needed later reinstate it via a system property?


 it wouldn't surprise me that some of the WARNING log messages are
things that the user shouldn't necessarily be warned about.


From my point of view, if the warning messages are of no interest to 
the user, maybe they shouldn't be warning level.




- Nir



On Thu, May 3, 2018 at 2:42 PM, Kevin Rushforth 
> wrote:




On 5/2/2018 6:25 PM, Nir Lisker wrote:


Not sure what you mean by "that file".


Sorry, I meant the log/config file.


I see. This isn't something that a library like JavaFX should read.


keeping the verbose flag but putting it on a System property


Then why not get a minimum level from a system property instead
of a general on/off flag?


Because that would be duplicating functionality that should be
handled by the logger configuration itself (we can't set the
logging level when using the PlatformLogger wrapper utility to
System.Logger).


Also, why is webkit.mediaplayer special in its usage of the
logger (that it requires its own property)?


That's the real question: are the needs of the WebView media
component so special that it justifies its own mechanism /
property. I doubt it, which is why removing the verbose flag
altogether seems the better choice as long as it isn't too
intrusive / noisy.

-- Kevin




- Nir

On Thu, May 3, 2018 at 3:31 AM, Kevin Rushforth
>
wrote:

inline


On 5/2/2018 4:52 PM, Nir Lisker wrote:

Thanks Murali,

I won’t suggest reading level value from log/config file.


Is that file user facing? If so, wouldn't ignoring the
level set in the
file break current behavior? Would there need to be
follow-up changes to
this file to remove the level setting from it?


Not sure what you mean by "that file". The WCMediaPlayer
file? No, it isn't user-facing. Or did you mean something else?

About option (a), wouldn't removing the verbose flag
(After changing INFO
to FINE) cause all the log messages to appear by default,
as you've stated
in the first point, and we want to avoid that? We don't
have a minimum log
level setting.


By default the log level for all loggers is set at INFO --
thus the suggestion to change all of the INFO messages to
FINE, which will not be logged by default. If we still end up
with a bunch of extra WARNING or SEVERE log messages from
from ordinary situations, then that would be a problem. Given
that the implementation of WCMediaPlayer produces "noisier
than typical" INFO log messages, it wouldn't surprise me that
some of the WARNING log messages are things that the user
shouldn't necessarily be warned about.

In any case, the second suggestion of keeping the verbose
flag but putting it on a System property might be less
intrusive. And like the current solution, puts the control in
the hands of the user.

-- Kevin


-Nir


On Wed, May 2, 2018 at 11:21 PM, Murali Billa
>
wrote:

Hi Nir,



1) Regarding “verbose” 

Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-03 Thread Nir Lisker
>
> Also, why is webkit.mediaplayer special in its usage of the logger (that
>> it requires its own property)?
>
>
> That's the real question: are the needs of the WebView media component so
> special that it justifies its own mechanism / property. I doubt it, which
> is why removing the verbose flag altogether seems the better choice as long
> as it isn't too intrusive / noisy.
>

Is the intrusiveness something that needs to be and can be tested? Can we
remove the flag and if needed later reinstate it via a system property?

 it wouldn't surprise me that some of the WARNING log messages are things
> that the user shouldn't necessarily be warned about.
>

>From my point of view, if the warning messages are of no interest to the
user, maybe they shouldn't be warning level.

- Nir

On Thu, May 3, 2018 at 2:42 PM, Kevin Rushforth 
wrote:

>
>
> On 5/2/2018 6:25 PM, Nir Lisker wrote:
>
> Not sure what you mean by "that file".
>
>
> Sorry, I meant the log/config file.
>
>
> I see. This isn't something that a library like JavaFX should read.
>
> keeping the verbose flag but putting it on a System property
>
>
> Then why not get a minimum level from a system property instead of a
> general on/off flag?
>
>
> Because that would be duplicating functionality that should be handled by
> the logger configuration itself (we can't set the logging level when using
> the PlatformLogger wrapper utility to System.Logger).
>
> Also, why is webkit.mediaplayer special in its usage of the logger (that
> it requires its own property)?
>
>
> That's the real question: are the needs of the WebView media component so
> special that it justifies its own mechanism / property. I doubt it, which
> is why removing the verbose flag altogether seems the better choice as long
> as it isn't too intrusive / noisy.
>
> -- Kevin
>
>
>
> - Nir
>
> On Thu, May 3, 2018 at 3:31 AM, Kevin Rushforth <
> kevin.rushfo...@oracle.com> wrote:
>
>> inline
>>
>>
>> On 5/2/2018 4:52 PM, Nir Lisker wrote:
>>
>>> Thanks Murali,
>>>
>>> I won’t suggest reading level value from log/config file.
>>>
>>>
>>> Is that file user facing? If so, wouldn't ignoring the level set in the
>>> file break current behavior? Would there need to be follow-up changes to
>>> this file to remove the level setting from it?
>>>
>>
>> Not sure what you mean by "that file". The WCMediaPlayer file? No, it
>> isn't user-facing. Or did you mean something else?
>>
>> About option (a), wouldn't removing the verbose flag (After changing INFO
>>> to FINE) cause all the log messages to appear by default, as you've
>>> stated
>>> in the first point, and we want to avoid that? We don't have a minimum
>>> log
>>> level setting.
>>>
>>
>> By default the log level for all loggers is set at INFO -- thus the
>> suggestion to change all of the INFO messages to FINE, which will not be
>> logged by default. If we still end up with a bunch of extra WARNING or
>> SEVERE log messages from from ordinary situations, then that would be a
>> problem. Given that the implementation of WCMediaPlayer produces "noisier
>> than typical" INFO log messages, it wouldn't surprise me that some of the
>> WARNING log messages are things that the user shouldn't necessarily be
>> warned about.
>>
>> In any case, the second suggestion of keeping the verbose flag but
>> putting it on a System property might be less intrusive. And like the
>> current solution, puts the control in the hands of the user.
>>
>> -- Kevin
>>
>>
>> -Nir
>>>
>>>
>>> On Wed, May 2, 2018 at 11:21 PM, Murali Billa 
>>> wrote:
>>>
>>> Hi Nir,



 1)  Regarding “verbose” flag usage:

 · Currently verbose flag is used to show log Levels
 (FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl.  I feel
 it is not desirable to remove this flag as all these logs will start
 appearing now by default.

 · We can try 2 options:

 a)   1st Option: We can change all INFO log messages to FINE  under
 verbose flag (by leaving all log messages that use Level other than INFO
 unchanged) and verbose flag can be removed.

 b)  If 1st option results in too much noise for WARNING log
 messages,
 then we can keep the verbose flag and introduce a System Property (for
 ex:
 javafx.web.verbose) to enable the flag. I won’t suggest reading level
 value
 from log/config file.



 2)  Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”,  I need
 to
 check few more things (since we use “addHandler” in drt) and will get
 back
 to you.



 Please let me know, if you have any queries for 1.

 Thanks,

 Murali

 *From:* Nir Lisker 
 *Sent:* Saturday, April 28, 2018 1:06 AM
 *To:* Murali Billa 
 *Cc:* openjfx-dev@openjdk.java.net Mailing <
 openjfx-dev@openjdk.java.net>

Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-03 Thread Kevin Rushforth



On 5/2/2018 6:25 PM, Nir Lisker wrote:


Not sure what you mean by "that file".


Sorry, I meant the log/config file.


I see. This isn't something that a library like JavaFX should read.


keeping the verbose flag but putting it on a System property


Then why not get a minimum level from a system property instead of a 
general on/off flag?


Because that would be duplicating functionality that should be handled 
by the logger configuration itself (we can't set the logging level when 
using the PlatformLogger wrapper utility to System.Logger).


Also, why is webkit.mediaplayer special in its usage of the logger 
(that it requires its own property)?


That's the real question: are the needs of the WebView media component 
so special that it justifies its own mechanism / property. I doubt it, 
which is why removing the verbose flag altogether seems the better 
choice as long as it isn't too intrusive / noisy.


-- Kevin



- Nir

On Thu, May 3, 2018 at 3:31 AM, Kevin Rushforth 
> wrote:


inline


On 5/2/2018 4:52 PM, Nir Lisker wrote:

Thanks Murali,

I won’t suggest reading level value from log/config file.


Is that file user facing? If so, wouldn't ignoring the level
set in the
file break current behavior? Would there need to be follow-up
changes to
this file to remove the level setting from it?


Not sure what you mean by "that file". The WCMediaPlayer file? No,
it isn't user-facing. Or did you mean something else?

About option (a), wouldn't removing the verbose flag (After
changing INFO
to FINE) cause all the log messages to appear by default, as
you've stated
in the first point, and we want to avoid that? We don't have a
minimum log
level setting.


By default the log level for all loggers is set at INFO -- thus
the suggestion to change all of the INFO messages to FINE, which
will not be logged by default. If we still end up with a bunch of
extra WARNING or SEVERE log messages from from ordinary
situations, then that would be a problem. Given that the
implementation of WCMediaPlayer produces "noisier than typical"
INFO log messages, it wouldn't surprise me that some of the
WARNING log messages are things that the user shouldn't
necessarily be warned about.

In any case, the second suggestion of keeping the verbose flag but
putting it on a System property might be less intrusive. And like
the current solution, puts the control in the hands of the user.

-- Kevin


-Nir


On Wed, May 2, 2018 at 11:21 PM, Murali Billa
>
wrote:

Hi Nir,



1)      Regarding “verbose” flag usage:

·         Currently verbose flag is used to show log Levels
(FINER/FINE/INFO/WARNING) in WCMediaPlayer &
WCMediaPlayerImpl.  I feel
it is not desirable to remove this flag as all these logs
will start
appearing now by default.

·         We can try 2 options:

a)       1st Option: We can change all INFO log messages
to FINE  under
verbose flag (by leaving all log messages that use Level
other than INFO
unchanged) and verbose flag can be removed.

b)      If 1st option results in too much noise for
WARNING log messages,
then we can keep the verbose flag and introduce a System
Property (for ex:
javafx.web.verbose) to enable the flag. I won’t suggest
reading level value
from log/config file.



2)      Regarding 
“com.sun.javafx.webkit.drt.DumpRenderTree”, I need to
check few more things (since we use “addHandler” in drt)
and will get back
to you.



Please let me know, if you have any queries for 1.

Thanks,

Murali

*From:* Nir Lisker >
*Sent:* Saturday, April 28, 2018 1:06 AM
*To:* Murali Billa >
*Cc:* openjfx-dev@openjdk.java.net
 Mailing
>
*Subject:* JDK-8195974: Replace use of java.util.logging
in javafx with
System logger



Hi Murali,



Can you have a look at
https://bugs.openjdk.java.net/browse/JDK-8195974

please?



There are some usages of j.u.l in the web module I'd like
 

Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-02 Thread Nir Lisker
>
> Not sure what you mean by "that file".


Sorry, I meant the log/config file.

keeping the verbose flag but putting it on a System property


Then why not get a minimum level from a system property instead of a
general on/off flag?

Also, why is webkit.mediaplayer special in its usage of the logger (that it
requires its own property)?

- Nir

On Thu, May 3, 2018 at 3:31 AM, Kevin Rushforth 
wrote:

> inline
>
>
> On 5/2/2018 4:52 PM, Nir Lisker wrote:
>
>> Thanks Murali,
>>
>> I won’t suggest reading level value from log/config file.
>>
>>
>> Is that file user facing? If so, wouldn't ignoring the level set in the
>> file break current behavior? Would there need to be follow-up changes to
>> this file to remove the level setting from it?
>>
>
> Not sure what you mean by "that file". The WCMediaPlayer file? No, it
> isn't user-facing. Or did you mean something else?
>
> About option (a), wouldn't removing the verbose flag (After changing INFO
>> to FINE) cause all the log messages to appear by default, as you've stated
>> in the first point, and we want to avoid that? We don't have a minimum log
>> level setting.
>>
>
> By default the log level for all loggers is set at INFO -- thus the
> suggestion to change all of the INFO messages to FINE, which will not be
> logged by default. If we still end up with a bunch of extra WARNING or
> SEVERE log messages from from ordinary situations, then that would be a
> problem. Given that the implementation of WCMediaPlayer produces "noisier
> than typical" INFO log messages, it wouldn't surprise me that some of the
> WARNING log messages are things that the user shouldn't necessarily be
> warned about.
>
> In any case, the second suggestion of keeping the verbose flag but putting
> it on a System property might be less intrusive. And like the current
> solution, puts the control in the hands of the user.
>
> -- Kevin
>
>
> -Nir
>>
>>
>> On Wed, May 2, 2018 at 11:21 PM, Murali Billa 
>> wrote:
>>
>> Hi Nir,
>>>
>>>
>>>
>>> 1)  Regarding “verbose” flag usage:
>>>
>>> · Currently verbose flag is used to show log Levels
>>> (FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl.  I feel
>>> it is not desirable to remove this flag as all these logs will start
>>> appearing now by default.
>>>
>>> · We can try 2 options:
>>>
>>> a)   1st Option: We can change all INFO log messages to FINE  under
>>> verbose flag (by leaving all log messages that use Level other than INFO
>>> unchanged) and verbose flag can be removed.
>>>
>>> b)  If 1st option results in too much noise for WARNING log messages,
>>> then we can keep the verbose flag and introduce a System Property (for
>>> ex:
>>> javafx.web.verbose) to enable the flag. I won’t suggest reading level
>>> value
>>> from log/config file.
>>>
>>>
>>>
>>> 2)  Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”,  I need
>>> to
>>> check few more things (since we use “addHandler” in drt) and will get
>>> back
>>> to you.
>>>
>>>
>>>
>>> Please let me know, if you have any queries for 1.
>>>
>>> Thanks,
>>>
>>> Murali
>>>
>>> *From:* Nir Lisker 
>>> *Sent:* Saturday, April 28, 2018 1:06 AM
>>> *To:* Murali Billa 
>>> *Cc:* openjfx-dev@openjdk.java.net Mailing >> >
>>> *Subject:* JDK-8195974: Replace use of java.util.logging in javafx with
>>> System logger
>>>
>>>
>>>
>>> Hi Murali,
>>>
>>>
>>>
>>> Can you have a look at https://bugs.openjdk.java.net/browse/JDK-8195974
>>> please?
>>>
>>>
>>>
>>> There are some usages of j.u.l in the web module I'd like your opinion
>>> on.
>>> I'm not familiar with the intent of these pieces of code and would like
>>> to
>>> know what the options are for advancing with this issue on that front.
>>>
>>>
>>>
>>> Thanks,
>>>
>>> Nir
>>>
>>>
>


Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-02 Thread Kevin Rushforth

inline


On 5/2/2018 4:52 PM, Nir Lisker wrote:

Thanks Murali,

I won’t suggest reading level value from log/config file.


Is that file user facing? If so, wouldn't ignoring the level set in the
file break current behavior? Would there need to be follow-up changes to
this file to remove the level setting from it?


Not sure what you mean by "that file". The WCMediaPlayer file? No, it 
isn't user-facing. Or did you mean something else?



About option (a), wouldn't removing the verbose flag (After changing INFO
to FINE) cause all the log messages to appear by default, as you've stated
in the first point, and we want to avoid that? We don't have a minimum log
level setting.


By default the log level for all loggers is set at INFO -- thus the 
suggestion to change all of the INFO messages to FINE, which will not be 
logged by default. If we still end up with a bunch of extra WARNING or 
SEVERE log messages from from ordinary situations, then that would be a 
problem. Given that the implementation of WCMediaPlayer produces 
"noisier than typical" INFO log messages, it wouldn't surprise me that 
some of the WARNING log messages are things that the user shouldn't 
necessarily be warned about.


In any case, the second suggestion of keeping the verbose flag but 
putting it on a System property might be less intrusive. And like the 
current solution, puts the control in the hands of the user.


-- Kevin



-Nir


On Wed, May 2, 2018 at 11:21 PM, Murali Billa 
wrote:


Hi Nir,



1)  Regarding “verbose” flag usage:

· Currently verbose flag is used to show log Levels
(FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl.  I feel
it is not desirable to remove this flag as all these logs will start
appearing now by default.

· We can try 2 options:

a)   1st Option: We can change all INFO log messages to FINE  under
verbose flag (by leaving all log messages that use Level other than INFO
unchanged) and verbose flag can be removed.

b)  If 1st option results in too much noise for WARNING log messages,
then we can keep the verbose flag and introduce a System Property (for ex:
javafx.web.verbose) to enable the flag. I won’t suggest reading level value
from log/config file.



2)  Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”,  I need to
check few more things (since we use “addHandler” in drt) and will get back
to you.



Please let me know, if you have any queries for 1.

Thanks,

Murali

*From:* Nir Lisker 
*Sent:* Saturday, April 28, 2018 1:06 AM
*To:* Murali Billa 
*Cc:* openjfx-dev@openjdk.java.net Mailing 
*Subject:* JDK-8195974: Replace use of java.util.logging in javafx with
System logger



Hi Murali,



Can you have a look at https://bugs.openjdk.java.net/browse/JDK-8195974
please?



There are some usages of j.u.l in the web module I'd like your opinion on.
I'm not familiar with the intent of these pieces of code and would like to
know what the options are for advancing with this issue on that front.



Thanks,

Nir





Re: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-02 Thread Nir Lisker
Thanks Murali,

I won’t suggest reading level value from log/config file.


Is that file user facing? If so, wouldn't ignoring the level set in the
file break current behavior? Would there need to be follow-up changes to
this file to remove the level setting from it?

About option (a), wouldn't removing the verbose flag (After changing INFO
to FINE) cause all the log messages to appear by default, as you've stated
in the first point, and we want to avoid that? We don't have a minimum log
level setting.

-Nir


On Wed, May 2, 2018 at 11:21 PM, Murali Billa 
wrote:

> Hi Nir,
>
>
>
> 1)  Regarding “verbose” flag usage:
>
> · Currently verbose flag is used to show log Levels
> (FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl.  I feel
> it is not desirable to remove this flag as all these logs will start
> appearing now by default.
>
> · We can try 2 options:
>
> a)   1st Option: We can change all INFO log messages to FINE  under
> verbose flag (by leaving all log messages that use Level other than INFO
> unchanged) and verbose flag can be removed.
>
> b)  If 1st option results in too much noise for WARNING log messages,
> then we can keep the verbose flag and introduce a System Property (for ex:
> javafx.web.verbose) to enable the flag. I won’t suggest reading level value
> from log/config file.
>
>
>
> 2)  Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”,  I need to
> check few more things (since we use “addHandler” in drt) and will get back
> to you.
>
>
>
> Please let me know, if you have any queries for 1.
>
> Thanks,
>
> Murali
>
> *From:* Nir Lisker 
> *Sent:* Saturday, April 28, 2018 1:06 AM
> *To:* Murali Billa 
> *Cc:* openjfx-dev@openjdk.java.net Mailing 
> *Subject:* JDK-8195974: Replace use of java.util.logging in javafx with
> System logger
>
>
>
> Hi Murali,
>
>
>
> Can you have a look at https://bugs.openjdk.java.net/browse/JDK-8195974
> please?
>
>
>
> There are some usages of j.u.l in the web module I'd like your opinion on.
> I'm not familiar with the intent of these pieces of code and would like to
> know what the options are for advancing with this issue on that front.
>
>
>
> Thanks,
>
> Nir
>


RE: JDK-8195974: Replace use of java.util.logging in javafx with System logger

2018-05-02 Thread Murali Billa
Hi Nir,

 

1)  Regarding “verbose” flag usage:  

· Currently verbose flag is used to show log Levels 
(FINER/FINE/INFO/WARNING) in WCMediaPlayer & WCMediaPlayerImpl.  I feel it is 
not desirable to remove this flag as all these logs will start appearing now by 
default.

· We can try 2 options: 

a)   1st Option: We can change all INFO log messages to FINE  under verbose 
flag (by leaving all log messages that use Level other than INFO unchanged) and 
verbose flag can be removed.

b)  If 1st option results in too much noise for WARNING log messages, then 
we can keep the verbose flag and introduce a System Property (for ex: 
javafx.web.verbose) to enable the flag. I won’t suggest reading level value 
from log/config file.

 

2)  Regarding  “com.sun.javafx.webkit.drt.DumpRenderTree”,  I need to check 
few more things (since we use “addHandler” in drt) and will get back to you.

 

Please let me know, if you have any queries for 1.

Thanks,

Murali

From: Nir Lisker  
Sent: Saturday, April 28, 2018 1:06 AM
To: Murali Billa 
Cc: openjfx-dev@openjdk.java.net Mailing 
Subject: JDK-8195974: Replace use of java.util.logging in javafx with System 
logger

 

Hi Murali,

 

Can you have a look at https://bugs.openjdk.java.net/browse/JDK-8195974 please?

 

There are some usages of j.u.l in the web module I'd like your opinion on. I'm 
not familiar with the intent of these pieces of code and would like to know 
what the options are for advancing with this issue on that front.

 

Thanks,

Nir